linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Larry Finger <Larry.Finger@lwfinger.net>
To: Frederic Leroy <fredo@starox.org>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	Hin-Tak Leung <hintak.leung@gmail.com>,
	Marcel Holtmann <marcel@holtmann.org>,
	Chris Mayo <aklhfex@googlemail.com>,
	linux-wireless@vger.kernel.org, Greg KH <greg@kroah.com>
Subject: Re: staging: No option to select rtl8192su in linux-2.6.33-rc2
Date: Sun, 14 Feb 2010 11:21:46 -0600	[thread overview]
Message-ID: <4B78312A.7060308@lwfinger.net> (raw)
In-Reply-To: <20100214131810.1a8b49ac@houba>

On 02/14/2010 06:18 AM, Frederic Leroy wrote:
>  
> It was my impression too, but as you say, YMMV. It will be my first
> driver, although I already hacked the kernel, make netfilters modules
> and read lwn ;).
> 
> For the process, it is still unclear for me. I copy rtl8180*[ch] to
> rtl8192*[ch], and start to hack into to make clean git patches ?
> 
> On which tree should I base my work : 
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git ?

Before you start with the mainline driver, I would suggest a little cleanup of
the driver in staging. The Realtek drivers are full of dead code such as

//	unused code that is not a comment

or

	if (0) {
	....
	} else {
	...
	}
or

#ifdef SOMECONFIGVARIABLE
	....
#else
	....
#endif

and finally

	if (priv->config_var) {
		....
	} else {
		....
	}

where config_var is set to some initial value and _NEVER_ changed.

When you are trying to translate their code to a new driver, such dead code is a
major distraction. My recommendations are as follows:

* Make certain the driver in staging works in its present form.

* Prepare a patch to eliminate the unused branches of if(0) and if(1) code and
the code that has been commented out. After the driver is tested to check for
silly mistakes and the patch passes scripts/checkpatch, push the patch to GregKH
for inclusion in future releases. If something prevents you from reaching the
goal of a mainline driver, at least your efforts will be not be lost.

* Next pick one of the ifdef variables, determine if the existing make file sets
it, and eliminate it from the code. After successful testing, push that patch to
Greg. Repeat until all such configuration variables are eliminated.

* Now start working on the priv->config_var stuff that never changes.

* The next step would be to convert the driver to use the standard EEPROM module
rather than their code. I just did that for the RTL8187SE driver. I can provide
a copy of that patch if it has not reached wireless-testing by the time you need it.

After the staging driver is leaner and cleaner, then you can start on the port.
As the RTL8192SU is a USB device, you may want to start with rtl8187 rather than
rtl8180, which is a PCI device. Once you reach the point of getting a driver
that has minimal functionality, you can use the USB capture routines to compare
your version with the one that works.

Writing a new driver is a very involved process as I have found with the
RTL8187SE. In fact, my first two attempts failed, in large part because I did
not do the steps above. The current attempt looks more promising as it has
successfully read the EEPROM and is about 1000 register read/write steps toward
getting the device initialized.

Larry

  reply	other threads:[~2010-02-14 17:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-28 12:20 staging: No option to select rtl8192su in linux-2.6.33-rc2 Chris Mayo
2009-12-28 21:59 ` Marcel Holtmann
2009-12-31 14:38   ` Hin-Tak Leung
2010-02-13 22:44     ` Frederic Leroy
2010-02-14  0:40       ` John W. Linville
2010-02-14 12:18         ` Frederic Leroy
2010-02-14 17:21           ` Larry Finger [this message]
2010-02-14 17:44             ` Frederic Leroy
2010-02-17  0:44           ` Hin-Tak Leung
2010-02-17  8:17             ` Frederic Leroy
2010-02-17 16:37               ` Larry Finger

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=4B78312A.7060308@lwfinger.net \
    --to=larry.finger@lwfinger.net \
    --cc=aklhfex@googlemail.com \
    --cc=fredo@starox.org \
    --cc=greg@kroah.com \
    --cc=hintak.leung@gmail.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=marcel@holtmann.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).