From: Michael Buesch <mb@bu3sch.de>
To: Harvey Harrison <harvey.harrison@gmail.com>
Cc: linux-wireless <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 3/8] b43: remove b43_radio_{read|write}16 from lo.c
Date: Sat, 4 Oct 2008 00:35:35 +0200 [thread overview]
Message-ID: <200810040035.35990.mb@bu3sch.de> (raw)
In-Reply-To: <200810032356.38798.mb@bu3sch.de>
On Friday 03 October 2008 23:56:38 Michael Buesch wrote:
> On Friday 03 October 2008 23:33:04 Harvey Harrison wrote:
> > OK, but you should still look at patch 1 for the unaligned access
> > helpers.
>
> Yeah it's broken (it removes bounds checks at the end)
> and unaligned is not needed.
>
> > The rest I'll kick to the bitbucket...although I think you should look
> > at the portion of patch 5 that touches the function b43_radio_init2060
> > as it points out the _one_ place where a register is masking the value
> > of some other register...which is absolutely impossible to see unless
> > you do a series of patches like this.
> >
> > It may be that it is intentional, and it has been this way as far back
> > in the git history as I can find, but at least now it can be seen.
> >
> > - b43_radio_write16(dev, 0x0005,
> > - (b43_radio_read16(dev, 0x0081) & ~0x0008) | 0x0008);
>
> That's a bug in the dead code. We have more such bugs in the A-PHY code,
> but nobody cares.
> If you want to fix it, please read the specs and find out what should be
> here instead. Then send a patch that fixes this single line of code.
>
ah and you can also send a patch that removes all radio_read16/write16, if
you like to.
This can be proven correct by comparing the .ko sha1sum checksum before
and after applying it.
But no replacing of expressions by maskset and stuff. These can't be proven
correct (easily) and are simply not worth the danger of typo-regressions.
--
Greetings Michael.
prev parent reply other threads:[~2008-10-03 22:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-03 20:48 [PATCH 3/8] b43: remove b43_radio_{read|write}16 from lo.c Harvey Harrison
2008-10-03 21:17 ` Michael Buesch
2008-10-03 21:33 ` Harvey Harrison
2008-10-03 21:56 ` Michael Buesch
2008-10-03 22:35 ` Michael Buesch [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200810040035.35990.mb@bu3sch.de \
--to=mb@bu3sch.de \
--cc=harvey.harrison@gmail.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).