Linux wireless drivers development
 help / color / mirror / Atom feed
* pull-request: iwlwifi-next 2019-02-20
From: Luca Coelho @ 2019-02-20 18:56 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, linuxwifi, david.e.box, joe.konno

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

Hi Kalle,

This is the fifth and last batch of patches intended for v5.1.  This
includes the last two patchsets I sent.  Usual development work.  More
details about the contents in the tag description.

As agreed, I'm sending this before kbuildbot finished its tests, but we
can fix any problems that may arise.

Please let me know if there are any issues.

Cheers,
Luca.


The following changes since commit 6c88e0ce5ebcbac8ac930c0149c217c6aa7ab0a3:

  Merge ath-next from git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git (2019-02-20 20:01:27 +0200)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-next.git tags/iwlwifi-next-for-kalle-2019-02-20

for you to fetch changes up to 085486de833e383705bce61dbde8d31077f5b8ca:

  iwlwifi: add new cards for 22000 and killer series and change the market name (2019-02-20 20:48:01 +0200)

----------------------------------------------------------------
Fifth batch of iwlwifi patches intended for v5.1

* Some small fixes and continued work on the new debugging
  infrastructure;
* Greg's debugfs clean-ups;
* Some janitorial patches from the community;
* Fix to one false-positive compiler warning;
* VHT extended NSS support;
* New PCI IDs for 9260 and 22000 series;
* Other general bugfixes and cleanups;

----------------------------------------------------------------
Andrei Otcheretianski (1):
      iwlwifi: mvm: Set TX_CMD_FLG_PROT_REQUIRE correctly

Arnd Bergmann (1):
      iwlwifi: fix false-positive maybe-uninitialized warning

Greg Kroah-Hartman (3):
      iwlwifi: dvm: no need to check return value of debugfs_create functions
      iwlwifi: fw: no need to check return value of debugfs_create functions
      iwlwifi: iwl-drv: no need to check return value of debugfs_create functions

Gustavo A. R. Silva (2):
      iwlwifi: eeprom-parse: use struct_size() in kzalloc()
      iwlwifi: nvm-parse: use struct_size() in kzalloc()

Ihab Zhaika (2):
      iwlwifi: add new card for 9260 series
      iwlwifi: add new cards for 22000 and killer series and change the market name

Ilan Peer (2):
      iwlwifi: mvm: Don't request HW restart if already requested
      iwlwifi: mvm: Allow retries for probe responses

Johannes Berg (4):
      iwlwifi: mvm: fix error path in iwl_mvm_mac_setup_register()
      iwlwifi: nvm-parse: advertise IEEE80211_VHT_EXT_NSS_BW_CAPABLE in VHT
      iwlwifi: mvm: remove IWL_MVM_INIT_STATUS_REG_HW_INIT_COMPLETE
      iwlwifi: mvm: implement VHT extended NSS support in rs.c

Liad Kaufman (1):
      iwlwifi: mvm: add read debugfs for he_sniffer_params

Sara Sharon (3):
      iwlwifi: mvm: reject new beacons when in inject mode
      iwlwifi: mvm: support non-transmitting AP
      iwlwifi: mvm: add some debug data to TX path

Shahar S Matityahu (8):
      iwlwifi: remove redundant condition from prior alive dump flow
      iwlwifi: dbg_ini: fix infinite time ignore consecutive dumps
      iwlwifi: dbg_ini: add print to iwl_dump_ini_mem in case of invalid range
      iwlwifi: dbg_ini: make memory dump get_size handler include headers
      iwlwifi: dbg_ini: make fill_range handler accept generic range pointer
      iwlwifi: dbg_ini: add region to fill_header handler
      iwlwifi: dbg_ini: implement Tx fifos dump
      iwlwifi: dbg_ini: implement Rx fifos dump

Triebitz (1):
      iwlwifi: pcie: allocate rb_stts's for all queues in one place

 drivers/net/wireless/intel/iwlwifi/cfg/22000.c        |   8 +-
 drivers/net/wireless/intel/iwlwifi/dvm/agn.h          |   9 +-
 drivers/net/wireless/intel/iwlwifi/dvm/debugfs.c      |  44 +--------
 drivers/net/wireless/intel/iwlwifi/dvm/main.c         |   5 +-
 drivers/net/wireless/intel/iwlwifi/fw/api/dbg-tlv.h   |  13 +--
 drivers/net/wireless/intel/iwlwifi/fw/dbg.c           | 508 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------
 drivers/net/wireless/intel/iwlwifi/fw/debugfs.c       |  11 +--
 drivers/net/wireless/intel/iwlwifi/fw/debugfs.h       |   9 +-
 drivers/net/wireless/intel/iwlwifi/fw/error-dump.h    |  32 +++++-
 drivers/net/wireless/intel/iwlwifi/fw/runtime.h       |   1 +
 drivers/net/wireless/intel/iwlwifi/iwl-config.h       |   2 +-
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c          |  22 -----
 drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c |   3 +-
 drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c    |  25 +++--
 drivers/net/wireless/intel/iwlwifi/mvm/constants.h    |   1 +
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c      |  32 +++++-
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c           |   8 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c     |   8 ++
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c     |  18 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h          |   3 +-
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c          |   8 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c           |  23 ++++-
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c           |  27 ++++--
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c         |  99 +++++++++++--------
 drivers/net/wireless/intel/iwlwifi/pcie/internal.h    |   9 +-
 drivers/net/wireless/intel/iwlwifi/pcie/rx.c          |  66 ++++++++++---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c       |   4 +-
 27 files changed, 690 insertions(+), 308 deletions(-)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Nathan Chancellor @ 2019-02-20 17:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, Kalle Valo, linux-wireless, Networking,
	Linux Kernel Mailing List, Nick Desaulniers
In-Reply-To: <CAK8P3a1d6Zksdv-Or+paawyn4N0u4EpVw9QgyDGHYvX_1s3JrQ@mail.gmail.com>

On Wed, Feb 20, 2019 at 11:51:34AM +0100, Arnd Bergmann wrote:
> On Tue, Feb 19, 2019 at 7:22 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> 
> > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > index e9822a3ec373..92b22250eb7d 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > @@ -462,7 +462,7 @@ static void iwl_mvm_debug_range_resp(struct iwl_mvm *mvm, u8 index,
> >  {
> >         s64 rtt_avg = res->ftm.rtt_avg * 100;
> >
> > -       do_div(rtt_avg, 6666);
> > +       div64_s64(rtt_avg, 6666);
> 
> This is wrong: div64_s64 does not modify its argument like do_div(), but
> it returns the result instead. You also don't want to divide by a 64-bit
> value when the second argument is a small constant.
> 
> I think the correct way should be
> 
>        s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, 6666);
> 
> If you know that the value is positive, using unsigned types
> and div_u64() would be slightly faster.
> 
>       Arnd

Thanks for the review and explanation, Arnd.

Luca, could you drop this version so I can resend it?

Nathan

^ permalink raw reply

* Re: [PATCH v2 1/3] libertas_tf: move hardware callbacks to a separate structure
From: Kalle Valo @ 2019-02-20 17:56 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Steve deRosier, linux-wireless, linux-kernel, Lubomir Rintel
In-Reply-To: <20190220105840.793525-2-lkundrak@v3.sk>

Lubomir Rintel <lkundrak@v3.sk> wrote:

> We'll need to talk to the firmware to get a hardware address before
> device is registered with ieee80211 subsystem at the end of
> lbtf_add_card(). Hooking the callbacks after that is too late.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

3 patches applied to wireless-drivers-next.git, thanks.

be9d0d3fe139 libertas_tf: move hardware callbacks to a separate structure
baa0280f08c7 libertas_tf: don't defer firmware loading until start()
5d04b22b881d libertas_tf: get the MAC address before registering the device

-- 
https://patchwork.kernel.org/patch/10821819/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH wireless-drivers-next] brcmfmac: add basic validation of shared RAM address
From: Kalle Valo @ 2019-02-20 17:55 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Arend van Spriel, linux-wireless, brcm80211-dev-list.pdl,
	brcm80211-dev-list, Rafał Miłecki
In-Reply-To: <20190220103047.8960-1-zajec5@gmail.com>

Rafał Miłecki wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> While experimenting with firmware loading I ended up in a state of
> firmware reporting shared RAM address 0x04000001. It was causing:
> [   94.448015] Unable to handle kernel paging request at virtual address cd680001
> due to reading out of the mapped memory.
> 
> This patch adds some basic validation to avoid kernel crashes due to the
> unexpected firmware behavior.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>

Patch applied to wireless-drivers-next.git, thanks.

e0a8ef4d7b43 brcmfmac: add basic validation of shared RAM address

-- 
https://patchwork.kernel.org/patch/10821803/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH][next] rtlwifi: rtl8192ce: fix typo, "PairwiseENcAlgorithm" -> "PairwiseEncAlgorithm"
From: Kalle Valo @ 2019-02-20 17:55 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, Larry Finger, linux-wireless,
	netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190220092727.17119-1-colin.king@canonical.com>

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> There is an uppercase 'N' that should be a lowercase 'n', fix this.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

0421dd4167ec rtlwifi: rtl8192ce: fix typo, "PairwiseENcAlgorithm" -> "PairwiseEncAlgorithm"

-- 
https://patchwork.kernel.org/patch/10821751/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH V4] brcmfmac: use bphy_err() in all wiphy-related code
From: Kalle Valo @ 2019-02-20 17:54 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Arend van Spriel, linux-wireless, brcm80211-dev-list.pdl,
	brcm80211-dev-list, Rafał Miłecki
In-Reply-To: <20190219224219.31746-1-zajec5@gmail.com>

Rafał Miłecki wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This recently added macro provides more meaningful error messages thanks
> to identifying a specific wiphy. It's especially important on systems
> with few cards supported by the same (brcmfmac) driver.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>

Patch applied to wireless-drivers-next.git, thanks.

dcb1471bc6d0 brcmfmac: use bphy_err() in all wiphy-related code

-- 
https://patchwork.kernel.org/patch/10820843/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Lorenzo Bianconi @ 2019-02-20 16:36 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Felix Fietkau, Stefan Wahren, Alan Stern, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <20190220163229.GB14165@redhat.com>

> On Wed, Feb 20, 2019 at 05:22:18PM +0100, Lorenzo Bianconi wrote:
> > > On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > > > >> >> The way I see it, we have two choices.
> > > > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > > > >> > 
> > > > > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > > > > >> > if this is not a bug on other platforms as well.
> > > > > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > > > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > > > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > > > > >> any further improvements later on.
> > > > > > > 
> > > > > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > > > > SG should be red flag.
> > > > > > I think we're simply dealing with multiple issues here, only some of
> > > > > > which are fixed by Lorenzo's patches.
> > > > > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > > > should be up to the controller driver to deal with that.
> > > > > 
> > > > > Agree.
> > > > > 
> > > > > > dwc2 tries to do that, but that has limitations which I already pointed
> > > > > > out and which are properly dealt with by Lorenzo's patches.
> > > > > 
> > > > > I planed to just accept current solution, but I started to work on patch
> > > > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > > > > related code is now tangled.
> > > > > 
> > > > > Would be ok to send this patch with proper changelog as fix for RPI
> > > > > against wireless-drivers and cc:stable (assuming it works and really
> > > > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > > > 
> > > > Hi Stanislaw,
> > > > 
> > > > what is the advantage of doing so?
> > > To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> > > and have simpler code.
> > 
> > merging the series I sent we will have a pretty simple approach, just a
> > single routine that allocates the rx buffers in the control path according to
> > the operating mode.
> 
> So will you do the backport of your patches and post them to -stable ?
> Do you think greg-kh will accept those ?

I will take care of it

Regards,
Lorenzo

> 
> Stanislaw 

^ permalink raw reply

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Stanislaw Gruszka @ 2019-02-20 16:32 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Felix Fietkau, Stefan Wahren, Alan Stern, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <20190220162217.GH2626@localhost.localdomain>

On Wed, Feb 20, 2019 at 05:22:18PM +0100, Lorenzo Bianconi wrote:
> > On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > > >> >> The way I see it, we have two choices.
> > > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > > >> > 
> > > > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > > > >> > if this is not a bug on other platforms as well.
> > > > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > > > >> any further improvements later on.
> > > > > > 
> > > > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > > > SG should be red flag.
> > > > > I think we're simply dealing with multiple issues here, only some of
> > > > > which are fixed by Lorenzo's patches.
> > > > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > > should be up to the controller driver to deal with that.
> > > > 
> > > > Agree.
> > > > 
> > > > > dwc2 tries to do that, but that has limitations which I already pointed
> > > > > out and which are properly dealt with by Lorenzo's patches.
> > > > 
> > > > I planed to just accept current solution, but I started to work on patch
> > > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > > > related code is now tangled.
> > > > 
> > > > Would be ok to send this patch with proper changelog as fix for RPI
> > > > against wireless-drivers and cc:stable (assuming it works and really
> > > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > > 
> > > Hi Stanislaw,
> > > 
> > > what is the advantage of doing so?
> > To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> > and have simpler code.
> 
> merging the series I sent we will have a pretty simple approach, just a
> single routine that allocates the rx buffers in the control path according to
> the operating mode.

So will you do the backport of your patches and post them to -stable ?
Do you think greg-kh will accept those ?

Stanislaw 

^ permalink raw reply

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Lorenzo Bianconi @ 2019-02-20 16:22 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Felix Fietkau, Stefan Wahren, Alan Stern, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <20190220161415.GA14165@redhat.com>

> On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > >> >> The way I see it, we have two choices.
> > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > >> > 
> > > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > > >> > if this is not a bug on other platforms as well.
> > > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > > >> any further improvements later on.
> > > > > 
> > > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > > SG should be red flag.
> > > > I think we're simply dealing with multiple issues here, only some of
> > > > which are fixed by Lorenzo's patches.
> > > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > should be up to the controller driver to deal with that.
> > > 
> > > Agree.
> > > 
> > > > dwc2 tries to do that, but that has limitations which I already pointed
> > > > out and which are properly dealt with by Lorenzo's patches.
> > > 
> > > I planed to just accept current solution, but I started to work on patch
> > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > > related code is now tangled.
> > > 
> > > Would be ok to send this patch with proper changelog as fix for RPI
> > > against wireless-drivers and cc:stable (assuming it works and really
> > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > 
> > Hi Stanislaw,
> > 
> > what is the advantage of doing so?
> To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> and have simpler code.

merging the series I sent we will have a pretty simple approach, just a
single routine that allocates the rx buffers in the control path according to
the operating mode.

> 
> > You have duplicated most of the fields that are
> > already in the urb data structure and you use transfer_buffer (no SG I/O).
> URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern
> this can be optimized by packing num_sgs, len, done into fields variable.
> 
> > Moreover I have ready a series that removes 99% of the dual allocation code and
> > maintain it in the control path (instead of the datapath one).
> > I need just to rebase it ontop of your series. I will post it soon.
> So I would ask what is the point to adding bunch of code and remove it in very
> next patch?

In the first series I fixed the issue, in this one I improved the code, I have
no added any new feature

Regards,
Lorenzo

> 
> Stanislaw 

^ permalink raw reply

* [PATCH v3 3/3] mt76usb: remove usb_mcu.c
From: Stanislaw Gruszka @ 2019-02-20 16:15 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi
In-Reply-To: <1550679320-14343-1-git-send-email-sgruszka@redhat.com>

Don't need separate file just for kmalloc/kfree.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
v2: use dev_kmalloc()
v3: fix Makefile

 drivers/net/wireless/mediatek/mt76/Makefile        |  2 +-
 drivers/net/wireless/mediatek/mt76/mt76.h          |  2 --
 drivers/net/wireless/mediatek/mt76/mt76x0/usb.c    |  5 ----
 .../net/wireless/mediatek/mt76/mt76x2/usb_init.c   |  5 ----
 drivers/net/wireless/mediatek/mt76/usb.c           |  5 ++++
 drivers/net/wireless/mediatek/mt76/usb_mcu.c       | 34 ----------------------
 6 files changed, 6 insertions(+), 47 deletions(-)
 delete mode 100644 drivers/net/wireless/mediatek/mt76/usb_mcu.c

diff --git a/drivers/net/wireless/mediatek/mt76/Makefile b/drivers/net/wireless/mediatek/mt76/Makefile
index fa7a44edd02d..0d6a76f6e2ba 100644
--- a/drivers/net/wireless/mediatek/mt76/Makefile
+++ b/drivers/net/wireless/mediatek/mt76/Makefile
@@ -7,7 +7,7 @@ mt76-y := \
 	mmio.o util.o trace.o dma.o mac80211.o debugfs.o eeprom.o \
 	tx.o agg-rx.o mcu.o
 
-mt76-usb-y := usb.o usb_trace.o usb_mcu.o
+mt76-usb-y := usb.o usb_trace.o
 
 CFLAGS_trace.o := -I$(src)
 CFLAGS_usb_trace.o := -I$(src)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index c9b5eb9b0582..a689af307a71 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -771,7 +771,5 @@ struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
 				      unsigned long expires);
 
 void mt76u_mcu_complete_urb(struct urb *urb);
-int mt76u_mcu_init_rx(struct mt76_dev *dev);
-void mt76u_mcu_deinit(struct mt76_dev *dev);
 
 #endif
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
index f0c33890f1a5..91718647da02 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
@@ -79,7 +79,6 @@ static void mt76x0u_cleanup(struct mt76x02_dev *dev)
 	clear_bit(MT76_STATE_INITIALIZED, &dev->mt76.state);
 	mt76x0_chip_onoff(dev, false, false);
 	mt76u_queues_deinit(&dev->mt76);
-	mt76u_mcu_deinit(&dev->mt76);
 }
 
 static void mt76x0u_mac_stop(struct mt76x02_dev *dev)
@@ -193,10 +192,6 @@ static int mt76x0u_register_device(struct mt76x02_dev *dev)
 	if (err < 0)
 		goto out_err;
 
-	err = mt76u_mcu_init_rx(&dev->mt76);
-	if (err < 0)
-		goto out_err;
-
 	err = mt76x0u_init_hardware(dev);
 	if (err < 0)
 		goto out_err;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
index 090aaf71b3ef..1da90e58d942 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
@@ -214,10 +214,6 @@ int mt76x2u_register_device(struct mt76x02_dev *dev)
 	if (err < 0)
 		goto fail;
 
-	err = mt76u_mcu_init_rx(&dev->mt76);
-	if (err < 0)
-		goto fail;
-
 	err = mt76x2u_init_hardware(dev);
 	if (err < 0)
 		goto fail;
@@ -259,5 +255,4 @@ void mt76x2u_cleanup(struct mt76x02_dev *dev)
 	mt76x02_mcu_set_radio_state(dev, false);
 	mt76x2u_stop_hw(dev);
 	mt76u_queues_deinit(&dev->mt76);
-	mt76u_mcu_deinit(&dev->mt76);
 }
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 5c3b7f735aae..792f5012f1b1 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -577,9 +577,14 @@ EXPORT_SYMBOL_GPL(mt76u_submit_rx_buffers);
 
 static int mt76u_alloc_rx(struct mt76_dev *dev)
 {
+	struct mt76_usb *usb = &dev->usb;
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
 	int i, err;
 
+	usb->mcu.data = devm_kmalloc(dev->dev, MCU_RESP_URB_SIZE, GFP_KERNEL);
+	if (!usb->mcu.data)
+		return -ENOMEM;
+
 	spin_lock_init(&q->rx_page_lock);
 	spin_lock_init(&q->lock);
 	q->entry = devm_kcalloc(dev->dev,
diff --git a/drivers/net/wireless/mediatek/mt76/usb_mcu.c b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
deleted file mode 100644
index 747231edc57d..000000000000
--- a/drivers/net/wireless/mediatek/mt76/usb_mcu.c
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Copyright (C) 2018 Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
- *
- * Permission to use, copy, modify, and/or distribute this software for any
- * purpose with or without fee is hereby granted, provided that the above
- * copyright notice and this permission notice appear in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
- * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
- * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
- * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
- * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
- * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- */
-
-#include "mt76.h"
-
-int mt76u_mcu_init_rx(struct mt76_dev *dev)
-{
-	struct mt76_usb *usb = &dev->usb;
-
-	usb->mcu.data = kmalloc(MCU_RESP_URB_SIZE, GFP_KERNEL);
-	return usb->mcu.data ? 0 : -ENOMEM;
-}
-EXPORT_SYMBOL_GPL(mt76u_mcu_init_rx);
-
-void mt76u_mcu_deinit(struct mt76_dev *dev)
-{
-	struct mt76_usb *usb = &dev->usb;
-
-	kfree(usb->mcu.data);
-}
-EXPORT_SYMBOL_GPL(mt76u_mcu_deinit);
-- 
2.7.5


^ permalink raw reply related

* [PATCH v3 2/3] mt76usb: use synchronous msg for mcu command responses
From: Stanislaw Gruszka @ 2019-02-20 16:15 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi
In-Reply-To: <1550679320-14343-1-git-send-email-sgruszka@redhat.com>

Use usb_bulk_msg for reading MCU command responses. This simplify code
a lot.

Together with 97a3005759c ("mt76usb: allow mt76u_bulk_msg be used
for reads") it also fix possible problems with rx data buffers
not being aligned and contained within single page. After doing
page_frag_alloc(1024) consecutive page_frag_alloc(PAGE_SIZE) will
alloc PAGE_SIZE buffer at PAGE_SIZE - 1024 offset.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76.h          |  3 +-
 drivers/net/wireless/mediatek/mt76/mt76x0/usb.c    | 11 --------
 .../net/wireless/mediatek/mt76/mt76x02_usb_mcu.c   | 32 +++++++---------------
 drivers/net/wireless/mediatek/mt76/mt76x2/usb.c    | 11 --------
 drivers/net/wireless/mediatek/mt76/usb.c           |  1 -
 drivers/net/wireless/mediatek/mt76/usb_mcu.c       | 31 +++------------------
 6 files changed, 15 insertions(+), 74 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 6092646014c4..c9b5eb9b0582 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -384,8 +384,7 @@ struct mt76_usb {
 
 	struct mt76u_mcu {
 		struct mutex mutex;
-		struct completion cmpl;
-		struct mt76u_buf res;
+		u8 *data;
 		u32 msg_seq;
 
 		/* multiple reads */
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
index da9d05f6074d..f0c33890f1a5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
@@ -311,13 +311,11 @@ static int __maybe_unused mt76x0_suspend(struct usb_interface *usb_intf,
 					 pm_message_t state)
 {
 	struct mt76x02_dev *dev = usb_get_intfdata(usb_intf);
-	struct mt76_usb *usb = &dev->mt76.usb;
 
 	mt76u_stop_queues(&dev->mt76);
 	mt76x0u_mac_stop(dev);
 	clear_bit(MT76_STATE_MCU_RUNNING, &dev->mt76.state);
 	mt76x0_chip_onoff(dev, false, false);
-	usb_kill_urb(usb->mcu.res.urb);
 
 	return 0;
 }
@@ -328,15 +326,6 @@ static int __maybe_unused mt76x0_resume(struct usb_interface *usb_intf)
 	struct mt76_usb *usb = &dev->mt76.usb;
 	int ret;
 
-	reinit_completion(&usb->mcu.cmpl);
-	ret = mt76u_submit_buf(&dev->mt76, USB_DIR_IN,
-			       MT_EP_IN_CMD_RESP,
-			       &usb->mcu.res, GFP_KERNEL,
-			       mt76u_mcu_complete_urb,
-			       &usb->mcu.cmpl);
-	if (ret < 0)
-		goto err;
-
 	ret = mt76u_submit_rx_buffers(&dev->mt76);
 	if (ret < 0)
 		goto err;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
index f497c8e4332a..0cb8751321a1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
@@ -61,33 +61,21 @@ mt76x02u_multiple_mcu_reads(struct mt76_dev *dev, u8 *data, int len)
 static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
 {
 	struct mt76_usb *usb = &dev->usb;
-	struct mt76u_buf *buf = &usb->mcu.res;
-	struct urb *urb = buf->urb;
-	u8 *data = buf->buf;
-	int i, ret;
+	u8 *data = usb->mcu.data;
+	int i, len, ret;
 	u32 rxfce;
 
 	for (i = 0; i < 5; i++) {
-		if (!wait_for_completion_timeout(&usb->mcu.cmpl,
-						 msecs_to_jiffies(300)))
+		ret = mt76u_bulk_msg(dev, data, MCU_RESP_URB_SIZE, &len, 300);
+		if (ret == -ETIMEDOUT)
 			continue;
-
-		if (urb->status)
-			return -EIO;
+		if (ret)
+			goto out;
 
 		if (usb->mcu.rp)
-			mt76x02u_multiple_mcu_reads(dev, data + 4,
-						    urb->actual_length - 8);
+			mt76x02u_multiple_mcu_reads(dev, data + 4, len - 8);
 
 		rxfce = get_unaligned_le32(data);
-		ret = mt76u_submit_buf(dev, USB_DIR_IN,
-				       MT_EP_IN_CMD_RESP,
-				       buf, GFP_KERNEL,
-				       mt76u_mcu_complete_urb,
-				       &usb->mcu.cmpl);
-		if (ret)
-			return ret;
-
 		if (seq == FIELD_GET(MT_RX_FCE_INFO_CMD_SEQ, rxfce) &&
 		    FIELD_GET(MT_RX_FCE_INFO_EVT_TYPE, rxfce) == EVT_CMD_DONE)
 			return 0;
@@ -96,9 +84,9 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
 			FIELD_GET(MT_RX_FCE_INFO_EVT_TYPE, rxfce),
 			seq, FIELD_GET(MT_RX_FCE_INFO_CMD_SEQ, rxfce));
 	}
-
-	dev_err(dev->dev, "error: %s timed out\n", __func__);
-	return -ETIMEDOUT;
+out:
+	dev_err(dev->dev, "error: %s failed with %d\n", __func__, ret);
+	return ret;
 }
 
 static int
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
index f81a85e96922..ddb6b2c48e01 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
@@ -100,11 +100,9 @@ static int __maybe_unused mt76x2u_suspend(struct usb_interface *intf,
 					  pm_message_t state)
 {
 	struct mt76x02_dev *dev = usb_get_intfdata(intf);
-	struct mt76_usb *usb = &dev->mt76.usb;
 
 	mt76u_stop_queues(&dev->mt76);
 	mt76x2u_stop_hw(dev);
-	usb_kill_urb(usb->mcu.res.urb);
 
 	return 0;
 }
@@ -115,15 +113,6 @@ static int __maybe_unused mt76x2u_resume(struct usb_interface *intf)
 	struct mt76_usb *usb = &dev->mt76.usb;
 	int err;
 
-	reinit_completion(&usb->mcu.cmpl);
-	err = mt76u_submit_buf(&dev->mt76, USB_DIR_IN,
-			       MT_EP_IN_CMD_RESP,
-			       &usb->mcu.res, GFP_KERNEL,
-			       mt76u_mcu_complete_urb,
-			       &usb->mcu.cmpl);
-	if (err < 0)
-		goto err;
-
 	err = mt76u_submit_rx_buffers(&dev->mt76);
 	if (err < 0)
 		goto err;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 78191968b4fa..5c3b7f735aae 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -933,7 +933,6 @@ int mt76u_init(struct mt76_dev *dev,
 	INIT_DELAYED_WORK(&usb->stat_work, mt76u_tx_status_data);
 	skb_queue_head_init(&dev->rx_skb[MT_RXQ_MAIN]);
 
-	init_completion(&usb->mcu.cmpl);
 	mutex_init(&usb->mcu.mutex);
 
 	mutex_init(&usb->usb_ctrl_mtx);
diff --git a/drivers/net/wireless/mediatek/mt76/usb_mcu.c b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
index 72c8607da4b4..747231edc57d 100644
--- a/drivers/net/wireless/mediatek/mt76/usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
@@ -16,42 +16,19 @@
 
 #include "mt76.h"
 
-void mt76u_mcu_complete_urb(struct urb *urb)
-{
-	struct completion *cmpl = urb->context;
-
-	complete(cmpl);
-}
-EXPORT_SYMBOL_GPL(mt76u_mcu_complete_urb);
-
 int mt76u_mcu_init_rx(struct mt76_dev *dev)
 {
 	struct mt76_usb *usb = &dev->usb;
-	int err;
 
-	err = mt76u_buf_alloc(dev, &usb->mcu.res, MCU_RESP_URB_SIZE,
-			      MCU_RESP_URB_SIZE, GFP_KERNEL);
-	if (err < 0)
-		return err;
-
-	err = mt76u_submit_buf(dev, USB_DIR_IN, MT_EP_IN_CMD_RESP,
-			       &usb->mcu.res, GFP_KERNEL,
-			       mt76u_mcu_complete_urb,
-			       &usb->mcu.cmpl);
-	if (err < 0)
-		mt76u_buf_free(&usb->mcu.res);
-
-	return err;
+	usb->mcu.data = kmalloc(MCU_RESP_URB_SIZE, GFP_KERNEL);
+	return usb->mcu.data ? 0 : -ENOMEM;
 }
 EXPORT_SYMBOL_GPL(mt76u_mcu_init_rx);
 
 void mt76u_mcu_deinit(struct mt76_dev *dev)
 {
-	struct mt76u_buf *buf = &dev->usb.mcu.res;
+	struct mt76_usb *usb = &dev->usb;
 
-	if (buf->urb) {
-		usb_kill_urb(buf->urb);
-		mt76u_buf_free(buf);
-	}
+	kfree(usb->mcu.data);
 }
 EXPORT_SYMBOL_GPL(mt76u_mcu_deinit);
-- 
2.7.5


^ permalink raw reply related

* [PATCH v3 1/3] mt76usb: allow mt76u_bulk_msg be used for reads
From: Stanislaw Gruszka @ 2019-02-20 16:15 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

Extend mt76u_bulk_msg() such it can be used for synchronous bulk reads.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
v2: pass NULL actual_len to usb_bulk_msg

 drivers/net/wireless/mediatek/mt76/mt76.h            | 12 ++++++++----
 drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c |  4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index f55dc621e060..6092646014c4 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -729,16 +729,20 @@ static inline u8 q2ep(u8 qid)
 }
 
 static inline int
-mt76u_bulk_msg(struct mt76_dev *dev, void *data, int len, int timeout)
+mt76u_bulk_msg(struct mt76_dev *dev, void *data, int len, int *actual_len,
+	       int timeout)
 {
 	struct usb_interface *intf = to_usb_interface(dev->dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct mt76_usb *usb = &dev->usb;
 	unsigned int pipe;
-	int sent;
 
-	pipe = usb_sndbulkpipe(udev, usb->out_ep[MT_EP_OUT_INBAND_CMD]);
-	return usb_bulk_msg(udev, pipe, data, len, &sent, timeout);
+	if (actual_len)
+		pipe = usb_rcvbulkpipe(udev, usb->in_ep[MT_EP_IN_CMD_RESP]);
+	else
+		pipe = usb_sndbulkpipe(udev, usb->out_ep[MT_EP_OUT_INBAND_CMD]);
+
+	return usb_bulk_msg(udev, pipe, data, len, actual_len, timeout);
 }
 
 int mt76u_vendor_request(struct mt76_dev *dev, u8 req,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
index e469e383cb88..f497c8e4332a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
@@ -126,7 +126,7 @@ __mt76x02u_mcu_send_msg(struct mt76_dev *dev, struct sk_buff *skb,
 	if (ret)
 		return ret;
 
-	ret = mt76u_bulk_msg(dev, skb->data, skb->len, 500);
+	ret = mt76u_bulk_msg(dev, skb->data, skb->len, NULL, 500);
 	if (ret)
 		return ret;
 
@@ -271,7 +271,7 @@ __mt76x02u_mcu_fw_send_data(struct mt76x02_dev *dev, u8 *data,
 
 	data_len = MT_CMD_HDR_LEN + len + sizeof(info);
 
-	err = mt76u_bulk_msg(&dev->mt76, data, data_len, 1000);
+	err = mt76u_bulk_msg(&dev->mt76, data, data_len, NULL, 1000);
 	if (err) {
 		dev_err(dev->mt76.dev, "firmware upload failed: %d\n", err);
 		return err;
-- 
2.7.5


^ permalink raw reply related

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Stanislaw Gruszka @ 2019-02-20 16:14 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Felix Fietkau, Stefan Wahren, Alan Stern, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <20190220132206.GF2626@localhost.localdomain>

On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > >> >> The way I see it, we have two choices.
> > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > >> > 
> > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > >> > if this is not a bug on other platforms as well.
> > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > >> any further improvements later on.
> > > > 
> > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > SG should be red flag.
> > > I think we're simply dealing with multiple issues here, only some of
> > > which are fixed by Lorenzo's patches.
> > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > since the Linux USB API supports non-aligned transfer buffers and it
> > > should be up to the controller driver to deal with that.
> > 
> > Agree.
> > 
> > > dwc2 tries to do that, but that has limitations which I already pointed
> > > out and which are properly dealt with by Lorenzo's patches.
> > 
> > I planed to just accept current solution, but I started to work on patch
> > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > related code is now tangled.
> > 
> > Would be ok to send this patch with proper changelog as fix for RPI
> > against wireless-drivers and cc:stable (assuming it works and really
> > fix things on RPI) and revert Lorenzo's patches in -next ?
> 
> Hi Stanislaw,
> 
> what is the advantage of doing so?
To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
and have simpler code.

> You have duplicated most of the fields that are
> already in the urb data structure and you use transfer_buffer (no SG I/O).
URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern
this can be optimized by packing num_sgs, len, done into fields variable.

> Moreover I have ready a series that removes 99% of the dual allocation code and
> maintain it in the control path (instead of the datapath one).
> I need just to rebase it ontop of your series. I will post it soon.
So I would ask what is the point to adding bunch of code and remove it in very
next patch?

Stanislaw 

^ permalink raw reply

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Alan Stern @ 2019-02-20 15:25 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Stefan Wahren, Lorenzo Bianconi, Felix Fietkau, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <20190220102050.GA7195@redhat.com>

On Wed, 20 Feb 2019, Stanislaw Gruszka wrote:

> On Tue, Feb 19, 2019 at 10:40:47AM -0500, Alan Stern wrote:
> > On Tue, 19 Feb 2019, Stanislaw Gruszka wrote:
> > 
> > > It would be interesting why urb->num_sgs = 0 & urb->sg cause
> > > the troubles. This is how usb_sg_init() submit urbs for sg_tablesize = 0
> > > controllers. So either are there are some requirement on urb->sg
> > > mapped via dma_map_page() (which mt76usb does not meet) not needed
> > > for urb->transfer_buffer mapped via dma_map_single() or there
> > > is something wrong in dwc2 with sg and this driver will not
> > > work with urb_sg_init() as well. I don't have hardware to investigate
> > > this and don't want to bother you with more patches.
> > 
> > urb->sg != NULL and urb->num_sgs == 0 means that the transfer buffer 
> > must fit into a single page, which is pointed to by the first element 
> > of the scatterlist.
> 
> I asked about that in other thread 
> https://lore.kernel.org/linux-wireless/2cc5674a-a3a0-d8fe-65f5-4357da9b85d3@arm.com/
> 
> the answer was it is weird but valid. However I think do dma_map_page()
> on buffer not fit in the single page is asking for troubles. I just posted
> patch that should fix this for mt76usb.
> 
> > But now that I look at the code in usb_sg_init(), it seems odd that the
> > !use_sg case doesn't increment sg during each loop iteration.  I don't
> > see how that could possibly work -- it looks like a bug!
> 
> Looks for me that this is done via for_each_sg(sg, sg, io->entries, i) loop.

Ah, of course.  Thanks for straightening me out; it's surprising what 
blind spots one's brain can develop.

Alan Stern


^ permalink raw reply

* bug report: iwlwifi: mvm: support mac80211 TXQs model
From: Colin Ian King @ 2019-02-20 14:40 UTC (permalink / raw)
  To: Sara Sharon, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, Sara Sharon, linux-wireless@vger.kernel.org,
	netdev
  Cc: Kalle Valo, David S. Miller, linux-kernel@vger.kernel.org

Hi,

Static analysis by CoverityScan has detected an uninitialized variable
error in drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c

Variable used_hw_queues in iwl_mvm_mac_ctxt_init() is no longer being
assigned an initial value, causing garbage to be found when calling
find_first_zero_bit() on used_hw_queues.

CoverityScan reports this as follows:

363        for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
   CID 1477001 (#1 of 1): Uninitialized scalar variable (UNINIT)
uninit_use_in_call: Using uninitialized value used_hw_queues when
calling find_first_zero_bit.

364                u8 queue = find_first_zero_bit(&used_hw_queues,
queue_limit);
365

This issue was caused by the following commit:

cfbc6c4c5b91c7725ef14465b98ac347d31f2334 ("iwlwifi: mvm: support
mac80211 TXQs model")

..when the used_hw_queues initialization was removed:

@@ -360,8 +300,6 @@ int iwl_mvm_mac_ctxt_init(struct iwl_mvm *mvm,
struct ieee80211_vif *vif)
                mvm->hw, IEEE80211_IFACE_ITER_RESUME_ALL,
                iwl_mvm_mac_iface_iterator, &data);

-       used_hw_queues = iwl_mvm_get_used_hw_queues(mvm, vif);
-
        /*
         * In the case we're getting here during resume, it's similar to
         * firmware restart, and with RESUME_ALL the iterator will find


I'm not 100% sure if the right thing to do is just to now initialize
used_hw_queues to zero; it's not entirely clear what the initial value
should be now.

Colin

^ permalink raw reply

* [PATCH] ath10k: Add peer delete response event
From: Rakesh Pillai @ 2019-02-20 14:30 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Dundi Raviteja, Rakesh Pillai

From: Dundi Raviteja <dundi@codeaurora.org>

Peer creation in firmware fails if last peer deletion is still
in progress.

The firmware sends a peer delete response event if it advertises
the service WMI_SERVICE_SYNC_DELETE_CMDS. This peer delete response
event is used to synchronize the peer deletion.

Add peer delete response event and wait for the event after
deleting every peer from host driver to synchronize with firmware.

Tested HW: WCN3990
Tested FW: WLAN.HL.2.0-01188-QCAHLSWMTPLZ-1

Signed-off-by: Dundi Raviteja <dundi@codeaurora.org>
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c    |  3 ++-
 drivers/net/wireless/ath/ath10k/core.h    |  3 ++-
 drivers/net/wireless/ath/ath10k/mac.c     | 41 +++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 23 ++++++++++++++++-
 drivers/net/wireless/ath/ath10k/wmi-tlv.h |  6 ++++-
 drivers/net/wireless/ath/ath10k/wmi.h     |  9 ++++++-
 6 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a1b2aea..21f60cd 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -3105,6 +3105,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 	init_completion(&ar->vdev_setup_done);
 	init_completion(&ar->thermal.wmi_sync);
 	init_completion(&ar->bss_survey_done);
+	init_completion(&ar->peer_delete_done);
 
 	INIT_DELAYED_WORK(&ar->scan.timeout, ath10k_scan_timeout_work);
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 2f43600..99951ed 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -1197,6 +1197,7 @@ struct ath10k {
 	struct ath10k_radar_found_info last_radar_info;
 	struct work_struct radar_confirmation_work;
 	struct ath10k_bus_params bus_param;
+	struct completion peer_delete_done;
 
 	/* must be last */
 	u8 drv_priv[0] __aligned(sizeof(void *));
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 795446d..a9a2448 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -704,6 +704,26 @@ ath10k_mac_get_any_chandef_iter(struct ieee80211_hw *hw,
 	*def = &conf->def;
 }
 
+static void ath10k_wait_for_peer_delete_done(struct ath10k *ar, u32 vdev_id,
+					     const u8 *addr)
+{
+	unsigned long time_left;
+	int ret;
+
+	if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+		ret = ath10k_wait_for_peer_deleted(ar, vdev_id, addr);
+		if (ret) {
+			ath10k_warn(ar, "failed wait for peer deleted");
+			return;
+		}
+
+		time_left = wait_for_completion_timeout(&ar->peer_delete_done,
+							5 * HZ);
+		if (!time_left)
+			ath10k_warn(ar, "Timeout in receiving peer delete response\n");
+	}
+}
+
 static int ath10k_peer_create(struct ath10k *ar,
 			      struct ieee80211_vif *vif,
 			      struct ieee80211_sta *sta,
@@ -748,7 +768,7 @@ static int ath10k_peer_create(struct ath10k *ar,
 		spin_unlock_bh(&ar->data_lock);
 		ath10k_warn(ar, "failed to find peer %pM on vdev %i after creation\n",
 			    addr, vdev_id);
-		ath10k_wmi_peer_delete(ar, vdev_id, addr);
+		ath10k_wait_for_peer_delete_done(ar, vdev_id, addr);
 		return -ENOENT;
 	}
 
@@ -830,6 +850,18 @@ static int ath10k_peer_delete(struct ath10k *ar, u32 vdev_id, const u8 *addr)
 	if (ret)
 		return ret;
 
+	if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+		unsigned long time_left;
+
+		time_left = wait_for_completion_timeout
+			    (&ar->peer_delete_done, 5 * HZ);
+
+		if (!time_left) {
+			ath10k_warn(ar, "Timeout in receiving peer delete response\n");
+			return -ETIMEDOUT;
+		}
+	}
+
 	ar->num_peers--;
 
 	return 0;
@@ -5426,8 +5458,11 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 
 err_peer_delete:
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP ||
-	    arvif->vdev_type == WMI_VDEV_TYPE_IBSS)
+	    arvif->vdev_type == WMI_VDEV_TYPE_IBSS) {
 		ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+		ath10k_wait_for_peer_delete_done(ar, arvif->vdev_id,
+						 vif->addr);
+	}
 
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
@@ -5492,6 +5527,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 			ath10k_warn(ar, "failed to submit AP/IBSS self-peer removal on vdev %i: %d\n",
 				    arvif->vdev_id, ret);
 
+		ath10k_wait_for_peer_delete_done(ar, arvif->vdev_id,
+						 vif->addr);
 		kfree(arvif->u.ap.noa_data);
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 5d05bff..143eb88 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -468,6 +468,24 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, struct sk_buff *skb)
 	kfree(tb);
 }
 
+static int ath10k_wmi_tlv_event_peer_delete_resp(struct ath10k *ar,
+						 struct sk_buff *skb)
+{
+	struct wmi_peer_delete_resp_ev_arg *arg;
+	struct wmi_tlv *tlv_hdr;
+
+	tlv_hdr = (struct wmi_tlv *)skb->data;
+	arg = (struct wmi_peer_delete_resp_ev_arg *)tlv_hdr->value;
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "vdev id %d", arg->vdev_id);
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "peer mac addr %pM", &arg->peer_addr);
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv peer delete response\n");
+
+	complete(&ar->peer_delete_done);
+
+	return 0;
+}
+
 /***********/
 /* TLV ops */
 /***********/
@@ -617,6 +635,9 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb)
 	case WMI_TLV_TDLS_PEER_EVENTID:
 		ath10k_wmi_event_tdls_peer(ar, skb);
 		break;
+	case WMI_TLV_PEER_DELETE_RESP_EVENTID:
+		ath10k_wmi_tlv_event_peer_delete_resp(ar, skb);
+		break;
 	case WMI_TLV_MGMT_TX_COMPLETION_EVENTID:
 		ath10k_wmi_event_mgmt_tx_compl(ar, skb);
 		break;
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.h b/drivers/net/wireless/ath/ath10k/wmi-tlv.h
index 4fba581..9e3335f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.h
@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -315,6 +315,8 @@ enum wmi_tlv_event_id {
 	WMI_TLV_PEER_TX_FAIL_CNT_THR_EVENTID,
 	WMI_TLV_PEER_ESTIMATED_LINKSPEED_EVENTID,
 	WMI_TLV_PEER_STATE_EVENTID,
+	WMI_TLV_PEER_ASSOC_CONF_EVENTID,
+	WMI_TLV_PEER_DELETE_RESP_EVENTID,
 	WMI_TLV_MGMT_RX_EVENTID = WMI_TLV_EV(WMI_TLV_GRP_MGMT),
 	WMI_TLV_HOST_SWBA_EVENTID,
 	WMI_TLV_TBTTOFFSET_UPDATE_EVENTID,
@@ -1557,6 +1559,8 @@ wmi_tlv_svc_map(const __le32 *in, unsigned long *out, size_t len)
 	       WMI_SERVICE_SAP_AUTH_OFFLOAD, len);
 	SVCMAP(WMI_TLV_SERVICE_MGMT_TX_WMI,
 	       WMI_SERVICE_MGMT_TX_WMI, len);
+	SVCMAP(WMI_TLV_SERVICE_SYNC_DELETE_CMDS,
+	       WMI_SERVICE_SYNC_DELETE_CMDS, len);
 }
 
 static inline void
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 0e27878..5d1a17e 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -211,6 +211,7 @@ enum wmi_service {
 	WMI_SERVICE_RTT_RESPONDER_ROLE,
 	WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
 	WMI_SERVICE_REPORT_AIRTIME,
+	WMI_SERVICE_SYNC_DELETE_CMDS,
 
 	/* Remember to add the new value to wmi_service_name()! */
 
@@ -502,6 +503,7 @@ static inline char *wmi_service_name(enum wmi_service service_id)
 	SVCSTR(WMI_SERVICE_RTT_RESPONDER_ROLE);
 	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
 	SVCSTR(WMI_SERVICE_REPORT_AIRTIME);
+	SVCSTR(WMI_SERVICE_SYNC_DELETE_CMDS);
 
 	case WMI_SERVICE_MAX:
 		return NULL;
@@ -6748,6 +6750,11 @@ struct wmi_tlv_mgmt_tx_bundle_compl_ev_arg {
 	const __le32 *status;
 };
 
+struct wmi_peer_delete_resp_ev_arg {
+	__le32 vdev_id;
+	struct wmi_mac_addr peer_addr;
+};
+
 struct wmi_mgmt_rx_ev_arg {
 	__le32 channel;
 	__le32 snr;
-- 
2.7.4


^ permalink raw reply related

* [RFC 3/3] mt76: usb: simplify mt76u_tx_build_sg routine
From: lorenzo @ 2019-02-20 14:05 UTC (permalink / raw)
  To: nbd; +Cc: linux-wireless, sgruszka, lorenzo.bianconi
In-Reply-To: <cover.1550671183.git.lorenzo@kernel.org>

From: Lorenzo Bianconi <lorenzo@kernel.org>

Use skb_to_sgvec instad of skb_to_sgvec_nomark in mt76u_tx_build_sg.
Moreover remove nsgs check since it is already performed in skb_to_sgvec

Suggested-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/usb.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 24bf96de9f9d..87b89d5f1d03 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -724,23 +724,12 @@ static int
 mt76u_tx_build_sg(struct mt76_dev * dev, struct sk_buff *skb,
 		  struct urb *urb)
 {
-	struct sk_buff *iter;
-	int nsgs;
-
 	if (!dev->usb.sg_en)
 		return 0;
 
-	nsgs = 1 + skb_shinfo(skb)->nr_frags;
-	skb_walk_frags(skb, iter)
-		nsgs += 1 + skb_shinfo(iter)->nr_frags;
-
-	memset(urb->sg, 0, sizeof(*urb->sg) * MT_SG_MAX_SIZE);
-
-	nsgs = min_t(int, MT_SG_MAX_SIZE, nsgs);
-	sg_init_marker(urb->sg, nsgs);
-	urb->num_sgs = nsgs;
-
-	return skb_to_sgvec_nomark(skb, urb->sg, 0, skb->len);
+	sg_init_table(urb->sg, MT_SG_MAX_SIZE);
+	urb->num_sgs = skb_to_sgvec(skb, urb->sg, 0, skb->len);
+	return urb->num_sgs;
 }
 
 static int
-- 
2.20.1


^ permalink raw reply related

* [RFC 2/3] mt76: usb: simplify rx buffer allocation
From: lorenzo @ 2019-02-20 14:05 UTC (permalink / raw)
  To: nbd; +Cc: linux-wireless, sgruszka, lorenzo.bianconi
In-Reply-To: <cover.1550671183.git.lorenzo@kernel.org>

From: Lorenzo Bianconi <lorenzo@kernel.org>

Squash mt76u_buf_alloc_sg and mt76u_buf_alloc and remove duplicated code

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  2 -
 drivers/net/wireless/mediatek/mt76/usb.c  | 78 +++++++++--------------
 2 files changed, 29 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 972f0ca7c818..51a4721df400 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -754,8 +754,6 @@ void mt76u_single_wr(struct mt76_dev *dev, const u8 req,
 		     const u16 offset, const u32 val);
 int mt76u_init(struct mt76_dev *dev, struct usb_interface *intf);
 void mt76u_deinit(struct mt76_dev *dev);
-int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
-		    int len, int data_len, gfp_t gfp);
 void mt76u_buf_free(struct mt76u_buf *buf);
 int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
 		     struct mt76u_buf *buf, gfp_t gfp,
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index c84415caaeaf..24bf96de9f9d 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -324,41 +324,42 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
 }
 
 static int
-mt76u_buf_alloc_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
-		   int nsgs, int len, int sglen, gfp_t gfp)
+mt76u_refill_rx(struct mt76_dev *dev, struct mt76_queue *q,
+		struct mt76u_buf *buf, int nsgs, gfp_t gfp)
 {
-	buf->urb = usb_alloc_urb(0, gfp);
-	if (!buf->urb)
-		return -ENOMEM;
-
-	buf->urb->sg = devm_kcalloc(dev->dev, nsgs, sizeof(*buf->urb->sg),
-				    gfp);
-	if (!buf->urb->sg)
-		return -ENOMEM;
-
-	sg_init_table(buf->urb->sg, nsgs);
-	buf->dev = dev;
-
-	return mt76u_fill_rx_sg(dev, buf, nsgs, len, sglen);
+	if (dev->usb.sg_en) {
+		return mt76u_fill_rx_sg(dev, buf, nsgs, q->buf_size,
+					SKB_WITH_OVERHEAD(q->buf_size));
+	} else {
+		buf->buf = page_frag_alloc(&q->rx_page, q->buf_size, gfp);
+		return buf->buf ? 0 : -ENOMEM;
+	}
 }
 
-int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
-		    int len, int data_len, gfp_t gfp)
+static int
+mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
+		gfp_t gfp)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
 
-	buf->urb = usb_alloc_urb(0, gfp);
+	buf->len = SKB_WITH_OVERHEAD(q->buf_size);
+	buf->dev = dev;
+
+	buf->urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!buf->urb)
 		return -ENOMEM;
 
-	buf->buf = page_frag_alloc(&q->rx_page, len, gfp);
-	if (!buf->buf)
-		return -ENOMEM;
+	if (dev->usb.sg_en) {
+		buf->urb->sg = devm_kcalloc(dev->dev, MT_SG_MAX_SIZE,
+					    sizeof(*buf->urb->sg),
+					    GFP_KERNEL);
+		if (!buf->urb->sg)
+			return -ENOMEM;
 
-	buf->len = data_len;
-	buf->dev = dev;
+		sg_init_table(buf->urb->sg, MT_SG_MAX_SIZE);
+	}
 
-	return 0;
+	return mt76u_refill_rx(dev, q, buf, MT_SG_MAX_SIZE, gfp);
 }
 
 void mt76u_buf_free(struct mt76u_buf *buf)
@@ -519,20 +520,6 @@ static void mt76u_complete_rx(struct urb *urb)
 	spin_unlock_irqrestore(&q->lock, flags);
 }
 
-static int
-mt76u_refill_rx(struct mt76_dev *dev, struct mt76_queue *q,
-		struct mt76u_buf *buf, int nsgs)
-{
-	if (dev->usb.sg_en) {
-		return mt76u_fill_rx_sg(dev, buf, nsgs, q->buf_size,
-					SKB_WITH_OVERHEAD(q->buf_size));
-	} else {
-		buf->buf = page_frag_alloc(&q->rx_page, q->buf_size,
-					   GFP_ATOMIC);
-		return buf->buf ? 0 : -ENOMEM;
-	}
-}
-
 static void mt76u_rx_tasklet(unsigned long data)
 {
 	struct mt76_dev *dev = (struct mt76_dev *)data;
@@ -549,7 +536,8 @@ static void mt76u_rx_tasklet(unsigned long data)
 
 		count = mt76u_process_rx_entry(dev, buf);
 		if (count > 0) {
-			err = mt76u_refill_rx(dev, q, buf, count);
+			err = mt76u_refill_rx(dev, q, buf, count,
+					      GFP_ATOMIC);
 			if (err < 0)
 				break;
 		}
@@ -605,16 +593,8 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
 	q->buf_size = dev->usb.sg_en ? MT_RX_BUF_SIZE : PAGE_SIZE;
 	q->ndesc = MT_NUM_RX_ENTRIES;
 	for (i = 0; i < q->ndesc; i++) {
-		if (dev->usb.sg_en)
-			err = mt76u_buf_alloc_sg(dev, &q->entry[i].ubuf,
-					MT_SG_MAX_SIZE, q->buf_size,
-					SKB_WITH_OVERHEAD(q->buf_size),
-					GFP_KERNEL);
-		else
-			err = mt76u_buf_alloc(dev, &q->entry[i].ubuf,
-					      q->buf_size,
-					      SKB_WITH_OVERHEAD(q->buf_size),
-					      GFP_KERNEL);
+		err = mt76u_buf_alloc(dev, &q->entry[i].ubuf,
+				      GFP_KERNEL);
 		if (err < 0)
 			return err;
 	}
-- 
2.20.1


^ permalink raw reply related

* [RFC 1/3] mt76: usb: introduce mt76u_fill_bulk_urb routine
From: lorenzo @ 2019-02-20 14:05 UTC (permalink / raw)
  To: nbd; +Cc: linux-wireless, sgruszka, lorenzo.bianconi
In-Reply-To: <cover.1550671183.git.lorenzo@kernel.org>

From: Lorenzo Bianconi <lorenzo@kernel.org>

Add mt76u_fill_bulk_urb routine and remove duplicate code
use to initialize tx/rx urb

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/usb.c | 45 ++++++++++++++----------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 62dfd70232fe..c84415caaeaf 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -381,9 +381,10 @@ void mt76u_buf_free(struct mt76u_buf *buf)
 }
 EXPORT_SYMBOL_GPL(mt76u_buf_free);
 
-int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
-		     struct mt76u_buf *buf, gfp_t gfp,
-		     usb_complete_t complete_fn, void *context)
+static void
+mt76u_fill_bulk_urb(struct mt76_dev *dev, int dir, int index,
+		    struct mt76u_buf *buf, usb_complete_t complete_fn,
+		    void *context)
 {
 	struct usb_interface *intf = to_usb_interface(dev->dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
@@ -397,6 +398,14 @@ int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
 
 	usb_fill_bulk_urb(buf->urb, udev, pipe, data, buf->len,
 			  complete_fn, context);
+}
+
+int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
+		     struct mt76u_buf *buf, gfp_t gfp,
+		     usb_complete_t complete_fn, void *context)
+{
+	mt76u_fill_bulk_urb(dev, dir, index, buf, complete_fn,
+			    context);
 	trace_submit_urb(dev, buf->urb);
 
 	return usb_submit_urb(buf->urb, gfp);
@@ -732,11 +741,16 @@ static void mt76u_complete_tx(struct urb *urb)
 }
 
 static int
-mt76u_tx_build_sg(struct sk_buff *skb, struct urb *urb)
+mt76u_tx_build_sg(struct mt76_dev *dev, struct sk_buff *skb,
+		  struct urb *urb)
 {
-	int nsgs = 1 + skb_shinfo(skb)->nr_frags;
 	struct sk_buff *iter;
+	int nsgs;
+
+	if (!dev->usb.sg_en)
+		return 0;
 
+	nsgs = 1 + skb_shinfo(skb)->nr_frags;
 	skb_walk_frags(skb, iter)
 		nsgs += 1 + skb_shinfo(iter)->nr_frags;
 
@@ -754,12 +768,8 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 		   struct sk_buff *skb, struct mt76_wcid *wcid,
 		   struct ieee80211_sta *sta)
 {
-	struct usb_interface *intf = to_usb_interface(dev->dev);
-	struct usb_device *udev = interface_to_usbdev(intf);
-	u8 *data = NULL, ep = q2ep(q->hw_idx);
 	struct mt76u_buf *buf;
 	u16 idx = q->tail;
-	unsigned int pipe;
 	int err;
 
 	if (q->queued == q->ndesc)
@@ -771,19 +781,16 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 		return err;
 
 	buf = &q->entry[idx].ubuf;
+	buf->buf = skb->data;
+	buf->len = skb->len;
 	buf->done = false;
 
-	if (dev->usb.sg_en) {
-		err = mt76u_tx_build_sg(skb, buf->urb);
-		if (err < 0)
-			return err;
-	} else {
-		data = skb->data;
-	}
+	err = mt76u_tx_build_sg(dev, skb, buf->urb);
+	if (err < 0)
+		return err;
 
-	pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[ep]);
-	usb_fill_bulk_urb(buf->urb, udev, pipe, data, skb->len,
-			  mt76u_complete_tx, buf);
+	mt76u_fill_bulk_urb(dev, USB_DIR_OUT, q2ep(q->hw_idx),
+			    buf, mt76u_complete_tx, buf);
 
 	q->tail = (q->tail + 1) % q->ndesc;
 	q->entry[idx].skb = skb;
-- 
2.20.1


^ permalink raw reply related

* [RFC 0/3] simplify usb layer buffer allocation
From: lorenzo @ 2019-02-20 14:05 UTC (permalink / raw)
  To: nbd; +Cc: linux-wireless, sgruszka, lorenzo.bianconi

From: Lorenzo Bianconi <lorenzo@kernel.org>

Simplify usb rx buffer allocation.
Simplify mt76u_tx_build_sg routine

Lorenzo Bianconi (3):
  mt76: usb: introduce mt76u_fill_bulk_urb routine
  mt76: usb: simplify rx buffer allocation
  mt76: usb: simplify mt76u_tx_build_sg routine

 drivers/net/wireless/mediatek/mt76/mt76.h |   2 -
 drivers/net/wireless/mediatek/mt76/usb.c  | 134 +++++++++-------------
 2 files changed, 55 insertions(+), 81 deletions(-)

-- 
2.20.1


^ permalink raw reply

* Re: [PATCH v2 3/3] mt76usb: remove usb_mcu.c
From: Lorenzo Bianconi @ 2019-02-20 13:56 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless
In-Reply-To: <1550666897-3099-3-git-send-email-sgruszka@redhat.com>

> Don't need separate file just for kmalloc/kfree.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> v2: use dev_kmalloc()
> 
>  drivers/net/wireless/mediatek/mt76/mt76.h          |  2 --
>  drivers/net/wireless/mediatek/mt76/mt76x0/usb.c    |  5 ----
>  .../net/wireless/mediatek/mt76/mt76x2/usb_init.c   |  5 ----
>  drivers/net/wireless/mediatek/mt76/usb.c           |  5 ++++
>  drivers/net/wireless/mediatek/mt76/usb_mcu.c       | 34 ----------------------
>  5 files changed, 5 insertions(+), 46 deletions(-)
>  delete mode 100644 drivers/net/wireless/mediatek/mt76/usb_mcu.c

You need to remove usb_mcu.o from the Makefile.

Regards,
Lorenzo

> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index c9b5eb9b0582..a689af307a71 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -771,7 +771,5 @@ struct sk_buff *mt76_mcu_get_response(struct mt76_dev *dev,
>  				      unsigned long expires);
>  
>  void mt76u_mcu_complete_urb(struct urb *urb);
> -int mt76u_mcu_init_rx(struct mt76_dev *dev);
> -void mt76u_mcu_deinit(struct mt76_dev *dev);
>  
>  #endif
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
> index f0c33890f1a5..91718647da02 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
> @@ -79,7 +79,6 @@ static void mt76x0u_cleanup(struct mt76x02_dev *dev)
>  	clear_bit(MT76_STATE_INITIALIZED, &dev->mt76.state);
>  	mt76x0_chip_onoff(dev, false, false);
>  	mt76u_queues_deinit(&dev->mt76);
> -	mt76u_mcu_deinit(&dev->mt76);
>  }
>  
>  static void mt76x0u_mac_stop(struct mt76x02_dev *dev)
> @@ -193,10 +192,6 @@ static int mt76x0u_register_device(struct mt76x02_dev *dev)
>  	if (err < 0)
>  		goto out_err;
>  
> -	err = mt76u_mcu_init_rx(&dev->mt76);
> -	if (err < 0)
> -		goto out_err;
> -
>  	err = mt76x0u_init_hardware(dev);
>  	if (err < 0)
>  		goto out_err;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
> index 090aaf71b3ef..1da90e58d942 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
> @@ -214,10 +214,6 @@ int mt76x2u_register_device(struct mt76x02_dev *dev)
>  	if (err < 0)
>  		goto fail;
>  
> -	err = mt76u_mcu_init_rx(&dev->mt76);
> -	if (err < 0)
> -		goto fail;
> -
>  	err = mt76x2u_init_hardware(dev);
>  	if (err < 0)
>  		goto fail;
> @@ -259,5 +255,4 @@ void mt76x2u_cleanup(struct mt76x02_dev *dev)
>  	mt76x02_mcu_set_radio_state(dev, false);
>  	mt76x2u_stop_hw(dev);
>  	mt76u_queues_deinit(&dev->mt76);
> -	mt76u_mcu_deinit(&dev->mt76);
>  }
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index 5c3b7f735aae..792f5012f1b1 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -577,9 +577,14 @@ EXPORT_SYMBOL_GPL(mt76u_submit_rx_buffers);
>  
>  static int mt76u_alloc_rx(struct mt76_dev *dev)
>  {
> +	struct mt76_usb *usb = &dev->usb;
>  	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
>  	int i, err;
>  
> +	usb->mcu.data = devm_kmalloc(dev->dev, MCU_RESP_URB_SIZE, GFP_KERNEL);
> +	if (!usb->mcu.data)
> +		return -ENOMEM;
> +
>  	spin_lock_init(&q->rx_page_lock);
>  	spin_lock_init(&q->lock);
>  	q->entry = devm_kcalloc(dev->dev,
> diff --git a/drivers/net/wireless/mediatek/mt76/usb_mcu.c b/drivers/net/wireless/mediatek/mt76/usb_mcu.c
> deleted file mode 100644
> index 747231edc57d..000000000000
> --- a/drivers/net/wireless/mediatek/mt76/usb_mcu.c
> +++ /dev/null
> @@ -1,34 +0,0 @@
> -/*
> - * Copyright (C) 2018 Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
> - *
> - * Permission to use, copy, modify, and/or distribute this software for any
> - * purpose with or without fee is hereby granted, provided that the above
> - * copyright notice and this permission notice appear in all copies.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> - */
> -
> -#include "mt76.h"
> -
> -int mt76u_mcu_init_rx(struct mt76_dev *dev)
> -{
> -	struct mt76_usb *usb = &dev->usb;
> -
> -	usb->mcu.data = kmalloc(MCU_RESP_URB_SIZE, GFP_KERNEL);
> -	return usb->mcu.data ? 0 : -ENOMEM;
> -}
> -EXPORT_SYMBOL_GPL(mt76u_mcu_init_rx);
> -
> -void mt76u_mcu_deinit(struct mt76_dev *dev)
> -{
> -	struct mt76_usb *usb = &dev->usb;
> -
> -	kfree(usb->mcu.data);
> -}
> -EXPORT_SYMBOL_GPL(mt76u_mcu_deinit);
> -- 
> 2.7.5
> 

^ permalink raw reply

* update on netdev 0x13 conference
From: Jamal Hadi Salim @ 2019-02-20 13:24 UTC (permalink / raw)
  To: netdev@vger.kernel.org, linux-wireless, netfilter-devel,
	netfilter, lartc

This is a small update to the community on the Netdev 0x13 conference
(March 20-22, in Prague, Czech Republic)
https://www.netdevconf.org/0x13

Early registration fees end today at 23:59 EST.
To Register: https://www.netdevconf.org/0x13/registration.html

Our bursaries are also going to close today at 23:59 EST.
Any member of the community may request for a bursary (sponsorship) to
attend.
https://www.netdevconf.org/0x13/bursaries.html

Many hours of exciting working discussions and talks.
https://www.netdevconf.org/0x13/accepted-sessions.html
https://www.netdevconf.org/0x13/schedule.html

Because the schedule is packed we will be providing lunch
this time at the conference location.

For regular updates, please subscribe to people@lists.netdevconf.org
(more info at: https://lists.netdevconf.org/cgi-bin/mailman/listinfo/people)

If twitter is your thing then follow us: @netdev01
and use hashtag #netdevconf

cheers,
jamal

^ permalink raw reply

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Lorenzo Bianconi @ 2019-02-20 13:22 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Felix Fietkau, Stefan Wahren, Alan Stern, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <20190220130009.GA2377@redhat.com>

> On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > >> >> The way I see it, we have two choices.
> > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > >> > 
> > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > >> > if this is not a bug on other platforms as well.
> > >> >From what I can see, using Lorenzo's patches seems to be the better
> > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > >> drivers as well). I will apply them and then we'll see if we need to do
> > >> any further improvements later on.
> > > 
> > > They work on rpi dwc2, but they do not address root of the problem.
> > > There is clearly something wrong how mt76usb handle SG, what is not
> > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > SG should be red flag.
> > I think we're simply dealing with multiple issues here, only some of
> > which are fixed by Lorenzo's patches.
> > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > since the Linux USB API supports non-aligned transfer buffers and it
> > should be up to the controller driver to deal with that.
> 
> Agree.
> 
> > dwc2 tries to do that, but that has limitations which I already pointed
> > out and which are properly dealt with by Lorenzo's patches.
> 
> I planed to just accept current solution, but I started to work on patch
> that remove len, sglen arguments from mt76u_buf_alloc() and use
> q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> related code is now tangled.
> 
> Would be ok to send this patch with proper changelog as fix for RPI
> against wireless-drivers and cc:stable (assuming it works and really
> fix things on RPI) and revert Lorenzo's patches in -next ?

Hi Stanislaw,

what is the advantage of doing so? You have duplicated most of the fields that are
already in the urb data structure and you use transfer_buffer (no SG I/O).
Moreover I have ready a series that removes 99% of the dual allocation code and
maintain it in the control path (instead of the datapath one).
I need just to rebase it ontop of your series. I will post it soon.

Regards,
Lorenzo

> 
> Stanislaw
> 
> From 4f8d7d3f4031b0a97b3bb147cb7e52533886e7cc Mon Sep 17 00:00:00 2001
> From: Stanislaw Gruszka <sgruszka@redhat.com>
> Date: Wed, 20 Feb 2019 13:29:42 +0100
> Subject: [PATCH] mt76usb: use urb transfer_buffer for one segment
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76.h     |  2 +
>  .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |  4 +-
>  drivers/net/wireless/mediatek/mt76/usb.c      | 75 +++++++++++--------
>  3 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 2e5bcb3fdff7..7e0680daeee6 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -86,6 +86,8 @@ struct mt76_queue_buf {
>  struct mt76u_buf {
>  	struct mt76_dev *dev;
>  	struct urb *urb;
> +	struct scatterlist *sg;
> +	int num_sgs;
>  	size_t len;
>  	bool done;
>  };
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> index da299b8a1334..75561910d630 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
> @@ -90,7 +90,7 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
>  		if (urb->status)
>  			return -EIO;
>  
> -		data = sg_virt(&urb->sg[0]);
> +		data = sg_virt(&buf->sg[0]);
>  		if (usb->mcu.rp)
>  			mt76x02u_multiple_mcu_reads(dev, data + 4,
>  						    urb->actual_length - 8);
> @@ -266,7 +266,7 @@ static int
>  __mt76x02u_mcu_fw_send_data(struct mt76x02_dev *dev, struct mt76u_buf *buf,
>  			    const void *fw_data, int len, u32 dst_addr)
>  {
> -	u8 *data = sg_virt(&buf->urb->sg[0]);
> +	u8 *data = sg_virt(&buf->sg[0]);
>  	DECLARE_COMPLETION_ONSTACK(cmpl);
>  	__le32 info;
>  	u32 val;
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index a1811c39415e..57bb16eaff06 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -277,7 +277,6 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
>  		 int nsgs, int len, int sglen)
>  {
>  	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
> -	struct urb *urb = buf->urb;
>  	int i;
>  
>  	spin_lock_bh(&q->rx_page_lock);
> @@ -292,21 +291,21 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
>  
>  		page = virt_to_head_page(data);
>  		offset = data - page_address(page);
> -		sg_set_page(&urb->sg[i], page, sglen, offset);
> +		sg_set_page(&buf->sg[i], page, sglen, offset);
>  	}
>  	spin_unlock_bh(&q->rx_page_lock);
>  
>  	if (i < nsgs) {
>  		int j;
>  
> -		for (j = nsgs; j < urb->num_sgs; j++)
> -			skb_free_frag(sg_virt(&urb->sg[j]));
> -		urb->num_sgs = i;
> +		for (j = nsgs; j < buf->num_sgs; j++)
> +			skb_free_frag(sg_virt(&buf->sg[j]));
> +		buf->num_sgs = i;
>  	}
>  
> -	urb->num_sgs = max_t(int, i, urb->num_sgs);
> -	buf->len = urb->num_sgs * sglen,
> -	sg_init_marker(urb->sg, urb->num_sgs);
> +	buf->num_sgs = max_t(int, i, buf->num_sgs);
> +	buf->len = buf->num_sgs * sglen,
> +	sg_init_marker(buf->sg, buf->num_sgs);
>  
>  	return i ? : -ENOMEM;
>  }
> @@ -318,13 +317,14 @@ int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
>  	if (!buf->urb)
>  		return -ENOMEM;
>  
> -	buf->urb->sg = devm_kcalloc(dev->dev, nsgs, sizeof(*buf->urb->sg),
> +	buf->sg = devm_kcalloc(dev->dev, nsgs, sizeof(*buf->sg),
>  				    gfp);
> -	if (!buf->urb->sg)
> +	if (!buf->sg)
>  		return -ENOMEM;
>  
> -	sg_init_table(buf->urb->sg, nsgs);
> +	sg_init_table(buf->sg, nsgs);
>  	buf->dev = dev;
> +	buf->num_sgs = nsgs;
>  
>  	return mt76u_fill_rx_sg(dev, buf, nsgs, len, sglen);
>  }
> @@ -332,12 +332,11 @@ EXPORT_SYMBOL_GPL(mt76u_buf_alloc);
>  
>  void mt76u_buf_free(struct mt76u_buf *buf)
>  {
> -	struct urb *urb = buf->urb;
>  	struct scatterlist *sg;
>  	int i;
>  
> -	for (i = 0; i < urb->num_sgs; i++) {
> -		sg = &urb->sg[i];
> +	for (i = 0; i < buf->num_sgs; i++) {
> +		sg = &buf->sg[i];
>  		if (!sg)
>  			continue;
>  
> @@ -347,9 +346,10 @@ void mt76u_buf_free(struct mt76u_buf *buf)
>  }
>  EXPORT_SYMBOL_GPL(mt76u_buf_free);
>  
> -int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
> -		     struct mt76u_buf *buf, gfp_t gfp,
> -		     usb_complete_t complete_fn, void *context)
> +static void
> +mt76u_fill_bulk_urb(struct mt76_dev *dev, int dir, int index,
> +		    struct mt76u_buf *buf, usb_complete_t complete_fn,
> +		    void *context)
>  {
>  	struct usb_interface *intf = to_usb_interface(dev->dev);
>  	struct usb_device *udev = interface_to_usbdev(intf);
> @@ -360,9 +360,22 @@ int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
>  	else
>  		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index]);
>  
> -	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, buf->len,
> -			  complete_fn, context);
> +	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, buf->len, complete_fn,
> +			  context);
> +
> +	if (buf->num_sgs > 1) {
> +		buf->urb->num_sgs = buf->num_sgs;
> +		buf->urb->sg = buf->sg;
> +	} else {
> +		buf->urb->transfer_buffer = sg_virt(buf->sg);
> +	}
> +}
>  
> +int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
> +		     struct mt76u_buf *buf, gfp_t gfp,
> +		     usb_complete_t complete_fn, void *context)
> +{
> +	mt76u_fill_bulk_urb(dev, dir, index, buf, complete_fn, context);
>  	return usb_submit_urb(buf->urb, gfp);
>  }
>  EXPORT_SYMBOL_GPL(mt76u_submit_buf);
> @@ -672,7 +685,7 @@ static void mt76u_complete_tx(struct urb *urb)
>  }
>  
>  static int
> -mt76u_tx_build_sg(struct sk_buff *skb, struct urb *urb)
> +mt76u_tx_build_sg(struct sk_buff *skb, struct mt76u_buf *buf)
>  {
>  	int nsgs = 1 + skb_shinfo(skb)->nr_frags;
>  	struct sk_buff *iter;
> @@ -680,13 +693,14 @@ mt76u_tx_build_sg(struct sk_buff *skb, struct urb *urb)
>  	skb_walk_frags(skb, iter)
>  		nsgs += 1 + skb_shinfo(iter)->nr_frags;
>  
> -	memset(urb->sg, 0, sizeof(*urb->sg) * MT_SG_MAX_SIZE);
> +	memset(buf->sg, 0, sizeof(*buf->sg) * MT_SG_MAX_SIZE);
>  
>  	nsgs = min_t(int, MT_SG_MAX_SIZE, nsgs);
> -	sg_init_marker(urb->sg, nsgs);
> -	urb->num_sgs = nsgs;
> +	sg_init_marker(buf->sg, nsgs);
> +	buf->num_sgs = nsgs;
> +	buf->len = skb->len;
>  
> -	return skb_to_sgvec_nomark(skb, urb->sg, 0, skb->len);
> +	return skb_to_sgvec_nomark(skb, buf->sg, 0, skb->len);
>  }
>  
>  static int
> @@ -694,12 +708,9 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
>  		   struct sk_buff *skb, struct mt76_wcid *wcid,
>  		   struct ieee80211_sta *sta)
>  {
> -	struct usb_interface *intf = to_usb_interface(dev->dev);
> -	struct usb_device *udev = interface_to_usbdev(intf);
>  	u8 ep = q2ep(q->hw_idx);
>  	struct mt76u_buf *buf;
>  	u16 idx = q->tail;
> -	unsigned int pipe;
>  	int err;
>  
>  	if (q->queued == q->ndesc)
> @@ -712,13 +723,11 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
>  	buf = &q->entry[idx].ubuf;
>  	buf->done = false;
>  
> -	err = mt76u_tx_build_sg(skb, buf->urb);
> +	err = mt76u_tx_build_sg(skb, buf);
>  	if (err < 0)
>  		return err;
>  
> -	pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[ep]);
> -	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, skb->len,
> -			  mt76u_complete_tx, buf);
> +	mt76u_fill_bulk_urb(dev, USB_DIR_OUT, ep, buf, mt76u_complete_tx, buf);
>  
>  	q->tail = (q->tail + 1) % q->ndesc;
>  	q->entry[idx].skb = skb;
> @@ -776,8 +785,8 @@ static int mt76u_alloc_tx(struct mt76_dev *dev)
>  			if (!buf->urb)
>  				return -ENOMEM;
>  
> -			buf->urb->sg = devm_kzalloc(dev->dev, size, GFP_KERNEL);
> -			if (!buf->urb->sg)
> +			buf->sg = devm_kzalloc(dev->dev, size, GFP_KERNEL);
> +			if (!buf->sg)
>  				return -ENOMEM;
>  		}
>  	}
> -- 
> 2.20.1
> 

^ permalink raw reply

* Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
From: Stanislaw Gruszka @ 2019-02-20 13:00 UTC (permalink / raw)
  To: Felix Fietkau
  Cc: Stefan Wahren, Lorenzo Bianconi, Alan Stern, Doug Anderson,
	Minas Harutyunyan, USB list, linux-wireless
In-Reply-To: <58097bb1-d726-c48e-2a40-2e12098dfb15@nbd.name>

On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> >> >> The way I see it, we have two choices.
> >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> >> > 
> >> > I agree, if this is only needed for dwc2. Though I would investigate
> >> > if this is not a bug on other platforms as well.
> >> >From what I can see, using Lorenzo's patches seems to be the better
> >> solution, since they avoid these corner cases in dwc2 (and maybe other
> >> drivers as well). I will apply them and then we'll see if we need to do
> >> any further improvements later on.
> > 
> > They work on rpi dwc2, but they do not address root of the problem.
> > There is clearly something wrong how mt76usb handle SG, what is not
> > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > SG should be red flag.
> I think we're simply dealing with multiple issues here, only some of
> which are fixed by Lorenzo's patches.
> I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> since the Linux USB API supports non-aligned transfer buffers and it
> should be up to the controller driver to deal with that.

Agree.

> dwc2 tries to do that, but that has limitations which I already pointed
> out and which are properly dealt with by Lorenzo's patches.

I planed to just accept current solution, but I started to work on patch
that remove len, sglen arguments from mt76u_buf_alloc() and use
q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
related code is now tangled.

Would be ok to send this patch with proper changelog as fix for RPI
against wireless-drivers and cc:stable (assuming it works and really
fix things on RPI) and revert Lorenzo's patches in -next ?

Stanislaw

From 4f8d7d3f4031b0a97b3bb147cb7e52533886e7cc Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Wed, 20 Feb 2019 13:29:42 +0100
Subject: [PATCH] mt76usb: use urb transfer_buffer for one segment

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/mediatek/mt76/mt76.h     |  2 +
 .../wireless/mediatek/mt76/mt76x02_usb_mcu.c  |  4 +-
 drivers/net/wireless/mediatek/mt76/usb.c      | 75 +++++++++++--------
 3 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 2e5bcb3fdff7..7e0680daeee6 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -86,6 +86,8 @@ struct mt76_queue_buf {
 struct mt76u_buf {
 	struct mt76_dev *dev;
 	struct urb *urb;
+	struct scatterlist *sg;
+	int num_sgs;
 	size_t len;
 	bool done;
 };
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
index da299b8a1334..75561910d630 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
@@ -90,7 +90,7 @@ static int mt76x02u_mcu_wait_resp(struct mt76_dev *dev, u8 seq)
 		if (urb->status)
 			return -EIO;
 
-		data = sg_virt(&urb->sg[0]);
+		data = sg_virt(&buf->sg[0]);
 		if (usb->mcu.rp)
 			mt76x02u_multiple_mcu_reads(dev, data + 4,
 						    urb->actual_length - 8);
@@ -266,7 +266,7 @@ static int
 __mt76x02u_mcu_fw_send_data(struct mt76x02_dev *dev, struct mt76u_buf *buf,
 			    const void *fw_data, int len, u32 dst_addr)
 {
-	u8 *data = sg_virt(&buf->urb->sg[0]);
+	u8 *data = sg_virt(&buf->sg[0]);
 	DECLARE_COMPLETION_ONSTACK(cmpl);
 	__le32 info;
 	u32 val;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index a1811c39415e..57bb16eaff06 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -277,7 +277,6 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
 		 int nsgs, int len, int sglen)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
-	struct urb *urb = buf->urb;
 	int i;
 
 	spin_lock_bh(&q->rx_page_lock);
@@ -292,21 +291,21 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
 
 		page = virt_to_head_page(data);
 		offset = data - page_address(page);
-		sg_set_page(&urb->sg[i], page, sglen, offset);
+		sg_set_page(&buf->sg[i], page, sglen, offset);
 	}
 	spin_unlock_bh(&q->rx_page_lock);
 
 	if (i < nsgs) {
 		int j;
 
-		for (j = nsgs; j < urb->num_sgs; j++)
-			skb_free_frag(sg_virt(&urb->sg[j]));
-		urb->num_sgs = i;
+		for (j = nsgs; j < buf->num_sgs; j++)
+			skb_free_frag(sg_virt(&buf->sg[j]));
+		buf->num_sgs = i;
 	}
 
-	urb->num_sgs = max_t(int, i, urb->num_sgs);
-	buf->len = urb->num_sgs * sglen,
-	sg_init_marker(urb->sg, urb->num_sgs);
+	buf->num_sgs = max_t(int, i, buf->num_sgs);
+	buf->len = buf->num_sgs * sglen,
+	sg_init_marker(buf->sg, buf->num_sgs);
 
 	return i ? : -ENOMEM;
 }
@@ -318,13 +317,14 @@ int mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf,
 	if (!buf->urb)
 		return -ENOMEM;
 
-	buf->urb->sg = devm_kcalloc(dev->dev, nsgs, sizeof(*buf->urb->sg),
+	buf->sg = devm_kcalloc(dev->dev, nsgs, sizeof(*buf->sg),
 				    gfp);
-	if (!buf->urb->sg)
+	if (!buf->sg)
 		return -ENOMEM;
 
-	sg_init_table(buf->urb->sg, nsgs);
+	sg_init_table(buf->sg, nsgs);
 	buf->dev = dev;
+	buf->num_sgs = nsgs;
 
 	return mt76u_fill_rx_sg(dev, buf, nsgs, len, sglen);
 }
@@ -332,12 +332,11 @@ EXPORT_SYMBOL_GPL(mt76u_buf_alloc);
 
 void mt76u_buf_free(struct mt76u_buf *buf)
 {
-	struct urb *urb = buf->urb;
 	struct scatterlist *sg;
 	int i;
 
-	for (i = 0; i < urb->num_sgs; i++) {
-		sg = &urb->sg[i];
+	for (i = 0; i < buf->num_sgs; i++) {
+		sg = &buf->sg[i];
 		if (!sg)
 			continue;
 
@@ -347,9 +346,10 @@ void mt76u_buf_free(struct mt76u_buf *buf)
 }
 EXPORT_SYMBOL_GPL(mt76u_buf_free);
 
-int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
-		     struct mt76u_buf *buf, gfp_t gfp,
-		     usb_complete_t complete_fn, void *context)
+static void
+mt76u_fill_bulk_urb(struct mt76_dev *dev, int dir, int index,
+		    struct mt76u_buf *buf, usb_complete_t complete_fn,
+		    void *context)
 {
 	struct usb_interface *intf = to_usb_interface(dev->dev);
 	struct usb_device *udev = interface_to_usbdev(intf);
@@ -360,9 +360,22 @@ int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
 	else
 		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index]);
 
-	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, buf->len,
-			  complete_fn, context);
+	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, buf->len, complete_fn,
+			  context);
+
+	if (buf->num_sgs > 1) {
+		buf->urb->num_sgs = buf->num_sgs;
+		buf->urb->sg = buf->sg;
+	} else {
+		buf->urb->transfer_buffer = sg_virt(buf->sg);
+	}
+}
 
+int mt76u_submit_buf(struct mt76_dev *dev, int dir, int index,
+		     struct mt76u_buf *buf, gfp_t gfp,
+		     usb_complete_t complete_fn, void *context)
+{
+	mt76u_fill_bulk_urb(dev, dir, index, buf, complete_fn, context);
 	return usb_submit_urb(buf->urb, gfp);
 }
 EXPORT_SYMBOL_GPL(mt76u_submit_buf);
@@ -672,7 +685,7 @@ static void mt76u_complete_tx(struct urb *urb)
 }
 
 static int
-mt76u_tx_build_sg(struct sk_buff *skb, struct urb *urb)
+mt76u_tx_build_sg(struct sk_buff *skb, struct mt76u_buf *buf)
 {
 	int nsgs = 1 + skb_shinfo(skb)->nr_frags;
 	struct sk_buff *iter;
@@ -680,13 +693,14 @@ mt76u_tx_build_sg(struct sk_buff *skb, struct urb *urb)
 	skb_walk_frags(skb, iter)
 		nsgs += 1 + skb_shinfo(iter)->nr_frags;
 
-	memset(urb->sg, 0, sizeof(*urb->sg) * MT_SG_MAX_SIZE);
+	memset(buf->sg, 0, sizeof(*buf->sg) * MT_SG_MAX_SIZE);
 
 	nsgs = min_t(int, MT_SG_MAX_SIZE, nsgs);
-	sg_init_marker(urb->sg, nsgs);
-	urb->num_sgs = nsgs;
+	sg_init_marker(buf->sg, nsgs);
+	buf->num_sgs = nsgs;
+	buf->len = skb->len;
 
-	return skb_to_sgvec_nomark(skb, urb->sg, 0, skb->len);
+	return skb_to_sgvec_nomark(skb, buf->sg, 0, skb->len);
 }
 
 static int
@@ -694,12 +708,9 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 		   struct sk_buff *skb, struct mt76_wcid *wcid,
 		   struct ieee80211_sta *sta)
 {
-	struct usb_interface *intf = to_usb_interface(dev->dev);
-	struct usb_device *udev = interface_to_usbdev(intf);
 	u8 ep = q2ep(q->hw_idx);
 	struct mt76u_buf *buf;
 	u16 idx = q->tail;
-	unsigned int pipe;
 	int err;
 
 	if (q->queued == q->ndesc)
@@ -712,13 +723,11 @@ mt76u_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 	buf = &q->entry[idx].ubuf;
 	buf->done = false;
 
-	err = mt76u_tx_build_sg(skb, buf->urb);
+	err = mt76u_tx_build_sg(skb, buf);
 	if (err < 0)
 		return err;
 
-	pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[ep]);
-	usb_fill_bulk_urb(buf->urb, udev, pipe, NULL, skb->len,
-			  mt76u_complete_tx, buf);
+	mt76u_fill_bulk_urb(dev, USB_DIR_OUT, ep, buf, mt76u_complete_tx, buf);
 
 	q->tail = (q->tail + 1) % q->ndesc;
 	q->entry[idx].skb = skb;
@@ -776,8 +785,8 @@ static int mt76u_alloc_tx(struct mt76_dev *dev)
 			if (!buf->urb)
 				return -ENOMEM;
 
-			buf->urb->sg = devm_kzalloc(dev->dev, size, GFP_KERNEL);
-			if (!buf->urb->sg)
+			buf->sg = devm_kzalloc(dev->dev, size, GFP_KERNEL);
+			if (!buf->sg)
 				return -ENOMEM;
 		}
 	}
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH wireless-drivers-next] brcmfmac: add basic validation of shared RAM address
From: Arend Van Spriel @ 2019-02-20 12:49 UTC (permalink / raw)
  To: Rafał Miłecki, Kalle Valo
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	Rafał Miłecki
In-Reply-To: <20190220103047.8960-1-zajec5@gmail.com>

On 2/20/2019 11:30 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> While experimenting with firmware loading I ended up in a state of
> firmware reporting shared RAM address 0x04000001. It was causing:
> [   94.448015] Unable to handle kernel paging request at virtual address cd680001
> due to reading out of the mapped memory.
> 
> This patch adds some basic validation to avoid kernel crashes due to the
> unexpected firmware behavior.

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++++++
>   1 file changed, 6 insertions(+)

^ 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