Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH v3 wireless-drivers 2/3] mt76: mt76u: introduce mt76u_ep data structure
From: Lorenzo Bianconi @ 2019-06-13 21:43 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, nbd, lorenzo.bianconi, sgruszka
In-Reply-To: <cover.1560461404.git.lorenzo@kernel.org>

Introduce mt76u_ep data structure as a container for usb endpoint info.
This is a preliminary patch to compute proper usb buffer size and avoid
always copy the first part of received frames

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

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 889b76deb703..1c51d6d48e60 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -382,6 +382,11 @@ enum mt76u_out_ep {
 	__MT_EP_OUT_MAX,
 };
 
+struct mt76u_ep {
+	u16 max_packet;
+	u8 ep;
+};
+
 #define MT_SG_MAX_SIZE		8
 #define MT_NUM_TX_ENTRIES	256
 #define MT_NUM_RX_ENTRIES	128
@@ -393,10 +398,8 @@ struct mt76_usb {
 	struct tasklet_struct rx_tasklet;
 	struct delayed_work stat_work;
 
-	u8 out_ep[__MT_EP_OUT_MAX];
-	u16 out_max_packet;
-	u8 in_ep[__MT_EP_IN_MAX];
-	u16 in_max_packet;
+	struct mt76u_ep out_ep[__MT_EP_OUT_MAX];
+	struct mt76u_ep in_ep[__MT_EP_IN_MAX];
 	bool sg_en;
 
 	struct mt76u_mcu {
@@ -786,9 +789,10 @@ mt76u_bulk_msg(struct mt76_dev *dev, void *data, int len, int *actual_len,
 	unsigned int pipe;
 
 	if (actual_len)
-		pipe = usb_rcvbulkpipe(udev, usb->in_ep[MT_EP_IN_CMD_RESP]);
+		pipe = usb_rcvbulkpipe(udev, usb->in_ep[MT_EP_IN_CMD_RESP].ep);
 	else
-		pipe = usb_sndbulkpipe(udev, usb->out_ep[MT_EP_OUT_INBAND_CMD]);
+		pipe = usb_sndbulkpipe(udev,
+				       usb->out_ep[MT_EP_OUT_INBAND_CMD].ep);
 
 	return usb_bulk_msg(udev, pipe, data, len, actual_len, timeout);
 }
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 12d60d31cb51..1ee54a9b302e 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -260,19 +260,22 @@ mt76u_set_endpoints(struct usb_interface *intf,
 	struct usb_host_interface *intf_desc = intf->cur_altsetting;
 	struct usb_endpoint_descriptor *ep_desc;
 	int i, in_ep = 0, out_ep = 0;
+	struct mt76u_ep *ep;
 
 	for (i = 0; i < intf_desc->desc.bNumEndpoints; i++) {
 		ep_desc = &intf_desc->endpoint[i].desc;
 
 		if (usb_endpoint_is_bulk_in(ep_desc) &&
 		    in_ep < __MT_EP_IN_MAX) {
-			usb->in_ep[in_ep] = usb_endpoint_num(ep_desc);
-			usb->in_max_packet = usb_endpoint_maxp(ep_desc);
+			ep = &usb->in_ep[in_ep];
+			ep->max_packet = usb_endpoint_maxp(ep_desc);
+			ep->ep = usb_endpoint_num(ep_desc);
 			in_ep++;
 		} else if (usb_endpoint_is_bulk_out(ep_desc) &&
 			   out_ep < __MT_EP_OUT_MAX) {
-			usb->out_ep[out_ep] = usb_endpoint_num(ep_desc);
-			usb->out_max_packet = usb_endpoint_maxp(ep_desc);
+			ep = &usb->out_ep[out_ep];
+			ep->max_packet = usb_endpoint_maxp(ep_desc);
+			ep->ep = usb_endpoint_num(ep_desc);
 			out_ep++;
 		}
 	}
@@ -386,9 +389,9 @@ mt76u_fill_bulk_urb(struct mt76_dev *dev, int dir, int index,
 	unsigned int pipe;
 
 	if (dir == USB_DIR_IN)
-		pipe = usb_rcvbulkpipe(udev, dev->usb.in_ep[index]);
+		pipe = usb_rcvbulkpipe(udev, dev->usb.in_ep[index].ep);
 	else
-		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index]);
+		pipe = usb_sndbulkpipe(udev, dev->usb.out_ep[index].ep);
 
 	urb->dev = udev;
 	urb->pipe = pipe;
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3 wireless-drivers 1/3] mt76: usb: fix rx A-MSDU support
From: Lorenzo Bianconi @ 2019-06-13 21:43 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, nbd, lorenzo.bianconi, sgruszka
In-Reply-To: <cover.1560461404.git.lorenzo@kernel.org>

Commit f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes
for rx") breaks A-MSDU support. When A-MSDU is enable the device can
receive frames up to q->buf_size but they will be discarded in
mt76u_process_rx_entry since there is no enough room for
skb_shared_info. Fix the issue reallocating the skb and copying in the
linear area the first 128B of the received frames and in the frag_list
the remaining part.

Fixes: f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes for rx")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  1 +
 drivers/net/wireless/mediatek/mt76/usb.c  | 49 ++++++++++++++++++-----
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 8ecbf81a906f..889b76deb703 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -30,6 +30,7 @@
 #define MT_TX_RING_SIZE     256
 #define MT_MCU_RING_SIZE    32
 #define MT_RX_BUF_SIZE      2048
+#define MT_SKB_HEAD_LEN     128
 
 struct mt76_dev;
 struct mt76_wcid;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index bbaa1365bbda..12d60d31cb51 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -429,6 +429,45 @@ static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
 	return dma_len;
 }
 
+static struct sk_buff *
+mt76u_build_rx_skb(u8 *data, int len, int buf_size)
+{
+	struct sk_buff *skb;
+
+	if (SKB_WITH_OVERHEAD(buf_size) < MT_DMA_HDR_LEN + len) {
+		struct page *page;
+		int offset;
+
+		/* slow path, not enough space for data and
+		 * skb_shared_info
+		 */
+		skb = alloc_skb(MT_SKB_HEAD_LEN, GFP_ATOMIC);
+		if (!skb)
+			return NULL;
+
+		skb_put_data(skb, data + MT_DMA_HDR_LEN, MT_SKB_HEAD_LEN);
+		data += (MT_SKB_HEAD_LEN + MT_DMA_HDR_LEN);
+		page = virt_to_head_page(data);
+		offset = data - (u8 *)page_address(page);
+
+		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+				page, offset, len - MT_SKB_HEAD_LEN,
+				buf_size);
+
+		return skb;
+	}
+
+	/* fast path */
+	skb = build_skb(data, buf_size);
+	if (!skb)
+		return NULL;
+
+	skb_reserve(skb, MT_DMA_HDR_LEN);
+	__skb_put(skb, len);
+
+	return skb;
+}
+
 static int
 mt76u_process_rx_entry(struct mt76_dev *dev, struct urb *urb)
 {
@@ -446,19 +485,11 @@ mt76u_process_rx_entry(struct mt76_dev *dev, struct urb *urb)
 		return 0;
 
 	data_len = min_t(int, len, data_len - MT_DMA_HDR_LEN);
-	if (MT_DMA_HDR_LEN + data_len > SKB_WITH_OVERHEAD(q->buf_size)) {
-		dev_err_ratelimited(dev->dev, "rx data too big %d\n", data_len);
-		return 0;
-	}
-
-	skb = build_skb(data, q->buf_size);
+	skb = mt76u_build_rx_skb(data, data_len, q->buf_size);
 	if (!skb)
 		return 0;
 
-	skb_reserve(skb, MT_DMA_HDR_LEN);
-	__skb_put(skb, data_len);
 	len -= data_len;
-
 	while (len > 0 && nsgs < urb->num_sgs) {
 		data_len = min_t(int, len, urb->sg[nsgs].length);
 		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3 wireless-drivers 0/3] mt76: usb: fix A-MSDU support
From: Lorenzo Bianconi @ 2019-06-13 21:43 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, nbd, lorenzo.bianconi, sgruszka

Reallocate the skb if there is no enough space to manage the AMSDU rx packets.
Do not always copy the first part of received frames if A-MSDU is enabled
for SG capable devices

Changes since v2:
- simplify mt76u_build_rx_skb
- add patch 2/3: mt76u: introduce mt76u_ep data structure
- align usb buffer size to usb max endpoint length
- set buf_size to PAGE_SIZE even for sg case

Changes since v1:
- do not allocate multiple page buffers but rely on fragmented skbs
  if there is no enough space to manage the AMSDU rx packets

Lorenzo Bianconi (3):
  mt76: usb: fix rx A-MSDU support
  mt76: mt76u: introduce mt76u_ep data structure
  mt76: usb: do not always copy the first part of received frames

 drivers/net/wireless/mediatek/mt76/mt76.h | 17 +++--
 drivers/net/wireless/mediatek/mt76/usb.c  | 75 +++++++++++++++++------
 2 files changed, 67 insertions(+), 25 deletions(-)

-- 
2.21.0


^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: Abort at too short BSS descriptor element
From: Brian Norris @ 2019-06-13 20:26 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-wireless, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Kalle Valo, huangwen, Solar Designer,
	Marcus Meissner
In-Reply-To: <20190613183835.GA16432@google.com>

On Thu, Jun 13, 2019 at 11:38 AM Brian Norris <briannorris@chromium.org> wrote:
> So, I might say:
>
>         /* Vendor IEs must at least contain the OUI. */
>         if (total_ie_len < offsetof(struct ieee80211_vendor_ie, oui_type))
>                 return -EINVAL;
>
>         /* If the IE still isn't long enough, it's not a match. */
>         if (element_len < sizeof(wpa_oui))
>                 continue;

That would of course need to be break, not continue, to properly skip
to the next IE.

Brian

^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: Abort at too short BSS descriptor element
From: Brian Norris @ 2019-06-13 18:38 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-wireless, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Kalle Valo, huangwen, Solar Designer,
	Marcus Meissner
In-Reply-To: <s5hmuile4mg.wl-tiwai@suse.de>

On Thu, Jun 13, 2019 at 08:12:39PM +0200, Takashi Iwai wrote:
> On Thu, 13 Jun 2019 19:49:40 +0200,
> Brian Norris wrote:
> > On Wed, May 29, 2019 at 02:52:20PM +0200, Takashi Iwai wrote:
> > > --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> > > @@ -1269,6 +1269,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
> > >  			break;
> > >  
> > >  		case WLAN_EID_FH_PARAMS:
> > > +			if (element_len + 2 < sizeof(*fh_param_set))
> > 
> > "element_len + 2" would be much more readable as "total_ie_len". (Same for
> > several other usages in this patch.) I can send such a patch myself as a
> > follow-up I suppose.
> 
> Yes, please.

I'll wait until we straighten out the other (potentially) real bug.
Don't want to make needless conflicts.

> > > +				return -EINVAL;
> > >  			fh_param_set =
> > >  				(struct ieee_types_fh_param_set *) current_ptr;
> > >  			memcpy(&bss_entry->phy_param_set.fh_param_set,
> > 
> > [...]
> > 
> > > @@ -1349,6 +1361,9 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
> > >  			break;
> > >  
> > >  		case WLAN_EID_VENDOR_SPECIFIC:
> > > +			if (element_len + 2 < sizeof(vendor_ie->vend_hdr))
> > 
> > Why 'sizeof(vendor_ie->vend_hdr)'? The (mwifiex-specific compare with the
> > ieee80211.h generic struct ieee80211_vendor_ie) ieee_types_vendor_header struct
> > includes the 'oui_subtype' and 'version' fields, which are not standard
> > requirements for the vendor header (in fact, even the 4th byte of the
> > OUI -- "oui_type" -- doesn't appear to be in the 802.11 specification).
> > So it looks to me like you might be rejecting valid vendor headers (that
> > we should just be skipping) that might have vendor-specific content with
> > length 0 or 1 bytes.
> > 
> > It seems like we should only be validating the standard pieces (e.g., up to the
> > length/OUI), and only after an appropriate OUI match, *then* validating the rest of
> > the vendor element (the pieces we'll use later).
> 
> Hm, right, that looks too strict.  Instead we need to check right
> before both memcmp()'s of OUI.

I think this is the right place for one check (the memcmp() is right
after this line anyway) -- the minimum length just should be smaller.

e.g.:

sizeof(struct ieee80211_vendor_ie) // this is still technically 1 byte too large I think
or
offsetof(struct ieee80211_vendor_ie, oui_type) // not sure if this is the cleanest...

If it's smaller than that, we can still say -EINVAL.

Then, we can go with:

if (element_len < sizeof(wpa_oui)
	continue;

or similar.

So, I might say:

	/* Vendor IEs must at least contain the OUI. */
	if (total_ie_len < offsetof(struct ieee80211_vendor_ie, oui_type))
		return -EINVAL;

	/* If the IE still isn't long enough, it's not a match. */
	if (element_len < sizeof(wpa_oui))
		continue;

Brian

^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: Abort at too short BSS descriptor element
From: Takashi Iwai @ 2019-06-13 18:12 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-wireless, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Kalle Valo, huangwen, Solar Designer,
	Marcus Meissner
In-Reply-To: <20190613174938.GA260350@google.com>

On Thu, 13 Jun 2019 19:49:40 +0200,
Brian Norris wrote:
> 
> Hi Takashi,
> 
> On Wed, May 29, 2019 at 02:52:20PM +0200, Takashi Iwai wrote:
> > Currently mwifiex_update_bss_desc_with_ie() implicitly assumes that
> > the source descriptor entries contain the enough size for each type
> > and performs copying without checking the source size.  This may lead
> > to read over boundary.
> > 
> > Fix this by putting the source size check in appropriate places.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/net/wireless/marvell/mwifiex/scan.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> > index 64ab6fe78c0d..c269a0de9413 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> > @@ -1269,6 +1269,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
> >  			break;
> >  
> >  		case WLAN_EID_FH_PARAMS:
> > +			if (element_len + 2 < sizeof(*fh_param_set))
> 
> "element_len + 2" would be much more readable as "total_ie_len". (Same for
> several other usages in this patch.) I can send such a patch myself as a
> follow-up I suppose.

Yes, please.

> > +				return -EINVAL;
> >  			fh_param_set =
> >  				(struct ieee_types_fh_param_set *) current_ptr;
> >  			memcpy(&bss_entry->phy_param_set.fh_param_set,
> 
> [...]
> 
> > @@ -1349,6 +1361,9 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
> >  			break;
> >  
> >  		case WLAN_EID_VENDOR_SPECIFIC:
> > +			if (element_len + 2 < sizeof(vendor_ie->vend_hdr))
> 
> Why 'sizeof(vendor_ie->vend_hdr)'? The (mwifiex-specific compare with the
> ieee80211.h generic struct ieee80211_vendor_ie) ieee_types_vendor_header struct
> includes the 'oui_subtype' and 'version' fields, which are not standard
> requirements for the vendor header (in fact, even the 4th byte of the
> OUI -- "oui_type" -- doesn't appear to be in the 802.11 specification).
> So it looks to me like you might be rejecting valid vendor headers (that
> we should just be skipping) that might have vendor-specific content with
> length 0 or 1 bytes.
> 
> It seems like we should only be validating the standard pieces (e.g., up to the
> length/OUI), and only after an appropriate OUI match, *then* validating the rest of
> the vendor element (the pieces we'll use later).

Hm, right, that looks too strict.  Instead we need to check right
before both memcmp()'s of OUI.


thanks,

Takashi

^ permalink raw reply

* Re: [PATCH] mmc: core: Prevent processing SDIO IRQs when the card is suspended
From: Doug Anderson @ 2019-06-13 18:05 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Linux MMC List, Adrian Hunter, Brian Norris, Shawn Lin,
	Guenter Roeck, Heiko Stuebner, Kalle Valo, linux-wireless, # 4.0+
In-Reply-To: <CAPDyKFqR-xSKdYZYBTK5kKOt1dk7dx_BjedHiDOKs7-X4od=dg@mail.gmail.com>

Hi,

On Thu, Jun 13, 2019 at 2:30 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> > > @@ -937,6 +937,10 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
> > >   */
> > >  static int mmc_sdio_suspend(struct mmc_host *host)
> > >  {
> > > +       /* Prevent processing of SDIO IRQs in suspended state. */
> > > +       mmc_card_set_suspended(host->card);
> >
> > Do you need to claim / release the host around the call to
> > mmc_card_set_suspended() to avoid races?
>
> The intent is that the races should be taken care of like this:
> 1) In MMC_CAP2_SDIO_IRQ_NOTHREAD case, the call to
> cancel_delayed_work_sync() below, will make sure that if there are any
> new work scheduled beyond that point, mmc_card_suspended() will be set
> and process_sdio_pending_irqs() will bail out.
>
> 2. In the non MMC_CAP2_SDIO_IRQ_NOTHREAD case, the call to
> mmc_claim_host() below will make sure if there is any new attempt to
> claim the host from the kthread, mmc_card_suspended() will be set and
> process_sdio_pending_irqs() bails out.
>
> Ideally in the long run and want to remove the SDIO kthread, so
> perhaps this is good enough for now, what do you think?

I was more worried about the safety of mmc_card_set_suspended()
itself.  That is:

#define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)

...so it's doing a read-modify-write of "state".  Is that safe to do
without any type of locking?


> BTW, the important point is that the call to
> cancel_delayed_work_sync(), must not be done while keeping the host
> claimed, as then it could deadlock.

Definitely.  I was thinking of this sequence:

mmc_claim_host(host);
mmc_card_set_suspended(host->card);
mmc_release_host(host);

cancel_delayed_work_sync(&host->sdio_irq_work);

mmc_claim_host(host);


> > >         if (!err && host->sdio_irqs) {
> > >                 if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD))
> > >                         wake_up_process(host->sdio_irq_thread);
> > > diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> > > index 931e6226c0b3..9f54a259a1b3 100644
> > > --- a/drivers/mmc/core/sdio_irq.c
> > > +++ b/drivers/mmc/core/sdio_irq.c
> > > @@ -34,6 +34,10 @@ static int process_sdio_pending_irqs(struct mmc_host *host)
> > >         unsigned char pending;
> > >         struct sdio_func *func;
> > >
> > > +       /* Don't process SDIO IRQs if the card is suspended. */
> > > +       if (mmc_card_suspended(card))
> > > +               return 0;
> > > +
> >
> > Is it really OK to just return like this?  I guess there are two
> > (somewhat opposite) worries I'd have.  See A) and B) below:
>
> Let me comment on A) and B) below, for sure there are more problems to address.
>
> The main reason to why I think it's okay to bail out here, is because
> I think it still improves the current behavior a lot. So, rather than
> solving all problems at once, I wanted to take a step by step
> approach.
>
> >
> > A) Do we need to do anything extra to make sure we actually call the
> > interrupt handler after we've resumed?  I guess we can't actually
> > "lose" the interrupt since it will be sitting asserted in CCCR_INTx
> > until we deal with it (right?), but maybe we need to do something to
> > ensure the handler gets called once we're done resuming?
>
> Good point!
>
> Although, it also depends on if we are going to power off the SDIO
> card or not. In other words, if the SDIO IRQ are configured as a
> system wakeup.

Even if it's not a system wakeup, we still don't want to drop the
interrupt on the ground though, do we?  For instance, think about a
level-triggered GPIO interrupt that's _not_ a wakeup interrupt.  If
that gets asserted in suspend then we won't wakeup the system, but as
soon as the system gets to a certain point in the resume sequence then
we should pass the interrupt on to the handler.  If an edge triggered
(but non-wakeup) interrupt fires when the system is resuming then we
should similarly not drop it, should we?


> Moreover there is another related problem, if the SDIO IRQ are
> configured as a system wakeup, and if there is an IRQ raised during
> the system suspend process, the system suspend process should be
> aborted but it may not. This is another issue that currently isn't
> supported. The PM core helps to deals with this, but to take advantage
> of that, the host controller device device must be configured via the
> common wakeup interfaces, such as the device_init_wakeup(), for
> example.

As per earlier discussions I don't have any good examples of SDIO IRQs
being able to wakeup the device to poke at.  ...but from GPIO-based
wakeups I'm used to the suspend code masking the interrupt (so it
doesn't fire anymore after the suspend call) but leaving it enabled
and configured as a wakeup.  I guess we'd have to think about how that
translates.  Your patch seems to be acting as a "mask" of the
interrupt, at least on my dw_mmc tests where the hardware presents the
interrupt like it was edge triggered.  ...so it would work OK there
I'd guess.


> > A2): new MMC_CAP2_SDIO_IRQ_NOTHREAD case
> >
> > Should we do something to re-kick things?  We could call
> > sdio_signal_irq() in mmc_sdio_resume() I guess?  I was worried that
> > might conflict with those that call sdio_run_irqs() directly but it
> > seems like that's nobody as of commit 89f3c365f3e1 ("mmc: sdhci: Fix
> > SDIO IRQ thread deadlock").
>
> Good point!
>
> Again, whether we should re-kick things depends on if the SDIO IRQ is
> configured as wakeup, but in general using sdio_signal_irq() should
> work.
>
> The other part I am considering is to disable the SDIO irq, in case of
> "mmc_card_keep_power() && !mmc_card_wake_sdio_irq()".
>
> Moreover, if !mmc_card_keep_power(), then there really shouldn't be
> any IRQs registered so perhaps we should add a special check for that
> and return an error code.

I haven't looked through all the details here but I can dig if you
want.  On other drivers it's generally OK to leave your interrupt
registered (just disabled and/or masked) across suspend/resume, but
maybe that's not OK for SDIO cards without keep power?


> In regards to other callers of sdio_run_irqs(). I have a patch that
> makes it this function static, as it really should not need to be used
> other than from the work queue path. Let me post it asap to cover that
> gap. Again, thanks for pointing this out!

Yeah, I was thinking of posting that too, but happy to have you do it!  :-)


> > ...side note: overall looking at this code path, two additional
> > questions come up for me.  One is why sdio_run_irqs() hardcodes
> > "sdio_irq_pending" as true.  That means we won't _ever_ poll CCCR_INTx
> > in the 1-function case, right?  That seems wrong.  The other is why
>
> In the 1-function case, the idea is that we don't have to read the
> CCCR_INTx to find out what func number the IRQ belongs to.
>
> This is the same behavior consistent as with the kthread case, see
> mmc_signal_sdio_irq(), unless I am mistaken.

I think there's at least the bug that nothing will ever set
"sdio_irq_pending" to false in the MMC_CAP2_SDIO_IRQ_NOTHREAD case,
right?  So we'll set it to true the first time and from then on out it
will never be false again?


> > mmc_sdio_resume() always calls host->ops->enable_sdio_irq(host, 1) at
> > resume time when nobody ever turned the IRQs off.
>
> That's correct and it leads to unbalanced calls of
> host->ops->enable_sdio_irq(). This needs to be fixed as well.
>
> >
> > ===
> >
> > B) Are there any instances where the interrupt will just keep firing
> > over and over again because we don't handle it?
> >
> > As per above, this _isn't_ happening on dw_mmc on my setup because
> > dw_mmc seems to treat the SDIO interrupt as edge triggered.  ...but is
> > this true everywhere?  If we were using SDIO in 1-bit mode on dw_mmc,
> > would the interrupt re-assert right away?  If dw_mmc were configured
> > to use a dedicated pin would it re-assert right away?  What about
> > other host controllers?
> >
> > If you're sure no host controllers will keep asserting the interrupt
> > over and over then I guess we don't need to worry about it?
> > ...otherwise we'd need to find some way to mask the interrupt and we'd
> > need to make sure whatever we do doesn't interfere with anyone who
> > supports the SDIO interrupt as a wake source, right?
>
> For the MMC_CAP2_SDIO_IRQ_NOTHREAD case, the expected behavior by the
> host driver is to prior calling sdio_signal_irq(), is should temporary
> disable the SDIO IRQ. Then, when the host->ops->ack_sdio_irq is called
> from the work, the IRQ has been processed, which tells the host driver
> to re-enable the SDIO IRQ.

So what I'm imagining is this:

1. mmc_sdio_suspend() starts; calls mmc_card_set_suspended() and
cancel_delayed_work_sync().

2. SDIO interrupt comes in; host controller calls sdio_signal_irq()

3. sdio_signal_irq() queues delayed work, which gets scheduled right away.

4. sdio_run_irqs() calls process_sdio_pending_irqs() which is a no-op
(because we're suspended)

5. sdio_run_irqs() calls host->ops->ack_sdio_irq(), which re-enables
more interrupts.

6. If SDIO interrupt was truly level triggered, we'll go straight back
to #2 because we never actually removed the true source of the
level-triggered interrupt by handling it.


We'll run steps #2 - #6 above ad nauseam until we finally manage to
get to the point in the suspend process where the system actually
masks/disables all driver interrupts.  This happens sometime _after_
the host controller's suspend call happens.  Technically this might
not really hurt anything (other than burning CPU cycles) because the
system workqueue isn't all that high priority so I think the suspend
can continue happening while we're looping.  ...but it still doesn't
seem great.

We don't end up in the above situation in my tests because the SDIO
interrupt was acting as an edge triggered interrupt.  ...and because,
as per below, we eventually turn the clock off.


> In the kthread case, this is managed by mmc_signal_sdio_irq() and the
> sdio_irq_thread() that calls host->ops->enable_sdio_irq() both to
> enable/disable the IRQ (but there are other problems with that).
>
> >
> > ======
> >
> > Overall, I can confirm that on my system your patch actually does
> > work.  ...so if all of the above concerns are moot and won't cause
> > anyone else problems then I can say that they don't seem to cause any
> > problems on my system.  On rk3288-veyron-jerry:
> >
> > - Before your patch, I got failures at iteration 18, then 32, then 55,
> > then 7, then 26.
> >
> > - After your patch I could do 100 iterations of suspend/resume with no
> > failures.  I also put printouts to confirm your patch was having an
> > effect.
>
> Great news, thanks a lot for testing and sharing these result.
>
> One more thing to consider. After the system suspend callback have
> been called for the mmc host driver (assuming SDIO IRQ isn't
> configured as system wakeup), the host driver shouldn't really receive
> SDIO IRQs and nor should it signal them via sdio_signal_irq(), simply
> because it has suspended its device/controller and beyond that point,
> the behavior might be undefined. Can you check to see if this is
> happening, or possibly you already know that this is the case and that
> we are "lucky"?

It's happening fine as long as we're loose with the term "after".  :-)
 Most certainly when we just finished executing the last line of the
host controller's suspend call then the system can't have done
anything to prevent interrupts from going off.  Even if the very next
thing that the core OS did was to disable interrupts there would still
be at least a few CPU instructions in there where we could have
finished the suspend call and interrupts were still enabled at the
system level.

It looks like the actual suspension of interrupts is in
suspend_device_irqs() which is called right before the "no irq" calls
are made.  ...so in theory we could still get interrupts for quite a
while after the host controller's suspend call.

In practice it actually looks to be impossible for dw_mmc, though.
...part of dw_mmc's suspend call turns off both the ciu (card clock)
and biu (bus clock).  I believe this means that the controller is
fully unclocked and there's no way it could give an interrupt.

In fact, the only time we actually get into trouble in dw_mmc is right
at the beginning of the resume code where we start re-initting the
host controller (and turning its clocks on) and then the interrupt
fires before we're quite ready.


-Doug

^ permalink raw reply

* Cleanup of -Wunused-const-variable in drivers/net/wireless/ti/wl18xx/main.c
From: Nathan Huckleberry @ 2019-06-13 18:00 UTC (permalink / raw)
  To: eliad, kvalo, davem; +Cc: netdev, linux-wireless, clang-built-linux

Hey all,

I'm looking into cleaning up ignored warnings in the kernel so we can
remove compiler flags to ignore warnings.

There are two unused variables ('wl18xx_iface_ap_cl_limits' and
'wl18xx_iface_ap_go_limits') in drivers/net/wireless/ti/wl18xx/main.c.
These appear to be limits when using p2p devices, yet they are never
used.

Wanted to reach out for the best course of action to fix the warning.

https://github.com/ClangBuiltLinux/linux/issues/530

Thanks,
Nathan Huckleberry

^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: Abort at too short BSS descriptor element
From: Brian Norris @ 2019-06-13 17:49 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-wireless, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Kalle Valo, huangwen, Solar Designer,
	Marcus Meissner
In-Reply-To: <20190529125220.17066-3-tiwai@suse.de>

Hi Takashi,

On Wed, May 29, 2019 at 02:52:20PM +0200, Takashi Iwai wrote:
> Currently mwifiex_update_bss_desc_with_ie() implicitly assumes that
> the source descriptor entries contain the enough size for each type
> and performs copying without checking the source size.  This may lead
> to read over boundary.
> 
> Fix this by putting the source size check in appropriate places.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 64ab6fe78c0d..c269a0de9413 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1269,6 +1269,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
>  			break;
>  
>  		case WLAN_EID_FH_PARAMS:
> +			if (element_len + 2 < sizeof(*fh_param_set))

"element_len + 2" would be much more readable as "total_ie_len". (Same for
several other usages in this patch.) I can send such a patch myself as a
follow-up I suppose.

> +				return -EINVAL;
>  			fh_param_set =
>  				(struct ieee_types_fh_param_set *) current_ptr;
>  			memcpy(&bss_entry->phy_param_set.fh_param_set,

[...]

> @@ -1349,6 +1361,9 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
>  			break;
>  
>  		case WLAN_EID_VENDOR_SPECIFIC:
> +			if (element_len + 2 < sizeof(vendor_ie->vend_hdr))

Why 'sizeof(vendor_ie->vend_hdr)'? The (mwifiex-specific compare with the
ieee80211.h generic struct ieee80211_vendor_ie) ieee_types_vendor_header struct
includes the 'oui_subtype' and 'version' fields, which are not standard
requirements for the vendor header (in fact, even the 4th byte of the
OUI -- "oui_type" -- doesn't appear to be in the 802.11 specification).
So it looks to me like you might be rejecting valid vendor headers (that
we should just be skipping) that might have vendor-specific content with
length 0 or 1 bytes.

It seems like we should only be validating the standard pieces (e.g., up to the
length/OUI), and only after an appropriate OUI match, *then* validating the rest of
the vendor element (the pieces we'll use later).

Brian

> +				return -EINVAL;
> +
>  			vendor_ie = (struct ieee_types_vendor_specific *)
>  					current_ptr;
>  
> -- 
> 2.16.4
> 

^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-12 21:59 UTC (permalink / raw)
  To: Larry Finger, Christoph Hellwig
  Cc: Aaro Koskinen, linux-wireless, linux-kernel, Christian Zigotzky,
	linuxppc-dev
In-Reply-To: <d6d82c0d-4a40-a191-0414-6b9a64547f65@lwfinger.net>

On Wed, 2019-06-12 at 14:41 -0500, Larry Finger wrote:
> On 6/12/19 1:55 AM, Christoph Hellwig wrote:
> > 
> > Ooops, yes.  But I think we could just enable ZONE_DMA on 32-bit
> > powerpc.  Crude enablement hack below:
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 8c1c636308c8..1dd71a98b70c 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -372,7 +372,7 @@ config PPC_ADV_DEBUG_DAC_RANGE
> >    
> >    config ZONE_DMA
> >        bool
> > -     default y if PPC_BOOK3E_64
> > +     default y
> >    
> >    config PGTABLE_LEVELS
> >        int
> > 
> 
> With the patch for Kconfig above, and the original patch setting 
> ARCH_ZONE_DMA_BITS to 30, everything works.
> 
> Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
> Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?

I think CONFIG_PPC32 is fine

Ben.


^ permalink raw reply

* Re: [PATCH] mmc: core: Prevent processing SDIO IRQs when the card is suspended
From: Doug Anderson @ 2019-06-12 22:20 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Linux MMC List, Adrian Hunter, Brian Norris, Shawn Lin,
	Guenter Roeck, Heiko Stuebner, Kalle Valo, linux-wireless, # 4.0+
In-Reply-To: <20190611123221.11580-1-ulf.hansson@linaro.org>

Hi,

On Tue, Jun 11, 2019 at 5:32 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Processing of SDIO IRQs must obviously be prevented while the card is
> system suspended, otherwise we may end up trying to communicate with an
> uninitialized SDIO card.
>
> Reports throughout the years shows that this is not only a theoretical
> problem, but a real issue. So, let's finally fix this problem, by keeping
> track of the state for the card and bail out before processing the SDIO
> IRQ, in case the card is suspended.
>
> Cc: stable@vger.kernel.org
> Reported-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> This has only been compile tested so far, any help for real test on HW is
> greatly appreciated.

Thanks for sending this!


> Note that, this is only the initial part of what is needed to make power
> management of SDIO card more robust, but let's start somewhere and continue to
> improve things.
>
> The next step I am looking at right now, is to make sure the SDIO IRQ is turned
> off during system suspend, unless it's supported as a system wakeup (and enabled
> to be used).

My gut says that the partway solution is going to be a problem on some
controllers / systems, even though it seems to work OK on mine.  See
my thoughts below and let me know what you think.


> ---
>  drivers/mmc/core/sdio.c     | 7 +++++++
>  drivers/mmc/core/sdio_irq.c | 4 ++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index d1aa1c7577bb..9951295d3220 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -937,6 +937,10 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
>   */
>  static int mmc_sdio_suspend(struct mmc_host *host)
>  {
> +       /* Prevent processing of SDIO IRQs in suspended state. */
> +       mmc_card_set_suspended(host->card);

Do you need to claim / release the host around the call to
mmc_card_set_suspended() to avoid races?



> +       cancel_delayed_work_sync(&host->sdio_irq_work);
> +
>         mmc_claim_host(host);
>
>         if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
> @@ -985,6 +989,9 @@ static int mmc_sdio_resume(struct mmc_host *host)
>                 err = sdio_enable_4bit_bus(host->card);
>         }
>
> +       /* Allow SDIO IRQs to be processed again. */
> +       mmc_card_clr_suspended(host->card);
> +

Do you need to check for "!err" before calling
mmc_card_clr_suspended()?  ...or add an "if (err) goto exit" type
thing and get rid of the "!err" check below?


>         if (!err && host->sdio_irqs) {
>                 if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD))
>                         wake_up_process(host->sdio_irq_thread);
> diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> index 931e6226c0b3..9f54a259a1b3 100644
> --- a/drivers/mmc/core/sdio_irq.c
> +++ b/drivers/mmc/core/sdio_irq.c
> @@ -34,6 +34,10 @@ static int process_sdio_pending_irqs(struct mmc_host *host)
>         unsigned char pending;
>         struct sdio_func *func;
>
> +       /* Don't process SDIO IRQs if the card is suspended. */
> +       if (mmc_card_suspended(card))
> +               return 0;
> +

Is it really OK to just return like this?  I guess there are two
(somewhat opposite) worries I'd have.  See A) and B) below:

A) Do we need to do anything extra to make sure we actually call the
interrupt handler after we've resumed?  I guess we can't actually
"lose" the interrupt since it will be sitting asserted in CCCR_INTx
until we deal with it (right?), but maybe we need to do something to
ensure the handler gets called once we're done resuming?

A1) old SDIO thread case

I think we'll be OK in the old SDIO thread case.  We'll call
wake_up_process() after we clear the suspended state and then we'll
either see "sdio_irq_pending" was set to true or we'll poll CCCR_INTx.

--

A2): new MMC_CAP2_SDIO_IRQ_NOTHREAD case

Should we do something to re-kick things?  We could call
sdio_signal_irq() in mmc_sdio_resume() I guess?  I was worried that
might conflict with those that call sdio_run_irqs() directly but it
seems like that's nobody as of commit 89f3c365f3e1 ("mmc: sdhci: Fix
SDIO IRQ thread deadlock").

NOTE: I put a bunch of debug printouts and I'm fairly convinced that
this is a real problem.  Sort of.  Specifically I confirmed that in
dw_mmc the SDIO interrupt seems to be treated as an edge-triggered
interrupt.  AKA: in dw_mci_interrupt() when we write to "RINTSTS" as
we're handling the interrupt the interrupt immediately stops
asserting.  It doesn't actually fire again until the Marvell SDIO
resume functions run.  I didn't dig enough to figure out what
specifically makes the interrupt fire again in the Marvell resume
functions, but it seems a little concerning that we're relying on
something in that driver to re-kick the host controller interrupt.

...side note: overall looking at this code path, two additional
questions come up for me.  One is why sdio_run_irqs() hardcodes
"sdio_irq_pending" as true.  That means we won't _ever_ poll CCCR_INTx
in the 1-function case, right?  That seems wrong.  The other is why
mmc_sdio_resume() always calls host->ops->enable_sdio_irq(host, 1) at
resume time when nobody ever turned the IRQs off.

===

B) Are there any instances where the interrupt will just keep firing
over and over again because we don't handle it?

As per above, this _isn't_ happening on dw_mmc on my setup because
dw_mmc seems to treat the SDIO interrupt as edge triggered.  ...but is
this true everywhere?  If we were using SDIO in 1-bit mode on dw_mmc,
would the interrupt re-assert right away?  If dw_mmc were configured
to use a dedicated pin would it re-assert right away?  What about
other host controllers?

If you're sure no host controllers will keep asserting the interrupt
over and over then I guess we don't need to worry about it?
...otherwise we'd need to find some way to mask the interrupt and we'd
need to make sure whatever we do doesn't interfere with anyone who
supports the SDIO interrupt as a wake source, right?

======

Overall, I can confirm that on my system your patch actually does
work.  ...so if all of the above concerns are moot and won't cause
anyone else problems then I can say that they don't seem to cause any
problems on my system.  On rk3288-veyron-jerry:

- Before your patch, I got failures at iteration 18, then 32, then 55,
then 7, then 26.

- After your patch I could do 100 iterations of suspend/resume with no
failures.  I also put printouts to confirm your patch was having an
effect.


I also confirmed that rk3288-veyron-minnie (which has Broadcom WiFi) I
could still suspend/resume fine with your patch.



-Doug

^ permalink raw reply

* Re: [PATCH-v3 2/2] mac80211: add assoc-at-ms support.
From: Ben Greear @ 2019-06-12 22:53 UTC (permalink / raw)
  To: linux-wireless
In-Reply-To: <20190415172123.6532-2-greearb@candelatech.com>

On 4/15/19 10:21 AM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Report when sta becomes associated.

While rebasing, I notice this file is just sitting in patchwork.

Any chance this patch and the previous one in the series can be accepted?

Thanks,
Ben

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


^ permalink raw reply

* iwlwifi/brcmfmac public action frames crash
From: James Prestwood @ 2019-06-13  0:18 UTC (permalink / raw)
  To: linux-wireless

Hi,

Both iwlwifi/brcmfmac seem to be unable to send public action frames to
an unassociated AP. I am attempting to do a GAS ANQP request with a
public action frame (via CMD_FRAME). Immediately after CMD_FRAME any of
the following happens depending on the card:

Intel 7260 (iwlwifi) - System lockup freeze (must hard reboot)
Intel 3160 (iwlwifi) - CMD_FRAME returns -EINVAL
BCM43602 (brcmfmac) - Kernel crash (below)
AR9462 (ath9k) - works
Random USB adapter (rt2800usb) - works

iwlwifi (on 7260) completely locks the system, where the only way to
recover is hard reboot. I have reproduced this on two separate systems,
both with a 7260. I *have* seen it not lock the system once although
lately it seems to happen every time. The 3160 did not cause a hang
with my limited testing, though it did not accept CMD_FRAME which is
likely why it never hung.

Not sure how I can get any more info about the iwlwifi problem as the
system is completely hung, but if there is a way I'll be happy to do
that.

Here is the brcmfmac crash:

[19735.643941] BUG: unable to handle kernel NULL pointer dereference at
0000000000000000
[19735.643965] PGD 80000001874aa067 P4D 80000001874aa067 PUD 2735fe067
PMD 0
[19735.643984] Oops: 0000 [#1] SMP PTI
[19735.643993] CPU: 7 PID: 5051 Comm: iwd Tainted: G        W
I       4.19.0-rc2-custom #27
[19735.644002] Hardware name: System manufacturer System Product
Name/SABERTOOTH X58, BIOS 1402    08/09/2012
[19735.644027] RIP: 0010:brcmf_p2p_send_action_frame+0x23a/0x850
[brcmfmac]
[19735.644037] Code: 41 c7 86 e0 00 00 00 00 00 00 00 f0 41 80 66 20 bf
f0 41 80 66 20 7f 49 8b 46 48 b9 24 07 00 00 48 89 da 48 c7 c6 3d 00 8f
c0 <48> 8b 38 e8 3e d7 ff ff 85 c0 41 89 c5 0f 85 c4 00 00 00 8b 03 49
[19735.644051] RSP: 0018:ffffa879c8477a00 EFLAGS: 00010246
[19735.644059] RAX: 0000000000000000 RBX: ffff954a2e059000 RCX:
0000000000000724
[19735.644067] RDX: ffff954a2e059000 RSI: ffffffffc08f003d RDI:
0000000000000002
[19735.644075] RBP: ffffa879c8477a50 R08: 000000000000001c R09:
0000000000000999
[19735.644083] R10: ffff954b157a2f00 R11: ffffffffc0720000 R12:
ffff954c32f26021
[19735.644091] R13: ffff954a2e059000 R14: ffff954c32f26000 R15:
00000000ffffffff
[19735.644099] FS:  00007f8d5aa30740(0000) GS:ffff954c369c0000(0000)
knlGS:0000000000000000
[19735.644108] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[19735.644115] CR2: 0000000000000000 CR3: 00000001845c8000 CR4:
00000000000006e0
[19735.644123] Call Trace:
[19735.644133]  ? _cond_resched+0x19/0x40
[19735.644153]  brcmf_cfg80211_mgmt_tx+0x170/0x2f0 [brcmfmac]
[19735.644192]  cfg80211_mlme_mgmt_tx+0x115/0x2f0 [cfg80211]
[19735.644219]  nl80211_tx_mgmt+0x24d/0x3d0 [cfg80211]
[19735.644228]  genl_family_rcv_msg+0x1fe/0x3f0
[19735.644237]  ? nlmon_xmit+0x2c/0x30
[19735.644246]  ? dev_hard_start_xmit+0xa8/0x210
[19735.644254]  genl_rcv_msg+0x4c/0x90
[19735.644261]  ? genl_family_rcv_msg+0x3f0/0x3f0
[19735.644268]  netlink_rcv_skb+0x54/0x130
[19735.644275]  genl_rcv+0x28/0x40
[19735.644281]  netlink_unicast+0x1ab/0x250
[19735.644288]  netlink_sendmsg+0x2d1/0x3d0
[19735.644297]  sock_sendmsg+0x3e/0x50
[19735.644304]  __sys_sendto+0x13f/0x180
[19735.644313]  ? do_epoll_wait+0xb0/0xc0
[19735.644321]  __x64_sys_sendto+0x28/0x30
[19735.644329]  do_syscall_64+0x5a/0x120
[19735.644336]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[19735.644344] RIP: 0033:0x7f8d5a352c4d
[19735.644350] Code: ff ff ff ff eb b6 0f 1f 80 00 00 00 00 48 8d 05 c1
dc 2c 00 41 89 ca 8b 00 85 c0 75 20 45 31 c9 45 31 c0 b8 2c 00 00 00 0f
05 <48> 3d 00 f0 ff ff 77 6b f3 c3 66 0f 1f 84 00 00 00 00 00 41 56 41
[19735.644365] RSP: 002b:00007ffc9a618048 EFLAGS: 00000246 ORIG_RAX:
000000000000002c
[19735.644374] RAX: ffffffffffffffda RBX: 00000000007077d0 RCX:
00007f8d5a352c4d
[19735.644382] RDX: 0000000000000068 RSI: 000000000072bc40 RDI:
0000000000000004
[19735.644390] RBP: 0000000000733510 R08: 0000000000000000 R09:
0000000000000000
[19735.644397] R10: 0000000000000000 R11: 0000000000000246 R12:
00007ffc9a618094
[19735.644405] R13: 00007ffc9a61809c R14: 0000000000000000 R15:
0000000000000000
[19735.644414] Modules linked in: ccm algif_aead snd_hda_codec_realtek
snd_hda_codec_generic snd_hda_codec_hdmi binfmt_misc arc4 nouveau
gpio_ich ath9k mxm_wmi ath9k_common video rt2800usb intel_powerclamp
snd_hda_intel ath9k_hw rt2x00usb iwlmvm rt2800lib snd_hda_codec
rt2x00lib ath snd_seq_midi snd_seq_midi_event coretemp ttm mac80211
snd_hda_core brcmfmac snd_hwdep snd_rawmidi iwlwifi intel_cstate
drm_kms_helper brcmutil snd_seq drm snd_pcm input_leds serio_raw
lpc_ich cfg80211 snd_seq_device i2c_algo_bit snd_timer fb_sys_fops
syscopyarea sysfillrect snd sysimgblt i5500_temp wmi asus_atk0110
soundcore mac_hid i7core_edac sch_fq_codel kvm_intel kvm vfio_pci
vfio_virqfd irqbypass vfio_iommu_type1 vfio pci_stub parport_pc ppdev
lp parport ip_tables x_tables autofs4 pata_acpi hid_generic usbhid hid
firewire_ohci
[19735.644521]  realtek psmouse firewire_core crc_itu_t r8169 i2c_i801
ahci libahci
[19735.644538] CR2: 0000000000000000
[19735.653612] ---[ end trace 30dbecd734da3b73 ]---
[19735.653641] RIP: 0010:brcmf_p2p_send_action_frame+0x23a/0x850
[brcmfmac]
[19735.653651] Code: 41 c7 86 e0 00 00 00 00 00 00 00 f0 41 80 66 20 bf
f0 41 80 66 20 7f 49 8b 46 48 b9 24 07 00 00 48 89 da 48 c7 c6 3d 00 8f
c0 <48> 8b 38 e8 3e d7 ff ff 85 c0 41 89 c5 0f 85 c4 00 00 00 8b 03 49
[19735.653659] RSP: 0018:ffffa879c8477a00 EFLAGS: 00010246
[19735.653672] RAX: 0000000000000000 RBX: ffff954a2e059000 RCX:
0000000000000724
[19735.653680] RDX: ffff954a2e059000 RSI: ffffffffc08f003d RDI:
0000000000000002
[19735.653688] RBP: ffffa879c8477a50 R08: 000000000000001c R09:
0000000000000999
[19735.653697] R10: ffff954b157a2f00 R11: ffffffffc0720000 R12:
ffff954c32f26021
[19735.653705] R13: ffff954a2e059000 R14: ffff954c32f26000 R15:
00000000ffffffff
[19735.653714] FS:  00007f8d5aa30740(0000) GS:ffff954c369c0000(0000)
knlGS:0000000000000000
[19735.653725] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[19735.653731] CR2: 0000000000000000 CR3: 00000001845c8000 CR4:
00000000000006e0

Thanks,
James


^ permalink raw reply

* Re: [PATCH v4 18/28] docs: convert docs to ReST and rename to *.rst
From: Srivatsa S. Bhat @ 2019-06-13  0:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet,
	Sebastian Reichel, Rafael J. Wysocki, Viresh Kumar, Len Brown,
	Pavel Machek, Nishanth Menon, Stephen Boyd, Liam Girdwood,
	Mark Brown, Mathieu Poirier, Suzuki K Poulose, Harry Wei,
	Alex Shi, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, x86, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	David Airlie, Daniel Vetter, Bjorn Helgaas, Johannes Berg,
	David S. Miller, linux-pm, linux-arm-kernel, intel-gfx, dri-devel,
	linux-pci, linux-wireless, netdev
In-Reply-To: <fac44e1fbab5ea755a93601a4fdfa34fcc57ae9e.1560361364.git.mchehab+samsung@kernel.org>

On 6/12/19 10:52 AM, Mauro Carvalho Chehab wrote:
> Convert the PM documents to ReST, in order to allow them to
> build with Sphinx.
> 
> The conversion is actually:
>   - add blank lines and identation in order to identify paragraphs;
>   - fix tables markups;
>   - add some lists markups;
>   - mark literal blocks;
>   - adjust title markups.
> 
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---

[...]

> diff --git a/Documentation/power/suspend-and-cpuhotplug.txt b/Documentation/power/suspend-and-cpuhotplug.rst
> similarity index 90%
> rename from Documentation/power/suspend-and-cpuhotplug.txt
> rename to Documentation/power/suspend-and-cpuhotplug.rst
> index a8751b8df10e..9df664f5423a 100644
> --- a/Documentation/power/suspend-and-cpuhotplug.txt
> +++ b/Documentation/power/suspend-and-cpuhotplug.rst
> @@ -1,10 +1,15 @@
> +====================================================================
>  Interaction of Suspend code (S3) with the CPU hotplug infrastructure
> +====================================================================
>  
> -     (C) 2011 - 2014 Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> +(C) 2011 - 2014 Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>  
>  
> -I. How does the regular CPU hotplug code differ from how the Suspend-to-RAM
> -   infrastructure uses it internally? And where do they share common code?
> +I. Differences between CPU hotplug and Suspend-to-RAM
> +======================================================
> +
> +How does the regular CPU hotplug code differ from how the Suspend-to-RAM
> +infrastructure uses it internally? And where do they share common code?
>  
>  Well, a picture is worth a thousand words... So ASCII art follows :-)
>  

[...]

> @@ -101,7 +108,7 @@ execution during resume):
>  
>  It is to be noted here that the system_transition_mutex lock is acquired at the very
>  beginning, when we are just starting out to suspend, and then released only
> -after the entire cycle is complete (i.e., suspend + resume).
> +after the entire cycle is complete (i.e., suspend + resume)::
>  

I think that should be a period, not a colon, because it is clarifying
the text above it (as opposed to referring to the example below it).

Other than that, for suspend-and-cpuhotplug.txt:

Acked-by: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>
 
Regards,
Srivatsa
VMware Photon OS

^ permalink raw reply

* Re: [PATCH] wlcore/wl18xx: Add invert-irq OF property for physically inverted IRQ
From: Harish Jenny K N @ 2019-06-13  4:36 UTC (permalink / raw)
  To: Eugeniu Rosca, Marc Zyngier
  Cc: Geert Uytterhoeven, Tony Lindgren, Kalle Valo, Eyal Reizer,
	Simon Horman, David S. Miller, Greg Kroah-Hartman, Randy Dunlap,
	Ulf Hansson, John Stultz, linux-wireless, netdev,
	Linux Kernel Mailing List, Spyridon Papageorgiou, Joshua Frkuska,
	George G . Davis, Andrey Gusakov, Linux-Renesas, Eugeniu Rosca,
	Thomas Gleixner, Jason Cooper, Linus Walleij
In-Reply-To: <20190612150644.GA22002@vmlxhi-102.adit-jv.com>


On 12/06/19 8:36 PM, Eugeniu Rosca wrote:
> Hi Marc,
>
> Thanks for your comment.
>
> On Wed, Jun 12, 2019 at 11:17:10AM +0100, Marc Zyngier wrote:
>> Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
>>> On Tue, Jun 11, 2019 at 10:00:41AM +0100, Marc Zyngier wrote:
> [..]
>>>> We already have plenty of that in the tree, the canonical example
>>>> probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty
>>>> easy to turn this driver into something more generic.
>>> I don't think drivers/irqchip/irq-mtk-sysirq.c can serve the
>>> use-case/purpose of this patch. The MTK driver seems to be dealing with
>>> the polarity inversion of on-SoC interrupts which are routed to GiC,
>>> whereas in this patch we are talking about an off-chip interrupt
>>> wired to R-Car GPIO controller.
>> And how different is that? The location of the interrupt source is
>> pretty irrelevant here.
> The main difference which I sense is that a driver like irq-mtk-sysirq
> mostly (if not exclusively) deals with internal kernel implementation
> detail (tuned via DT) whilst adding an inverter for GPIO IRQs raises
> a whole bunch of new questions (e.g. how to arbitrate between
> kernel-space and user-space IRQ polarity configuration?).
>
>> The point is that there is already a general
>> scheme to deal with these "signal altering widgets", and that we
>> should try to reuse at least the concept, if not the code.
> Since Harish Jenny K N might be working on a new driver doing GPIO IRQ
> inversion, I have CC-ed him as well to avoid any overlapping work.


Sorry I am not completely aware of the background discussion.

But here is the link to my proposal for new consumer driver to provide a new virtual
gpio controller to configure the polarity of the gpio pins used by the userspace.

https://www.spinics.net/lists/linux-gpio/msg39681.html


>
>>> It looks to me that the nice DTS sketch shared by Linus Walleij in [5]
>>> might come closer to the concept proposed by Geert? FWIW, the
>>> infrastructure/implementation to make this possible is still not
>>> ready.
>> Which looks like what I'm suggesting.
> Then we are on the same page. Thanks.
>
>> 	M.
>>
>> -- 
>> Jazz is not dead, it just smells funny.


^ permalink raw reply

* iwlwifi/brcmfmac public action frames crash (RESENDING)
From: James Prestwood @ 2019-06-13 16:45 UTC (permalink / raw)
  To: linux-wireless

Sorry if this comes in twice, I sent it ~12 hours ago but never saw it
hit the list, nor in the archives so I am resending it.

Hi,

Both iwlwifi/brcmfmac seem to be unable to send public action frames to
an unassociated AP. I am attempting to do a GAS ANQP request with a
public action frame (via CMD_FRAME). Immediately after CMD_FRAME any of
the following happens depending on the card:

Intel 7260 (iwlwifi) - System lockup freeze (must hard reboot)
Intel 3160 (iwlwifi) - CMD_FRAME returns -EINVAL
BCM43602 (brcmfmac) - Kernel crash (below)
AR9462 (ath9k) - works
Random USB adapter (rt2800usb) - works

iwlwifi (on 7260) completely locks the system, where the only way to
recover is hard reboot. I have reproduced this on two separate systems,
both with a 7260. I *have* seen it not lock the system once although
lately it seems to happen every time. The 3160 did not cause a hang
with my limited testing, though it did not accept CMD_FRAME which is
likely why it never hung.

Not sure how I can get any more info about the iwlwifi problem as the
system is completely hung, but if there is a way I'll be happy to do
that.

Here is the brcmfmac crash:

[19735.643941] BUG: unable to handle kernel NULL pointer dereference at
0000000000000000
[19735.643965] PGD 80000001874aa067 P4D 80000001874aa067 PUD 2735fe067
PMD 0
[19735.643984] Oops: 0000 [#1] SMP PTI
[19735.643993] CPU: 7 PID: 5051 Comm: iwd Tainted: G        W
I       4.19.0-rc2-custom #27
[19735.644002] Hardware name: System manufacturer System Product
Name/SABERTOOTH X58, BIOS 1402    08/09/2012
[19735.644027] RIP: 0010:brcmf_p2p_send_action_frame+0x23a/0x850
[brcmfmac]
[19735.644037] Code: 41 c7 86 e0 00 00 00 00 00 00 00 f0 41 80 66 20 bf
f0 41 80 66 20 7f 49 8b 46 48 b9 24 07 00 00 48 89 da 48 c7 c6 3d 00 8f
c0 <48> 8b 38 e8 3e d7 ff ff 85 c0 41 89 c5 0f 85 c4 00 00 00 8b 03 49
[19735.644051] RSP: 0018:ffffa879c8477a00 EFLAGS: 00010246
[19735.644059] RAX: 0000000000000000 RBX: ffff954a2e059000 RCX:
0000000000000724
[19735.644067] RDX: ffff954a2e059000 RSI: ffffffffc08f003d RDI:
0000000000000002
[19735.644075] RBP: ffffa879c8477a50 R08: 000000000000001c R09:
0000000000000999
[19735.644083] R10: ffff954b157a2f00 R11: ffffffffc0720000 R12:
ffff954c32f26021
[19735.644091] R13: ffff954a2e059000 R14: ffff954c32f26000 R15:
00000000ffffffff
[19735.644099] FS:  00007f8d5aa30740(0000) GS:ffff954c369c0000(0000)
knlGS:0000000000000000
[19735.644108] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[19735.644115] CR2: 0000000000000000 CR3: 00000001845c8000 CR4:
00000000000006e0
[19735.644123] Call Trace:
[19735.644133]  ? _cond_resched+0x19/0x40
[19735.644153]  brcmf_cfg80211_mgmt_tx+0x170/0x2f0 [brcmfmac]
[19735.644192]  cfg80211_mlme_mgmt_tx+0x115/0x2f0 [cfg80211]
[19735.644219]  nl80211_tx_mgmt+0x24d/0x3d0 [cfg80211]
[19735.644228]  genl_family_rcv_msg+0x1fe/0x3f0
[19735.644237]  ? nlmon_xmit+0x2c/0x30
[19735.644246]  ? dev_hard_start_xmit+0xa8/0x210
[19735.644254]  genl_rcv_msg+0x4c/0x90
[19735.644261]  ? genl_family_rcv_msg+0x3f0/0x3f0
[19735.644268]  netlink_rcv_skb+0x54/0x130
[19735.644275]  genl_rcv+0x28/0x40
[19735.644281]  netlink_unicast+0x1ab/0x250
[19735.644288]  netlink_sendmsg+0x2d1/0x3d0
[19735.644297]  sock_sendmsg+0x3e/0x50
[19735.644304]  __sys_sendto+0x13f/0x180
[19735.644313]  ? do_epoll_wait+0xb0/0xc0
[19735.644321]  __x64_sys_sendto+0x28/0x30
[19735.644329]  do_syscall_64+0x5a/0x120
[19735.644336]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[19735.644344] RIP: 0033:0x7f8d5a352c4d
[19735.644350] Code: ff ff ff ff eb b6 0f 1f 80 00 00 00 00 48 8d 05 c1
dc 2c 00 41 89 ca 8b 00 85 c0 75 20 45 31 c9 45 31 c0 b8 2c 00 00 00 0f
05 <48> 3d 00 f0 ff ff 77 6b f3 c3 66 0f 1f 84 00 00 00 00 00 41 56 41
[19735.644365] RSP: 002b:00007ffc9a618048 EFLAGS: 00000246 ORIG_RAX:
000000000000002c
[19735.644374] RAX: ffffffffffffffda RBX: 00000000007077d0 RCX:
00007f8d5a352c4d
[19735.644382] RDX: 0000000000000068 RSI: 000000000072bc40 RDI:
0000000000000004
[19735.644390] RBP: 0000000000733510 R08: 0000000000000000 R09:
0000000000000000
[19735.644397] R10: 0000000000000000 R11: 0000000000000246 R12:
00007ffc9a618094
[19735.644405] R13: 00007ffc9a61809c R14: 0000000000000000 R15:
0000000000000000
[19735.644414] Modules linked in: ccm algif_aead snd_hda_codec_realtek
snd_hda_codec_generic snd_hda_codec_hdmi binfmt_misc arc4 nouveau
gpio_ich ath9k mxm_wmi ath9k_common video rt2800usb intel_powerclamp
snd_hda_intel ath9k_hw rt2x00usb iwlmvm rt2800lib snd_hda_codec
rt2x00lib ath snd_seq_midi snd_seq_midi_event coretemp ttm mac80211
snd_hda_core brcmfmac snd_hwdep snd_rawmidi iwlwifi intel_cstate
drm_kms_helper brcmutil snd_seq drm snd_pcm input_leds serio_raw
lpc_ich cfg80211 snd_seq_device i2c_algo_bit snd_timer fb_sys_fops
syscopyarea sysfillrect snd sysimgblt i5500_temp wmi asus_atk0110
soundcore mac_hid i7core_edac sch_fq_codel kvm_intel kvm vfio_pci
vfio_virqfd irqbypass vfio_iommu_type1 vfio pci_stub parport_pc ppdev
lp parport ip_tables x_tables autofs4 pata_acpi hid_generic usbhid hid
firewire_ohci
[19735.644521]  realtek psmouse firewire_core crc_itu_t r8169 i2c_i801
ahci libahci
[19735.644538] CR2: 0000000000000000
[19735.653612] ---[ end trace 30dbecd734da3b73 ]---
[19735.653641] RIP: 0010:brcmf_p2p_send_action_frame+0x23a/0x850
[brcmfmac]
[19735.653651] Code: 41 c7 86 e0 00 00 00 00 00 00 00 f0 41 80 66 20 bf
f0 41 80 66 20 7f 49 8b 46 48 b9 24 07 00 00 48 89 da 48 c7 c6 3d 00 8f
c0 <48> 8b 38 e8 3e d7 ff ff 85 c0 41 89 c5 0f 85 c4 00 00 00 8b 03 49
[19735.653659] RSP: 0018:ffffa879c8477a00 EFLAGS: 00010246
[19735.653672] RAX: 0000000000000000 RBX: ffff954a2e059000 RCX:
0000000000000724
[19735.653680] RDX: ffff954a2e059000 RSI: ffffffffc08f003d RDI:
0000000000000002
[19735.653688] RBP: ffffa879c8477a50 R08: 000000000000001c R09:
0000000000000999
[19735.653697] R10: ffff954b157a2f00 R11: ffffffffc0720000 R12:
ffff954c32f26021
[19735.653705] R13: ffff954a2e059000 R14: ffff954c32f26000 R15:
00000000ffffffff
[19735.653714] FS:  00007f8d5aa30740(0000) GS:ffff954c369c0000(0000)
knlGS:0000000000000000
[19735.653725] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[19735.653731] CR2: 0000000000000000 CR3: 00000001845c8000 CR4:
00000000000006e0

Thanks,
James


^ permalink raw reply

* RE: [EXT] Re: mwifiex: update set_mac_address logic
From: Ganapathi Bhat @ 2019-06-13  6:55 UTC (permalink / raw)
  To: Kalle Valo
  Cc: linux-wireless@vger.kernel.org, Cathy Luo, Zhiyuan Yang,
	James Cao, Rakesh Parmar, Brian Norris
In-Reply-To: <87pnqbxhra.fsf@purkki.adurom.net>

Hi Kalle,

I did resend the patch with 'git send-email': https://patchwork.kernel.org/patch/10990209/

Kindly review;

Thanks,
Ganapathi

^ permalink raw reply

* [PATCH v2 1/2] mt76: mt7615: fix incorrect settings in mesh mode
From: Ryder Lee @ 2019-06-13  7:13 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, linux-wireless,
	linux-mediatek, linux-kernel, Ryder Lee
In-Reply-To: <ec6ccb9ad8e083bbc960c84adcb3164a4e190c7c.1560338170.git.ryder.lee@mediatek.com>

Fix wrong settings that will drop packets due to hardware's RX table
searching flow.

Fixes: f072c7ba2150 ("mt76: mt7615: enable support for mesh")
Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 76431d00a8ac..e82086eb8aa4 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -970,7 +970,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct ieee80211_vif *vif,
 		.rx_wtbl = {
 			.tag = cpu_to_le16(WTBL_RX),
 			.len = cpu_to_le16(sizeof(struct wtbl_rx)),
-			.rca1 = vif->type == NL80211_IFTYPE_STATION,
+			.rca1 = vif->type != NL80211_IFTYPE_AP,
 			.rca2 = 1,
 			.rv = 1,
 		},
-- 
2.18.0


^ permalink raw reply related

* [PATCH v3] mt76: mt7615: add support for per-chain signal strength reporting
From: Ryder Lee @ 2019-06-13  7:13 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, linux-wireless,
	linux-mediatek, linux-kernel, Ryder Lee

Fill in RX status->chain_signal to avoid empty value.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
Changes since v3 - use hweight8 instead.
Changes since v2 - correct calculation sequence.
---
 .../net/wireless/mediatek/mt76/mt7615/mac.c   | 22 ++++++++++++++++++-
 .../net/wireless/mediatek/mt76/mt7615/mac.h   |  5 +++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mac.c b/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
index b60d42b5923d..1eb0e9c9970c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
@@ -13,6 +13,11 @@
 #include "../dma.h"
 #include "mac.h"
 
+static inline s8 to_rssi(u32 field, u32 rxv)
+{
+	return (FIELD_GET(field, rxv) - 220) / 2;
+}
+
 static struct mt76_wcid *mt7615_rx_get_wcid(struct mt7615_dev *dev,
 					    u8 idx, bool unicast)
 {
@@ -120,6 +125,7 @@ int mt7615_mac_fill_rx(struct mt7615_dev *dev, struct sk_buff *skb)
 	if (rxd0 & MT_RXD0_NORMAL_GROUP_3) {
 		u32 rxdg0 = le32_to_cpu(rxd[0]);
 		u32 rxdg1 = le32_to_cpu(rxd[1]);
+		u32 rxdg3 = le32_to_cpu(rxd[3]);
 		u8 stbc = FIELD_GET(MT_RXV1_HT_STBC, rxdg0);
 		bool cck = false;
 
@@ -169,7 +175,21 @@ int mt7615_mac_fill_rx(struct mt7615_dev *dev, struct sk_buff *skb)
 
 		status->enc_flags |= RX_ENC_FLAG_STBC_MASK * stbc;
 
-		/* TODO: RSSI */
+		status->chains = dev->mt76.antenna_mask;
+		status->chain_signal[0] = to_rssi(MT_RXV4_RCPI0, rxdg3);
+		status->chain_signal[1] = to_rssi(MT_RXV4_RCPI1, rxdg3);
+		status->chain_signal[2] = to_rssi(MT_RXV4_RCPI2, rxdg3);
+		status->chain_signal[3] = to_rssi(MT_RXV4_RCPI3, rxdg3);
+		status->signal = status->chain_signal[0];
+
+		for (i = 1; i < hweight8(dev->mt76.antenna_mask); i++) {
+			if (!(status->chains & BIT(i)))
+				continue;
+
+			status->signal = max(status->signal,
+					     status->chain_signal[i]);
+		}
+
 		rxd += 6;
 		if ((u8 *)rxd - skb->data >= skb->len)
 			return -EINVAL;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mac.h b/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
index 18ad4b8a3807..b00ce8db58e9 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
@@ -98,6 +98,11 @@ enum rx_pkt_type {
 #define MT_RXV2_GROUP_ID		GENMASK(26, 21)
 #define MT_RXV2_LENGTH			GENMASK(20, 0)
 
+#define MT_RXV4_RCPI3			GENMASK(31, 24)
+#define MT_RXV4_RCPI2			GENMASK(23, 16)
+#define MT_RXV4_RCPI1			GENMASK(15, 8)
+#define MT_RXV4_RCPI0			GENMASK(7, 0)
+
 enum tx_header_format {
 	MT_HDR_FORMAT_802_3,
 	MT_HDR_FORMAT_CMD,
-- 
2.18.0


^ permalink raw reply related

* [PATCH v2 2/2] mt76: mt7615: update peer's bssid when state transition occurs
From: Ryder Lee @ 2019-06-13  7:13 UTC (permalink / raw)
  To: Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, linux-wireless,
	linux-mediatek, linux-kernel, Ryder Lee
In-Reply-To: <3065a01998dfa04a5d2d680e820a17cb5c110d0f.1560330906.git.ryder.lee@mediatek.com>

This makes sure that the driver update peer's bssid when state
transition occurs.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
Changes since v2 - remove unnecessary changes
---
 .../net/wireless/mediatek/mt76/mt7615/main.c  |  5 ++--
 .../net/wireless/mediatek/mt76/mt7615/mcu.c   | 27 ++++++++++---------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index d21407ddda31..e0824392c019 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -283,9 +283,8 @@ static void mt7615_bss_info_changed(struct ieee80211_hw *hw,
 
 	mutex_lock(&dev->mt76.mutex);
 
-	/* TODO: sta mode connect/disconnect
-	 * BSS_CHANGED_ASSOC | BSS_CHANGED_BSSID
-	 */
+	if (changed & BSS_CHANGED_ASSOC)
+		mt7615_mcu_set_bss_info(dev, vif, info->assoc);
 
 	/* TODO: update beacon content
 	 * BSS_CHANGED_BEACON
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index e82086eb8aa4..52d89f3533bf 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -759,22 +759,23 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
 		conn_type = CONNECTION_INFRA_AP;
 		break;
 	case NL80211_IFTYPE_STATION: {
-		struct ieee80211_sta *sta;
-		struct mt7615_sta *msta;
-
-		rcu_read_lock();
-
-		sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
-		if (!sta) {
+		/* TODO: enable BSS_INFO_UAPSD & BSS_INFO_PM */
+		if (en) {
+			struct ieee80211_sta *sta;
+			struct mt7615_sta *msta;
+
+			rcu_read_lock();
+			sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
+			if (!sta) {
+				rcu_read_unlock();
+				return -EINVAL;
+			}
+
+			msta = (struct mt7615_sta *)sta->drv_priv;
+			tx_wlan_idx = msta->wcid.idx;
 			rcu_read_unlock();
-			return -EINVAL;
 		}
-
-		msta = (struct mt7615_sta *)sta->drv_priv;
-		tx_wlan_idx = msta->wcid.idx;
 		conn_type = CONNECTION_INFRA_STA;
-
-		rcu_read_unlock();
 		break;
 	}
 	default:
-- 
2.18.0


^ permalink raw reply related

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-13  7:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Larry Finger, Christoph Hellwig, Aaro Koskinen, linux-wireless,
	linux-kernel, Christian Zigotzky, linuxppc-dev
In-Reply-To: <05db995c55ad7c1002124374c139d2c0812ad034.camel@kernel.crashing.org>

On Thu, Jun 13, 2019 at 07:59:51AM +1000, Benjamin Herrenschmidt wrote:
> > With the patch for Kconfig above, and the original patch setting 
> > ARCH_ZONE_DMA_BITS to 30, everything works.
> > 
> > Do you have any ideas on what should trigger the change in ARCH_ZONE_BITS? 
> > Should it be CONFIG_PPC32 defined, or perhaps CONFIG_G4_CPU defined?
> 
> I think CONFIG_PPC32 is fine

I'll cook up a patch unless someone beats me to it.

^ permalink raw reply

* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Stanislaw Gruszka @ 2019-06-13  7:51 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612144400.GB7016@localhost.localdomain>

On Wed, Jun 12, 2019 at 04:44:01PM +0200, Lorenzo Bianconi wrote:
> On Jun 12, Stanislaw Gruszka wrote:
> > On Wed, Jun 12, 2019 at 02:59:05PM +0200, Stanislaw Gruszka wrote:
> > > > > If max RX AMSDU size is 3839B I do not see reason why we allocate
> > > > > MT_SG_MAX_SIZE=8 of MT_RX_BUF_SIZE=2k buffers for sg_en case.
> > > > > I thought the reason is that max AMSDU size is 16kB so it fit into
> > > > > 8 sg buffers of 2k.
> > > > > 
> > > > > In other words, for me, looks like either
> > > > > - we can not handle AMSDU for non sg case because we do not
> > > > > allocate big enough buffer
> > > > 
> > > > I think AMSDU is mandatory and we currently support it even for non-sg case
> > > > (since max rx AMSDU is 3839B)
> > > > 
> > > > > or
> > > > > - we can just use one PAGE_SIZE buffer for rx and remove sg
> > > > > buffers for rx completely 
> > > > 
> > > > using sg buffers we can support bigger rx AMSDU size in the future without using
> > > > huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
> > > > mt76x2u)
> > > 
> > > I think it would be simpler just to allocate 2 pages for 7935B .
> > 
> > And if we could determine that there is no true need to use sg for rx,
> > I think best approach here would be revert f8f527b16db5 in v5.2 to fix
> > regression and remove rx sg in -next. That would make code simpler,
> > allocate 4k instead 16k per packet, allow to use build_skb (4096 - 3839
> > give enough space for shared info) and not use usb hcd bounce buffer.
> 
> I do not think we should drop sg support since:
> - it allow us to rx huge amsdu frames (e.g. IEEE80211_MAX_MPDU_LEN_VHT_11454)
>   using multiple one page buffer. I think there will be new usb devices where we can
>   increase amsdu size (we can increase it even on mt76x2u usb 3.0 devices)

I looked at intel wifi drivers and this is handled by amsdu_size module
parameter, supported values are 4k, 8k and 12k. RX allocation size and
proper values in vht_cap & ht_cap are set accordingly. Assuming (some)
mt76 HW and FW can handle bigger AMSDUs I think we should do similar
thing.

Otherwise looks for me, we just waste memory and have not needed code
for no true reason. 

>   space needed for skb_shared_info is 320B on a x86_64 device

Uhh, I haven't expected that sk_shared_info() is that big, so indeed build_skb
could not used and 128B copy fallback will be necessary.


Stanislaw

^ permalink raw reply

* Re: [EXT] Re: mwifiex: update set_mac_address logic
From: Kalle Valo @ 2019-06-13  7:52 UTC (permalink / raw)
  To: Ganapathi Bhat
  Cc: linux-wireless@vger.kernel.org, Cathy Luo, Zhiyuan Yang,
	James Cao, Rakesh Parmar, Brian Norris
In-Reply-To: <MN2PR18MB26370061E95E43F826F4E53FA0EF0@MN2PR18MB2637.namprd18.prod.outlook.com>

Ganapathi Bhat <gbhat@marvell.com> writes:

> I did resend the patch with 'git send-email':
> https://patchwork.kernel.org/patch/10990209/

Thanks, I see it now.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Stanislaw Gruszka @ 2019-06-13  7:55 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190612142741.GA7016@localhost.localdomain>

On Wed, Jun 12, 2019 at 04:27:42PM +0200, Lorenzo Bianconi wrote:
> > On Wed, Jun 12, 2019 at 02:28:48PM +0200, Lorenzo Bianconi wrote:
> 
> [...]
> 
> > > 
> > > using sg buffers we can support bigger rx AMSDU size in the future without using
> > > huge buffers (e.g. we can try to use IEEE80211_MAX_MPDU_LEN_HT_7935 with
> > > mt76x2u)
> > I think it would be simpler just to allocate 2 pages for 7935B .
> > 
> 
> We should avoid increasing buffer size to more than PAGE_SIZE for
> performance reasons.

Interesting, at what place and how this affect performance negatively ?

> As suggested by Felix what about of setting buf_size to
> PAGE_SIZE for both sg and non-sg cases and for sg setting usb data size to
> 
> SKB_WITH_OVERHEAD(q->buf_size) & (usb_endpoint_maxp() - 1);

Increasing to PAGE_SIZE for sg looks reasonable to me.

Not sure if understand data_size logic. If this mean 'PAGE_SIZE - usb_endpint_maxp()',
looks ok to me as well (for first segment), but would require one extra segment
to avoid coping (i.e. 2 pages for 3839 , 3 pages for 7935 ...) If we would 
like to stay with 128B copy fallback, we can have 1 page for 3839 , 2 for 7935 ...

It would be interesting how frequently AMSDU of max size is sent by
remote station. If this is rare situation I would be opting for
smaller allocation and 128B copy fallback. If this is frequent
for bigger allocation to assure we can always use build_skb().

Stanislaw 

^ permalink raw reply

* Re: [PATCH v2 2/2] mt76: usb: do not always copy the first part of received frames
From: Lorenzo Bianconi @ 2019-06-13  8:26 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, kvalo, linux-wireless
In-Reply-To: <20190613075108.GA24650@redhat.com>

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

[...]

> I looked at intel wifi drivers and this is handled by amsdu_size module
> parameter, supported values are 4k, 8k and 12k. RX allocation size and
> proper values in vht_cap & ht_cap are set accordingly. Assuming (some)
> mt76 HW and FW can handle bigger AMSDUs I think we should do similar
> thing.
> 
> Otherwise looks for me, we just waste memory and have not needed code
> for no true reason. 
> 
> >   space needed for skb_shared_info is 320B on a x86_64 device
> 
> Uhh, I haven't expected that sk_shared_info() is that big, so indeed build_skb
> could not used and 128B copy fallback will be necessary.

Hi Stanislaw,

reviewing the original patch I think we can't trigger any IOMMU bug since the
usb buffer length is actually 2048 and not 2048 + skb_shared_info_size:

in mt76u_fill_rx_sg()
data_size = SKB_WITH_OVERHEAD(q->buf_size);

q->buf_size = MT_BUF_WITH_OVERHEAD(MT_RX_BUF_SIZE);

where MT_BUF_WITH_OVERHEAD is
((x) + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))

anyway we can even set
q->buf_size = PAGE_SIZE
data_size = rounddown(SKB_WITH_OVERHEAD(q->buf_size), usb_endpoint_maxp())

Regards,
Lorenzo

> 
> 
> Stanislaw

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ 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