Search Linux Wireless

Re: [PATCH 5/5] b43: move the register write into radio2050_rfover_val

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux