Skip to contentSkip to conversation

b43: implement baseband init for LP-PHY <= rev1

PATCH RESEND Superseded Gábor Stefanik 3 participants 11 messages Aug 2, 2009 – Aug 3, 2009 Networking drivers (wireless)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH RESEND 1/1 b43: implement baseband init for LP-PHY <= rev1

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

Cc Michael Buesch<[email protected]>
Cc Larry Finger<[email protected]>

---

Hopefully this mail will survive the trip through the arcane world of Gmail's servers...

If this very-very-very-long line (including a few tab characters) doesn't get mangled, chances are that the mail didn't get mangled. More text to make this line even longer.

 b43.h        |   11 ++++++
 phy_common.c |    7 ++++
 phy_lp.c     |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 phy_lp.h     |    8 +++-
 phy_n.c      |    3 +
 5 files changed, 120 insertions(+), 4 deletions(-)
drivers/net/wireless/b43/b43.h +11 -0
index 4e8ad84..41ca727 100644
@@ -142,6 +142,17 @@
 #define B43_BFL_BTCMOD			0x4000	/* BFL_BTCOEXIST is given in alternate GPIOs */
 #define B43_BFL_ALTIQ			0x8000	/* alternate I/Q settings */
 
+/* SPROM boardflags_hi values */
+#define B43_BFH_NOPA			0x0001	/* has no PA */
+#define B43_BFH_RSSIINV			0x0002	/* RSSI uses positive slope (not TSSI) */
+#define B43_BFH_PAREF			0x0004	/* uses the PARef LDO */
+#define B43_BFH_3TSWITCH		0x0008	/* uses a triple throw switch shared
+						 * with bluetooth */
+#define B43_BFH_PHASESHIFT		0x0010	/* can support phase shifter */
+#define B43_BFH_BUCKBOOST		0x0020	/* has buck/booster */
+#define B43_BFH_FEM_BT			0x0040	/* has FEM and switch to share antenna
+						 * with bluetooth */
+
 /* GPIO register offset, in both ChipCommon and PCI core. */
 #define B43_GPIO_CONTROL		0x6c
 
drivers/net/wireless/b43/phy_common.c +7 -0
index e176b6e..999e0bd 100644
@@ -240,6 +240,13 @@ void b43_phy_write(struct b43_wldev *dev, u16 reg, u16 value)
 	dev->phy.ops->phy_write(dev, reg, value);
 }
 
+void b43_phy_copy(struct b43_wldev *dev, u16 srcreg, u16 destreg)
+{
+	assert_mac_suspended(dev);
+	dev->phy.ops->phy_write(dev, destreg,
+		dev->phy.ops->phy_read(dev, srcreg));
+}
+
 void b43_phy_mask(struct b43_wldev *dev, u16 offset, u16 mask)
 {
 	b43_phy_write(dev, offset,
drivers/net/wireless/b43/phy_lp.c +94 -1

101 lines not on this page · raw message

drivers/net/wireless/b43/phy_lp.h +6 -2
index 18370b4..d0e67e4 100644
@@ -273,12 +273,16 @@
 #define B43_LPPHY_AFE_DDFS_POINTER_INIT		B43_PHY_OFDM(0xB8) /* AFE DDFS pointer init */
 #define B43_LPPHY_AFE_DDFS_INCR_INIT		B43_PHY_OFDM(0xB9) /* AFE DDFS incr init */
 #define B43_LPPHY_MRCNOISEREDUCTION		B43_PHY_OFDM(0xBA) /* mrcNoiseReduction */
-#define B43_LPPHY_TRLOOKUP3			B43_PHY_OFDM(0xBB) /* TRLookup3 */
-#define B43_LPPHY_TRLOOKUP4			B43_PHY_OFDM(0xBC) /* TRLookup4 */
+#define B43_LPPHY_TR_LOOKUP_3			B43_PHY_OFDM(0xBB) /* TR Lookup 3 */
+#define B43_LPPHY_TR_LOOKUP_4			B43_PHY_OFDM(0xBC) /* TR Lookup 4 */
 #define B43_LPPHY_RADAR_FIFO_STAT		B43_PHY_OFDM(0xBD) /* Radar FIFO Status */
 #define B43_LPPHY_GPIO_OUTEN			B43_PHY_OFDM(0xBE) /* GPIO Out enable */
 #define B43_LPPHY_GPIO_SELECT			B43_PHY_OFDM(0xBF) /* GPIO Select */
 #define B43_LPPHY_GPIO_OUT			B43_PHY_OFDM(0xC0) /* GPIO Out */
+#define B43_LPPHY_TR_LOOKUP_5			B43_PHY_OFDM(0xC7) /* TR Lookup 5 */
+#define B43_LPPHY_TR_LOOKUP_6			B43_PHY_OFDM(0xC8) /* TR Lookup 6 */
+#define B43_LPPHY_TR_LOOKUP_7			B43_PHY_OFDM(0xC9) /* TR Lookup 7 */
+#define B43_LPPHY_TR_LOOKUP_8			B43_PHY_OFDM(0xCA) /* TR Lookup 8 */
 
 
 
drivers/net/wireless/b43/phy_n.c +2 -1
index 8bcfda5..14ad95a 100644
@@ -137,7 +137,8 @@ static void b43_radio_init2055_post(struct b43_wldev *dev)
 
 	b43_radio_mask(dev, B2055_MASTER1, 0xFFF3);
 	msleep(1);
-	if ((sprom->revision != 4) || !(sprom->boardflags_hi&  0x0002)) {
+	if ((sprom->revision != 4) ||
+	   !(sprom->boardflags_hi&  B43_BFH_RSSIINV)) {
 		if ((binfo->vendor != PCI_VENDOR_ID_BROADCOM) ||
 		    (binfo->type != 0x46D) ||
 		    (binfo->rev<  0x41)) {
#1 lore raw

2009/8/3 Gábor Stefanik <[email protected]>:

Quoted 10 lines

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

Cc Michael Buesch<[email protected]>
Cc Larry Finger<[email protected]>

---

Looks like this one survived.

Signature · 1 line
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
#2 lore raw
In reply to Gábor Stefanik
Quoted 48 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

Cc Michael Buesch<[email protected]>
Cc Larry Finger<[email protected]>

---

Hopefully this mail will survive the trip through the arcane world of Gmail's servers...

If this very-very-very-long line (including a few tab characters) doesn't get mangled, chances are that the mail didn't get mangled. More text to make this line even longer.

  b43.h        |   11 ++++++
  phy_common.c |    7 ++++
  phy_lp.c     |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
  phy_lp.h     |    8 +++-
  phy_n.c      |    3 +
  5 files changed, 120 insertions(+), 4 deletions(-)
drivers/net/wireless/b43/b43.h +11 -0
index 4e8ad84..41ca727 100644
@@ -142,6 +142,17 @@
  #define B43_BFL_BTCMOD			0x4000	/* BFL_BTCOEXIST is given in alternate GPIOs */
  #define B43_BFL_ALTIQ			0x8000	/* alternate I/Q settings */
 
+/* SPROM boardflags_hi values */
+#define B43_BFH_NOPA			0x0001	/* has no PA */
+#define B43_BFH_RSSIINV			0x0002	/* RSSI uses positive slope (not TSSI) */
+#define B43_BFH_PAREF			0x0004	/* uses the PARef LDO */
+#define B43_BFH_3TSWITCH		0x0008	/* uses a triple throw switch shared
+						 * with bluetooth */
+#define B43_BFH_PHASESHIFT		0x0010	/* can support phase shifter */
+#define B43_BFH_BUCKBOOST		0x0020	/* has buck/booster */
+#define B43_BFH_FEM_BT			0x0040	/* has FEM and switch to share antenna
+						 * with bluetooth */
+
  /* GPIO register offset, in both ChipCommon and PCI core. */
  #define B43_GPIO_CONTROL		0x6c
 
drivers/net/wireless/b43/phy_common.c +0 -0
index e176b6e..999e0bd 100644

drivers/net/wireless/b43/phy_common.c void b43_phy_write(struct b43_wldev *dev, u16 reg, u16 value) · line 248

Quoted 9 lines
drivers/net/wireless/b43/phy_common.c +6 -0
@@ -240,6 +240,13 @@ void b43_phy_write(struct b43_wldev *dev, u16 reg, u16 value)
  	dev->phy.ops->phy_write(dev, reg, value);
  }
 
+void b43_phy_copy(struct b43_wldev *dev, u16 srcreg, u16 destreg)
+{
+	assert_mac_suspended(dev);
+	dev->phy.ops->phy_write(dev, destreg,
+		dev->phy.ops->phy_read(dev, srcreg));
+}

Hm, can you reverse the srcreg/destreg order? All C-standard copy functions use func(dest, src). I think we should stick with it.

Quoted 9 lines
drivers/net/wireless/b43/phy_common.c +1 -0
+
  void b43_phy_mask(struct b43_wldev *dev, u16 offset, u16 mask)
  {
  	b43_phy_write(dev, offset,
drivers/net/wireless/b43/phy_lp.c +0 -0
index 58e319d..dbaa2e4 100644

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 73

Quoted 9 lines
drivers/net/wireless/b43/phy_lp.c +5 -1
@@ -66,7 +66,100 @@ static void lpphy_table_init(struct b43_wldev *dev)
 
  static void lpphy_baseband_rev0_1_init(struct b43_wldev *dev)
  {
-	B43_WARN_ON(1);//TODO rev<  2 not supported, yet.
+	struct ssb_bus *bus = dev->dev->bus;
+	struct b43_phy_lp *lpphy = dev->phy.lp;
+	u16 tmp, tmp2;
+
+	if (dev->phy.rev == 1&&
                             ^

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 74

Quoted 1 line
drivers/net/wireless/b43/phy_lp.c +1 -0
+	   (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)) {
                                    ^

whitespaces

Quoted 8 lines
drivers/net/wireless/b43/phy_lp.c +8 -0
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0x3F00, 0x0900);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xC0FF, 0x0B00);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xC0FF, 0x0400);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xC0FF, 0x0B00);

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 92

Quoted 10 lines
drivers/net/wireless/b43/phy_lp.c +10 -0
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_5, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_5, 0xC0FF, 0x0900);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_6, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_6, 0xC0FF, 0x0B00);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_7, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_7, 0xC0FF, 0x0900);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_8, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_8, 0xC0FF, 0x0B00);
+	} else if (b43_current_band(dev->wl) == IEEE80211_BAND_5GHZ ||
+		  (bus->boardinfo.type == 0x048A) || ((dev->phy.rev == 0)&&
                                                                         ^

whitespace

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 93

Quoted 1 line
drivers/net/wireless/b43/phy_lp.c +1 -0
+		  (bus->sprom.boardflags_lo&  B43_BFL_FEM))) {
                                           ^

whitespace

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 103

Quoted 10 lines
drivers/net/wireless/b43/phy_lp.c +10 -0
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xFFC0, 0x0001);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xC0FF, 0x0400);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xFFC0, 0x0001);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xC0FF, 0x0500);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xFFC0, 0x0002);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xC0FF, 0x0800);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xFFC0, 0x0002);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xC0FF, 0x0A00);
+	} else if (dev->phy.rev == 1 ||
+		  (bus->sprom.boardflags_lo&  B43_BFL_FEM)) {
                                           ^

whitespace

Quoted 13 lines
drivers/net/wireless/b43/phy_lp.c +13 -0
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xFFC0, 0x0004);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xC0FF, 0x0800);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xFFC0, 0x0004);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xC0FF, 0x0C00);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xFFC0, 0x0002);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xC0FF, 0x0100);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xFFC0, 0x0002);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xC0FF, 0x0300);
+	} else {
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_1, 0xC0FF, 0x0900);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xFFC0, 0x000A);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_2, 0xC0FF, 0x0B00);

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 126

Quoted 10 lines
drivers/net/wireless/b43/phy_lp.c +10 -0
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xFFC0, 0x0006);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_3, 0xC0FF, 0x0500);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xFFC0, 0x0006);
+		b43_phy_maskset(dev, B43_LPPHY_TR_LOOKUP_4, 0xC0FF, 0x0700);
+	}
+	if (dev->phy.rev == 1) {
+		b43_phy_copy(dev, B43_LPPHY_TR_LOOKUP_1, B43_LPPHY_TR_LOOKUP_5);
+		b43_phy_copy(dev, B43_LPPHY_TR_LOOKUP_2, B43_LPPHY_TR_LOOKUP_6);
+		b43_phy_copy(dev, B43_LPPHY_TR_LOOKUP_3, B43_LPPHY_TR_LOOKUP_7);
+		b43_phy_copy(dev, B43_LPPHY_TR_LOOKUP_4, B43_LPPHY_TR_LOOKUP_8);

(Reverse them, if you reverse the meaning of b43_phy_copy())

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 129

Quoted 3 lines
drivers/net/wireless/b43/phy_lp.c +3 -0
+	}
+	if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+	   (bus->chip_id == 0x5354)&&

Lots of whitespace needed here...

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 134

Quoted 5 lines
drivers/net/wireless/b43/phy_lp.c +5 -0
+	   (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+		b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+		b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+		b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+		b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);

^^ ^ ^
b43_hf_read is not called. You OR the function pointer. And whitespace at the wrong spot.

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 141

Quoted 7 lines
drivers/net/wireless/b43/phy_lp.c +7 -0
+	}
+	if (b43_current_band(dev->wl) == IEEE80211_BAND_2GHZ) {
+		b43_phy_set(dev, B43_LPPHY_LP_PHY_CTL, 0x8000);
+		b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0040);
+		b43_phy_maskset(dev, B43_LPPHY_MINPWR_LEVEL, 0x00FF, 0xA400);
+		b43_phy_maskset(dev, B43_LPPHY_CRSGAIN_CTL, 0xF0FF, 0x0B00);
+		b43_phy_maskset(dev, 0x030, 0xFFF8, 0x0007);

Any chance to define a name for the register? Even if it's just B43_LPPHY_030 or something like that.

Quoted 10 lines
drivers/net/wireless/b43/phy_lp.c +10 -0
+		b43_phy_maskset(dev, B43_LPPHY_DSSS_CONFIRM_CNT, 0xFFF8, 0x0003);
+		b43_phy_maskset(dev, B43_LPPHY_DSSS_CONFIRM_CNT, 0xFFC7, 0x0020);
+		b43_phy_mask(dev, B43_LPPHY_IDLEAFTERPKTRXTO, 0x00FF);
+	} else { /* 5GHz */
+		b43_phy_mask(dev, 0x448, 0x7FFF);
+		b43_phy_mask(dev, B43_LPPHY_CRSGAIN_CTL, 0xFFBF);
+	}
+	if (dev->phy.rev == 1) {
+		tmp = b43_read(dev, B43_LPPHY_CLIPCTRTHRESH);
+		tmp2 = (tmp&  0x03E0)>>  5

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 161

Quoted 10 lines
drivers/net/wireless/b43/phy_lp.c +10 -0
+		tmp2 |= tmp<<  5;
+		b43_phy_write(dev, 0x4C3, tmp2);
+		tmp = b43_read(dev, B43_LPPHY_OFDMSYNCTHRESH0);
+		tmp2 = (tmp&  0x1F00)>>  8
+		tmp2 |= tmp<<  5;
+		b43_phy_write(dev, 0x4C4, tmp2);
+		tmp = b43_read(dev, B43_LPPHY_VERYLOWGAINDB);
+		tmp2 = tmp&  0x00FF
+		tmp2 |= tmp<<  8;
+		b43_phy_write(dev, 0x4C5, tmp2);

Lots of whitespace problems in this code block.
And please define the registers. Even if you just assign numeric names (like above). (hint: also use the B43_PHY_OFDM() macro)

Quoted 32 lines
drivers/net/wireless/b43/phy_lp.c +1 -0
+	}
  }
 
  static void lpphy_baseband_rev2plus_init(struct b43_wldev *dev)
drivers/net/wireless/b43/phy_lp.h +6 -2
index 18370b4..d0e67e4 100644
@@ -273,12 +273,16 @@
  #define B43_LPPHY_AFE_DDFS_POINTER_INIT		B43_PHY_OFDM(0xB8) /* AFE DDFS pointer init */
  #define B43_LPPHY_AFE_DDFS_INCR_INIT		B43_PHY_OFDM(0xB9) /* AFE DDFS incr init */
  #define B43_LPPHY_MRCNOISEREDUCTION		B43_PHY_OFDM(0xBA) /* mrcNoiseReduction */
-#define B43_LPPHY_TRLOOKUP3			B43_PHY_OFDM(0xBB) /* TRLookup3 */
-#define B43_LPPHY_TRLOOKUP4			B43_PHY_OFDM(0xBC) /* TRLookup4 */
+#define B43_LPPHY_TR_LOOKUP_3			B43_PHY_OFDM(0xBB) /* TR Lookup 3 */
+#define B43_LPPHY_TR_LOOKUP_4			B43_PHY_OFDM(0xBC) /* TR Lookup 4 */
  #define B43_LPPHY_RADAR_FIFO_STAT		B43_PHY_OFDM(0xBD) /* Radar FIFO Status */
  #define B43_LPPHY_GPIO_OUTEN			B43_PHY_OFDM(0xBE) /* GPIO Out enable */
  #define B43_LPPHY_GPIO_SELECT			B43_PHY_OFDM(0xBF) /* GPIO Select */
  #define B43_LPPHY_GPIO_OUT			B43_PHY_OFDM(0xC0) /* GPIO Out */
+#define B43_LPPHY_TR_LOOKUP_5			B43_PHY_OFDM(0xC7) /* TR Lookup 5 */
+#define B43_LPPHY_TR_LOOKUP_6			B43_PHY_OFDM(0xC8) /* TR Lookup 6 */
+#define B43_LPPHY_TR_LOOKUP_7			B43_PHY_OFDM(0xC9) /* TR Lookup 7 */
+#define B43_LPPHY_TR_LOOKUP_8			B43_PHY_OFDM(0xCA) /* TR Lookup 8 */
 
 
 
drivers/net/wireless/b43/phy_n.c +0 -0
index 8bcfda5..14ad95a 100644

drivers/net/wireless/b43/phy_n.c static void b43_radio_init2055_post(struct b43_wldev *dev) · line 141

Quoted 6 lines
drivers/net/wireless/b43/phy_n.c +2 -1
@@ -137,7 +137,8 @@ static void b43_radio_init2055_post(struct b43_wldev *dev)
 
  	b43_radio_mask(dev, B2055_MASTER1, 0xFFF3);
  	msleep(1);
-	if ((sprom->revision != 4) || !(sprom->boardflags_hi&  0x0002)) {
+	if ((sprom->revision != 4) ||
+	   !(sprom->boardflags_hi&  B43_BFH_RSSIINV)) {
                                 ^

whitespace

Quoted 7 lines
drivers/net/wireless/b43/phy_n.c +0 -0
  		if ((binfo->vendor != PCI_VENDOR_ID_BROADCOM) ||
  		    (binfo->type != 0x46D) ||
  		    (binfo->rev<  0x41)) {
Signature · 1 line
Greetings, Michael.
#3 lore raw
In reply to Gábor Stefanik

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 135

Quoted 13 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

drivers/net/wireless/b43/phy_lp.c +8 -0
+	if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+	   (bus->chip_id == 0x5354)&&
+	   (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+		b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+		b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+		b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+		b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);
+	}

The HF write is wrong. Read the specification:
http://bcm-v4.sipsolutions.net/802.11/Mhf

Patch otherwise looks ok.

Signature · 1 line
Greetings, Michael.
#4 lore raw
Quoted 18 lines · On Monday 03 August 2009 11:13:37 Michael Buesch wrote:

drivers/net/wireless/b43/phy_lp.c static void lpphy_table_init(struct b43_wldev *dev) · line 135

Quoted 13 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

drivers/net/wireless/b43/phy_lp.c +8 -0
+	if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+	   (bus->chip_id == 0x5354)&&
+	   (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+		b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+		b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+		b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+		b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);
+	}

The HF write is wrong. Read the specification:
http://bcm-v4.sipsolutions.net/802.11/Mhf

Quoted 1 line

Patch otherwise looks ok.

Sorry, I replied to the wrong mail. But this does also apply to V2 patch.

Signature · 1 line
Greetings, Michael.
#5 lore raw
Quoted 19 lines · Michael Buesch
Quoted 18 lines · On Monday 03 August 2009 11:13:37 Michael Buesch wrote:
Quoted 5 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

Quoted 8 lines

+   if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+      (bus->chip_id == 0x5354)&&
+      (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+           b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+           b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+           b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+           b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);
+   }

The HF write is wrong. Read the specification:
http://bcm-v4.sipsolutions.net/802.11/Mhf

Quoted 7 lines
Quoted 1 line

Patch otherwise looks ok.

Sorry, I replied to the wrong mail. But this does also apply to V2 patch.

--
Greetings, Michael.

In V2, this line is as follows (b43_hf_read corrected):

b43_hf_write(dev, b43_hf_read(dev) | 0x0800ULL << 32)

The command in the specs is this:

mhf(2, 0x800, 0x800, 1)

2 means B43_SHM_SH_HOSTFHI, 0x800 is the bit to set, and 1 is allbands, which, per Larry, can be ignored in our current implementation (it is specific to the caching behavior of the mips driver).

Quoted 1 line

From what I read in b43_hf_write, writing to HOSTFHI can be achieved

by left-shifting by 32 (16 for HOSTFMI).

Is the problem that we write all 3 hostflags registers here?

(BTW are there any other known host flags? If there are, maybe we should #define them.)

Signature · 1 line
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
#6 lore raw
Quoted 41 lines · On Monday 03 August 2009 15:55:29 Gábor Stefanik wrote:
Quoted 19 lines · Michael Buesch
Quoted 18 lines · On Monday 03 August 2009 11:13:37 Michael Buesch wrote:
Quoted 5 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

Quoted 8 lines

+   if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+      (bus->chip_id == 0x5354)&&
+      (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+           b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+           b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+           b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+           b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);
+   }

The HF write is wrong. Read the specification:
http://bcm-v4.sipsolutions.net/802.11/Mhf

Quoted 7 lines
Quoted 1 line

Patch otherwise looks ok.

Sorry, I replied to the wrong mail. But this does also apply to V2 patch.

--
Greetings, Michael.

In V2, this line is as follows (b43_hf_read corrected):

b43_hf_write(dev, b43_hf_read(dev) | 0x0800ULL << 32)

The command in the specs is this:

mhf(2, 0x800, 0x800, 1)

2 means B43_SHM_SH_HOSTFHI, 0x800 is the bit to set, and 1 is allbands, which, per Larry, can be ignored in our current implementation (it is specific to the caching behavior of the mips driver).

Quoted 8 lines

From what I read in b43_hf_write, writing to HOSTFHI can be achieved by left-shifting by 32 (16 for HOSTFMI).

Is the problem that we write all 3 hostflags registers here?

(BTW are there any other known host flags? If there are, maybe we should #define them.)

My point is that update_mhf does not write to actual hardware, as far as I understand the code. Larry, can you please explain that part of the specs?

Signature · 1 line
Greetings, Michael.
#7 lore raw

2009/8/3 Michael Buesch <[email protected]>:

Quoted 51 lines · On Monday 03 August 2009 15:55:29 Gábor Stefanik wrote:
Quoted 41 lines · Michael Buesch
Quoted 19 lines · On Monday 03 August 2009 11:13:37 Michael Buesch wrote:
Quoted 18 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:
Quoted 5 lines

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

Quoted 8 lines

+   if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+      (bus->chip_id == 0x5354)&&
+      (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+           b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+           b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+           b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+           b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);
+   }

The HF write is wrong. Read the specification:
http://bcm-v4.sipsolutions.net/802.11/Mhf

Quoted 7 lines
Quoted 1 line

Patch otherwise looks ok.

Sorry, I replied to the wrong mail. But this does also apply to V2 patch.

--
Greetings, Michael.

In V2, this line is as follows (b43_hf_read corrected):

b43_hf_write(dev, b43_hf_read(dev) | 0x0800ULL << 32)

The command in the specs is this:

mhf(2, 0x800, 0x800, 1)

2 means B43_SHM_SH_HOSTFHI, 0x800 is the bit to set, and 1 is allbands, which, per Larry, can be ignored in our current implementation (it is specific to the caching behavior of the mips driver).

Quoted 8 lines

From what I read in b43_hf_write, writing to HOSTFHI can be achieved by left-shifting by 32 (16 for HOSTFMI).

Is the problem that we write all 3 hostflags registers here?

(BTW are there any other known host flags? If there are, maybe we should #define them.)

Quoted 6 lines

My point is that update_mhf does not write to actual hardware, as far as I understand the code. Larry, can you please explain that part of the specs?

From 802.11/Mhf:

"If core->clk is not zero AND band->mhfs[idx] is not equal to tmp:
1. Write band->mhfs[idx] to Shared Memory Address addr[idx]"

This looks to me like it does indeed write to SHM, though the actual mhf() tries to avoid writing to SHM if possible, while b43_hf_write doesn't perform this optimization. (We don't have a band->mhfs, nor a bandstate[] array.)

Signature · 1 line
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
#8 lore raw
Quoted 52 lines · Gábor Stefanik

2009/8/3 Michael Buesch <[email protected]>:

Quoted 50 lines · On Monday 03 August 2009 15:55:29 Gábor Stefanik wrote:
Quoted 37 lines · Michael Buesch
Quoted 14 lines · On Monday 03 August 2009 11:13:37 Michael Buesch wrote:
Quoted 13 lines · On Monday 03 August 2009 00:18:22 Gábor Stefanik wrote:
Quoted 12 lines

Implement baseband init for rev.0 and rev.1 LP PHYs. Convert boardflags_hi values to defines.
Implement b43_phy_copy for easier copying between registers, as needed by LP-PHY init.

drivers/net/wireless/b43/phy_lp.c +8 -0
+   if (bus->sprom.boardflags_hi&  B43_BFH_FEM_BT)&&
+      (bus->chip_id == 0x5354)&&
+      (bus->chip_package == SSB_CHIPPACK_BCM4712S)) {
+           b43_phy_set(dev, B43_LPPHY_CRSGAIN_CTL, 0x0006);
+           b43_phy_write(dev, B43_LPPHY_GPIO_SELECT, 0x0005);
+           b43_phy_write(dev, B43_LPPHY_GPIO_OUTEN, 0xFFFF);
+           b43_hf_write(dev, b43_hf_read | 0x0800ULL<<  32);
+   }
Quoted 9 lines
Quoted 4 lines

The HF write is wrong. Read the specification:
http://bcm-v4.sipsolutions.net/802.11/Mhf

Patch otherwise looks ok.

Sorry, I replied to the wrong mail. But this does also apply to V2 patch.

--
Greetings, Michael.

In V2, this line is as follows (b43_hf_read corrected):

b43_hf_write(dev, b43_hf_read(dev) | 0x0800ULL << 32)

The command in the specs is this:

mhf(2, 0x800, 0x800, 1)

2 means B43_SHM_SH_HOSTFHI, 0x800 is the bit to set, and 1 is allbands, which, per Larry, can be ignored in our current implementation (it is specific to the caching behavior of the mips driver).

Quoted 8 lines

From what I read in b43_hf_write, writing to HOSTFHI can be achieved by left-shifting by 32 (16 for HOSTFMI).

Is the problem that we write all 3 hostflags registers here?

(BTW are there any other known host flags? If there are, maybe we should #define them.)

My point is that update_mhf does not write to actual hardware, as far as I understand the code. Larry, can you please explain that part of the specs?

Quoted 8 lines
Quoted 1 line

From 802.11/Mhf:

"If core->clk is not zero AND band->mhfs[idx] is not equal to tmp:
1. Write band->mhfs[idx] to Shared Memory Address addr[idx]"

This looks to me like it does indeed write to SHM, though the actual mhf() tries to avoid writing to SHM if possible, while b43_hf_write doesn't perform this optimization. (We don't have a band->mhfs, nor a bandstate[] array.)

Gábor states it the way the Broadcom routine is written. They have the flags divided into 3 16-bit values - high, middle, and low. The values are kept in arrays - one set is for the current band and the other is for both bands. When the routine is entered, the appropriate quantity is saved in a temporary, then the array value is maskset. Only when the resulting value changes is the shared memory location updated. The implication is that shared memory writes are expensive. Is that true?

Larry

#9 lore raw
Quoted 7 lines · On Monday 03 August 2009 22:58:30 Larry Finger wrote:

Gábor states it the way the Broadcom routine is written. They have the flags divided into 3 16-bit values - high, middle, and low. The values are kept in arrays - one set is for the current band and the other is for both bands. When the routine is entered, the appropriate quantity is saved in a temporary, then the array value is maskset. Only when the resulting value changes is the shared memory location updated. The implication is that shared memory writes are expensive. Is that true?

No. I think it has other reasons.

Signature · 1 line
Greetings, Michael.
#10 lore raw
Quoted 9 lines · Michael Buesch
Quoted 7 lines · On Monday 03 August 2009 22:58:30 Larry Finger wrote:

Gábor states it the way the Broadcom routine is written. They have the flags divided into 3 16-bit values - high, middle, and low. The values are kept in arrays - one set is for the current band and the other is for both bands. When the routine is entered, the appropriate quantity is saved in a temporary, then the array value is maskset. Only when the resulting value changes is the shared memory location updated. The implication is that shared memory writes are expensive. Is that true?

Quoted 5 lines

No. I think it has other reasons.

--
Greetings, Michael.

In that case, is the code correct:

Signature · 1 line
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)