All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5/5] b43: move the register write into radio2050_rfover_val
@ 2008-05-14 18:56 Harvey Harrison
  2008-05-14 19:22 ` Michael Buesch
  0 siblings, 1 reply; 6+ messages in thread
From: Harvey Harrison @ 2008-05-14 18:56 UTC (permalink / raw)
  To: Michael Buesch; +Cc: linux-wireless, John Linville

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.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/net/wireless/b43/phy.c |  125 ++++++++++++++++++----------------------
 1 files changed, 56 insertions(+), 69 deletions(-)

diff --git a/drivers/net/wireless/b43/phy.c b/drivers/net/wireless/b43/phy.c
index 4f1129b..3c5dc1d 100644
--- a/drivers/net/wireless/b43/phy.c
+++ b/drivers/net/wireless/b43/phy.c
@@ -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);
 		}
-- 
1.5.5.1.482.g0f174


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

* Re: [PATCH 5/5] b43: move the register write into radio2050_rfover_val
  2008-05-14 18:56 [PATCH 5/5] b43: move the register write into radio2050_rfover_val Harvey Harrison
@ 2008-05-14 19:22 ` Michael Buesch
  2008-05-14 19:35   ` Harvey Harrison
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Buesch @ 2008-05-14 19:22 UTC (permalink / raw)
  To: Harvey Harrison; +Cc: linux-wireless, John Linville

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.

-- 
Greetings Michael.

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

* Re: [PATCH 5/5] b43: move the register write into radio2050_rfover_val
  2008-05-14 19:22 ` Michael Buesch
@ 2008-05-14 19:35   ` Harvey Harrison
  2008-05-14 20:00     ` Johannes Berg
  2008-05-14 20:06     ` Michael Buesch
  0 siblings, 2 replies; 6+ messages in thread
From: Harvey Harrison @ 2008-05-14 19:35 UTC (permalink / raw)
  To: Michael Buesch; +Cc: linux-wireless, John Linville

On Wed, 2008-05-14 at 21:22 +0200, Michael Buesch wrote:
> 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


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

* Re: [PATCH 5/5] b43: move the register write into radio2050_rfover_val
  2008-05-14 19:35   ` Harvey Harrison
@ 2008-05-14 20:00     ` Johannes Berg
  2008-05-14 21:53       ` Harvey Harrison
  2008-05-14 20:06     ` Michael Buesch
  1 sibling, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2008-05-14 20:00 UTC (permalink / raw)
  To: Harvey Harrison; +Cc: Michael Buesch, linux-wireless, John Linville

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

On Wed, 2008-05-14 at 12:35 -0700, Harvey Harrison wrote:
> On Wed, 2008-05-14 at 21:22 +0200, Michael Buesch wrote:
> > 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* Re: [PATCH 5/5] b43: move the register write into radio2050_rfover_val
  2008-05-14 19:35   ` Harvey Harrison
  2008-05-14 20:00     ` Johannes Berg
@ 2008-05-14 20:06     ` Michael Buesch
  1 sibling, 0 replies; 6+ messages in thread
From: Michael Buesch @ 2008-05-14 20:06 UTC (permalink / raw)
  To: Harvey Harrison; +Cc: linux-wireless, John Linville

On Wednesday 14 May 2008 21:35:57 Harvey Harrison wrote:
> On Wed, 2008-05-14 at 21:22 +0200, Michael Buesch wrote:
> > 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.

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

-- 
Greetings Michael.

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

* Re: [PATCH 5/5] b43: move the register write into radio2050_rfover_val
  2008-05-14 20:00     ` Johannes Berg
@ 2008-05-14 21:53       ` Harvey Harrison
  0 siblings, 0 replies; 6+ messages in thread
From: Harvey Harrison @ 2008-05-14 21:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Michael Buesch, linux-wireless, John Linville

On Wed, 2008-05-14 at 22:00 +0200, Johannes Berg wrote:
> On Wed, 2008-05-14 at 12:35 -0700, Harvey Harrison wrote:
> > On Wed, 2008-05-14 at 21:22 +0200, Michael Buesch wrote:
> > > 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.
> 
> 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


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

end of thread, other threads:[~2008-05-14 21:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-05-14 18:56 [PATCH 5/5] b43: move the register write into radio2050_rfover_val Harvey Harrison
2008-05-14 19:22 ` Michael Buesch
2008-05-14 19:35   ` Harvey Harrison
2008-05-14 20:00     ` Johannes Berg
2008-05-14 21:53       ` Harvey Harrison
2008-05-14 20:06     ` Michael Buesch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.