Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH] wireless: Convert mwifiex/pcie to dev_pm_ops from legacy pm ops
From: Shuah Khan @ 2013-07-02 19:24 UTC (permalink / raw)
  To: Hauke Mehrtens
  Cc: bzhao@marvell.com, linville@tuxdriver.com, rjw@sisk.pl,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shuahkhan@gmail.com, Shuah Khan
In-Reply-To: <51D32116.4010109@hauke-m.de>

On 07/02/2013 12:51 PM, Hauke Mehrtens wrote:
> On 07/02/2013 05:24 PM, Shuah Khan wrote:
>> Convert the mwifiex/pci driver to use dev_pm_ops for power management and
>> remove Legacy PM handling. This change re-uses existing suspend and resume
>> interfaces for dev_pm_ops.
>>
>> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
>> ---
>>   drivers/net/wireless/mwifiex/pcie.c |   34 ++++++++++++++++++++++++++++------
>>   1 file changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mwifiex/pcie.c b/drivers/net/wireless/mwifiex/pcie.c
>> index 20c9c4c..b169318 100644
>> --- a/drivers/net/wireless/mwifiex/pcie.c
>> +++ b/drivers/net/wireless/mwifiex/pcie.c
>> @@ -85,7 +85,7 @@ static bool mwifiex_pcie_ok_to_access_hw(struct mwifiex_adapter *adapter)
>>    * If already not suspended, this function allocates and sends a host
>>    * sleep activate request to the firmware and turns off the traffic.
>>    */
>> -static int mwifiex_pcie_suspend(struct pci_dev *pdev, pm_message_t state)
>> +static int __mwifiex_pcie_suspend(struct pci_dev *pdev)
>>   {
>>   	struct mwifiex_adapter *adapter;
>>   	struct pcie_service_card *card;
>> @@ -112,6 +112,13 @@ static int mwifiex_pcie_suspend(struct pci_dev *pdev, pm_message_t state)
>>   	return 0;
>>   }
>>
>> +static int mwifiex_pcie_suspend(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +	return __mwifiex_pcie_suspend(pdev);
>> +}
>> +
>
> For what do you need __mwifiex_pcie_suspend() ? Why not make one
> function out of these two?

Originally, the reason is to keep __mwifiex_pcie_suspend() and 
__mwifiex_pcie_resume() parameters the same so there will be no 
confusion. However, I didn't think about using 
mwifiex_pcie_resume(&pdev->dev) as you mentioned below. Yes, I can make 
the change and not add two additional routines and send a v2 patch.

>
>>   /*
>>    * Kernel needs to suspend all functions separately. Therefore all
>>    * registered functions must have drivers with suspend and resume
>> @@ -120,7 +127,7 @@ static int mwifiex_pcie_suspend(struct pci_dev *pdev, pm_message_t state)
>>    * If already not resumed, this function turns on the traffic and
>>    * sends a host sleep cancel request to the firmware.
>>    */
>> -static int mwifiex_pcie_resume(struct pci_dev *pdev)
>> +static int __mwifiex_pcie_resume(struct pci_dev *pdev)
>>   {
>>   	struct mwifiex_adapter *adapter;
>>   	struct pcie_service_card *card;
>> @@ -150,6 +157,13 @@ static int mwifiex_pcie_resume(struct pci_dev *pdev)
>>
>>   	return 0;
>>   }
>> +
>> +static int mwifiex_pcie_resume(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +	return __mwifiex_pcie_resume(pdev);
>> +}
>>   #endif
>>
>>   /*
>> @@ -213,7 +227,7 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
>>   	if (user_rmmod) {
>>   #ifdef CONFIG_PM
>>   		if (adapter->is_suspended)
>> -			mwifiex_pcie_resume(pdev);
>> +			__mwifiex_pcie_resume(pdev);
>
> You could use mwifiex_pcie_resume(&pdev->dev) here and then the extra
> function __mwifiex_pcie_resume() is not needed any more.

Yes. That would eliminate the need for additional routine. I didn't 
think about that option.

>
>>   #endif
>>
>>   		for (i = 0; i < adapter->priv_num; i++)
>> @@ -249,6 +263,14 @@ static DEFINE_PCI_DEVICE_TABLE(mwifiex_ids) = {
>>
>>   MODULE_DEVICE_TABLE(pci, mwifiex_ids);
>>
>> +#ifdef CONFIG_PM
>> +/* Power Management Hooks */
>> +static const struct dev_pm_ops mwifiex_pcie_pm_ops = {
>> +	.suspend = mwifiex_pcie_suspend,
>> +	.resume = mwifiex_pcie_resume,
>> +};
>> +#endif
>> +
>
> Is it intended that you do not use SIMPLE_DEV_PM_OPS() like most of the
> other wifi drivers?

So the reason for this is SIMPLE_DEV_PM_OPS() is defined only when 
CONFIG_PM_SLEEP is enabled. This driver doesn't defines suspend and 
resume when CONFIG_PM is enabled and I didn't want to change that. Hope 
that helps understand why I didn't use SIMPLE_DEV_PM_OPS()

>
>>   /* PCI Device Driver */
>>   static struct pci_driver __refdata mwifiex_pcie = {
>>   	.name     = "mwifiex_pcie",
>> @@ -256,9 +278,9 @@ static struct pci_driver __refdata mwifiex_pcie = {
>>   	.probe    = mwifiex_pcie_probe,
>>   	.remove   = mwifiex_pcie_remove,
>>   #ifdef CONFIG_PM
>> -	/* Power Management Hooks */
>> -	.suspend  = mwifiex_pcie_suspend,
>> -	.resume   = mwifiex_pcie_resume,
>> +	.driver   = {
>> +		.pm = &mwifiex_pcie_pm_ops,
>> +	},
>>   #endif
>>   };
>>
>>
>
>

-- Shuah

Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research 
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658

^ permalink raw reply

* Re: [PATCH] wireless: Convert mwifiex/pcie to dev_pm_ops from legacy pm ops
From: Lars-Peter Clausen @ 2013-07-02 19:52 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Hauke Mehrtens, bzhao@marvell.com, linville@tuxdriver.com,
	rjw@sisk.pl, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	shuahkhan@gmail.com
In-Reply-To: <B8EFE96D1287C24090BAD9D858E15E61734462@sisaex02sj>

On 07/02/2013 09:24 PM, Shuah Khan wrote:
[...]
>>> +#ifdef CONFIG_PM
>>> +/* Power Management Hooks */
>>> +static const struct dev_pm_ops mwifiex_pcie_pm_ops = {
>>> +	.suspend = mwifiex_pcie_suspend,
>>> +	.resume = mwifiex_pcie_resume,
>>> +};
>>> +#endif
>>> +
>>
>> Is it intended that you do not use SIMPLE_DEV_PM_OPS() like most of the
>> other wifi drivers?
> 
> So the reason for this is SIMPLE_DEV_PM_OPS() is defined only when 
> CONFIG_PM_SLEEP is enabled. This driver doesn't defines suspend and 
> resume when CONFIG_PM is enabled and I didn't want to change that. Hope 
> that helps understand why I didn't use SIMPLE_DEV_PM_OPS()

Change the ifdefs to CONFIG_PM_SLEEP and use SIMPLE_DEV_PM_OPS. The driver does
not support runtime PM.

- Lars


^ permalink raw reply

* Re: [PATCH v2 2/9] wlcore: use irq_flags in pdata instead of hiding it behind a quirk
From: Luciano Coelho @ 2013-07-02 20:12 UTC (permalink / raw)
  To: balbi
  Cc: linux-wireless, tony, nsekhar, mturquette, mark.rutland,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702152657.GE7013@arwen.pp.htv.fi>

On Tue, 2013-07-02 at 18:26 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Jul 02, 2013 at 05:55:41PM +0300, Luciano Coelho wrote:
> > The platform_quirk element in the platform data was used to change the
> > way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
> > the irqflags used and treat edge trigger differently from the rest.
> > 
> > Instead of hiding this irq flag setting behind the quirk, export the
> > whole irq_flags element and let the board file define what to use.
> > This will be more meaningful than driver-specific quirks when we
> > switch to DT.
> > 
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > ---
> >  arch/arm/mach-davinci/board-da850-evm.c      |    2 +-
> >  arch/arm/mach-omap2/board-4430sdp.c          |    1 +
> >  arch/arm/mach-omap2/board-omap3evm.c         |    1 +
> >  arch/arm/mach-omap2/board-omap4panda.c       |    1 +
> >  arch/arm/mach-omap2/board-zoom-peripherals.c |    1 +
> >  drivers/net/wireless/ti/wlcore/debugfs.c     |    2 +-
> >  drivers/net/wireless/ti/wlcore/main.c        |   13 +++----------
> >  drivers/net/wireless/ti/wlcore/wlcore.h      |    5 ++---
> >  include/linux/wl12xx.h                       |    5 +----
> >  9 files changed, 12 insertions(+), 19 deletions(-)
> > 
> > diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
> > index 8a24b6c..d2a2a98 100644
> > --- a/arch/arm/mach-davinci/board-da850-evm.c
> > +++ b/arch/arm/mach-davinci/board-da850-evm.c
> > @@ -1377,8 +1377,8 @@ static const short da850_wl12xx_pins[] __initconst = {
> >  
> >  static struct wl12xx_platform_data da850_wl12xx_wlan_data __initdata = {
> >  	.irq			= -1,
> > +	.irq_flags		= IRQF_TRIGGER_RISING,
> >  	.board_ref_clock	= WL12XX_REFCLOCK_38,
> > -	.platform_quirks	= WL12XX_PLATFORM_QUIRK_EDGE_IRQ,
> >  };
> >  
> >  static __init int da850_wl12xx_init(void)
> > diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> > index 56a9a4f..c2334aa 100644
> > --- a/arch/arm/mach-omap2/board-4430sdp.c
> > +++ b/arch/arm/mach-omap2/board-4430sdp.c
> > @@ -693,6 +693,7 @@ static void __init omap4_sdp4430_wifi_mux_init(void)
> >  }
> >  
> >  static struct wl12xx_platform_data omap4_sdp4430_wlan_data __initdata = {
> > +	.irq_flags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> 
> couldn't you just call irq_set_irq_type() from the board-file itself ?
> Then on your driver you can just pass IRQF_ONESHOT (to make sure heh) to
> your request_threaded_irq_handler()

Good point.  This is an oldish patch from the time I still thought I'd
have to pass the flags in the device tree.  It turned out that I don't
need it anymore, so this can be totally removed from pdata and be set by
the board file itself.

As you saw in a later patch, I do make sure the driver sets
IRQF_ONESHOT. ;)

--
Cheers,
Luca.


^ permalink raw reply

* Re: [PATCH v2 4/9] wl12xx: use frequency instead of enumerations for pdata clocks
From: Luciano Coelho @ 2013-07-02 20:14 UTC (permalink / raw)
  To: balbi
  Cc: linux-wireless, tony, nsekhar, mturquette, mark.rutland,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702153113.GF7013@arwen.pp.htv.fi>

On Tue, 2013-07-02 at 18:31 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Jul 02, 2013 at 05:55:43PM +0300, Luciano Coelho wrote:
> > diff --git a/drivers/net/wireless/ti/wl12xx/main.c b/drivers/net/wireless/ti/wl12xx/main.c
> > index 1c627da..903dcb3 100644
> > --- a/drivers/net/wireless/ti/wl12xx/main.c
> > +++ b/drivers/net/wireless/ti/wl12xx/main.c
> > @@ -1701,6 +1701,42 @@ static struct ieee80211_sta_ht_cap wl12xx_ht_cap = {
> >  		},
> >  };
> >  
> > +static struct wl12xx_clock wl12xx_refclock_table[] = {
> 
> const
> 
> > +	{ 19200000,	false,	WL12XX_REFCLOCK_19	},
> > +	{ 26000000,	false,	WL12XX_REFCLOCK_26	},
> > +	{ 26000000,	true,	WL12XX_REFCLOCK_26_XTAL	},
> > +	{ 38400000,	false,	WL12XX_REFCLOCK_38	},
> > +	{ 38400000,	true,	WL12XX_REFCLOCK_38_XTAL	},
> > +	{ 52000000,	false,	WL12XX_REFCLOCK_52	},
> > +	{ 0,		false,	0 }
> > +};
> > +
> > +static struct wl12xx_clock wl12xx_tcxoclock_table[] = {
> 
> const

Duh! Thanks for noticing it.

--
Luca.


^ permalink raw reply

* Re: [PATCH v2 5/9] wlcore: always use one-shot IRQ
From: Luciano Coelho @ 2013-07-02 20:16 UTC (permalink / raw)
  To: balbi
  Cc: linux-wireless, tony, nsekhar, mturquette, mark.rutland,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702153229.GG7013@arwen.pp.htv.fi>

On Tue, 2013-07-02 at 18:32 +0300, Felipe Balbi wrote:
> On Tue, Jul 02, 2013 at 05:55:44PM +0300, Luciano Coelho wrote:
> > Since we are now using threaded IRQs without the primary handler, we
> > need to set IRQF_ONESHOT, otherwise our request will fail.
> > 
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> 
> good to see this happening, I remember we talked about this a while back
> :-)

Yeah, we talked about it and I did the patch immediately.  This is
needed because this is obviously not set automatically in the interrupts
created via device tree.


> Acked-by: Felipe Balbi <balbi@ti.com>
> 
> Still, if you call irq_set_irq_type() on board-file (since DT will do
> something similar for you anyway) then you can completely drop
> irq_flags.

Yep.

--
Luca.


^ permalink raw reply

* Re: [PATCH v2 8/9] wlcore: sdio: get clocks from device tree
From: Luciano Coelho @ 2013-07-02 20:19 UTC (permalink / raw)
  To: balbi
  Cc: linux-wireless, tony, nsekhar, mturquette, mark.rutland,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702153526.GI7013@arwen.pp.htv.fi>

On Tue, 2013-07-02 at 18:35 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Jul 02, 2013 at 05:55:47PM +0300, Luciano Coelho wrote:
> > @@ -294,6 +316,8 @@ static int wl1271_probe(struct sdio_func *func,
> >  	/* Use block mode for transferring over one block size of data */
> >  	func->card->quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
> >  
> > +	sdio_set_drvdata(func, glue);
> 
> is this move really necessary ?

It is, because I use the glue in wlcore_get_pdata_from_of(), so I need
to call sdio_set_drvdata() earlier.  I reckoned it wouldn't hurt to move
this, so I wouldn't have to pass the glue in the
wlcore_get_pdata_from_of() call.

--
Luca.


^ permalink raw reply

* Re: [PATCH v2 4/9] wl12xx: use frequency instead of enumerations for pdata clocks
From: Luciano Coelho @ 2013-07-02 20:27 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-wireless, tony, nsekhar, mturquette, mark.rutland, balbi,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702150208.GA4233@kahuna>

On Tue, 2013-07-02 at 10:02 -0500, Nishanth Menon wrote:
> On 17:55-20130702, Luciano Coelho wrote:
> > Instead of defining an enumeration with the FW specific values for the
> > different clock rates, use the actual frequency instead.  Also add a
> > boolean to specify whether the clock is XTAL or not.
> > 
> > Change all board files to reflect this.
> > 
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> > ---
> >  arch/arm/mach-davinci/board-da850-evm.c      |    3 +-
> >  arch/arm/mach-omap2/board-4430sdp.c          |    5 ++-
> ^^
> >  arch/arm/mach-omap2/board-omap3evm.c         |    3 +-
> >  arch/arm/mach-omap2/board-omap4panda.c       |    3 +-
> ^^
> Please do not add more platform data to platforms that are DT only.

Ah, I hadn't realized that board_omap4panda.c and board-4430sdp.c had
been removed in linux-next.  I base my tree on wireless-next and it
doesn't contain these changes yet.  I would only have noticed this when
I rebased my tree once the merge window is closed. ;)

Thanks for pointing out, I'll make sure these changes will not be there
when I send the pull request.

--
Cheers,
Luca.


^ permalink raw reply

* Re: [PATCH V2] brcmsmac: Fix WARNING caused by lack of calls to dma_mapping_error()
From: Arend van Spriel @ 2013-07-02 20:53 UTC (permalink / raw)
  To: Larry Finger
  Cc: linville, linux-wireless, netdev, Stable, Brett Rudley,
	Franky (Zhenhui) Lin, Hante Meuleman, brcm80211-dev-list
In-Reply-To: <1372781198-15218-1-git-send-email-Larry.Finger@lwfinger.net>

On 07/02/2013 06:06 PM, Larry Finger wrote:
> The driver fails to check the results of DMA mapping in twp places, which results
> in the following warning:
>
> [   28.078515] ------------[ cut here ]------------
> [   28.078529] WARNING: at lib/dma-debug.c:937 check_unmap+0x47e/0x930()
> [   28.078533] bcma-pci-bridge 0000:0e:00.0: DMA-API: device driver failed to check map error[device address=0x00000000b5d60d6c] [size=1876 bytes] [mapped as
>   single]
> [   28.078536] Modules linked in: bnep bluetooth vboxpci(O) vboxnetadp(O) vboxnetflt(O) vboxdrv(O) ipv6 b43 brcmsmac rtl8192cu rtl8192c_common rtlwifi mac802
> 11 brcmutil cfg80211 snd_hda_codec_conexant rng_core snd_hda_intel kvm_amd snd_hda_codec ssb kvm mmc_core snd_pcm snd_seq snd_timer snd_seq_device snd k8temp
>   cordic joydev serio_raw hwmon sr_mod sg pcmcia pcmcia_core soundcore cdrom i2c_nforce2 i2c_core forcedeth bcma snd_page_alloc autofs4 ext4 jbd2 mbcache crc1
> 6 scsi_dh_alua scsi_dh_hp_sw scsi_dh_rdac scsi_dh_emc scsi_dh ata_generic pata_amd
> [   28.078602] CPU: 1 PID: 2570 Comm: NetworkManager Tainted: G           O 3.10.0-rc7-wl+ #42
> [   28.078605] Hardware name: Hewlett-Packard HP Pavilion dv2700 Notebook PC/30D6, BIOS F.27 11/27/2008
> [   28.078607]  0000000000000009 ffff8800bbb03ad8 ffffffff8144f898 ffff8800bbb03b18
> [   28.078612]  ffffffff8103e1eb 0000000000000002 ffff8800b719f480 ffff8800b7b9c010
> [   28.078617]  ffffffff824204c0 ffffffff81754d57 0000000000000754 ffff8800bbb03b78
> [   28.078622] Call Trace:
> [   28.078624]  <IRQ>  [<ffffffff8144f898>] dump_stack+0x19/0x1b
> [   28.078634]  [<ffffffff8103e1eb>] warn_slowpath_common+0x6b/0xa0
> [   28.078638]  [<ffffffff8103e2c1>] warn_slowpath_fmt+0x41/0x50
> [   28.078650]  [<ffffffff8122d7ae>] check_unmap+0x47e/0x930
> [   28.078655]  [<ffffffff8122de4c>] debug_dma_unmap_page+0x5c/0x70
> [   28.078679]  [<ffffffffa04a808c>] dma64_getnextrxp+0x10c/0x190 [brcmsmac]
> [   28.078691]  [<ffffffffa04a9042>] dma_rx+0x62/0x240 [brcmsmac]
> [   28.078707]  [<ffffffffa0479101>] brcms_c_dpc+0x211/0x9d0 [brcmsmac]
> [   28.078717]  [<ffffffffa046d927>] ? brcms_dpc+0x27/0xf0 [brcmsmac]
> [   28.078731]  [<ffffffffa046d947>] brcms_dpc+0x47/0xf0 [brcmsmac]
> [   28.078736]  [<ffffffff81047dcc>] tasklet_action+0x6c/0xf0
> --snip--
> [   28.078974]  [<ffffffff813891bd>] SyS_sendmsg+0xd/0x20
> [   28.078979]  [<ffffffff81455c24>] tracesys+0xdd/0xe2
> [   28.078982] ---[ end trace 6164d1a08148e9c8 ]---
> [   28.078984] Mapped at:
> [   28.078985]  [<ffffffff8122c8fd>] debug_dma_map_page+0x9d/0x150
> [   28.078989]  [<ffffffffa04a9322>] dma_rxfill+0x102/0x3d0 [brcmsmac]
> [   28.079001]  [<ffffffffa047a13d>] brcms_c_init+0x87d/0x1100 [brcmsmac]
> [   28.079010]  [<ffffffffa046d851>] brcms_init+0x21/0x30 [brcmsmac]
> [   28.079018]  [<ffffffffa04786e0>] brcms_c_up+0x150/0x430 [brcmsmac]
>
> As the patch adds a new failure mechanism to dma_rxfill(). When I changed the
> comment at the start of the routine to add that information, I also polished
> the wording.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> Cc: Brett Rudley <brudley@broadcom.com>
> Cc: Arend van Spriel <arend@broadcom.com>
> Cc: Franky (Zhenhui) Lin <frankyl@broadcom.com>
> Cc: Hante Meuleman <meuleman@broadcom.com>
> Cc: brcm80211-dev-list@broadcom.com
> ---
>
> V2 - fixed two patch errors.

Hi Larry,

Are you planning to do a V3, ie. address my review comments sent earlier 
today? I can resend them if needed.

Regards,
Arend

> ---
>   drivers/net/wireless/brcm80211/brcmsmac/dma.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
> index 1860c57..6260571 100644
> --- a/drivers/net/wireless/brcm80211/brcmsmac/dma.c
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
> @@ -1015,9 +1015,10 @@ static bool dma64_txidle(struct dma_info *di)
>
>   /*
>    * post receive buffers
> - *  return false is refill failed completely and ring is empty this will stall
> - *  the rx dma and user might want to call rxfill again asap. This unlikely
> - *  happens on memory-rich NIC, but often on memory-constrained dongle
> + *  return false is refill failed completely or dma mapping failed. The ring
> + *  is empty, which will stall the rx dma and user might want to call rxfill
> + *  again asap. This is unlikely to happen on a memory-rich NIC, but often on
> + *  memory-constrained dongle
>    */
>   bool dma_rxfill(struct dma_pub *pub)
>   {
> @@ -1078,6 +1079,8 @@ bool dma_rxfill(struct dma_pub *pub)
>
>   		pa = dma_map_single(di->dmadev, p->data, di->rxbufsize,
>   				    DMA_FROM_DEVICE);
> +		if (dma_mapping_error(di->dmadev, pa))
> +			return false;
>
>   		/* save the free packet pointer */
>   		di->rxp[rxout] = p;
> @@ -1284,7 +1287,8 @@ static void dma_txenq(struct dma_info *di, struct sk_buff *p)
>
>   	/* get physical address of buffer start */
>   	pa = dma_map_single(di->dmadev, data, len, DMA_TO_DEVICE);
> -
> +	if (dma_mapping_error(di->dmadev, pa))
> +		return;
>   	/* With a DMA segment list, Descriptor table is filled
>   	 * using the segment list instead of looping over
>   	 * buffers in multi-chain DMA. Therefore, EOF for SGLIST
>



^ permalink raw reply

* Re: [PATCH] brcmfmac: Turn off ARP offloading when configured for AP.
From: Arend van Spriel @ 2013-07-02 20:57 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, linux-wireless, Hante Meuleman, John W. Linville
In-Reply-To: <20130619165217.GC5419@kroah.com>

On 06/19/2013 06:52 PM, Greg KH wrote:
> On Wed, Jun 19, 2013 at 05:51:46PM +0200, Arend van Spriel wrote:
>> On 06/19/2013 04:19 PM, Greg KH wrote:
>>> On Wed, Jun 19, 2013 at 09:27:09AM +0200, Arend van Spriel wrote:
>>>> On 06/06/2013 10:55 AM, Arend van Spriel wrote:
>>>>> From: Hante Meuleman <meuleman@broadcom.com>
>>>>
>>>> Hi Greg,
>>>>
>>>> I noticed your review announcement for v3.9.7 and did not see the
>>>> change below. I sent it to stable because the original upstream
>>>> commit did not apply. Did I miss some step in the process?
>>>
>>> Ah, somehow I missed your patch, sorry about that, I do have it in my
>>> mbox.  What kernel tree(s) do you want it to be applied to?  I'll pick
>>> it up in my next round of releases.
>>
>> Thanks, Greg
>>
>> I backported it for the 3.9 tree. It does not apply to 3.8 so I will
>> have to create another backport if I need it there.
>
> 3.8 is long dead and not maintained by me anymore, but thanks.

Hi Greg,

Forgot to ask, but it seems like Canonical is maintaining additional 
stable kernel branches. I guess I should contact them for those kernel 
versions (if I care), right?

Regards,
Arend

> Next time you submit stable patches, please let us know somewhere in the
> patch/subject what tree it applies to, that makes our lives much easier,
> otherwise I just guess and ususally get it wrong :)
>
> thanks,
>
> greg k-h
>



^ permalink raw reply

* Re: [PATCH v2 7/9] wlcore: sdio: add wilink clock providers
From: Felipe Balbi @ 2013-07-02 21:30 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Luciano Coelho, linux-wireless, tony, nsekhar, mturquette,
	mark.rutland, grant.likely, rob.herring, devicetree-discuss,
	linux-doc, linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702153423.GH7013@arwen.pp.htv.fi>

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

Hi,

On Tue, Jul 02, 2013 at 06:34:23PM +0300, Felipe Balbi wrote:
> On Tue, Jul 02, 2013 at 05:55:46PM +0300, Luciano Coelho wrote:
> > Add refclock and tcxoclock as clock providers in WiLink.  These clocks
> > are not accesible outside the WiLink module, but they are registered
> > in the clock framework anyway.  Only the WiLink chip consumes these
> > clocks.
> > 
> > In theory, the WiLink chip could be connected to external clocks
> > instead of using these internal clocks, so make the clock consumer
> > code generic enough.  If external clocks are used, then the internal
> > clock device tree nodes are not necessary, but the external ones must
> > be specified.
> > 
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> 
> Acked-by: Felipe Balbi <balbi@ti.com>

I guess this should've been

Reviewed-by: Felipe Balbi <balbi@ti.com>

instead

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 8/9] wlcore: sdio: get clocks from device tree
From: Felipe Balbi @ 2013-07-02 21:32 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: balbi, linux-wireless, tony, nsekhar, mturquette, mark.rutland,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <1372796394.21065.89.camel@cumari.coelho.fi>

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Tue, Jul 02, 2013 at 11:19:54PM +0300, Luciano Coelho wrote:
> On Tue, 2013-07-02 at 18:35 +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Tue, Jul 02, 2013 at 05:55:47PM +0300, Luciano Coelho wrote:
> > > @@ -294,6 +316,8 @@ static int wl1271_probe(struct sdio_func *func,
> > >  	/* Use block mode for transferring over one block size of data */
> > >  	func->card->quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
> > >  
> > > +	sdio_set_drvdata(func, glue);
> > 
> > is this move really necessary ?
> 
> It is, because I use the glue in wlcore_get_pdata_from_of(), so I need
> to call sdio_set_drvdata() earlier.  I reckoned it wouldn't hurt to move
> this, so I wouldn't have to pass the glue in the
> wlcore_get_pdata_from_of() call.

that's alright, it does look like it deserves a mention in changelog
though. Other than that:

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 8/9] wlcore: sdio: get clocks from device tree
From: Luciano Coelho @ 2013-07-02 21:38 UTC (permalink / raw)
  To: balbi
  Cc: linux-wireless, tony, nsekhar, mturquette, mark.rutland,
	grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <20130702213208.GC10388@arwen.pp.htv.fi>

On Wed, 2013-07-03 at 00:32 +0300, Felipe Balbi wrote:
> On Tue, Jul 02, 2013 at 11:19:54PM +0300, Luciano Coelho wrote:
> > On Tue, 2013-07-02 at 18:35 +0300, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Tue, Jul 02, 2013 at 05:55:47PM +0300, Luciano Coelho wrote:
> > > > @@ -294,6 +316,8 @@ static int wl1271_probe(struct sdio_func *func,
> > > >  	/* Use block mode for transferring over one block size of data */
> > > >  	func->card->quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
> > > >  
> > > > +	sdio_set_drvdata(func, glue);
> > > 
> > > is this move really necessary ?
> > 
> > It is, because I use the glue in wlcore_get_pdata_from_of(), so I need
> > to call sdio_set_drvdata() earlier.  I reckoned it wouldn't hurt to move
> > this, so I wouldn't have to pass the glue in the
> > wlcore_get_pdata_from_of() call.
> 
> that's alright, it does look like it deserves a mention in changelog
> though. Other than that:

Uh, ok.  This was so tiny (and I thought so obvious) a change that I
didn't mention it.  But if you asked about it, it was not obvious
enough. ;) I'll add a small comment about it in the commit message.


> Reviewed-by: Felipe Balbi <balbi@ti.com>

Thanks!

--
Luca.


^ permalink raw reply

* [PATCH] ath10k:  Fix crash when using v1 hardware.
From: greearb @ 2013-07-02 22:42 UTC (permalink / raw)
  To: ath9k-devel; +Cc: linux-wireless, Ben Greear

From: Ben Greear <greearb@candelatech.com>

I put a v1 NIC from an TP-LINK AC 1750 AP in
a 64-bit PC, and the OS crashes on bootup.  I'm not
sure how broken my hardware is (possibly completely non
functional), but at least with this patch it will no longer
crash the OS.  Not sure it ever got far enough to try,
but I also do not have firmware for the NIC.

With this patch I get this info on module load:

ath10k_pci 0000:05:00.0: BAR 0: assigned [mem 0xf4400000-0xf45fffff 64bit]
ath10k_pci 0000:05:00.0: BAR 0: error updating (0xf4400004 != 0xffffffff)
ath10k_pci 0000:05:00.0: BAR 0: error updating (high 0x000000 != 0xffffffff)
ath10k_pci 0000:05:00.0: Refused to change power state, currently in D3
ath10k: MSI-X interrupt handling (8 intrs)
ath10k: Unable to wakeup target
ath10k: target takes too long to wake up (awake count 1)
ath10k: src_ring ffff88020c0d0a00:  write_index is out of bounds: 4294967295  nentries_mask: 15.
ath10k: dest_ring ffff88020db2c000:  write_index is out of bounds: 4294967295  nentries_mask: 511.
ath10k: dest_ring ffff880210d56400:  write_index is out of bounds: 4294967295  nentries_mask: 31.
ath10k: src_ring ffff880210d57600:  write_index is out of bounds: 4294967295  nentries_mask: 31.
ath10k: src_ring ffff88020fe70000:  write_index is out of bounds: 4294967295  nentries_mask: 2047.
ath10k: src_ring ffff880212989b40:  write_index is out of bounds: 4294967295  nentries_mask: 1.
ath10k: dest_ring ffff880212989960:  write_index is out of bounds: 4294967295  nentries_mask: 1.
ath10k: Failed to get pcie state addr: -5
ath10k: early firmware event indicated
------------[ cut here ]------------
WARNING: at /home/greearb/git/linux.wireless-testing/drivers/net/wireless/ath/ath10k/ce.c:771 ath10k_ce_per_engine_service+0x53/0x1b4 [ath10k_pci]()
....
(it hits the warning case about 5-6 times and then seems to quiesce OK).

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/ath/ath10k/ce.c |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 61a8ac7..56669f8 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -756,13 +756,23 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ce_state *ce_state = ar_pci->ce_id_to_state[ce_id];
-	u32 ctrl_addr = ce_state->ctrl_addr;
+	u32 ctrl_addr;
 	void *transfer_context;
 	u32 buf;
 	unsigned int nbytes;
 	unsigned int id;
 	unsigned int flags;
 
+	/* On v1 hardware at least, setup can fail, causing ce_id_state to
+	 * be cleaned up, but this method is still called a few times.  Check
+	 * for NULL here so we don't crash.  Probably a better fix is to stop
+	 * the ath10k_pci_ce_tasklet sooner.
+	 */
+	if (WARN_ONCE(!ce_state, "ce_id_to_state[%i] is NULL\n", ce_id))
+		return;
+
+	ctrl_addr = ce_state->ctrl_addr;
+
 	ath10k_pci_wake(ar);
 	spin_lock_bh(&ar_pci->ce_lock);
 
@@ -954,6 +964,16 @@ static int ath10k_ce_init_src_ring(struct ath10k *ar,
 
 	src_ring->write_index =
 		ath10k_ce_src_ring_write_index_get(ar, ctrl_addr);
+	/* Make sure the value above is sane.  Can get 0xFFFFFFFF
+	 * on a v1 board.
+	 */
+	if (src_ring->write_index > src_ring->nentries_mask) {
+		ath10k_err("src_ring %p:  write_index is out of bounds: %u  nentries_mask: %u.\n",
+			   src_ring, src_ring->write_index,
+			   src_ring->nentries_mask);
+		src_ring->write_index = 0;
+	}
+
 	ath10k_pci_sleep(ar);
 
 	src_ring->per_transfer_context = (void **)ptr;
@@ -1037,6 +1057,16 @@ static int ath10k_ce_init_dest_ring(struct ath10k *ar,
 	dest_ring->sw_index = ath10k_ce_dest_ring_read_index_get(ar, ctrl_addr);
 	dest_ring->write_index =
 		ath10k_ce_dest_ring_write_index_get(ar, ctrl_addr);
+	/* Make sure the value above is sane.  Can get 0xFFFFFFFF
+	 * on a v1 board.
+	 */
+	if (dest_ring->write_index > dest_ring->nentries_mask) {
+		ath10k_err("dest_ring %p:  write_index is out of bounds: %u  nentries_mask: %u.\n",
+			   dest_ring, dest_ring->write_index,
+			   dest_ring->nentries_mask);
+		dest_ring->write_index = 0;
+	}
+
 	ath10k_pci_sleep(ar);
 
 	dest_ring->per_transfer_context = (void **)ptr;
-- 
1.7.3.4


^ permalink raw reply related

* Re: [PATCH] wireless: Convert mwifiex/pcie to dev_pm_ops from legacy pm ops
From: Shuah Khan @ 2013-07-02 22:59 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Hauke Mehrtens, bzhao@marvell.com, linville@tuxdriver.com,
	rjw@sisk.pl, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	shuahkhan@gmail.com, Shuah Khan
In-Reply-To: <51D32F7F.4090606@metafoo.de>

On 07/02/2013 01:49 PM, Lars-Peter Clausen wrote:
> On 07/02/2013 09:24 PM, Shuah Khan wrote:
> [...]
>>>> +#ifdef CONFIG_PM
>>>> +/* Power Management Hooks */
>>>> +static const struct dev_pm_ops mwifiex_pcie_pm_ops = {
>>>> +	.suspend = mwifiex_pcie_suspend,
>>>> +	.resume = mwifiex_pcie_resume,
>>>> +};
>>>> +#endif
>>>> +
>>>
>>> Is it intended that you do not use SIMPLE_DEV_PM_OPS() like most of the
>>> other wifi drivers?
>>
>> So the reason for this is SIMPLE_DEV_PM_OPS() is defined only when
>> CONFIG_PM_SLEEP is enabled. This driver doesn't defines suspend and
>> resume when CONFIG_PM is enabled and I didn't want to change that. Hope
>> that helps understand why I didn't use SIMPLE_DEV_PM_OPS()
>
> Change the ifdefs to CONFIG_PM_SLEEP and use SIMPLE_DEV_PM_OPS. The driver does
> not support runtime PM.
>
> - Lars
>
>


Thanks. I will update the patch to use SIMPLE_DEV_PM_OPS.

-- Shuah

Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research 
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658

^ permalink raw reply

* Re: [PATCH] ath10k:  Fix crash when using v1 hardware.
From: Ben Greear @ 2013-07-02 23:08 UTC (permalink / raw)
  To: ath9k-devel; +Cc: linux-wireless
In-Reply-To: <1372804925-1701-1-git-send-email-greearb@candelatech.com>

Actually, I have no idea what type of hardware this is.  It was
suggested earlier to me that this AP had a v1 hardware in it, but
lspci shows this fairly unpromising thing, and the ath10k driver
appears to call 003c the V2 hardware....

05:00.0 Network controller: Atheros Communications Inc. Device 003c (rev ff) (prog-if ff)
	!!! Unknown header type 7f
	Kernel modules: ath10k_pci


On 07/02/2013 03:42 PM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> I put a v1 NIC from an TP-LINK AC 1750 AP in
> a 64-bit PC, and the OS crashes on bootup.  I'm not
> sure how broken my hardware is (possibly completely non
> functional), but at least with this patch it will no longer
> crash the OS.  Not sure it ever got far enough to try,
> but I also do not have firmware for the NIC.
>
> With this patch I get this info on module load:
>
> ath10k_pci 0000:05:00.0: BAR 0: assigned [mem 0xf4400000-0xf45fffff 64bit]
> ath10k_pci 0000:05:00.0: BAR 0: error updating (0xf4400004 != 0xffffffff)
> ath10k_pci 0000:05:00.0: BAR 0: error updating (high 0x000000 != 0xffffffff)
> ath10k_pci 0000:05:00.0: Refused to change power state, currently in D3
> ath10k: MSI-X interrupt handling (8 intrs)
> ath10k: Unable to wakeup target
> ath10k: target takes too long to wake up (awake count 1)
> ath10k: src_ring ffff88020c0d0a00:  write_index is out of bounds: 4294967295  nentries_mask: 15.
> ath10k: dest_ring ffff88020db2c000:  write_index is out of bounds: 4294967295  nentries_mask: 511.
> ath10k: dest_ring ffff880210d56400:  write_index is out of bounds: 4294967295  nentries_mask: 31.
> ath10k: src_ring ffff880210d57600:  write_index is out of bounds: 4294967295  nentries_mask: 31.
> ath10k: src_ring ffff88020fe70000:  write_index is out of bounds: 4294967295  nentries_mask: 2047.
> ath10k: src_ring ffff880212989b40:  write_index is out of bounds: 4294967295  nentries_mask: 1.
> ath10k: dest_ring ffff880212989960:  write_index is out of bounds: 4294967295  nentries_mask: 1.
> ath10k: Failed to get pcie state addr: -5
> ath10k: early firmware event indicated
> ------------[ cut here ]------------
> WARNING: at /home/greearb/git/linux.wireless-testing/drivers/net/wireless/ath/ath10k/ce.c:771 ath10k_ce_per_engine_service+0x53/0x1b4 [ath10k_pci]()
> ....
> (it hits the warning case about 5-6 times and then seems to quiesce OK).
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>   drivers/net/wireless/ath/ath10k/ce.c |   32 +++++++++++++++++++++++++++++++-
>   1 files changed, 31 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
> index 61a8ac7..56669f8 100644
> --- a/drivers/net/wireless/ath/ath10k/ce.c
> +++ b/drivers/net/wireless/ath/ath10k/ce.c
> @@ -756,13 +756,23 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
>   {
>   	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
>   	struct ce_state *ce_state = ar_pci->ce_id_to_state[ce_id];
> -	u32 ctrl_addr = ce_state->ctrl_addr;
> +	u32 ctrl_addr;
>   	void *transfer_context;
>   	u32 buf;
>   	unsigned int nbytes;
>   	unsigned int id;
>   	unsigned int flags;
>
> +	/* On v1 hardware at least, setup can fail, causing ce_id_state to
> +	 * be cleaned up, but this method is still called a few times.  Check
> +	 * for NULL here so we don't crash.  Probably a better fix is to stop
> +	 * the ath10k_pci_ce_tasklet sooner.
> +	 */
> +	if (WARN_ONCE(!ce_state, "ce_id_to_state[%i] is NULL\n", ce_id))
> +		return;
> +
> +	ctrl_addr = ce_state->ctrl_addr;
> +
>   	ath10k_pci_wake(ar);
>   	spin_lock_bh(&ar_pci->ce_lock);
>
> @@ -954,6 +964,16 @@ static int ath10k_ce_init_src_ring(struct ath10k *ar,
>
>   	src_ring->write_index =
>   		ath10k_ce_src_ring_write_index_get(ar, ctrl_addr);
> +	/* Make sure the value above is sane.  Can get 0xFFFFFFFF
> +	 * on a v1 board.
> +	 */
> +	if (src_ring->write_index > src_ring->nentries_mask) {
> +		ath10k_err("src_ring %p:  write_index is out of bounds: %u  nentries_mask: %u.\n",
> +			   src_ring, src_ring->write_index,
> +			   src_ring->nentries_mask);
> +		src_ring->write_index = 0;
> +	}
> +
>   	ath10k_pci_sleep(ar);
>
>   	src_ring->per_transfer_context = (void **)ptr;
> @@ -1037,6 +1057,16 @@ static int ath10k_ce_init_dest_ring(struct ath10k *ar,
>   	dest_ring->sw_index = ath10k_ce_dest_ring_read_index_get(ar, ctrl_addr);
>   	dest_ring->write_index =
>   		ath10k_ce_dest_ring_write_index_get(ar, ctrl_addr);
> +	/* Make sure the value above is sane.  Can get 0xFFFFFFFF
> +	 * on a v1 board.
> +	 */
> +	if (dest_ring->write_index > dest_ring->nentries_mask) {
> +		ath10k_err("dest_ring %p:  write_index is out of bounds: %u  nentries_mask: %u.\n",
> +			   dest_ring, dest_ring->write_index,
> +			   dest_ring->nentries_mask);
> +		dest_ring->write_index = 0;
> +	}
> +
>   	ath10k_pci_sleep(ar);
>
>   	dest_ring->per_transfer_context = (void **)ptr;
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Updates to wireless-regdb review - vendor namespaces and VHT80
From: Luis R. Rodriguez @ 2013-07-03  0:38 UTC (permalink / raw)
  To: linux-wireless, wireless-regdb@lists.infradead.org

Folks,

we have 802.11ac and 802.11ad now and I have a few pending patches
that I'd like to submit but unfortunately the requirements that we
have discussed for accepting patches do not meet the criteria we have
set in the community [0] for all the documentation. Additionally
getting all that documentation has proven quite difficult and at this
point I do not think we can easily get these upstream without making
an exception.

I'd like to propose a middle ground to also address another issue I've
noticed. Vendors can disagree and in order to give vendors a warm
fuzzy on ability to ensure their data is interpreted and provide the
ability to offload down to wireless-regdb even more interpretations
I'd like to propose the idea of embracing vendor namespaces within
wireless-regdb / crda / the kernel. The way I'd envision this is
'/sbin/crda US OUI' is passed upon a regulatory hint and in turn CRDA
will read the namespace for the OUI passed in regulatory.bin. Then at
our wireless summit kumbaya and with the development / enhancements of
intersect.c and union.c (not yet developed) we'd work on generalizing
the data. This would also allow vendors to supply their own rules
without the high bar that we are setting which so far has proven very
difficult to met.

In the meantime, while that gets developed, I'd still like to supply
patches to enable VHT80 for a few countries without hopefully such
high bar for documentation as I cannot get this information.

Let me know if this sounds reasonable.

[0] http://marc.info/?l=linux-wireless&m=128414096127554&w=2

  Luis

^ permalink raw reply

* [PATCH] mwifiex: don't ignore SDIO interrupts during shutdown
From: Daniel Drake @ 2013-07-03  1:56 UTC (permalink / raw)
  To: bzhao, linville; +Cc: linux-wireless

When the card is being removed, mwifiex_remove_card() immediately sets
surprise_removed to 1. This flag then causes the SDIO interrupt handler
to ignore all interrupts without even acking them.

If there are any async commands ongoing, it is very likely that interrupts
will be received during this time. Since they are not acked (via the MP reg
read in mwifiex_interrupt_status), it becomes an interrupt storm.

This interrupt storm is undesirable and can cause problems for the
bluetooth driver which also operates the 8787 SDIO card.

Make the driver continue to ack interrupts during shutdown to avoid
this. This is harder than it might sound.

We must be careful not to act upon the interrupt, only ack it.
Otherwise, we end up setting int_status to something. And hw_status is
set to MWIFIEX_HW_STATUS_CLOSING for obvious reasons. We would hit the
following infinite loop in mwifiex_main_process:

	process_start:
	do {
		if ((adapter->hw_status == MWIFIEX_HW_STATUS_CLOSING) ||
		    (adapter->hw_status == MWIFIEX_HW_STATUS_NOT_READY))
			break;
		[...]
	} while (true);
	if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter))
		goto process_start;

We must also observe that ACKing interrupts involves reading a load
of data into the mp_regs buffer. The driver doesn't do much in the
way of ensuring that interrupts are disabled before freeing buffers
such as mp_regs, but we do need something here to make sure that we
don't get any interrupts after mp_regs is freed.

This whole thing feels rather fragile, but I couldn't see a clean
way to do it, the driver seems a bit disorganised here. I would
welcome a review from the designers.

Signed-off-by: Daniel Drake <dsd@laptop.org>
---
 drivers/net/wireless/mwifiex/init.c |  5 +++++
 drivers/net/wireless/mwifiex/main.h |  1 +
 drivers/net/wireless/mwifiex/sdio.c | 14 ++++++++------
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/init.c b/drivers/net/wireless/mwifiex/init.c
index caaf4bd..0e656db 100644
--- a/drivers/net/wireless/mwifiex/init.c
+++ b/drivers/net/wireless/mwifiex/init.c
@@ -643,6 +643,11 @@ mwifiex_shutdown_drv(struct mwifiex_adapter *adapter)
 		}
 	}
 
+	/* Must be done before cleanup_if (in mwifiex_free_adapter) and can't
+	 * be done in atomic context. */
+	if (adapter->if_ops.disable_int)
+		adapter->if_ops.disable_int(adapter);
+
 	spin_lock(&adapter->mwifiex_lock);
 
 	if (adapter->if_ops.data_complete) {
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index 3da73d3..5162e8c 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -601,6 +601,7 @@ struct mwifiex_if_ops {
 	int (*register_dev) (struct mwifiex_adapter *);
 	void (*unregister_dev) (struct mwifiex_adapter *);
 	int (*enable_int) (struct mwifiex_adapter *);
+	int (*disable_int) (struct mwifiex_adapter *);
 	int (*process_int_status) (struct mwifiex_adapter *);
 	int (*host_to_card) (struct mwifiex_adapter *, u8, struct sk_buff *,
 			     struct mwifiex_tx_param *);
diff --git a/drivers/net/wireless/mwifiex/sdio.c b/drivers/net/wireless/mwifiex/sdio.c
index 5ee5ed0..25cfc30 100644
--- a/drivers/net/wireless/mwifiex/sdio.c
+++ b/drivers/net/wireless/mwifiex/sdio.c
@@ -948,7 +948,7 @@ static int mwifiex_check_fw_status(struct mwifiex_adapter *adapter,
 /*
  * This function reads the interrupt status from card.
  */
-static void mwifiex_interrupt_status(struct mwifiex_adapter *adapter)
+static void mwifiex_process_interrupt(struct mwifiex_adapter *adapter)
 {
 	struct sdio_mmc_card *card = adapter->card;
 	u8 sdio_ireg;
@@ -961,6 +961,9 @@ static void mwifiex_interrupt_status(struct mwifiex_adapter *adapter)
 		return;
 	}
 
+	if (adapter->surprise_removed)
+		return;
+
 	sdio_ireg = card->mp_regs[HOST_INTSTATUS_REG];
 	if (sdio_ireg) {
 		/*
@@ -975,6 +978,8 @@ static void mwifiex_interrupt_status(struct mwifiex_adapter *adapter)
 		adapter->int_status |= sdio_ireg;
 		spin_unlock_irqrestore(&adapter->int_lock, flags);
 	}
+
+	mwifiex_main_process(adapter);
 }
 
 /*
@@ -997,14 +1002,10 @@ mwifiex_sdio_interrupt(struct sdio_func *func)
 	}
 	adapter = card->adapter;
 
-	if (adapter->surprise_removed)
-		return;
-
 	if (!adapter->pps_uapsd_mode && adapter->ps_state == PS_STATE_SLEEP)
 		adapter->ps_state = PS_STATE_AWAKE;
 
-	mwifiex_interrupt_status(adapter);
-	mwifiex_main_process(adapter);
+	mwifiex_process_interrupt(adapter);
 }
 
 /*
@@ -1957,6 +1958,7 @@ static struct mwifiex_if_ops sdio_ops = {
 	.register_dev = mwifiex_register_dev,
 	.unregister_dev = mwifiex_unregister_dev,
 	.enable_int = mwifiex_sdio_enable_host_int,
+	.disable_int = mwifiex_sdio_disable_host_int,
 	.process_int_status = mwifiex_process_int_status,
 	.host_to_card = mwifiex_sdio_host_to_card,
 	.wakeup = mwifiex_pm_wakeup_card,
-- 
1.8.1.4


^ permalink raw reply related

* Re: [PATCH] brcmfmac: Turn off ARP offloading when configured for AP.
From: Rafał Miłecki @ 2013-07-03  4:54 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Greg KH, stable, linux-wireless, Hante Meuleman, John W. Linville
In-Reply-To: <51D33EC2.1090108@broadcom.com>

2013/7/2 Arend van Spriel <arend@broadcom.com>:
> On 06/19/2013 06:52 PM, Greg KH wrote:
>>
>> On Wed, Jun 19, 2013 at 05:51:46PM +0200, Arend van Spriel wrote:
>>>
>>> On 06/19/2013 04:19 PM, Greg KH wrote:
>>>>
>>>> On Wed, Jun 19, 2013 at 09:27:09AM +0200, Arend van Spriel wrote:
>>>>>
>>>>> On 06/06/2013 10:55 AM, Arend van Spriel wrote:
>>>>>>
>>>>>> From: Hante Meuleman <meuleman@broadcom.com>
>>>>>
>>>>>
>>>>> Hi Greg,
>>>>>
>>>>> I noticed your review announcement for v3.9.7 and did not see the
>>>>> change below. I sent it to stable because the original upstream
>>>>> commit did not apply. Did I miss some step in the process?
>>>>
>>>>
>>>> Ah, somehow I missed your patch, sorry about that, I do have it in my
>>>> mbox.  What kernel tree(s) do you want it to be applied to?  I'll pick
>>>> it up in my next round of releases.
>>>
>>>
>>> Thanks, Greg
>>>
>>> I backported it for the 3.9 tree. It does not apply to 3.8 so I will
>>> have to create another backport if I need it there.
>>
>>
>> 3.8 is long dead and not maintained by me anymore, but thanks.
>
>
> Hi Greg,
>
> Forgot to ask, but it seems like Canonical is maintaining additional stable
> kernel branches. I guess I should contact them for those kernel versions (if
> I care), right?

They track Greg's stables to pick up the patches. Know from my own
experience, I didn't have to ping anyone :)

-- 
Rafał

^ permalink raw reply

* Re: [PATCH] mac80211/minstrel_ht: fix cck rate sampling
From: Johannes Berg @ 2013-07-03  8:18 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless
In-Reply-To: <1372446275-43012-1-git-send-email-nbd@openwrt.org>

On Fri, 2013-06-28 at 21:04 +0200, Felix Fietkau wrote:
> The CCK group needs special treatment to set the right flags and rate
> index. Add this missing check to prevent setting broken rates for tx
> packets.

Applied.

johannes


^ permalink raw reply

* Use of 5GHz frequencies with Wi-Fi Direct
From: Kévin THIERRY @ 2013-07-03  8:37 UTC (permalink / raw)
  To: linux-wireless; +Cc: ronan lanoe

Hi,

Is there a way to force the use of 5GHz frequencies with Wi-Fi Direct ? 
(Or define the 5GHz frequencies as preferred ?)

I tried to add the freq_list option in wpa_supplicant config file 
without success. (I think this option should be set in a network block 
but there is no network block for Wi-Fi Direct...)
"freq_list=5180 5200 5220 5240 5260 5280 5300 5320 5500 5520 5540 5560 
5580 5600 5620 5640 5660 5680 5700 5745 5765 5785 5805 5825"

Thanks,

Kevin

^ permalink raw reply

* Re: [wireless-regdb] Updates to wireless-regdb review - vendor namespaces and VHT80
From: Bjørn Mork @ 2013-07-03  8:58 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless, wireless-regdb@lists.infradead.org
In-Reply-To: <CAB=NE6V_PKG2UBSLPRhziUDcFzFi8uOJNiNzoPNaK0oU7rmZuQ@mail.gmail.com>

"Luis R. Rodriguez" <mcgrof@do-not-panic.com> writes:

> In the meantime, while that gets developed, I'd still like to supply
> patches to enable VHT80 for a few countries without hopefully such
> high bar for documentation as I cannot get this information.

I believe VHT80 (and VHT160) is allowed by current regulations in a
large number of countries, and that the current db entries are in fact
wrong by stating any upper channel width limits at all in the 5 GHz
bands.

For example, regulations in most CEPT countries are likely based on

 "ECC/DEC/(04)08 on the harmonised use of the 5 GHz frequency bands for
  the implementation of Wireless Access Systems including Radio Local
  Area Networks (WAS/RLANs)"

or a previous version of that decision.  Which is available here:
http://www.erodocdb.dk/Docs/doc98/official/pdf/ECCDEC0408.PDF

Quoting:

 "considering
  ..

  e. that the systems covered by this ECC Decision operate typically in a 20 MHz channel bandwidth, other
    values for the channel bandwidth are also feasible provided they comply with the relevant maximum mean
    e.i.r.p. and the corresponding maximum mean e.i.r.p. density limits;
 "

and the continues deciding the mentioned power and power density limits
only, without any specific channelization.  This document clearly allows
and anticipates both wider and narrower channels, and so does most
likely the national regualations implementing the decision.

The list of CEPT countries implementating this decision is here:
http://www.erodocdb.dk/doks/implement_doc_adm.aspx?docid=2033



Bjørn

^ permalink raw reply

* Re: Use of 5GHz frequencies with Wi-Fi Direct
From: Gagan Goyal @ 2013-07-03  9:30 UTC (permalink / raw)
  To: Kévin THIERRY; +Cc: linux-wireless, ronan lanoe
In-Reply-To: <51D3E2C7.7020605@kaiwen.me>

Hi Kevin,

Plz add the following entries in wpa_supplicant.conf to use freq 5180
as operating ch.

p2p_oper_reg_class=115
p2p_oper_channel=36
country=US

Check, if this solve the requirement.

Cheers

Gagan

On 7/3/13, Kévin THIERRY <kevin.thierry@kaiwen.me> wrote:
> Hi,
>
> Is there a way to force the use of 5GHz frequencies with Wi-Fi Direct ?
> (Or define the 5GHz frequencies as preferred ?)
>
> I tried to add the freq_list option in wpa_supplicant config file
> without success. (I think this option should be set in a network block
> but there is no network block for Wi-Fi Direct...)
> "freq_list=5180 5200 5220 5240 5260 5280 5300 5320 5500 5520 5540 5560
> 5580 5600 5620 5640 5660 5680 5700 5745 5765 5785 5805 5825"
>
> Thanks,
>
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: Use of 5GHz frequencies with Wi-Fi Direct
From: Gagan Goyal @ 2013-07-03  9:59 UTC (permalink / raw)
  To: Kévin THIERRY; +Cc: linux-wireless, ronan lanoe
In-Reply-To: <51D3E2C7.7020605@kaiwen.me>

Hi Kevin,

plz make sure that driver advertise the support for both the bands and
provide all the supported frequencies.

During Supplicant init, supplicant read the driver capabilities and
along with other things the supported band and frequencies are stored.

P2p global data is then initialized with same data.

Cheers

Gagan

On 7/3/13, Kévin THIERRY <kevin.thierry@kaiwen.me> wrote:
> Hi,
>
> Is there a way to force the use of 5GHz frequencies with Wi-Fi Direct ?
> (Or define the 5GHz frequencies as preferred ?)
>
> I tried to add the freq_list option in wpa_supplicant config file
> without success. (I think this option should be set in a network block
> but there is no network block for Wi-Fi Direct...)
> "freq_list=5180 5200 5220 5240 5260 5280 5300 5320 5500 5520 5540 5560
> 5580 5600 5620 5640 5660 5680 5700 5745 5765 5785 5805 5825"
>
> Thanks,
>
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH v2 0/9] wilink: add device tree support
From: Grazvydas Ignotas @ 2013-07-03 10:13 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: linux-wireless@vger.kernel.org, Tony Lindgren, nsekhar,
	mturquette, mark.rutland, Felipe Balbi, grant.likely, rob.herring,
	devicetree-discuss, linux-doc, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	pavel
In-Reply-To: <1372776948-24840-1-git-send-email-coelho@ti.com>

Hi,

On Tue, Jul 2, 2013 at 5:55 PM, Luciano Coelho <coelho@ti.com> wrote:
> Hi,
>
> This is a follow-up on a previous patch set that had a smaller
> audience.  This time, I added the lists and people who were involved
> in the review of the bindings documentation, since most of my changes
> in v2 are coming from discussions there.
>
> This patch series adds device tree support to the wlcore_sdio driver,
> which is used by WiLink6, WiLink7 and WiLink8.

Could you perhaps consider doing device tree conversion for wl1251
too? With the knowledge you have from working on this series, it would
be much easier for you to do it than for someone else, and I don't
have much hope someone will do it at all. It's WiLink series chip
after all. Without this pandora and N900 are going to lose wifi
support after the switch to dt-only kernel.

I can offer you my help testing things on pandora and I'm sure someone
here could try it on N900.


--
Gražvydas

^ permalink raw reply

* Re: [PATCH v2 4/9] wl12xx: use frequency instead of enumerations for pdata clocks
From: Tony Lindgren @ 2013-07-03 11:33 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: Nishanth Menon, linux-wireless, nsekhar, mturquette, mark.rutland,
	balbi, grant.likely, rob.herring, devicetree-discuss, linux-doc,
	linux-kernel, linux-omap, linux-arm-kernel
In-Reply-To: <1372796850.21065.93.camel@cumari.coelho.fi>

* Luciano Coelho <coelho@ti.com> [130702 13:33]:
> On Tue, 2013-07-02 at 10:02 -0500, Nishanth Menon wrote:
> > On 17:55-20130702, Luciano Coelho wrote:
> > > Instead of defining an enumeration with the FW specific values for the
> > > different clock rates, use the actual frequency instead.  Also add a
> > > boolean to specify whether the clock is XTAL or not.
> > > 
> > > Change all board files to reflect this.
> > > 
> > > Cc: Tony Lindgren <tony@atomide.com>
> > > Cc: Sekhar Nori <nsekhar@ti.com>
> > > Signed-off-by: Luciano Coelho <coelho@ti.com>
> > > ---
> > >  arch/arm/mach-davinci/board-da850-evm.c      |    3 +-
> > >  arch/arm/mach-omap2/board-4430sdp.c          |    5 ++-
> > ^^
> > >  arch/arm/mach-omap2/board-omap3evm.c         |    3 +-
> > >  arch/arm/mach-omap2/board-omap4panda.c       |    3 +-
> > ^^
> > Please do not add more platform data to platforms that are DT only.
> 
> Ah, I hadn't realized that board_omap4panda.c and board-4430sdp.c had
> been removed in linux-next.  I base my tree on wireless-next and it
> doesn't contain these changes yet.  I would only have noticed this when
> I rebased my tree once the merge window is closed. ;)
> 
> Thanks for pointing out, I'll make sure these changes will not be there
> when I send the pull request.

Please separate out the minimal pdata and arch/arm/mach-omap2 changes int
a immutable branch on v3.11-rc1 that I can also pull in. For v3.12, we're
going to be making more boards device tree only, so these changes may
otherwise cause pointless merge conflicts.

Regards,

Tony

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox