Skip to contentSkip to conversation

b43: move the register write into radio2050_rfover_val

PATCH In review Harvey Harrison 3 participants 6 messages May 14, 2008 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH 5/5 b43: move the register write into radio2050_rfover_val

It is always called with the same dev, register value as the b43_phy_write that wraps around it, make it return void and move the register write into radio2050_rfover_val.

---
 drivers/net/wireless/b43/phy.c |  125 ++++++++++++++++++----------------------
 1 files changed, 56 insertions(+), 69 deletions(-)
drivers/net/wireless/b43/phy.c +56 -69
index 4f1129b..3c5dc1d 100644
@@ -3022,14 +3022,15 @@ static u16 b43_radio_core_calibration_value(struct b43_wldev *dev)
 }
 
 #define LPD(L, P, D)	(((L) << 2) | ((P) << 1) | ((D) << 0))
-static u16 radio2050_rfover_val(struct b43_wldev *dev,
+static void radio2050_rfover_val(struct b43_wldev *dev,
 				u16 phy_register, unsigned int lpd)
 {
 	struct b43_phy *phy = &dev->phy;
 	struct ssb_sprom *sprom = &(dev->dev->bus->sprom);
+	u16 value = 0;
 
 	if (!phy->gmode)
-		return 0;
+		goto write_value;
 
 	if (has_loopback_gain(phy)) {
 		int max_lb_gain = phy->max_lb_gain;
@@ -3063,37 +3064,46 @@ static u16 radio2050_rfover_val(struct b43_wldev *dev,
 		if ((phy->rev < 7) ||
 		    !(sprom->boardflags_lo & B43_BFL_EXTLNA)) {
 			if (phy_register == B43_PHY_RFOVER) {
-				return 0x1B3;
+				value =  0x1B3;
+				goto write_value;
 			} else if (phy_register == B43_PHY_RFOVERVAL) {
 				extlna |= (i << 8);
 				switch (lpd) {
 				case LPD(0, 1, 1):
-					return 0x0F92;
+					value = 0x0F92;
+					goto write_value;
 				case LPD(0, 0, 1):
 				case LPD(1, 0, 1):
-					return (0x0092 | extlna);
+					value = 0x0092 | extlna;
+					goto write_value;
 				case LPD(1, 0, 0):
-					return (0x0093 | extlna);
+					value = 0x0093 | extlna;
+					goto write_value;
 				}
 				B43_WARN_ON(1);
 			}
 			B43_WARN_ON(1);
 		} else {
 			if (phy_register == B43_PHY_RFOVER) {
-				return 0x9B3;
+				value = 0x9B3;
+				goto write_value;
 			} else if (phy_register == B43_PHY_RFOVERVAL) {
 				if (extlna)
 					extlna |= 0x8000;
 				extlna |= (i << 8);
 				switch (lpd) {
 				case LPD(0, 1, 1):
-					return 0x8F92;
+					value = 0x8F92;
+					goto write_value;
 				case LPD(0, 0, 1):
-					return (0x8092 | extlna);
+					value = 0x8092 | extlna;
+					goto write_value;
 				case LPD(1, 0, 1):
-					return (0x2092 | extlna);
+					value = 0x2092 | extlna;
+					goto write_value;
 				case LPD(1, 0, 0):
-					return (0x2093 | extlna);
+					value = 0x2093 | extlna;
+					goto write_value;
 				}
 				B43_WARN_ON(1);
 			}
@@ -3103,41 +3113,53 @@ static u16 radio2050_rfover_val(struct b43_wldev *dev,
 		if ((phy->rev < 7) ||
 		    !(sprom->boardflags_lo & B43_BFL_EXTLNA)) {
 			if (phy_register == B43_PHY_RFOVER) {
-				return 0x1B3;
+				value = 0x1B3;
+				goto write_value;
 			} else if (phy_register == B43_PHY_RFOVERVAL) {
 				switch (lpd) {
 				case LPD(0, 1, 1):
-					return 0x0FB2;
+					value = 0x0FB2;
+					goto write_value;
 				case LPD(0, 0, 1):
-					return 0x00B2;
+					value = 0x00B2;
+					goto write_value;
 				case LPD(1, 0, 1):
-					return 0x30B2;
+					value = 0x30B2;
+					goto write_value;
 				case LPD(1, 0, 0):
-					return 0x30B3;
+					value = 0x30B3;
+					goto write_value;
 				}
 				B43_WARN_ON(1);
 			}
 			B43_WARN_ON(1);
 		} else {
 			if (phy_register == B43_PHY_RFOVER) {
-				return 0x9B3;
+				value = 0x9B3;
+				goto write_value;
 			} else if (phy_register == B43_PHY_RFOVERVAL) {
 				switch (lpd) {
 				case LPD(0, 1, 1):
-					return 0x8FB2;
+					value = 0x8FB2;
+					goto write_value;
 				case LPD(0, 0, 1):
-					return 0x80B2;
+					value = 0x80B2;
+					goto write_value;
 				case LPD(1, 0, 1):
-					return 0x20B2;
+					value = 0x20B2;
+					goto write_value;
 				case LPD(1, 0, 0):
-					return 0x20B3;
+					value = 0x20B3;
+					goto write_value;
 				}
 				B43_WARN_ON(1);
 			}
 			B43_WARN_ON(1);
 		}
 	}
-	return 0;
+
+write_value:
+	b43_phy_write(dev, phy_register, value);
 }
 
 struct init2050_saved_values {
@@ -3216,11 +3238,8 @@ u16 b43_radio_init2050(struct b43_wldev *dev)
 			b43_phy_write(dev, B43_PHY_LO_CTL, 0);
 		}
 
-		b43_phy_write(dev, B43_PHY_RFOVERVAL,
-			      radio2050_rfover_val(dev, B43_PHY_RFOVERVAL,
-						   LPD(0, 1, 1)));
-		b43_phy_write(dev, B43_PHY_RFOVER,
-			      radio2050_rfover_val(dev, B43_PHY_RFOVER, 0));
+		radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(0, 1, 1));
+		radio2050_rfover_val(dev, B43_PHY_RFOVER, 0);
 	}
 	b43_write16(dev, 0x3E2, b43_read16(dev, 0x3E2) | 0x8000);
 
@@ -3246,16 +3265,12 @@ u16 b43_radio_init2050(struct b43_wldev *dev)
 	if (phy->type == B43_PHYTYPE_B)
 		b43_radio_write16(dev, 0x78, 0x26);
 	if (phy->gmode || phy->rev >= 2) {
-		b43_phy_write(dev, B43_PHY_RFOVERVAL,
-			      radio2050_rfover_val(dev, B43_PHY_RFOVERVAL,
-						   LPD(0, 1, 1)));
+		radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(0, 1, 1));
 	}
 	b43_phy_write(dev, B43_PHY_PGACTL, 0xBFAF);
 	b43_phy_write(dev, B43_PHY_CCK(0x2B), 0x1403);
 	if (phy->gmode || phy->rev >= 2) {
-		b43_phy_write(dev, B43_PHY_RFOVERVAL,
-			      radio2050_rfover_val(dev, B43_PHY_RFOVERVAL,
-						   LPD(0, 0, 1)));
+		radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(0, 0, 1));
 	}
 	b43_phy_write(dev, B43_PHY_PGACTL, 0xBFA0);
 	b43_radio_write16(dev, 0x51, b43_radio_read16(dev, 0x51)
@@ -3274,36 +3289,24 @@ u16 b43_radio_init2050(struct b43_wldev *dev)
 		b43_phy_write(dev, B43_PHY_CCK(0x59), 0xC810);
 		b43_phy_write(dev, B43_PHY_CCK(0x58), 0x000D);
 		if (phy->gmode || phy->rev >= 2) {
-			b43_phy_write(dev, B43_PHY_RFOVERVAL,
-				      radio2050_rfover_val(dev,
-							   B43_PHY_RFOVERVAL,
-							   LPD(1, 0, 1)));
+			radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 1));
 		}
 		b43_phy_write(dev, B43_PHY_PGACTL, 0xAFB0);
 		udelay(10);
 		if (phy->gmode || phy->rev >= 2) {
-			b43_phy_write(dev, B43_PHY_RFOVERVAL,
-				      radio2050_rfover_val(dev,
-							   B43_PHY_RFOVERVAL,
-							   LPD(1, 0, 1)));
+			radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 1));
 		}
 		b43_phy_write(dev, B43_PHY_PGACTL, 0xEFB0);
 		udelay(10);
 		if (phy->gmode || phy->rev >= 2) {
-			b43_phy_write(dev, B43_PHY_RFOVERVAL,
-				      radio2050_rfover_val(dev,
-							   B43_PHY_RFOVERVAL,
-							   LPD(1, 0, 0)));
+			radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 0));
 		}
 		b43_phy_write(dev, B43_PHY_PGACTL, 0xFFF0);
 		udelay(20);
 		tmp1 += b43_phy_read(dev, B43_PHY_LO_LEAKAGE);
 		b43_phy_write(dev, B43_PHY_CCK(0x58), 0);
 		if (phy->gmode || phy->rev >= 2) {
-			b43_phy_write(dev, B43_PHY_RFOVERVAL,
-				      radio2050_rfover_val(dev,
-							   B43_PHY_RFOVERVAL,
-							   LPD(1, 0, 1)));
+			radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 1));
 		}
 		b43_phy_write(dev, B43_PHY_PGACTL, 0xAFB0);
 	}
@@ -3322,40 +3325,24 @@ u16 b43_radio_init2050(struct b43_wldev *dev)
 			b43_phy_write(dev, B43_PHY_CCK(0x59), 0xC810);
 			b43_phy_write(dev, B43_PHY_CCK(0x58), 0x000D);
 			if (phy->gmode || phy->rev >= 2) {
-				b43_phy_write(dev, B43_PHY_RFOVERVAL,
-					      radio2050_rfover_val(dev,
-								   B43_PHY_RFOVERVAL,
-								   LPD(1, 0,
-								       1)));
+				radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 1));
 			}
 			b43_phy_write(dev, B43_PHY_PGACTL, 0xAFB0);
 			udelay(10);
 			if (phy->gmode || phy->rev >= 2) {
-				b43_phy_write(dev, B43_PHY_RFOVERVAL,
-					      radio2050_rfover_val(dev,
-								   B43_PHY_RFOVERVAL,
-								   LPD(1, 0,
-								       1)));
+				radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 1));
 			}
 			b43_phy_write(dev, B43_PHY_PGACTL, 0xEFB0);
 			udelay(10);
 			if (phy->gmode || phy->rev >= 2) {
-				b43_phy_write(dev, B43_PHY_RFOVERVAL,
-					      radio2050_rfover_val(dev,
-								   B43_PHY_RFOVERVAL,
-								   LPD(1, 0,
-								       0)));
+				radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 0));
 			}
 			b43_phy_write(dev, B43_PHY_PGACTL, 0xFFF0);
 			udelay(10);
 			tmp2 += b43_phy_read(dev, B43_PHY_LO_LEAKAGE);
 			b43_phy_write(dev, B43_PHY_CCK(0x58), 0);
 			if (phy->gmode || phy->rev >= 2) {
-				b43_phy_write(dev, B43_PHY_RFOVERVAL,
-					      radio2050_rfover_val(dev,
-								   B43_PHY_RFOVERVAL,
-								   LPD(1, 0,
-								       1)));
+				radio2050_rfover_val(dev, B43_PHY_RFOVERVAL, LPD(1, 0, 1));
 			}
 			b43_phy_write(dev, B43_PHY_PGACTL, 0xAFB0);
 		}
Signature · 1 line
1.5.5.1.482.g0f174
#1 lore raw
Quoted 3 lines · On Wednesday 14 May 2008 20:56:36 Harvey Harrison wrote:

It is always called with the same dev, register value as the b43_phy_write that wraps around it, make it return void and move the register write into radio2050_rfover_val.

NACK to the whole 5 patches.
See the list archives for an explanation.

Signature · 1 line
Greetings Michael.
#2 lore raw
Quoted 7 lines · Michael Buesch
Quoted 3 lines · On Wednesday 14 May 2008 20:56:36 Harvey Harrison wrote:

It is always called with the same dev, register value as the b43_phy_write that wraps around it, make it return void and move the register write into radio2050_rfover_val.

NACK to the whole 5 patches.
See the list archives for an explanation.

Any particular reference for nacking 5/5?

While I respectfully disagree with you Re: 1-4 I can't make you take it.

But if you apply all 5, just look at the two files side-by-side and it is much easier to follow with them applied.

Cheers,

Harvey

maintainer
#3 lore raw
Quoted 6 lines · Harvey Harrison
Quoted 5 lines · Michael Buesch
Quoted 3 lines · On Wednesday 14 May 2008 20:56:36 Harvey Harrison wrote:

It is always called with the same dev, register value as the b43_phy_write that wraps around it, make it return void and move the register write into radio2050_rfover_val.

Quoted 6 lines
Quoted 2 lines

NACK to the whole 5 patches.
See the list archives for an explanation.

Any particular reference for nacking 5/5?

While I respectfully disagree with you Re: 1-4 I can't make you take it.

You could prove that those are correct:

make the function you're going to use an inline, compile & objdump/md5sum, apply the patch conversion, compile & objdump/md5sum & compare, make function out of line again

johannes

#4 lore raw
Quoted 12 lines · Johannes Berg
Quoted 11 lines · Harvey Harrison
Quoted 7 lines · Michael Buesch
Quoted 3 lines · On Wednesday 14 May 2008 20:56:36 Harvey Harrison wrote:

It is always called with the same dev, register value as the b43_phy_write that wraps around it, make it return void and move the register write into radio2050_rfover_val.

NACK to the whole 5 patches.
See the list archives for an explanation.

Any particular reference for nacking 5/5?

Quoted 7 lines
Quoted 1 line

While I respectfully disagree with you Re: 1-4 I can't make you take it.

You could prove that those are correct:

make the function you're going to use an inline, compile & objdump/md5sum, apply the patch conversion, compile & objdump/md5sum & compare, make function out of line again

At this point I'll let sleeping dogs lie, it's not worth fighting over.

Harvey

#5 lore raw
In reply to Harvey Harrison
Quoted 10 lines · On Wednesday 14 May 2008 21:35:57 Harvey Harrison wrote:
Quoted 7 lines · Michael Buesch
Quoted 3 lines · On Wednesday 14 May 2008 20:56:36 Harvey Harrison wrote:

It is always called with the same dev, register value as the b43_phy_write that wraps around it, make it return void and move the register write into radio2050_rfover_val.

NACK to the whole 5 patches.
See the list archives for an explanation.

Any particular reference for nacking 5/5?

Well, I don't see a point.
I designed the function to return the actual value. Therefore it's named foobar_val().
You patches shuffle a lot of code around, but there's no real gain from it. There are only huge downsides, like the possibility of introducing a flipped bitmask or similiar bugs. That happened in the past. And I tell you, it's really hard to debug the PHY code. _Not_ because it's hard to read, but because nobody does understand what it really does.
And guess what; it's _me_ who will have to debug it in case bugs appear... . But I already explained all this earlier.

Quoted 2 lines

But if you apply all 5, just look at the two files side-by-side and it is much easier to follow with them applied.

Nobody is actually reading or touching this code. It's Good Code (tm). We don't know what it does and we must not modify its semantics. So I don't really see a point in making it more pretty. You won't understand it afterwards anyway.

_Please_ try to do something more useful, like hacking mac80211 or something like that. There are lots of places in wireless that need work. But this certainly not one of them, as it's good and _working_ code.

Signature · 1 line
Greetings Michael.