Search Linux Wireless

Re: [PATCH 4/6] rtlwifi: Add changes for new vendor driver version

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

 



On 08/22/2012 05:50 AM, Stanislaw Gruszka wrote:

Stanislaw,

Thanks for the review. I have in-line responses.

On Tue, Aug 21, 2012 at 10:00:53AM -0500, Larry Finger wrote:
From: Li Chaoming <chaoming_li@xxxxxxxxxxxxxx>

Realtek driver rtl_92ce_92se_92de_linux_mac80211_0005.1230.2011 includes
many changes not previously included. The main changes are as follows:

1. Support for the PHY mode switch implemented for some models.
2. Implementation of new routines for the dual-MAC devices.
3. Implement a mechanism for reaching the private data storage
    of the other unit in a dual-MAC device.
4. Impplementation of RX aggregation.
5. Storage of beacon statistics for roaming.
6. The RX routine has been rewritten to reduce the number of O(1) buffers.

Signed-off-by: Li Chaoming <chaoming_li@xxxxxxxxxxxxxx>
Signed-off-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
---
  drivers/net/wireless/rtlwifi/base.c  |  323 ++++++++++++++++--
  drivers/net/wireless/rtlwifi/base.h  |   10 +-
  drivers/net/wireless/rtlwifi/cam.c   |    7 +-
  drivers/net/wireless/rtlwifi/core.c  |   76 +++--
  drivers/net/wireless/rtlwifi/debug.h |    8 +
  drivers/net/wireless/rtlwifi/efuse.c |   40 ++-
  drivers/net/wireless/rtlwifi/efuse.h |    1 -
  drivers/net/wireless/rtlwifi/pci.c   |  624 +++++++++++++++++++---------------
  drivers/net/wireless/rtlwifi/pci.h   |    3 +
  drivers/net/wireless/rtlwifi/ps.c    |   93 ++++-
  drivers/net/wireless/rtlwifi/ps.h    |    3 +-
  drivers/net/wireless/rtlwifi/rc.c    |    3 +-
  drivers/net/wireless/rtlwifi/regd.c  |   18 +
  13 files changed, 851 insertions(+), 358 deletions(-)

Could Realsil/Realtek post small patches please?

See "Separate your changes." from Documentation/SubmittingPatches.

You must have separate patches in repositories already. Life is strange,
but I don't believe Realsil/Realtek develops driver without source
control :-)

Sorry for making the patch so large.

I have no idea what Realtek uses for version control, but I have no access to it. What I get from them is a complete new version of a driver. To discover what changes have been made, I diff the new version against the old and prepare the necessary patches for the in-kernel versions, which have diverged from the vendor version. Most of the changes are cosmetic, but not all of them fit that category. When patches are submitted, I mark them as from Realtek authors when that is appropriate. If the change is something that I instituted, then I claim authorship. In either case, I am the one that prepared the patch.

  	    /* IEEE80211_HW_SUPPORTS_CQM_RSSI | */
-	    IEEE80211_HW_REPORTS_TX_ACK_STATUS | 0;
+	    IEEE80211_HW_REPORTS_TX_ACK_STATUS |
+	    WIPHY_FLAG_IBSS_RSN |
This flags belongs to hw->wiphy->flags.

Apparently RSN IBSS functionality wasn't tested, so for now, this probably
should be removed.

OK.

+	init_timer(&rtlpriv->works.dualmac_easyconcurrent_retrytimer);
+	setup_timer(&rtlpriv->works.dualmac_easyconcurrent_retrytimer,
+		    rtl_easy_concurrent_retrytimer_callback, (unsigned long)hw);
Nit: init_timer is not needed if setup_timer is used.

Good to know.

  	mutex_init(&rtlpriv->locks.conf_mutex);
-	mutex_init(&rtlpriv->locks.ps_mutex);
  	spin_lock_init(&rtlpriv->locks.ips_lock);
  	spin_lock_init(&rtlpriv->locks.irq_th_lock);
  	spin_lock_init(&rtlpriv->locks.h2c_lock);
  	spin_lock_init(&rtlpriv->locks.rf_ps_lock);
  	spin_lock_init(&rtlpriv->locks.rf_lock);
+	spin_lock_init(&rtlpriv->locks.lps_lock);
This reverts various changes we did in kernel driver regarding PS
locking, which was started by commit:

commit 312d5479dcfaca2b8aa451201b5388fdb8c8684a
Author: Mike McCormack <mikem@xxxxxxxxxx>
Date:   Tue May 31 08:49:36 2011 +0900

     rtlwifi: Don't block interrupts in spinlocks

I wonder if this change does not reintroduce "disabling interrupts for
long time" problem.

Also below there is overwrite of IRQ_HANDLED fix. I did not check more,
but possibly patch also overwrite some other, more important fixes
we have in kernel version of rtlwifi driver. Larry put dozen of fixes to
driver, would be pity to lost them.

I got confused here. I certainly have no intent to undo my work.

+/* mac80211 have issues when receive del ba
+ * so here we just make a fake del_ba when we receive a ba_req
+ * but rx_agg was opened to let mac80211 release some ba
+ * related resources, so please this del_ba for tx
+ */

So this issue should be fixed in mac80211 instead of work around
in driver.

BTW, such approach is typical "platform problem" (see
http://lwn.net/Articles/443531/), which frustrate kernel developers.
Please participate in mac80211 and other kernel core components too.

I will pass that on to Chaoming, and I will remove that routine and its call. As soon as possible, I hope to have a patch for mac80211.

  static irqreturn_t _rtl_pci_interrupt(int irq, void *dev_id)
@@ -775,7 +865,6 @@ static irqreturn_t _rtl_pci_interrupt(int irq, void *dev_id)
  	unsigned long flags;
  	u32 inta = 0;
  	u32 intb = 0;
-	irqreturn_t ret = IRQ_HANDLED;

  	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);

@@ -783,10 +872,8 @@ static irqreturn_t _rtl_pci_interrupt(int irq, void *dev_id)
  	rtlpriv->cfg->ops->interrupt_recognized(hw, &inta, &intb);

  	/*Shared IRQ or HW disappared */
-	if (!inta || inta == 0xffff) {
-		ret = IRQ_NONE;
+	if (!inta || inta == 0xffff)
  		goto done;
-	}

  	/*<1> beacon related */
  	if (inta & rtlpriv->cfg->maps[RTL_IMR_TBDOK]) {
@@ -889,7 +976,7 @@ static irqreturn_t _rtl_pci_interrupt(int irq, void *dev_id)

  done:
  	spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, flags);
-	return ret;
+	return IRQ_HANDLED;
Overwrite of:

commit de2e56cea25c80f91a6c6699de40fb3fe8b2479d
Author: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
Date:   Wed Nov 23 21:30:19 2011 -0600

     rtlwifi: Fix incorrect return of IRQ_HANDLED


This will be fixed.

+/* this is used for other modules get
+ * hw pointer in rtl_pci_get_hw_pointer
+ */
+static struct ieee80211_hw *hw_export;
+
  int __devinit rtl_pci_probe(struct pci_dev *pdev,
  			    const struct pci_device_id *id)
  {
@@ -1785,6 +1832,7 @@ int __devinit rtl_pci_probe(struct pci_dev *pdev,
  		err = -ENOMEM;
  		goto fail1;
  	}
+	hw_export = hw;
How this is suppose to work, this variable will be overwritten by
any ->pci_probe ? In general this buddy/glabal_var stuff is very
fishy, but I did not looked in detail at that. Does all of
that actually work ?

The question of whether it works will need to be deferred to Chaoming. The situation is that the device has both a 2.4 GHz part and a 5 GHz part that register as two separate devices; however, each driver instance needs to have access to some of the private variables of the other. Can you point me to an example of a safe way to do this without using a global variable?

  	if (rtlpci->irq_alloc) {
+		synchronize_irq(rtlpci->pdev->irq);
  		free_irq(rtlpci->pdev->irq, hw);
Nit: not needed - free_irq do sync internally.

Thanks.

+static void _rtl_dump_channel_map(struct wiphy *wiphy)
+{
+	enum ieee80211_band band;
+	struct ieee80211_supported_band *sband;
+	struct ieee80211_channel *ch;
+	unsigned int i;
+
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+		if (!wiphy->bands[band])
+			continue;
+		sband = wiphy->bands[band];
+		for (i = 0; i < sband->n_channels; i++)
+			ch = &sband->channels[i];
+	}
+}
This functions doesn't do anything useful.

Agreed. I must have missed something. If not, it will go away.

Thanks again,

Larry

--
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 Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux