From: Ivo van Doorn <ivdoorn@gmail.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH 5/10] rt2x00: Register initialization fixes
Date: Mon, 28 Aug 2006 22:15:18 +0200 [thread overview]
Message-ID: <200608282215.18238.IvDoorn@gmail.com> (raw)
In-Reply-To: <20060828160834.GA14069@tuxdriver.com>
On Monday 28 August 2006 18:08, John W. Linville wrote:
> On Sun, Aug 27, 2006 at 05:39:14PM +0200, Ivo van Doorn wrote:
> > Various register initialization fixes to make the device work properly.
> > This will fix the RX/TX issue for rt61pci.
> >
> > Signed-off-by Ivo van Doorn <ivdoorn@gmail.com>
> >
> > ---
> >
> > diff -rU3 wireless-dev-rt2x00-interface/drivers/net/wireless/d80211/rt2x00/rt2400pci.c wireless-dev-rt2x00-register/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> > --- wireless-dev-rt2x00-interface/drivers/net/wireless/d80211/rt2x00/rt2400pci.c 2006-08-27 16:11:40.000000000 +0200
> > +++ wireless-dev-rt2x00-register/drivers/net/wireless/d80211/rt2x00/rt2400pci.c 2006-08-27 16:17:02.000000000 +0200
> > @@ -1192,11 +1192,7 @@
> > rt2x00_register_write(rt2x00dev, RXCSR0, reg);
> >
> > rt2x00_register_write(rt2x00dev, MACCSR0, cpu_to_le32(0x00217223));
> > -
> > - rt2x00_register_read(rt2x00dev, MACCSR1, ®);
> > - rt2x00_set_field32(®, MACCSR1_AUTO_TXBBP, 1);
> > - rt2x00_set_field32(®, MACCSR1_AUTO_RXBBP, 1);
> > - rt2x00_register_write(rt2x00dev, MACCSR1, reg);
> > + rt2x00_register_write(rt2x00dev, MACCSR1, cpu_to_le32(0x00235518));
> >
> > rt2x00_register_read(rt2x00dev, MACCSR2, ®);
> > rt2x00_set_field32(®, MACCSR2_DELAY, 64);
> > diff -rU3 wireless-dev-rt2x00-interface/drivers/net/wireless/d80211/rt2x00/rt2500pci.c wireless-dev-rt2x00-register/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> > --- wireless-dev-rt2x00-interface/drivers/net/wireless/d80211/rt2x00/rt2500pci.c 2006-08-27 16:12:03.000000000 +0200
> > +++ wireless-dev-rt2x00-register/drivers/net/wireless/d80211/rt2x00/rt2500pci.c 2006-08-27 16:17:56.000000000 +0200
> > @@ -1249,6 +1249,7 @@
> > return -EBUSY;
> >
> > rt2x00_register_write(rt2x00dev, PWRCSR0, cpu_to_le32(0x3f3b3100));
> > + rt2x00_register_write(rt2x00dev, PCICSR, cpu_to_le32(0x000003b8));
> >
> > rt2x00_register_write(rt2x00dev, PSCSR0, cpu_to_le32(0x00020002));
> > rt2x00_register_write(rt2x00dev, PSCSR1, cpu_to_le32(0x00000002));
> > @@ -1272,12 +1273,11 @@
> > rt2x00_set_field32(®, RXCSR0_DISABLE_RX, 0);
> > rt2x00_register_write(rt2x00dev, RXCSR0, reg);
> >
> > - rt2x00_register_write(rt2x00dev, MACCSR0, cpu_to_le32(0x00213223));
> > + rt2x00_register_write(rt2x00dev, GPIOCSR, cpu_to_le32(0x0000ff00));
> > + rt2x00_register_write(rt2x00dev, TESTCSR, cpu_to_le32(0x000000f0));
> >
> > - rt2x00_register_read(rt2x00dev, MACCSR1, ®);
> > - rt2x00_set_field32(®, MACCSR1_AUTO_TXBBP, 1);
> > - rt2x00_set_field32(®, MACCSR1_AUTO_RXBBP, 1);
> > - rt2x00_register_write(rt2x00dev, MACCSR1, reg);
> > + rt2x00_register_write(rt2x00dev, MACCSR0, cpu_to_le32(0x00213223));
> > + rt2x00_register_write(rt2x00dev, MACCSR1, cpu_to_le32(0x00235518));
> >
> > rt2x00_register_read(rt2x00dev, MACCSR2, ®);
> > rt2x00_set_field32(®, MACCSR2_DELAY, 64);
>
> Lots of magic numbers here...can we do something about that?
Only partially, there are currently a couple problems with:
- Some of the above registers are documented, but there is however a main problem with current documentation from Ralink,
not all register maps are correct. The driver indicates some other function for the register than our documentation claims.
- The register not defined, the documentation gives no details and I cannot understand the meaning of the value from
the original Ralink code. This is a main problem with the original code since its code contains a great deal of magical values,
most have them have been analysed and its meaning or origin was determined. There are however still some magical values left.
- In some other situations it is indeed possible to use the bitmaps as defined in the header,
but those would not always explain the meaning of the value clearer (GPIOCSR is for example just a list of BIT0, BIT1, BIT2 etc).
- Using the defines fom the headers and using the rt2x00_set_field32 would result in quite a lot of cpu_to_le32 calls,
this would be a waste on big endian machines when the particular register is only used once.
I will go over the values again and see which ones can be made clearer with comments, and if using the
rt2x00_set/get_field32 can be used instead to make things clearer.
Ivo
prev parent reply other threads:[~2006-08-28 20:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-08-27 15:39 [PATCH 5/10] rt2x00: Register initialization fixes Ivo van Doorn
2006-08-28 16:08 ` John W. Linville
2006-08-28 20:15 ` Ivo van Doorn [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=200608282215.18238.IvDoorn@gmail.com \
--to=ivdoorn@gmail.com \
--cc=linville@tuxdriver.com \
--cc=netdev@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).