Re: pull-request: mac80211 2019-10-08
On Wed, 09 Oct 2019 08:36:57 +0200, Johannes Berg wrote: > Hi Jakub, > > > Pulled into net. Let me know if did it wrong :) > > Oops, didn't know it was your "turn" again, guess I haven't been reading > netdev enough. It's more of a ad hoc whenever Dave needs to step away for a day or two thing, than a schedule. Also I'm quite happy to pick things up from patchwork and the mailing list, so no real need to CC me, anyway :) > Looks good, but I didn't think this could possibly go wrong :) > > > FWIW there was this little complaint from checkpatch: > [...] > > WARNING: Duplicate signature > > #14: > > Signed-off-by: Johannes Berg > > Hmm, yeah, so ... I was actually not sure about that and I guess it > slipped by. Most of the time, I've been editing it out, but what happens > is this: > > 1) I send a patch to our internal tree, to fix up some things. Unless > it's really urgent, I don't necessarily post it externally at the > same time. This obviously has my S-o-b. > 2) Luca goes through our internal tree and sends out the patches to the > list, adding his S-o-b. > 3) For the patches to the stack, I apply them, and git-am adds my S-o-b > again because it's not the last. > > So now we have > > S-o-b: Johannes > S-o-b: Luca > S-o-b: Johannes > > If I edit it just to be "S-o-b: Johannes", then it looks strange because > I've applied a patch from the list and dropped an S-o-b. It's still my > code, and Luca doesn't normally have to make any changes to it, but ... > This is what I've normally been doing I think, but it always felt a bit > weird because then it's not the patch I actually applied, it's like I > pretend the whole process described above never happened. > > If I edit and remove my first S-o-b then it's weird because the Author > isn't the first S-o-b, making it look like I didn't sign it off when I > authored it? > > If I edit and remove the last S-o-b, how did it end up in my tree? > > So basically my first S-o-b is certifying (a) or maybe occasionally (b) > under the DCO, while Luca's and my second are certifying (c) (and maybe > occasionally also (a) or (b) if any changes were made.) > > > Is there any convention on this that I could adhere to? :) Thanks for the explanation, seems like a reasonable stand so as long as you're aware this is happening, I'm happy :)
Re: pull-request: mac80211 2019-10-08
On Tue, 8 Oct 2019 14:31:10 +0200, Johannes Berg wrote: > Hi Dave, > > Another week, another set of fixes. > > Please pull and let me know if there's any problem. Pulled into net. Let me know if did it wrong :) FWIW there was this little complaint from checkpatch: --- 0006-mac80211-accept-deauth-frames-in-IBSS-mode.patch --- WARNING: Duplicate signature #14: Signed-off-by: Johannes Berg total: 0 errors, 1 warnings, 0 checks, 19 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. 0006-mac80211-accept-deauth-frames-in-IBSS-mode.patch has style problems, please review.
Re: [PATCH] mt7601u: use params->ssn value directly
On Fri, 12 Jul 2019 14:09:50 +0200, Stanislaw Gruszka wrote: > There is no point to use pointer to params->ssn. > > Signed-off-by: Stanislaw Gruszka Acked-by: Jakub Kicinski
Re: [PATCH v2 0/2] mt7601u: do not schedule {rx,tx}_tasklet when the device has been disconnected
On Fri, 7 Jun 2019 13:48:08 +0200, Lorenzo Bianconi wrote:
> Do not schedule {tx,rx}_tasklet when the usb dongle is disconnected in order
> to fix
> {TX,RX} urb mismatch warning
> Fix possible memory leak when the device is disconnected while passing
> traffic.
Acked-by: Jakub Kicinski
Thanks Lorenzo!
Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
On Fri, 7 Jun 2019 00:05:56 +0200, Lorenzo Bianconi wrote: > I guess this can be a future improvement, do you agree? Def. > Do I need to resubmit this patch? My only concern is the Fixes tag, then. In the good old days we could just wait a bit ourselves, now some bot will autoselect it, ugh. But up to you, for the code: Acked-by: Jakub Kicinski
Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
On Thu, 6 Jun 2019 23:10:15 +0200, Lorenzo Bianconi wrote:
> On Thu, Jun 6, 2019 at 8:49 PM Jakub Kicinski wrote:
> >
> > On Thu, 6 Jun 2019 17:34:16 +0200, Lorenzo Bianconi wrote:
> > > When the device is disconnected while passing traffic it is possible
> > > to receive out of order urbs causing a memory leak since the skb liked
> > > to the current tx urb is not removed. Fix the issue deallocating the skb
> > > cleaning up the tx ring. Moreover this patch fixes the following kernel
> > > warning
> >
> > Ugh if we don't have ordering guarantees then the entire "ring" scheme
> > no longer works :( Should we move to URB queues, I don't remember now,
> > but there seem to had been a better way to manage URBs :S
> >
>
> actually I have observed these issues on tx/rx side just during device
> disconnection while passing traffic.
> I guess we can assume a proper urb ordering during normal operation
> (and so tx/rx ring works fine).
Ah, phew, okay. That's fine then.
> Btw what do you mean with 'URB queues'? :)
I think it may have been URB anchors. I remember looking at carl9170
and liking how the DMAs operated there.
> > > [ 57.480771] usb 1-1: USB disconnect, device number 2
> > > [ 57.483451] [ cut here ]
> > > [ 57.483462] TX urb mismatch
> > > [ 57.483481] WARNING: CPU: 1 PID: 32 at
> > > drivers/net/wireless/mediatek/mt7601u/dma.c:245
> > > mt7601u_complete_tx+0x165/00
> > > [ 57.483483] Modules linked in:
> > > [ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
> > > [ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> > > 1.12.0-2.fc30 04/01/2014
> > > [ 57.483502] Workqueue: usb_hub_wq hub_event
> > > [ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
> > > [ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1
> > > cb e1 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
> > > [ 57.483513] RSP: :c90a0d28 EFLAGS: 00010092
> > > [ 57.483516] RAX: 000f RBX: 88802c0a62c0 RCX:
> > > c90a0c2c
> > > [ 57.483518] RDX: RSI: RDI:
> > > 810a8371
> > > [ 57.483520] RBP: 88803ced6858 R08: R09:
> > > 0001
> > > [ 57.483540] R10: 0002 R11: R12:
> > > 0046
> > > [ 57.483542] R13: 88802c0a6c88 R14: 88803baab540 R15:
> > > 88803a0cc078
> > > [ 57.483548] FS: () GS:88803eb0()
> > > knlGS:
> > > [ 57.483550] CS: 0010 DS: ES: CR0: 80050033
> > > [ 57.483552] CR2: 55e7f6780100 CR3: 28c86000 CR4:
> > > 06a0
> > > [ 57.483554] DR0: DR1: DR2:
> > >
> > > [ 57.483556] DR3: DR6: fffe0ff0 DR7:
> > > 0400
> > > [ 57.483559] Call Trace:
> > > [ 57.483561]
> > > [ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
> > > [ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
> > > [ 57.483574] handle_cmd_completion+0xf5b/0x12c0
> > > [ 57.483577] xhci_irq+0x1f6/0x1810
> > > [ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
> > > [ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
> > > [ 57.483588] __handle_irq_event_percpu+0x3a/0x260
> > > [ 57.483592] handle_irq_event_percpu+0x1c/0x60
> > > [ 57.483595] handle_irq_event+0x2f/0x4c
> > > [ 57.483599] handle_edge_irq+0x7e/0x1a0
> > > [ 57.483603] handle_irq+0x17/0x20
> > > [ 57.483607] do_IRQ+0x54/0x110
> > > [ 57.483610] common_interrupt+0xf/0xf
> > > [ 57.483612]
> > >
> > > Fixes: c869f77d6abb ("add mt7601u driver")
> > > Signed-off-by: Lorenzo Bianconi
> > > ---
> > > drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++--
> > > drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
> > > 2 files changed, 19 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > index e7703990b291..bbf1deed7f3b 100644
> > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> &
Re: [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
On Thu, 6 Jun 2019 23:02:08 +0200, Lorenzo Bianconi wrote:
> > On Thu, 6 Jun 2019 14:26:12 +0200, Lorenzo Bianconi wrote:
> Hi Jakub,
>
> thx for the fast review :)
I guess I'm used to the 24h "review timeout" on netdev :)
> > Is this a new thing? I def tested unplugging the dongle under
> > traffic, but that must had been in 3.19 days :S
>
> I do not know if the issue has been introduced in recent kernel, I am
> able to trigger it in a vm running
> wireless-drivers-next and it has been reported here:
> https://bugzilla.kernel.org/show_bug.cgi?id=203249
I see. I'm just worried that we make a mistake here and it will get
backported all the way back to very old kernels because of the fixes
tag. If old kernels worked perhaps it's not worth disturbing them.
> > > Fixes: c869f77d6abb ("add mt7601u driver")
> > > Signed-off-by: Lorenzo Bianconi
> > > ---
> > > I will post a patch to fix tx side as well
> > > ---
> > > drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++---
> > > 1 file changed, 16 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > index f7edeffb2b19..e7703990b291 100644
> > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > @@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
> > > struct mt7601u_rx_queue *q = &dev->rx_q;
> > > unsigned long flags;
> > >
> > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > + switch (urb->status) {
> > > + case -ECONNRESET:
> > > + case -ESHUTDOWN:
> > > + case -ENOENT:
> > > + return;
> >
> > So we assume this is non-recoverable? Everything will fail after?
> > Because pending is incremented linearly :S That's why there is a
> > warning here.
>
> AFAIK -ECONNRESET/-ENOENT are related to urb unlink/kill while
> -ESHUTDOWN is related to device disconnection.
> I guess we can assume the device has been removed if we get these errors
Makes sense. A bit of an implicit assumption, USB subsystem may break
this for us. Let's at least put a comment here so we can go back and
know that at the time of committing we did double check?
> > > + default:
> > > + dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
> > > + urb->status);
> > > + /* fall through */
> > > + case 0:
> > > + break;
> > > + }
> > >
> > > - if (mt7601u_urb_has_error(urb))
> > > - dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
> > > + spin_lock_irqsave(&dev->rx_lock, flags);
> > > if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
> > > goto out;
> > >
> > > @@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev,
> > > struct sk_buff *skb,
> > > static void mt7601u_kill_rx(struct mt7601u_dev *dev)
> > > {
> > > int i;
> > > - unsigned long flags;
> > > -
> > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > >
> > > - for (i = 0; i < dev->rx_q.entries; i++) {
> > > - int next = dev->rx_q.end;
> > > -
> > > - spin_unlock_irqrestore(&dev->rx_lock, flags);
> > > - usb_poison_urb(dev->rx_q.e[next].urb);
> > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > - }
> >
> > Why is there no need to take the lock? Admittedly it's not clear what
> > this lock is protecting here :P Perhaps a separate patch to remove the
> > unnecessary locking with an explanation?
>
> usb_poison_urb() can run concurrently with urb completion so I guess
> we do not need locks here.
> I guess we need to maintain this chunk in the same patch since now
> when the device is disconnected
> we do not increment dev->rx_q.end. What do you think?
Ah, I see! The completion used to run in between the unlock/lock!
Now we just poison out of order, because completion doesn't care about
ordering for errored URBs. Perhaps worth putting in the commit message?
Re: [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
On Thu, 6 Jun 2019 14:26:12 +0200, Lorenzo Bianconi wrote:
> Do not schedule rx_tasklet when the usb dongle is disconnected. This
> patch fixes the common kernel warning reported when the device is
> removed.
>
> [ 24.921354] usb 3-14: USB disconnect, device number 7
> [ 24.921593] [ cut here ]
> [ 24.921594] RX urb mismatch
> [ 24.921675] WARNING: CPU: 4 PID: 163 at
> drivers/net/wireless/mediatek/mt7601u/dma.c:200 mt7601u_complete_rx+0xcb/0xd0
> [mt7601u]
> [ 24.921769] CPU: 4 PID: 163 Comm: kworker/4:2 Tainted: G OE
> 4.19.31-041931-generic #201903231635
> [ 24.921770] Hardware name: To Be Filled By O.E.M. To Be Filled By
> O.E.M./Z97 Extreme4, BIOS P1.30 05/23/2014
> [ 24.921782] Workqueue: usb_hub_wq hub_event
> [ 24.921797] RIP: 0010:mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> [ 24.921800] RSP: 0018:9bd9cfd03d08 EFLAGS: 00010086
> [ 24.921802] RAX: RBX: 9bd9bf043540 RCX:
> 0006
> [ 24.921803] RDX: 0007 RSI: 0096 RDI:
> 9bd9cfd16420
> [ 24.921804] RBP: 9bd9cfd03d28 R08: 0002 R09:
> 03a8
> [ 24.921805] R10: 002f485fca34 R11: R12:
> 9bd9bf043c1c
> [ 24.921806] R13: 9bd9c62fa3c0 R14: 0082 R15:
>
> [ 24.921807] FS: () GS:9bd9cfd0()
> knlGS:
> [ 24.921808] CS: 0010 DS: ES: CR0: 80050033
> [ 24.921808] CR2: 7fb2648b CR3: 000142c0a004 CR4:
> 001606e0
> [ 24.921809] Call Trace:
> [ 24.921812]
> [ 24.921819] __usb_hcd_giveback_urb+0x8b/0x140
> [ 24.921821] usb_hcd_giveback_urb+0xca/0xe0
> [ 24.921828] xhci_giveback_urb_in_irq.isra.42+0x82/0xf0
> [ 24.921834] handle_cmd_completion+0xe02/0x10d0
> [ 24.921837] xhci_irq+0x274/0x4a0
> [ 24.921838] xhci_msi_irq+0x11/0x20
> [ 24.921851] __handle_irq_event_percpu+0x44/0x190
> [ 24.921856] handle_irq_event_percpu+0x32/0x80
> [ 24.921861] handle_irq_event+0x3b/0x5a
> [ 24.921867] handle_edge_irq+0x80/0x190
> [ 24.921874] handle_irq+0x20/0x30
> [ 24.921889] do_IRQ+0x4e/0xe0
> [ 24.921891] common_interrupt+0xf/0xf
> [ 24.921892]
> [ 24.921900] RIP: 0010:usb_hcd_flush_endpoint+0x78/0x180
> [ 24.921354] usb 3-14: USB disconnect, device number 7
Is this a new thing? I def tested unplugging the dongle under
traffic, but that must had been in 3.19 days :S
> Fixes: c869f77d6abb ("add mt7601u driver")
> Signed-off-by: Lorenzo Bianconi
> ---
> I will post a patch to fix tx side as well
> ---
> drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++---
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
> b/drivers/net/wireless/mediatek/mt7601u/dma.c
> index f7edeffb2b19..e7703990b291 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> @@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
> struct mt7601u_rx_queue *q = &dev->rx_q;
> unsigned long flags;
>
> - spin_lock_irqsave(&dev->rx_lock, flags);
> + switch (urb->status) {
> + case -ECONNRESET:
> + case -ESHUTDOWN:
> + case -ENOENT:
> + return;
So we assume this is non-recoverable? Everything will fail after?
Because pending is incremented linearly :S That's why there is a
warning here.
> + default:
> + dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
> + urb->status);
> + /* fall through */
> + case 0:
> + break;
> + }
>
> - if (mt7601u_urb_has_error(urb))
> - dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
> + spin_lock_irqsave(&dev->rx_lock, flags);
> if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
> goto out;
>
> @@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev,
> struct sk_buff *skb,
> static void mt7601u_kill_rx(struct mt7601u_dev *dev)
> {
> int i;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&dev->rx_lock, flags);
>
> - for (i = 0; i < dev->rx_q.entries; i++) {
> - int next = dev->rx_q.end;
> -
> - spin_unlock_irqrestore(&dev->rx_lock, flags);
> - usb_poison_urb(dev->rx_q.e[next].urb);
> - spin_lock_irqsave(&dev->rx_lock, flags);
> - }
Why is there no need to take the lock? Admittedly it's not clear what
this lock is protecting here :P Perhaps a separate patch to remove the
unnecessary locking with an explanation?
> - spin_unlock_irqrestore(&dev->rx_lock, flags);
> + for (i = 0; i < dev->rx_q.entries; i++)
> + usb_poison_urb(dev->rx_q.e[i].urb);
> + tasklet_kill(&dev->rx_tasklet);
> }
>
> static int mt7601u_submit_rx_buf(struct mt7601u_dev *dev,
> @@ -525,8 +526,6
Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
On Thu, 6 Jun 2019 17:34:16 +0200, Lorenzo Bianconi wrote:
> When the device is disconnected while passing traffic it is possible
> to receive out of order urbs causing a memory leak since the skb liked
> to the current tx urb is not removed. Fix the issue deallocating the skb
> cleaning up the tx ring. Moreover this patch fixes the following kernel
> warning
Ugh if we don't have ordering guarantees then the entire "ring" scheme
no longer works :( Should we move to URB queues, I don't remember now,
but there seem to had been a better way to manage URBs :S
> [ 57.480771] usb 1-1: USB disconnect, device number 2
> [ 57.483451] [ cut here ]
> [ 57.483462] TX urb mismatch
> [ 57.483481] WARNING: CPU: 1 PID: 32 at
> drivers/net/wireless/mediatek/mt7601u/dma.c:245 mt7601u_complete_tx+0x165/00
> [ 57.483483] Modules linked in:
> [ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
> [ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> 1.12.0-2.fc30 04/01/2014
> [ 57.483502] Workqueue: usb_hub_wq hub_event
> [ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
> [ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1 cb e1
> 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
> [ 57.483513] RSP: :c90a0d28 EFLAGS: 00010092
> [ 57.483516] RAX: 000f RBX: 88802c0a62c0 RCX:
> c90a0c2c
> [ 57.483518] RDX: RSI: RDI:
> 810a8371
> [ 57.483520] RBP: 88803ced6858 R08: R09:
> 0001
> [ 57.483540] R10: 0002 R11: R12:
> 0046
> [ 57.483542] R13: 88802c0a6c88 R14: 88803baab540 R15:
> 88803a0cc078
> [ 57.483548] FS: () GS:88803eb0()
> knlGS:
> [ 57.483550] CS: 0010 DS: ES: CR0: 80050033
> [ 57.483552] CR2: 55e7f6780100 CR3: 28c86000 CR4:
> 06a0
> [ 57.483554] DR0: DR1: DR2:
>
> [ 57.483556] DR3: DR6: fffe0ff0 DR7:
> 0400
> [ 57.483559] Call Trace:
> [ 57.483561]
> [ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
> [ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
> [ 57.483574] handle_cmd_completion+0xf5b/0x12c0
> [ 57.483577] xhci_irq+0x1f6/0x1810
> [ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
> [ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
> [ 57.483588] __handle_irq_event_percpu+0x3a/0x260
> [ 57.483592] handle_irq_event_percpu+0x1c/0x60
> [ 57.483595] handle_irq_event+0x2f/0x4c
> [ 57.483599] handle_edge_irq+0x7e/0x1a0
> [ 57.483603] handle_irq+0x17/0x20
> [ 57.483607] do_IRQ+0x54/0x110
> [ 57.483610] common_interrupt+0xf/0xf
> [ 57.483612]
>
> Fixes: c869f77d6abb ("add mt7601u driver")
> Signed-off-by: Lorenzo Bianconi
> ---
> drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++--
> drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
> b/drivers/net/wireless/mediatek/mt7601u/dma.c
> index e7703990b291..bbf1deed7f3b 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> @@ -238,14 +238,25 @@ static void mt7601u_complete_tx(struct urb *urb)
> struct sk_buff *skb;
> unsigned long flags;
>
> - spin_lock_irqsave(&dev->tx_lock, flags);
> + switch (urb->status) {
> + case -ECONNRESET:
> + case -ESHUTDOWN:
> + case -ENOENT:
> + return;
> + default:
> + dev_err_ratelimited(dev->dev, "tx urb failed: %d\n",
> + urb->status);
> + /* fall through */
> + case 0:
> + break;
> + }
>
> - if (mt7601u_urb_has_error(urb))
> - dev_err(dev->dev, "Error: TX urb failed:%d\n", urb->status);
> + spin_lock_irqsave(&dev->tx_lock, flags);
> if (WARN_ONCE(q->e[q->start].urb != urb, "TX urb mismatch"))
> goto out;
>
> skb = q->e[q->start].skb;
> + q->e[q->start].skb = NULL;
> trace_mt_tx_dma_done(dev, skb);
>
> __skb_queue_tail(&dev->tx_skb_done, skb);
> @@ -446,10 +457,10 @@ static void mt7601u_free_tx_queue(struct
> mt7601u_tx_queue *q)
> {
> int i;
>
> - WARN_ON(q->used);
> -
> for (i = 0; i < q->entries; i++) {
> usb_poison_urb(q->e[i].urb);
> + if (q->e[i].skb)
> + mt7601u_tx_status(q->dev, q->e[i].skb);
Perhaps a separate patch?
> usb_free_urb(q->e[i].urb);
> }
> }
> @@ -463,6 +474,7 @@ static void mt7601u_free_tx(struct mt7601u_dev *dev)
>
> for (i = 0; i < __MT_EP_OUT_MAX; i++)
> mt7601u_free_tx_queue(&dev->tx_q[i]);
> + tasklet_kill(&dev->
Re: [PATCH v2] mt7601u: check chip version on probe
On Thu, 7 Mar 2019 13:22:07 +0100, Stanislaw Gruszka wrote: > Since some USB device IDs are duplicated between mt7601u and mt76x0u > devices, check chip version on probe and return error if not match > 0x7601. > > Don't think this is serious issue, probe most likely will fail at > some other point for wrong device, but we do not have to configure > it if we know is not mt7601u device. > > Reported-by: Xose Vazquez Perez > Signed-off-by: Stanislaw Gruszka Acked-by: Jakub Kicinski Thanks!
Re: mediatek: duplicate usb devices
On Wed, 6 Mar 2019 12:03:30 +0100, Stanislaw Gruszka wrote:
> On Wed, Mar 06, 2019 at 11:09:12AM +0100, Lorenzo Bianconi wrote:
> > >
> > > On Tue, Mar 05, 2019 at 07:33:51PM +0100, Xose Vazquez Perez wrote:
> > > > There a three duplicate devices at:
> > > >
> > > > mt76/mt76x2/usb.c: { USB_DEVICE(0x0e8d, 0x7612) }, /* Alfa
> > > > AWUS036ACM */
> > > > mt76/mt76x2/usb.c: { USB_DEVICE(0x0e8d, 0x7612) }, /* Aukey
> > > > USB-AC1200 */
> > > One duplicated entry should be removed.
> > >
> > > > mt76/mt76x0/usb.c: { USB_DEVICE(0x148f, 0x760a) }, /* TP-Link
> > > > unknown */
> > > > mt7601u/usb.c: { USB_DEVICE(0x148f, 0x760a) },
> >
> > IIUC this is not in the mt7601u driver available on mtk website.
>
> The same apply to mt7610u. Vendor driver have only IDs for some
> reference devices. Both mt7601u and mt7610u drivers do not include
> 0x7610.
>
> For mt76x0u ids were provided by Hans
> https://lore.kernel.org/linux-wireless/alpine.LNX.2.00.1804092042320.8369@T420s/
> I'm not sure where they came from.
The ones in mt7601u came from Xose, the GitHub repo has fuller history:
https://github.com/kuba-moo/mt7601u/commit/5e6b565301d9882aab9af46b396a5cf489db6c78
I don't really mind which one stays :)
Re: [PATCH] mt7601u: do not use WARN_ON in the datapath
On Tue, 22 Jan 2019 16:39:34 +0100, Lorenzo Bianconi wrote: > Substitute WARN_ON with WARN_ON_ONCE in mt7601u_rx_next_seg_len > routine > > Signed-off-by: Lorenzo Bianconi Acked-by: Jakub Kicinski
Re: [PATCH] mt7601u: bump supported EEPROM version
On Tue, 22 Jan 2019 13:47:54 +0100, Stanislaw Gruszka wrote: > As reported by Michael eeprom 0d is supported and work with the driver. > > Dump of /sys/kernel/debug/ieee80211/phy1/mt7601u/eeprom_param > with 0d EEPORM looks like this: > > RSSI offset: 0 0 > Reference temp: f9 > LNA gain: 8 > Reg channels: 1-14 > Per rate power: >raw:05 bw20:05 bw40:05 >raw:05 bw20:05 bw40:05 >raw:03 bw20:03 bw40:03 >raw:03 bw20:03 bw40:03 >raw:04 bw20:04 bw40:04 >raw:00 bw20:00 bw40:00 >raw:00 bw20:00 bw40:00 >raw:00 bw20:00 bw40:00 >raw:02 bw20:02 bw40:02 >raw:00 bw20:00 bw40:00 > Per channel power: >tx_power ch1:09 ch2:09 >tx_power ch3:0a ch4:0a >tx_power ch5:0a ch6:0a >tx_power ch7:0b ch8:0b >tx_power ch9:0b ch10:0b >tx_power ch11:0b ch12:0b >tx_power ch13:0b ch14:0b > > Reported-and-tested-by: Michael > Signed-off-by: Stanislaw Gruszka Acked-by: Jakub Kicinski
Re: [PATCH] mt76: convert to DEFINE_SHOW_ATTRIBUTE
On Mon, 3 Dec 2018 08:40:28 -0500, Yangtao Li wrote: > Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code. > > Signed-off-by: Yangtao Li Acked-by: Jakub Kicinski
Re: [PATCH 0/2] enable 802.11w in mt7601u driver
On Mon, 9 Jul 2018 12:20:25 +0200, Lorenzo Bianconi wrote: > Lorenzo Bianconi (1): > mt7601u: use sw encryption for hw unsupported ciphers > > Davide Caratti (1): > mt7601u: expose 802.11w support > > drivers/net/wireless/mediatek/mt7601u/init.c | 1 + > drivers/net/wireless/mediatek/mt7601u/main.c | 11 +++ > 2 files changed, 12 insertions(+) Acked-by: Jakub Kicinski
Re: [PATCH] mt7601u: remove warning when avg_rssi is zero
On Sat, 9 Jun 2018 12:10:44 +0200, Stanislaw Gruszka wrote:
> It turned out that we can run calibration without already received
> frames with RSSI data. In such case just don't update AGC register
> and don't print the warning.
>
> Fixes: b305a6ab0247 ("mt7601u: use EWMA to calculate avg_rssi")
> Reported-by: Adam Borowski
> Signed-off-by: Stanislaw Gruszka
Acked-by: Jakub Kicinski
Re: [PATCH 2/2 v2] mt7601u: run calibration works after finishing scanning
On Mon, 16 Apr 2018 13:56:18 +0200, Stanislaw Gruszka wrote: > When finishing scanning we switch to operational channel sill with > SCANNING flag. This mean that we never perform calibration works after > scanning. To fix the problem queue calibration works on > .sw_scan_complete() routine. > > Signed-off-by: Stanislaw Gruszka Acked-by: Jakub Kicinski Thank you!!
Re: [PATCH 1/2] mt7601u: use EWMA to calculate avg_rssi
On Fri, 13 Apr 2018 16:44:37 +0200, Stanislaw Gruszka wrote: > avr_rssi is not calculated correctly as we do not divide result > by 256 (mt76 sum avg_rssi1 and avg_rssi2 and divide by 512). > However dividing by 256 will make avg_rssi almost the same as > last rssi value - not really an average. So use EWMA to calculate > avg_rssi. I've chosen weight_rcp=4 to convergence quicker on signal > strength changes. > > Signed-off-by: Stanislaw Gruszka Acked-by: Jakub Kicinski
Re: [PATCH 2/2] mt7601u: run calibration works after finishing scanning
On Fri, 13 Apr 2018 16:44:38 +0200, Stanislaw Gruszka wrote: > When finishing scanning we switch to operational channel sill with > SCANNING flag. This mean that we never perform calibration works after > scanning. To fix the problem cancel and queue calibration works on > .sw_scan_start() and .sw_scan_complete() routines. > > Signed-off-by: Stanislaw Gruszka IOW the stack will potentially ask us to return to the original channel before calling .sw_scan_complete()? Hm. That's unpleasant. I'm not entirely on board with the patch. Now normal channel setting will happen while calibration is running. Is that necessary? Would it not be sufficient to make sure work is scheduled from .sw_scan_complete()?
Re: [PATCH] mt7601u: phy: mark expected switch fall-through
On Fri, 30 Mar 2018 16:12:23 -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Signed-off-by: Gustavo A. R. Silva Acked-by: Jakub Kicinski
Re: [PATCH v3 20/20] mt7601u: use request_firmware_cache() to address cache on reboot
On Sat, 10 Mar 2018 06:15:01 -0800, Luis R. Rodriguez wrote: > request_firmware_cache() will ensure the firmware is available on resume > from suspend if on reboot the device retains the firmware. > > This optimization is in place given otherwise on reboot we have to > reload the firmware, the opmization saves us about max 1s, minimum 10ms. > > Cantabile has reported back this fixes his woes with both suspend and > hibernation. > > Reported-by: Cantabile > Tested-by: Cantabile > Signed-off-by: Luis R. Rodriguez FWIW: Acked-by: Jakub Kicinski Thanks Luis!!
Re: [PATCH] mt7601u: simplify mt7601u_mcu_msg_alloc signature
On Wed, 7 Mar 2018 10:25:50 +0100, Lorenzo Bianconi wrote: > Remove mt7601u_dev parameter from mt7601u_mcu_msg_alloc signature since > dev pointer is never used in routine body > > Signed-off-by: Lorenzo Bianconi Acked-by: Jakub Kicinski
Re: [PATCH] mt7601u: remove a warning in mt7601u_efuse_physical_size_check()
On Wed, 28 Feb 2018 15:26:57 +0100, Lorenzo Bianconi wrote: > Fix the following sparse warning in mt7601u_efuse_physical_size_check: > - drivers/net/wireless/mediatek/mt7601u/eeprom.c:77:27: warning: > Variable length array is used > > Signed-off-by: Lorenzo Bianconi Acked-by: Jakub Kicinski Thanks Lorenzo!
Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation
On Tue, 27 Feb 2018 16:54:51 +, Luis R. Rodriguez wrote: > On Tue, Feb 27, 2018 at 02:25:55PM +0200, cantabile wrote: > > On 27/02/18 04:28, Jakub Kicinski wrote: > > > On Sun, 25 Feb 2018 17:54:25 +, Luis R. Rodriguez wrote: > > > > > > I want to understand the case where the firmware is *not* available on > > > > resume? > > > > Why did that happen? I seem to have read that on a fresh reboot the > > > > firmware > > > > was not needed, and so on probe request_firmware() was not called? Why > > > > would > > > > firmware not be required on a reboot? > > > > > > Yes, that is a good question.. John, do you have a theory? My initial > > > thought was that the UEFI/BIOS loads it during pre-boot, but this is a > > > USB card, so it's a bit unlikely that UEFI will have a driver for it... > > > Does this happen when rebooting maybe? > > > > Yes, it happens when rebooting: > > 1) Plug in the dongle. Message about firmware appears in dmesg: > > > > mt7601u 2-3:1.0: ASIC revision: 76010001 MAC revision: 76010500 > > mt7601u 2-3:1.0: Firmware Version: 0.1.00 Build: 7640 Build time: > > 201302052146 > > mt7601u 2-3:1.0: Warning: unsupported EEPROM version 0d > > > > 2) `systemctl reboot`. Message about firmware does not appear in dmesg: > > > > mt7601u 2-3:1.0: ASIC revision: 76010001 MAC revision: 76010500 > > mt7601u 2-3:1.0: Warning: unsupported EEPROM version 0d > > > > The dongle is nevertheless perfectly functional after rebooting. > > > > I have no idea why it works like this. > > > > This is an older laptop, no UEFI. > > OK, this just confirms that firmware is not needed on reboot sometimes, > but it does not explain *why*. What driver and code lines are involved > so I can go read? mt7601u_load_firmware() is probably the place to look at. > Is the vendor involved or not really? Not really, we got the vendor to submit the FW to linux-firmware, that's about it. > I could imagine a situation where on reboot we just reset a device but > since poweroff is never fully issued the firmware is not lost. So let me > ask, why not do a full reset / shutdown of the device when the interface > goes down? > > If there is no gain from the behaviour observed, might as well make > the device work as much others rather than adding an API for a one-off > situation where there is no gain from it behaving in a unique way. IDK what the reset procedure is :S
Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation
On Sun, 25 Feb 2018 17:54:25 +, Luis R. Rodriguez wrote: > On Mon, Feb 19, 2018 at 05:01:27PM +0200, cantabile wrote: > > On 19/02/18 07:55, Jakub Kicinski wrote: > > > On Sat, 17 Feb 2018 13:23:29 +0200, cantabile wrote: > > > > > Thanks for the info. Would it be cleaner to EXPORT fw_add_devm_name() > > > > > and just call that in case driver sees FW is already loaded? That > > > > > should inform the fw subsystem that we want the image around in case > > > > > of > > > > > hibernation, but there is no need to load it immediately? > > > > > > > > No, I don't believe it's cleaner to expose a private function that you > > > > don't even really need. Remember that calling request_firmware every > > > > time your driver's probe and resume functions are called is normal. It's > > > > the expected behaviour. > > > > > > I'm asking you the extend functionality of a subsystem to be able to > > > cleanly communicate the intent. Not export internal functions. > > > > > > Requesting firmware you don't need and risking failing probe even if FW > > > is already pre-loaded is not correct. Reordering you suggest is > > > brittle and makes little logical sense unless someone guesses your use > > > case. > > > > > > Please at least try to do as advised. Otherwise: > > > > > > Nacked-by: Jakub Kicinski > > > > > > > You're right about the reordering not making sense to someone unfamiliar > > with the problem. I can fix that with a comment. > > > > I can change the patch so that request_firmware will only make the probe > > function fail if the firmware is not already running. > > Note that using request_firmware() on probe typically is also not an > outstanding idea given it delays boot. Not because looking for the firmware > takes time, but instead because processing firmware typically does on > the device. For instance cxgb4 is an example device where processing > firmware takes a long time. > > Delays on probe may mean the "feel good" immediate desktop coming up is > delyed. > > Specially if its networking... I see no reason why to process firmware on > probe. > > If one can use a workqueue to process verifying if it needs firmware and > loading > later, that's more advisable. Quite true, more advanced the FW the longer FW load takes :( Although I would be cautious not to cause issues for network/NFS boot... Perhaps it can wait for such workqueue to finish? > Now, that's all a side topic. > > I will for now agree that it seems pointless to request for firmware always > even if you don't need to, and all you want is to just cache the firmware > on suspend. So I welcome a patch but the justification for it really needs to > be documented very well, and the documentation extended as such. In fact > maybe rename the function to something more sensible. > > Another use case for the firmware cache (which we need to add the > documentation) > is that for hibernation we suspend all devices first, get a snapshot, and then > resume devices so we can then write the snapshot to disk. On that resume step > I don't think devices have access to the hard drive for firmware, so cache is > all we have. This may need some confirmation but I suspect this is the case. > Drivers needing firmware on resume for hibernation may need to cache their > firmware. > > I want to understand the case where the firmware is *not* available on resume? > Why did that happen? I seem to have read that on a fresh reboot the firmware > was not needed, and so on probe request_firmware() was not called? Why would > firmware not be required on a reboot? Yes, that is a good question.. John, do you have a theory? My initial thought was that the UEFI/BIOS loads it during pre-boot, but this is a USB card, so it's a bit unlikely that UEFI will have a driver for it... Does this happen when rebooting maybe?
Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation
On Sat, 17 Feb 2018 13:23:29 +0200, cantabile wrote: > > Thanks for the info. Would it be cleaner to EXPORT fw_add_devm_name() > > and just call that in case driver sees FW is already loaded? That > > should inform the fw subsystem that we want the image around in case of > > hibernation, but there is no need to load it immediately? > > No, I don't believe it's cleaner to expose a private function that you > don't even really need. Remember that calling request_firmware every > time your driver's probe and resume functions are called is normal. It's > the expected behaviour. I'm asking you the extend functionality of a subsystem to be able to cleanly communicate the intent. Not export internal functions. Requesting firmware you don't need and risking failing probe even if FW is already pre-loaded is not correct. Reordering you suggest is brittle and makes little logical sense unless someone guesses your use case. Please at least try to do as advised. Otherwise: Nacked-by: Jakub Kicinski
Re: [PATCH v2] mt7601u: make write with mask access atomic
On Fri, 16 Feb 2018 23:30:01 +0100, Lorenzo Bianconi wrote: > Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order > to make mt7601u_rmw and mt7601u_rmc atomic. This patch does not fix a > reported issue but makes the usb access more robust to concurrent > operations on the same register since it is theoretically possible that > read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved with > a different write operation on the same register. > Moreover using __mt7601u_rr and __mt7601u_vendor_single_wr in > mt7601u_rmw/mt7601u_rmc allows to grab vendor_req_mutex mutex once > instead of twice > > Signed-off-by: Lorenzo Bianconi Acked-by: Jakub Kicinski Thanks!
Re: [PATCH] mt7601u: make write with mask access atomic
On Fri, 16 Feb 2018 10:06:49 +0100, Lorenzo Bianconi wrote: > > Hm.. There should be no path in the driver where that could cause > > problems AFAIR. We have reg_atomic_mutex and hw_atomic_mutex to > > protect from concurrent access to the HW where they are possible. > > RMW operations are non-atomic by definition, it's supposed to work > > like PCIe register accesses would - 32bit reads/writes are atomic, > > but RMW is not. > > Yes, RMW accesses are non-atomic by default but since vendor_req_mutex mutex > is already there (and grabbed for RMW operations), why not use it to make > write > with mask access atomic without adding complexity? Moreover it would be a > micro-optimisation since vendor_req_mutex would be grabbed just once instead > of > twice > > > > > So I'm not sure what to do with this patch. Doesn't seem necessary... > > It is just a trivial rework of locking in usb read/write accesses, not > mandatory, so if you prefer we can just drop it You make good points :) Could you please respin stating clearly in the commit message that this is not a bug fix, but a micro-optimization and may be useful in the future?
Re: [PATCH] mt7601u: make write with mask access atomic
On Fri, 16 Feb 2018 01:02:31 +0100, Lorenzo Bianconi wrote: > > On Thu, 15 Feb 2018 23:59:24 +0100, Lorenzo Bianconi wrote: > >> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order > >> to make mt7601u_rmw and mt7601u_rmc atomic since it is possible that > >> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved > >> with a different write operation on the same register. > >> Moreover move write trace point in __mt7601u_vendor_single_wr > >> > >> Signed-off-by: Lorenzo Bianconi > > > > Could you provide an example of which accesses make it problematic? > > Is this fixing an actual bug? > > Hi Jakub, > > it is not an issue I had experimented, I noticed a theoretical race > reviewing the code. > AFAIU, based on the current implementation it is possible that mt7601u_rmw > (with mt7601u_rr) loads data from given register but its store access > (mt7601u_wr) is > preceded by another mt7601u_wr on the same register. In this case the > value configured by > the first mt7601u_wr executed is overwritten by the second one (the > store from mt7601u_rmw) > even if the first write is setting a different subfield respect to > mt7601u_rmw. Hm.. There should be no path in the driver where that could cause problems AFAIR. We have reg_atomic_mutex and hw_atomic_mutex to protect from concurrent access to the HW where they are possible. RMW operations are non-atomic by definition, it's supposed to work like PCIe register accesses would - 32bit reads/writes are atomic, but RMW is not. So I'm not sure what to do with this patch. Doesn't seem necessary...
Re: [PATCH] mt7601u: make write with mask access atomic
On Thu, 15 Feb 2018 23:59:24 +0100, Lorenzo Bianconi wrote: > Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order > to make mt7601u_rmw and mt7601u_rmc atomic since it is possible that > read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved > with a different write operation on the same register. > Moreover move write trace point in __mt7601u_vendor_single_wr > > Signed-off-by: Lorenzo Bianconi Could you provide an example of which accesses make it problematic? Is this fixing an actual bug?
Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation
On Thu, 15 Feb 2018 13:38:00 +0200, cantabile wrote: > On 15/02/18 02:45, Jakub Kicinski wrote: > > On Wed, 14 Feb 2018 13:34:38 +0200, cantabile wrote: > >> The firmware running on the device sometimes survives a reboot > >> (firmware_running returns 1). When this happens the driver never calls > >> request_firmware, which means the kernel's firmware handling code > >> doesn't know this firmware should be cached before hibernating. Upon > >> resuming from several hours of hibernation, the firmware is no longer > >> running on the device, so the driver calls request_firmware. Since the > >> firmware was never cached, it needs to be loaded from disk, and this is > >> when the system freezes, somewhere in the xfs driver. Fix this by always > >> requesting the firmware, whether it's already running on the device or not. > >> > >> Signed-off-by: John Smith > > > > Thanks for tracking this down, but this seems like the wrong > > direction. > > > > What's your hard drive? Is it some complex configuration which > > prevents the block device from coming online after resume? > > > > If it's really because of some peculiarities of XFS the fix should > > go there, no driver will be able to load FW on resume... > > > > My hard drive is a SATA SSD, with a regular MS-DOS partition table, with > three primary partitions: one ext4 mounted at /boot, one xfs mounted at > /, one xfs mounted at /home. No encryption, no software RAID, no logical > volumes, etc. > > The kernel has a firmware caching mechanism to make sure that no driver > needs to load firmware from disk during the resume process. Because > that's unreliable. See this bit of the documentation: [1] > > This is how it works: when the driver's probe callback calls > request_firmware, that function adds the firmware file's name in a > devres in your 'struct device'. When you suspend the system, the kernel > calls fw_pm_notify [2], which goes through all the 'struct device's and > looks for firmware file names previously added by request_firmware, and > loads into memory all the firmware files it finds. When you resume the > system, all calls to request_firmware ought to find the firmware files > already loaded in memory. After resuming, the kernel calls fw_pm_notify > again to release the cached firmware files (with some delay). > > This caching mechanism currently doesn't always work for your driver > because your driver doesn't always call request_firmware from the probe > callback. Because the firmware running on the device sometimes survives > a reboot. Thanks for the info. Would it be cleaner to EXPORT fw_add_devm_name() and just call that in case driver sees FW is already loaded? That should inform the fw subsystem that we want the image around in case of hibernation, but there is no need to load it immediately? > You can get some very useful messages if you compile firmware_class.c > with -DDEBUG [3]. This is how I figured out the problem. > > [1] > https://www.kernel.org/doc/html/v4.13/driver-api/firmware/request_firmware.html#considerations-for-suspend-and-resume > [2] > https://github.com/torvalds/linux/blob/master/drivers/base/firmware_class.c#L1804 > [3] https://www.kernel.org/doc/local/pr_debug.txt
Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation
On Wed, 14 Feb 2018 13:34:38 +0200, cantabile wrote:
> The firmware running on the device sometimes survives a reboot
> (firmware_running returns 1). When this happens the driver never calls
> request_firmware, which means the kernel's firmware handling code
> doesn't know this firmware should be cached before hibernating. Upon
> resuming from several hours of hibernation, the firmware is no longer
> running on the device, so the driver calls request_firmware. Since the
> firmware was never cached, it needs to be loaded from disk, and this is
> when the system freezes, somewhere in the xfs driver. Fix this by always
> requesting the firmware, whether it's already running on the device or not.
>
> Signed-off-by: John Smith
Thanks for tracking this down, but this seems like the wrong
direction.
What's your hard drive? Is it some complex configuration which
prevents the block device from coming online after resume?
If it's really because of some peculiarities of XFS the fix should
go there, no driver will be able to load FW on resume...
> --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> @@ -420,13 +420,15 @@
> mt7601u_wr(dev, MT_USB_DMA_CFG, (MT_USB_DMA_CFG_RX_BULK_EN |
>MT_USB_DMA_CFG_TX_BULK_EN));
>
> - if (firmware_running(dev))
> - return 0;
> -
> ret = request_firmware(&fw, MT7601U_FIRMWARE, dev->dev);
> if (ret)
> return ret;
>
> + if (firmware_running(dev)) {
> + release_firmware(fw);
> + return 0;
> + }
> +
> if (!fw || !fw->data || fw->size < sizeof(*hdr))
> goto err_inv_fw;
>
Re: [PATCH 2/2] mt7601u: set device mac address in mt7601u_add_interface()
On Fri, 9 Feb 2018 10:49:21 +0100, Lorenzo Bianconi wrote: > On Feb 08, Jakub Kicinski wrote: > > On Thu, 8 Feb 2018 23:08:09 +0100, Lorenzo Bianconi wrote: > > > If mac80211 adds a vif with a different mac address respect to > > > the eeprom one, the device will not be able to connect to the ap > > > since the hw address has not been updated. > > > Fix the issue updating hw mac address in mt7601u_add_interface routine > > > > > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1516935 > > > Signed-off-by: Lorenzo Bianconi > > > --- > > > drivers/net/wireless/mediatek/mt7601u/main.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c > > > b/drivers/net/wireless/mediatek/mt7601u/main.c > > > index 43ebd460ba86..3c9ea40d9584 100644 > > > --- a/drivers/net/wireless/mediatek/mt7601u/main.c > > > +++ b/drivers/net/wireless/mediatek/mt7601u/main.c > > > @@ -64,6 +64,9 @@ static int mt7601u_add_interface(struct ieee80211_hw > > > *hw, > > >*/ > > > mvif->idx = idx; > > > > > > + if (!ether_addr_equal(dev->macaddr, vif->addr)) > > > + mt7601u_set_macaddr(dev, vif->addr); > > > + > > > if (dev->wcid_mask[wcid / BITS_PER_LONG] & BIT(wcid % BITS_PER_LONG)) > > > return -ENOSPC; > > > dev->wcid_mask[wcid / BITS_PER_LONG] |= BIT(wcid % BITS_PER_LONG); > > > > Sorry, my recollection of mac80211 code is waning, but can't we have > > more than one vif as long as they are on the same channel? > > Hi Jakub, > > yep, you are right, but according to my understanding (please correct me if > it is wrong) current implementation supports just one interface in sta mode > (i.e. mvif->idx is always 0) so my patchset just fixes the issue highlighted > in the bugzilla since I do not know if the hw supports multiple concurrent > vifs > in client mode. If so, I can extend the support to multiple client vifs if it > is > a way to properly configure the rx filters to allow reception from multiple > mac > addresses. Ah, you're probably right, perhaps we would have one vif but multiple stas.
Re: [PATCH 0/2] mt7601u: update mac addr if different from eeprom one
On Thu, 8 Feb 2018 23:08:07 +0100, Lorenzo Bianconi wrote: > Lorenzo Bianconi (2): > mt7601u: move mt7601u_set_macaddr in mac related code > mt7601u: set device mac address in mt7601u_add_interface() > > drivers/net/wireless/mediatek/mt7601u/eeprom.c | 24 ++-- > drivers/net/wireless/mediatek/mt7601u/mac.c| 16 > drivers/net/wireless/mediatek/mt7601u/mac.h| 1 + > drivers/net/wireless/mediatek/mt7601u/main.c | 3 +++ > 4 files changed, 22 insertions(+), 22 deletions(-) > Acked-by: Jakub Kicinski
Re: [PATCH 2/2] mt7601u: set device mac address in mt7601u_add_interface()
On Thu, 8 Feb 2018 23:08:09 +0100, Lorenzo Bianconi wrote: > If mac80211 adds a vif with a different mac address respect to > the eeprom one, the device will not be able to connect to the ap > since the hw address has not been updated. > Fix the issue updating hw mac address in mt7601u_add_interface routine > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1516935 > Signed-off-by: Lorenzo Bianconi > --- > drivers/net/wireless/mediatek/mt7601u/main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c > b/drivers/net/wireless/mediatek/mt7601u/main.c > index 43ebd460ba86..3c9ea40d9584 100644 > --- a/drivers/net/wireless/mediatek/mt7601u/main.c > +++ b/drivers/net/wireless/mediatek/mt7601u/main.c > @@ -64,6 +64,9 @@ static int mt7601u_add_interface(struct ieee80211_hw *hw, >*/ > mvif->idx = idx; > > + if (!ether_addr_equal(dev->macaddr, vif->addr)) > + mt7601u_set_macaddr(dev, vif->addr); > + > if (dev->wcid_mask[wcid / BITS_PER_LONG] & BIT(wcid % BITS_PER_LONG)) > return -ENOSPC; > dev->wcid_mask[wcid / BITS_PER_LONG] |= BIT(wcid % BITS_PER_LONG); Sorry, my recollection of mac80211 code is waning, but can't we have more than one vif as long as they are on the same channel?
Re: [PATCH 1/3] ath9k: remove stray backslash in Makefile
On Mon, 27 Nov 2017 18:56:21 +0100, Matthias Schiffer wrote: > Signed-off-by: Matthias Schiffer > --- > drivers/net/wireless/ath/ath9k/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath9k/Makefile > b/drivers/net/wireless/ath/ath9k/Makefile > index 36a40ffdce15..90e4a341076c 100644 > --- a/drivers/net/wireless/ath/ath9k/Makefile > +++ b/drivers/net/wireless/ath/ath9k/Makefile > @@ -59,7 +59,7 @@ obj-$(CONFIG_ATH9K_HW) += ath9k_hw.o > obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o > ath9k_common-y:= common.o \ > common-init.o \ > - common-beacon.o \ > + common-beacon.o > It's not necessarily stray, there is nothing on the next line so it's OK, and if you add \ at the end of all lines, you don't have to touch the last line every time you add/remove something. Sort of like putting a , after last enum value. Just my $.02, not that I mind either way :) > ath9k_common-$(CONFIG_ATH9K_COMMON_DEBUG) += common-debug.o \ >common-spectral.o
Re: [PATCH v2] mt7601u: check memory allocation failure
On Tue, 22 Aug 2017 00:06:17 +0200, Christophe JAILLET wrote: > Check memory allocation failure and return -ENOMEM in such a case, as > already done a few lines below. > > As 'dev->tx_q' can be NULL, we also need to check for that in > 'mt7601u_free_tx()', and return early. > > Signed-off-by: Christophe JAILLET Acked-by: Jakub Kicinski
Re: [PATCH] mt7601u: check memory allocation failure
On Mon, 21 Aug 2017 14:34:30 -0700, Jakub Kicinski wrote: > On Mon, 21 Aug 2017 22:59:56 +0200, Christophe JAILLET wrote: > > Check memory allocation failure and return -ENOMEM in such a case, as > > already done a few lines below > > > > Signed-off-by: Christophe JAILLET > > Acked-by: Jakub Kicinski Wait, I take that back. This code is a bit weird. We would return an error, then mt7601u_dma_init() will call mt7601u_free_tx_queue() which doesn't check for tx_q == NULL condition. Looks like mt7601u_free_tx() has to check for dev->tx_q == NULL and return early if that's the case. Or mt7601u_alloc_tx() should really clean things up on it's own on failure. Ugh.
Re: [PATCH] mt7601u: check memory allocation failure
On Mon, 21 Aug 2017 22:59:56 +0200, Christophe JAILLET wrote: > Check memory allocation failure and return -ENOMEM in such a case, as > already done a few lines below > > Signed-off-by: Christophe JAILLET Acked-by: Jakub Kicinski Thanks!
Re: 'skb' buffer address information leakage
On Tue, 4 Jul 2017 13:12:18 +0800, Dison River wrote:
> drivers/net/ethernet/netronome/nfp/nfp_net_debugfs.c:167
> seq_printf(file, " frag=%p", skb);
FWIW that's actually not a skb pointer. The structure is defined like
this:
struct nfp_net_tx_buf {
union {
struct sk_buff *skb;
void *frag;
};
dma_addr_t dma_addr;
short int fidx;
u16 pkt_cnt;
u32 real_len;
};
So the line in question is actually reading the frag pointer, I just
reused the skb variable, because this has to be read via READ_ONCE()
and NULL-checked so I thought that doing it separately for skb and
frag is a waste of LOC especially in debug code. I will queue up a
clean up for after the merge window.
Thanks!
Re: [PATCH 1/1] mt7601u: check return value of alloc_skb
On Sun, 23 Apr 2017 15:00:23 +0800, Pan Bian wrote: > Function alloc_skb() will return a NULL pointer if there is no enough > memory. However, in function mt7601u_mcu_msg_alloc(), its return value > is not validated before it is used. This patch fixes it. > > Signed-off-by: Pan Bian Acked-by: Jakub Kicinski Thanks!
Re: [PATCH] mt7601u: wait for clear rxq when stopping mac
On Fri, 25 Nov 2016 03:13:34 -0800, Anthony Romano wrote: > mt7601u_mac_stop_hw should stop polling the rxq once it remains empty > but instead continues polling after the rxq status stays clear; bringing > down the interface takes about six seconds from this alone. > > Speed up path by exiting rxq loop once status repeatedly polls empty. > > Signed-off-by: Anthony Romano Looks correct, the condition was inverted and we don't have to msleep() when we see values go to zero. Thanks! Reviewed-by: Jakub Kicinski
[PATCHv8 0/4] register-field manipulation macros
Hi! Small improvement suggested by Daniel Borkmann - use two underscore prefix. https://www.mail-archive.com/netdev@vger.kernel.org/msg125423.html -- v6 blurb: This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro and have that macro compute shift at compilation time using FFS. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). I'm planning to use those macros in another driver soon, Felix may also use them when upstreaming mt76 if he chooses to do so. IMHO if accepted it would be best to push these through Kalle's wireless drivers' tree, since the only existing user is in drivers/net/wireless/. v8: - use two underscores for auxiliary macros (Daniel B). v7: - drop the explicit type marking (u32/u64) - depend on the type of the mask instead; - only allow compilation time constant masks; - barf at "type of register too small to ever match mask" on get; - rename PUT -> PREP. v6: - do a full rename in patch 2; - CC many people. v5: - repost. v4: - add documentation in the header. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (4): add basic register-field manipulation macros mt7601u: remove redefinition of GENMASK mt7601u: remove unnecessary include mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++- drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- drivers/net/wireless/mediatek/mt7601u/init.c| 10 +-- drivers/net/wireless/mediatek/mt7601u/mac.c | 38 +- drivers/net/wireless/mediatek/mt7601u/main.c| 1 - drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++--- drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- drivers/net/wireless/mediatek/mt7601u/phy.c | 44 ++-- drivers/net/wireless/mediatek/mt7601u/regs.h| 4 -- drivers/net/wireless/mediatek/mt7601u/tx.c | 19 ++--- drivers/net/wireless/mediatek/mt7601u/util.h| 77 include/linux/bitfield.h| 93 + include/linux/bug.h | 3 + 14 files changed, 177 insertions(+), 160 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1
[PATCHv8 4/4] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Signed-off-by: Jakub Kicinski
Reviewed-by: Dinan Gunawardena
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++--
drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++--
drivers/net/wireless/mediatek/mt7601u/init.c| 10 ++--
drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++--
drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++---
drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++---
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
10 files changed, 81 insertions(+), 155 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 57a80cfa39b1..a8bc064bc14f 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev,
u8 *data,
if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2]))
dev_err_once(dev->dev, "Error: RXWI zero fields are set\n");
- if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info)))
+ if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info)))
dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n");
trace_mt_rx(dev, rxwi, fce_info);
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..270d126880c0 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
@@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb,
*/
info = flags |
- MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
- MT76_SET(MT_TXD_INFO_D_PORT, d_port) |
- MT76_SET(MT_TXD_INFO_TYPE, type);
+ FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
+ FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) |
+ FIELD_PREP(MT_TXD_INFO_TYPE, type);
put_unaligned_le32(info, skb_push(skb, sizeof(info)));
return skb_put_padto(skb, round_up(skb->len, 4) + 4);
@@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb,
static inline int
mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags)
{
- flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel);
+ flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel);
return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags);
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
index 8d8ee0344f7b..da6faea092d6 100644
--- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
@@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8
*data,
val = mt76_rr(dev, MT_EFUSE_CTRL);
val &= ~(MT_EFUSE_CTRL_AIN |
MT_EFUSE_CTRL_MODE);
- val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
- MT76_SET(MT_EFUSE_CTRL_MODE, mode) |
+ val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
+ FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) |
MT_EFUSE_CTRL_KICK;
mt76_wr(dev, MT_EFUSE_CTRL, val);
@@ -128,8 +128,8 @@ mt7601u_set_chip_cap(struct mt7601u_dev *dev, u8 *eeprom)
if (!field_valid(nic_conf0 >> 8))
return;
- if (MT76_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 ||
- MT76_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1)
+ if (FIELD_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 ||
+ FIELD_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1)
dev_err(dev->dev,
"Error: device has more than 1 RX/TX stream!\n");
}
@@ -150,7 +150,7 @@ mt7601u_set_macaddr(struct mt7601u_dev *dev, const u8
*eeprom)
mt76_wr(dev, MT_MAC_ADDR_DW0, get_unaligned_le32(dev->macaddr));
mt76_wr(dev, MT_MAC_ADDR_DW1, get_unaligned_le16(dev->macaddr + 4) |
- MT76_SET(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
+ FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
return 0;
}
@@ -176,7 +176,7 @@ mt7601u_set_channel_power(struct mt7601u_dev *dev, u8
*eeprom)
u8 max_pwr;
val = mt7601u_rr(dev, MT_TX_ALC_CFG_0);
- max_pwr = MT76_GET(MT_TX_ALC_CFG_0_LIMIT_0, val)
[PATCHv8 2/4] mt7601u: remove redefinition of GENMASK
Remove redefinition of GENMASK which should not be there in the upstream version of the code. Signed-off-by: Jakub Kicinski Reviewed-by: Dinan Gunawardena --- drivers/net/wireless/mediatek/mt7601u/regs.h | 4 1 file changed, 4 deletions(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/regs.h b/drivers/net/wireless/mediatek/mt7601u/regs.h index afd8978e83fa..27a429d90cec 100644 --- a/drivers/net/wireless/mediatek/mt7601u/regs.h +++ b/drivers/net/wireless/mediatek/mt7601u/regs.h @@ -17,10 +17,6 @@ #include -#ifndef GENMASK -#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) -#endif - #define MT_ASIC_VERSION0x #define MT76XX_REV_E3 0x22 -- 1.9.1
[PATCHv8 3/4] mt7601u: remove unnecessary include
There is no need to include linux/version.h in a in-tree
driver.
Signed-off-by: Jakub Kicinski
Reviewed-by: Dinan Gunawardena
---
drivers/net/wireless/mediatek/mt7601u/main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c
b/drivers/net/wireless/mediatek/mt7601u/main.c
index e70dd9523911..43ebd460ba86 100644
--- a/drivers/net/wireless/mediatek/mt7601u/main.c
+++ b/drivers/net/wireless/mediatek/mt7601u/main.c
@@ -15,7 +15,6 @@
#include "mt7601u.h"
#include "mac.h"
#include
-#include
static int mt7601u_start(struct ieee80211_hw *hw)
{
--
1.9.1
[PATCHv8 1/4] add basic register-field manipulation macros
Common approach to accessing register fields is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define REG_FIELD 0x000ff000
field = FIELD_GET(REG_FIELD, reg);
reg &= ~REG_FIELD;
reg |= FIELD_PREP(REG_FIELD, field);
FIELD_{GET,PREP} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Attempts to use static inlines instead of macros failed due
to false positive triggering of BUILD_BUG_ON()s, especially with
GCC < 6.0.
Signed-off-by: Jakub Kicinski
Reviewed-by: Dinan Gunawardena
---
v8:
- use two underscores as prefix for __bf_shf() and
__BF_FIELD_CHECK()
include/linux/bitfield.h | 93
include/linux/bug.h | 3 ++
2 files changed, 96 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..f6505d83069d
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+
+/*
+ * Bitfield access macros
+ *
+ * FIELD_{GET,PREP} macros take as first parameter shifted mask
+ * from which they extract the base mask and shift amount.
+ * Mask must be a compilation time constant.
+ *
+ * Example:
+ *
+ * #define REG_FIELD_A GENMASK(6, 0)
+ * #define REG_FIELD_B BIT(7)
+ * #define REG_FIELD_C GENMASK(15, 8)
+ * #define REG_FIELD_D GENMASK(31, 16)
+ *
+ * Get:
+ * a = FIELD_GET(REG_FIELD_A, reg);
+ * b = FIELD_GET(REG_FIELD_B, reg);
+ *
+ * Set:
+ * reg = FIELD_PREP(REG_FIELD_A, 1) |
+ * FIELD_PREP(REG_FIELD_B, 0) |
+ * FIELD_PREP(REG_FIELD_C, c) |
+ * FIELD_PREP(REG_FIELD_D, 0x40);
+ *
+ * Modify:
+ * reg &= ~REG_FIELD_C;
+ * reg |= FIELD_PREP(REG_FIELD_C, c);
+ */
+
+#define __bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \
+ ({ \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \
+_pfx "mask is not constant"); \
+ BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");\
+ BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
+~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \
+_pfx "value too large for the field"); \
+ BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \
+_pfx "type of reg too small for mask"); \
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << __bf_shf(_mask))); \
+ })
+
+/**
+ * FIELD_PREP() - prepare a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: value to put in the field
+ *
+ * FIELD_PREP() masks and shifts up the value. The result should
+ * be combined with other fields of the bitfield using logical OR.
+ */
+#define FIELD_PREP(_mask, _val)
\
+ ({ \
+ __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");\
+ ((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask); \
+ })
+
+/**
+ * FIELD_GET() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_reg: 32bit value of entire bitfield
+ *
+ * FIELD_GET() extracts
Re: [PATCHv7 4/4] mt7601u: use linux/bitfield.h
On Mon, 22 Aug 2016 21:19:41 +0200, Arend Van Spriel wrote: > On 22-8-2016 12:52, Jakub Kicinski wrote: > > On Fri, 19 Aug 2016 21:02:02 +0200, Arend Van Spriel wrote: > >> On 19-8-2016 18:44, Jakub Kicinski wrote: > >>> Use the newly added linux/bitfield.h. > >>> > >>> Reviewed-by: Dinan Gunawardena > >>> Signed-off-by: Jakub Kicinski > >>> --- > > [snip] > > >>> diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> index 8d8ee0344f7b..da6faea092d6 100644 > >>> --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, > >>> u8 *data, > >>> val = mt76_rr(dev, MT_EFUSE_CTRL); > >>> val &= ~(MT_EFUSE_CTRL_AIN | > >>>MT_EFUSE_CTRL_MODE); > >>> - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > >>> -MT76_SET(MT_EFUSE_CTRL_MODE, mode) | > >>> + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > >>> +FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | > >>> MT_EFUSE_CTRL_KICK; > >> > >> MT_EFUSE_CTRL_KICK is probably a 1-bit field in MT_EFUSE_CTRL register. > >> It looks like you did not want to go all the way although you do give an > >> example in bitfield.h, ie. + * #define REG_FIELD_B BIT(7). > > > > True, I just wanted to show in the examples that BIT() is OK to use. > > My feeling is that ORing in always set flags is acceptable, or at least > > it was my feeling when I wrote this code ;) > > I find it tricky to have the user do the field clearing separately. > Especially if you allow the calling function to pass additional opaque > "flags". In my experience this is more error prone than anything else > when dealing with bit fields and as such it is unfortunate this aspect > is not addressed in your patches. I don't think anything in this set prevents people from adding a FIELD_SET() wrapper. Quite the opposite and I'd encourage it. > I would rather see: > > val = mt76_rr(dev, MT_EFUSE_CTRL); > FIELD_SET(val, MT_EFUSE_CTRL_AIN, addr & ~0xf); > FIELD_SET(val, MT_EFUSE_CTRL_MODE, mode); > FIELD_SET(val, MT_EFUSE_CTRL_KICK, 1); > mt76_wr(dev, MT_EFUSE_CTRL, val); > > in which FIELD_SET takes care of clearing the indicated field. Yes, I agree this is a good solution as well. The reason I didn't go with this approach (apart from modifying an argument of a macro ;)) is the experience with rt2x00 which followed this design and I wasn't particularly fond of the resulting code. I find PREP macro easier to read (less parameters). It's also often convenient to not have to zero the variable for pure write and be able to combine the values in a parameter list: + mt7601u_wr(dev, MT_TXOP_CTRL_CFG, + FIELD_PREP(MT_TXOP_TRUN_EN, 0x3f) | + FIELD_PREP(MT_TXOP_EXT_CCA_DLY, 0x58)); or: + reg = cpu_to_le32(FIELD_PREP(MT_TXD_INFO_TYPE, DMA_PACKET) | + FIELD_PREP(MT_TXD_INFO_D_PORT, CPU_TX_PORT) | + FIELD_PREP(MT_TXD_INFO_LEN, len)); So each approach has it's advantages and I think they complement each other. Please note that I'm just using mt7601u as an example, I really don't need SET in the new code I'm trying to use these macros for now [1]. [1] https://patchwork.ozlabs.org/patch/628779/
Re: [PATCHv7 4/4] mt7601u: use linux/bitfield.h
On Fri, 19 Aug 2016 21:02:02 +0200, Arend Van Spriel wrote:
> On 19-8-2016 18:44, Jakub Kicinski wrote:
> > Use the newly added linux/bitfield.h.
> >
> > Reviewed-by: Dinan Gunawardena
> > Signed-off-by: Jakub Kicinski
> > ---
> > drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +-
> > drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++--
> > drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++--
> > drivers/net/wireless/mediatek/mt7601u/init.c| 10 ++--
> > drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++--
> > drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++
> > drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
> > drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++---
> > drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++---
> > drivers/net/wireless/mediatek/mt7601u/util.h| 77
> > -
> > 10 files changed, 81 insertions(+), 155 deletions(-)
> > delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
> >
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > index 57a80cfa39b1..a8bc064bc14f 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > @@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev
> > *dev, u8 *data,
> >
> > if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2]))
> > dev_err_once(dev->dev, "Error: RXWI zero fields are set\n");
> > - if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info)))
> > + if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info)))
> > dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n");
> >
> > trace_mt_rx(dev, rxwi, fce_info);
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
> > b/drivers/net/wireless/mediatek/mt7601u/dma.h
> > index 978e8a90b87f..270d126880c0 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/dma.h
> > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
> > @@ -18,8 +18,6 @@
> > #include
> > #include
> >
> > -#include "util.h"
> > -
> > #define MT_DMA_HDR_LEN 4
> > #define MT_RX_INFO_LEN 4
> > #define MT_FCE_INFO_LEN4
> > @@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff
> > *skb,
> > */
> >
> > info = flags |
> > - MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
> > - MT76_SET(MT_TXD_INFO_D_PORT, d_port) |
> > - MT76_SET(MT_TXD_INFO_TYPE, type);
> > + FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
> > + FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) |
> > + FIELD_PREP(MT_TXD_INFO_TYPE, type);
>
> So what are those flags? Is there no field definition for those.
>
> > put_unaligned_le32(info, skb_push(skb, sizeof(info)));
> > return skb_put_padto(skb, round_up(skb->len, 4) + 4);
> > @@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff
> > *skb,
> > static inline int
> > mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32
> > flags)
> > {
> > - flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel);
> > + flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel);
>
> Ah. This is the flags being ORed in above. I suppose there are more
> callsites to mt7601u_dma_skb_wrap().
Yes, the field layout differs slightly between data and MCU commands.
Additionally some packet flags depend on mac80211 info and I didn't
want to pollute dma.h with mac80211 info so I just pass those flags
as opaque u32.
> > return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags);
> > }
> >
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
> > b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
> > index 8d8ee0344f7b..da6faea092d6 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
> > @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8
> > *data,
> > val = mt76_rr(dev, MT_EFUSE_CTRL);
> > val &= ~(MT_EFUSE_CTRL_AIN |
> > MT_EFUSE_CTRL_MODE);
> > - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
> > - MT76_SET(MT_EFUSE_CTRL_MODE, mode) |
> > + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
> > + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) |
> >MT_EFUSE_CTRL_KICK;
>
> MT_EFUSE_CTRL_KICK is probably a 1-bit field in MT_EFUSE_CTRL register.
> It looks like you did not want to go all the way although you do give an
> example in bitfield.h, ie. + * #define REG_FIELD_B BIT(7).
True, I just wanted to show in the examples that BIT() is OK to use.
My feeling is that ORing in always set flags is acceptable, or at least
it was my feeling when I wrote this code ;)
[PATCHv7 1/4] add basic register-field manipulation macros
Common approach to accessing register fields is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define REG_FIELD 0x000ff000
field = FIELD_GET(REG_FIELD, reg);
reg &= ~REG_FIELD;
reg |= FIELD_PREP(REG_FIELD, field);
FIELD_{GET,PREP} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Attempts to use static inlines instead of macros failed due
to false positive triggering of BUILD_BUG_ON()s, especially with
GCC < 6.0.
Reviewed-by: Dinan Gunawardena
Signed-off-by: Jakub Kicinski
---
include/linux/bitfield.h | 93
include/linux/bug.h | 3 ++
2 files changed, 96 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..32ca8863e66d
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+
+/*
+ * Bitfield access macros
+ *
+ * FIELD_{GET,PREP} macros take as first parameter shifted mask
+ * from which they extract the base mask and shift amount.
+ * Mask must be a compilation time constant.
+ *
+ * Example:
+ *
+ * #define REG_FIELD_A GENMASK(6, 0)
+ * #define REG_FIELD_B BIT(7)
+ * #define REG_FIELD_C GENMASK(15, 8)
+ * #define REG_FIELD_D GENMASK(31, 16)
+ *
+ * Get:
+ * a = FIELD_GET(REG_FIELD_A, reg);
+ * b = FIELD_GET(REG_FIELD_B, reg);
+ *
+ * Set:
+ * reg = FIELD_PREP(REG_FIELD_A, 1) |
+ * FIELD_PREP(REG_FIELD_B, 0) |
+ * FIELD_PREP(REG_FIELD_C, c) |
+ * FIELD_PREP(REG_FIELD_D, 0x40);
+ *
+ * Modify:
+ * reg &= ~REG_FIELD_C;
+ * reg |= FIELD_PREP(REG_FIELD_C, c);
+ */
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \
+ ({ \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \
+_pfx "mask is not constant"); \
+ BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");\
+ BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) : 0, \
+_pfx "value too large for the field"); \
+ BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \
+_pfx "type of reg too small for mask"); \
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << _bf_shf(_mask))); \
+ })
+
+/**
+ * FIELD_PREP() - prepare a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: value to put in the field
+ *
+ * FIELD_PREP() masks and shifts up the value. The result should
+ * be combined with other fields of the bitfield using logical OR.
+ */
+#define FIELD_PREP(_mask, _val)
\
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
+ ((typeof(_mask))(_val) << _bf_shf(_mask)) & (_mask);\
+ })
+
+/**
+ * FIELD_GET() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_reg: 32bit value of entire bitfield
+ *
+ * FIELD_GET() extracts the field specified by @_mask from the
+ * bitfield passed in as @_reg by masking and shifting it down.
+ */
+#define FIELD_GET(_mask, _reg)
[PATCHv7 3/4] mt7601u: remove unnecessary include
There is no need to include linux/version.h in a in-tree
driver.
Reviewed-by: Dinan Gunawardena
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c
b/drivers/net/wireless/mediatek/mt7601u/main.c
index e70dd9523911..43ebd460ba86 100644
--- a/drivers/net/wireless/mediatek/mt7601u/main.c
+++ b/drivers/net/wireless/mediatek/mt7601u/main.c
@@ -15,7 +15,6 @@
#include "mt7601u.h"
#include "mac.h"
#include
-#include
static int mt7601u_start(struct ieee80211_hw *hw)
{
--
1.9.1
[PATCHv7 2/4] mt7601u: remove redefinition of GENMASK
Remove redefinition of GENMASK which should not be there in the upstream version of the code. Reviewed-by: Dinan Gunawardena Signed-off-by: Jakub Kicinski --- drivers/net/wireless/mediatek/mt7601u/regs.h | 4 1 file changed, 4 deletions(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/regs.h b/drivers/net/wireless/mediatek/mt7601u/regs.h index afd8978e83fa..27a429d90cec 100644 --- a/drivers/net/wireless/mediatek/mt7601u/regs.h +++ b/drivers/net/wireless/mediatek/mt7601u/regs.h @@ -17,10 +17,6 @@ #include -#ifndef GENMASK -#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) -#endif - #define MT_ASIC_VERSION0x #define MT76XX_REV_E3 0x22 -- 1.9.1
[PATCHv7 4/4] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Reviewed-by: Dinan Gunawardena
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++--
drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++--
drivers/net/wireless/mediatek/mt7601u/init.c| 10 ++--
drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++--
drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++---
drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++---
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
10 files changed, 81 insertions(+), 155 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 57a80cfa39b1..a8bc064bc14f 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev,
u8 *data,
if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2]))
dev_err_once(dev->dev, "Error: RXWI zero fields are set\n");
- if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info)))
+ if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info)))
dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n");
trace_mt_rx(dev, rxwi, fce_info);
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..270d126880c0 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
@@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb,
*/
info = flags |
- MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
- MT76_SET(MT_TXD_INFO_D_PORT, d_port) |
- MT76_SET(MT_TXD_INFO_TYPE, type);
+ FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
+ FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) |
+ FIELD_PREP(MT_TXD_INFO_TYPE, type);
put_unaligned_le32(info, skb_push(skb, sizeof(info)));
return skb_put_padto(skb, round_up(skb->len, 4) + 4);
@@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb,
static inline int
mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags)
{
- flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel);
+ flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel);
return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags);
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
index 8d8ee0344f7b..da6faea092d6 100644
--- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
@@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8
*data,
val = mt76_rr(dev, MT_EFUSE_CTRL);
val &= ~(MT_EFUSE_CTRL_AIN |
MT_EFUSE_CTRL_MODE);
- val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
- MT76_SET(MT_EFUSE_CTRL_MODE, mode) |
+ val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
+ FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) |
MT_EFUSE_CTRL_KICK;
mt76_wr(dev, MT_EFUSE_CTRL, val);
@@ -128,8 +128,8 @@ mt7601u_set_chip_cap(struct mt7601u_dev *dev, u8 *eeprom)
if (!field_valid(nic_conf0 >> 8))
return;
- if (MT76_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 ||
- MT76_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1)
+ if (FIELD_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 ||
+ FIELD_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1)
dev_err(dev->dev,
"Error: device has more than 1 RX/TX stream!\n");
}
@@ -150,7 +150,7 @@ mt7601u_set_macaddr(struct mt7601u_dev *dev, const u8
*eeprom)
mt76_wr(dev, MT_MAC_ADDR_DW0, get_unaligned_le32(dev->macaddr));
mt76_wr(dev, MT_MAC_ADDR_DW1, get_unaligned_le16(dev->macaddr + 4) |
- MT76_SET(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
+ FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
return 0;
}
@@ -176,7 +176,7 @@ mt7601u_set_channel_power(struct mt7601u_dev *dev, u8
*eeprom)
u8 max_pwr;
val = mt7601u_rr(dev, MT_TX_ALC_CFG_0);
- max_pwr = MT76_GET(MT_TX_ALC_CFG_0_LIMIT_0, val)
[PATCHv7 0/4] register-field manipulation macros
Hi! Looks like we're good to go :) I've thrown in two mt7601u cleanups, macros are as approved by Linus in the RFC. I'm posting this already to give build bot a head start while we wait for any additional feedback on LKML. -- v6 blurb: This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro and have that macro compute shift at compilation time using FFS. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). I'm planning to use those macros in another driver soon, Felix may also use them when upstreaming mt76 if he chooses to do so. IMHO if accepted it would be best to push these through Kalle's wireless drivers' tree, since the only existing user is in drivers/net/wireless/. v7: - drop the explicit type marking (u32/u64) - depend on the type of the mask instead; - only allow compilation time constant masks; - barf at "type of register too small to ever match mask" on get; - rename PUT -> PREP. v6: - do a full rename in patch 2; - CC many people. v5: - repost. v4: - add documentation in the header. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (4): add basic register-field manipulation macros mt7601u: remove redefinition of GENMASK mt7601u: remove unnecessary include mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++- drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- drivers/net/wireless/mediatek/mt7601u/init.c| 10 +-- drivers/net/wireless/mediatek/mt7601u/mac.c | 38 +- drivers/net/wireless/mediatek/mt7601u/main.c| 1 - drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++--- drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- drivers/net/wireless/mediatek/mt7601u/phy.c | 44 ++-- drivers/net/wireless/mediatek/mt7601u/regs.h| 4 -- drivers/net/wireless/mediatek/mt7601u/tx.c | 19 ++--- drivers/net/wireless/mediatek/mt7601u/util.h| 77 include/linux/bitfield.h| 93 + include/linux/bug.h | 3 + 14 files changed, 177 insertions(+), 160 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1
[RFC (v7)] add basic register-field manipulation macros
Hi!
This is what I came up with. Changes:
- drop the explicit type marking (u32/u64) - depend on the type
of the mask instead;
- only allow compilation time constant masks;
- barf at "type of register too small to ever match mask" on get;
- rename PUT -> PREP.
I did not put the new marcos in bitops.h (where GENMASK lives)
because of the dependency on bug.h. If I did there would be
a circular dependency like this:
bitops.h -> linux/bug.h -> asm-generic/bug.h -> kernel.h -> bitops.h
Which makes things really ugly. I tried to remove the kernel.h
include from bug.h by separating panic-related definitions into
a new header. There were still cycles coming from the notifier.h
which I had to include for panic notifier...
Reviewed-by: Dinan Gunawardena
Signed-off-by: Jakub Kicinski
---
include/linux/bitfield.h | 93
include/linux/bug.h | 3 ++
2 files changed, 96 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..32ca8863e66d
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+
+/*
+ * Bitfield access macros
+ *
+ * FIELD_{GET,PREP} macros take as first parameter shifted mask
+ * from which they extract the base mask and shift amount.
+ * Mask must be a compilation time constant.
+ *
+ * Example:
+ *
+ * #define REG_FIELD_A GENMASK(6, 0)
+ * #define REG_FIELD_B BIT(7)
+ * #define REG_FIELD_C GENMASK(15, 8)
+ * #define REG_FIELD_D GENMASK(31, 16)
+ *
+ * Get:
+ * a = FIELD_GET(REG_FIELD_A, reg);
+ * b = FIELD_GET(REG_FIELD_B, reg);
+ *
+ * Set:
+ * reg = FIELD_PREP(REG_FIELD_A, 1) |
+ * FIELD_PREP(REG_FIELD_B, 0) |
+ * FIELD_PREP(REG_FIELD_C, c) |
+ * FIELD_PREP(REG_FIELD_D, 0x40);
+ *
+ * Modify:
+ * reg &= ~REG_FIELD_C;
+ * reg |= FIELD_PREP(REG_FIELD_C, c);
+ */
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \
+ ({ \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \
+_pfx "mask is not constant"); \
+ BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");\
+ BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) : 0, \
+_pfx "value too large for the field"); \
+ BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \
+_pfx "type of reg too small for mask"); \
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << _bf_shf(_mask))); \
+ })
+
+/**
+ * FIELD_PREP() - prepare a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: value to put in the field
+ *
+ * FIELD_PREP() masks and shifts up the value. The result should
+ * be combined with other fields of the bitfield using logical OR.
+ */
+#define FIELD_PREP(_mask, _val)
\
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
+ ((typeof(_mask))(_val) << _bf_shf(_mask)) & (_mask);\
+ })
+
+/**
+ * FIELD_GET() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_reg: 32bit value of entire bitfield
+ *
+ * FIELD_GET() extracts the field specified by @_mask from the
+ * bitfield passed in as @_reg by masking and shifting it down.
+ */
+#define FIELD_GET(_mask, _reg) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: ");\
+ (typeof(_mask))(((_reg) & (_mask)) >> _bf_shf(_mask)); \
+ })
+
+#endif
diff --git a/include/linux/bug.h b/include/linux/bug.h
index e51b0709e78d..292d6a10b0c2 100644
--- a/include/linux/bug.h
+++ b/include/linux/
Re: [PATCHv6 1/2] add basic register-field manipulation macros
On Wed, 17 Aug 2016 09:33:26 -0700, Linus Torvalds wrote: > On Wed, Aug 17, 2016 at 3:31 AM, Kalle Valo wrote: > > > > Are people ok with this? I think they are useful and I can take these > > through my tree, but I would prefer to get an ack from other maintainers > > first. Dave? Andrew? > > I'm not a huge fan, since the interface fundamentally seems to be > oddly designed (why pass in the mask rather than "start bit + > length"?). Would that not require start and length to have separate defines? I assume doing: #define REG_BLA_FIELD_FOO 3, 4 val = FIELD_GET(REG_BLA_FIELD_FOO, reg); is not acceptable. Attempts to define a single value brought us to the shifted mask. > I also don't like how this very much would match the GENMASK() macros > we have, but then it clashes with them in other ways > > - it's in a different header file I'll move to bitops.h. > - it has completely different naming (GENMASK_ULL vs FIELD_GET64}. > > I actually think the naming could/should be fixed by just > automatically doing the right thing based on sizes. For example, > GENMASK could just have something like > > #define GENMASK(end,start) __builtin_choose_expr((end)>31, > __GENMASK_64(end,start), __GENMASK_32(end,start)) > > and doing similar things with the FIELD_GET/SET ops. > > So I'm not entirely happy about this all. Seems feasible. > But if people love the interface, and think the above kind of cleanups > might be possible, I'd just want to make sure that there is also a > >BUILD_BUG_ON(!__builtin_constant_p(_mask)); > > because if the mask isn't a build-time constant, the code ends up > being *complete* shit. Also preferably something like > >BUILD_BUG_ON((_mask) > (typeof(_val)~0ull); > > to make sure you can't try to mask bits that don't exist in the value. > > Or something like that to make mis-uses *really* obvious. OK! > The FIELD_PUT macro also seems misnamed. It doesn't "put" anything > (unlike the GET macro). It just prepares the field for inserting > later. As exemplified by how you actually have to put things: > > First, "GET" - yeah, that looks like a "get" operation: > > * Get: > * a = FIELD_GET(REG_FIELD_A, reg); > > But then "PUT" isn't actually a PUT operation at all, but the comments > kind of gloss over it by talking about "Modify" instead: > > * Modify: > * reg &= ~REG_FIELD_C; > * reg |= FIELD_PUT(REG_FIELD_C, c); > > so I'm not entirely sure about the naming. > > I can live with the FIELD_PUT naming, because I see how it comes > about, even if I think it's a bit odd. I might have called it > "FIELD_PREP" instead, but I'm not really sure that's all that much > better. Yes, it used to be called FIELD_SET, which was even worse. I think PREP sounds good. Thanks!
[PATCHv6 2/2] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Signed-off-by: Jakub Kicinski
Reviewed-by: Dinan Gunawardena
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++--
drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++--
drivers/net/wireless/mediatek/mt7601u/init.c| 9 +--
drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++--
drivers/net/wireless/mediatek/mt7601u/mcu.c | 18 +++---
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
drivers/net/wireless/mediatek/mt7601u/phy.c | 36 ++--
drivers/net/wireless/mediatek/mt7601u/tx.c | 16 ++---
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
10 files changed, 72 insertions(+), 150 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 57a80cfa39b1..a8bc064bc14f 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev,
u8 *data,
if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2]))
dev_err_once(dev->dev, "Error: RXWI zero fields are set\n");
- if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info)))
+ if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info)))
dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n");
trace_mt_rx(dev, rxwi, fce_info);
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..46c6a7860c38 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
@@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb,
*/
info = flags |
- MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
- MT76_SET(MT_TXD_INFO_D_PORT, d_port) |
- MT76_SET(MT_TXD_INFO_TYPE, type);
+ FIELD_PUT(MT_TXD_INFO_LEN, round_up(skb->len, 4)) |
+ FIELD_PUT(MT_TXD_INFO_D_PORT, d_port) |
+ FIELD_PUT(MT_TXD_INFO_TYPE, type);
put_unaligned_le32(info, skb_push(skb, sizeof(info)));
return skb_put_padto(skb, round_up(skb->len, 4) + 4);
@@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb,
static inline int
mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags)
{
- flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel);
+ flags |= FIELD_PUT(MT_TXD_PKT_INFO_QSEL, qsel);
return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags);
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
index 8d8ee0344f7b..14ac2c0f8838 100644
--- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
@@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8
*data,
val = mt76_rr(dev, MT_EFUSE_CTRL);
val &= ~(MT_EFUSE_CTRL_AIN |
MT_EFUSE_CTRL_MODE);
- val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
- MT76_SET(MT_EFUSE_CTRL_MODE, mode) |
+ val |= FIELD_PUT(MT_EFUSE_CTRL_AIN, addr & ~0xf) |
+ FIELD_PUT(MT_EFUSE_CTRL_MODE, mode) |
MT_EFUSE_CTRL_KICK;
mt76_wr(dev, MT_EFUSE_CTRL, val);
@@ -128,8 +128,8 @@ mt7601u_set_chip_cap(struct mt7601u_dev *dev, u8 *eeprom)
if (!field_valid(nic_conf0 >> 8))
return;
- if (MT76_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 ||
- MT76_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1)
+ if (FIELD_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 ||
+ FIELD_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1)
dev_err(dev->dev,
"Error: device has more than 1 RX/TX stream!\n");
}
@@ -150,7 +150,7 @@ mt7601u_set_macaddr(struct mt7601u_dev *dev, const u8
*eeprom)
mt76_wr(dev, MT_MAC_ADDR_DW0, get_unaligned_le32(dev->macaddr));
mt76_wr(dev, MT_MAC_ADDR_DW1, get_unaligned_le16(dev->macaddr + 4) |
- MT76_SET(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
+ FIELD_PUT(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
return 0;
}
@@ -176,7 +176,7 @@ mt7601u_set_channel_power(struct mt7601u_dev *dev, u8
*eeprom)
u8 max_pwr;
val = mt7601u_rr(dev, MT_TX_ALC_CFG_0);
- max_pwr = MT76_GET(MT_TX_ALC_CFG_0_LIMIT_0, val);
+ max_pwr = FIELD_GET(MT_TX_ALC_CFG_0_LIMIT_0
[PATCHv6 1/2] add basic register-field manipulation macros
Common approach to accessing register fields is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define REG_FIELD 0x000ff000
field = FIELD_GET(REG_FIELD, reg);
reg &= ~REG_FIELD;
reg |= FIELD_PUT(REG_FIELD, field);
FIELD_{GET,PUT} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Attempts to use static inlines instead of macros failed due
to false positive triggering of BUILD_BUG_ON()s, especially with
GCC < 6.0.
Signed-off-by: Jakub Kicinski
Reviewed-by: Dinan Gunawardena
---
include/linux/bitfield.h | 109 +++
include/linux/bug.h | 3 ++
2 files changed, 112 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..ff9fd0af2ac7
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+#include
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _val) \
+ ({ \
+ BUILD_BUG_ON(!(_mask)); \
+ BUILD_BUG_ON(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) :\
+0);\
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << _bf_shf(_mask))); \
+ })
+
+/*
+ * Bitfield access macros
+ *
+ * This file contains macros which take as input shifted mask
+ * from which they extract the base mask and shift amount at
+ * compilation time. There are two separate sets of the macros
+ * one for 32bit registers and one for 64bit ones.
+ *
+ * Fields can be defined using GENMASK (which is usually
+ * less error-prone and easier to match with datasheets).
+ *
+ * FIELD_{GET,PUT} macros are designed to be used with masks which
+ * are compilation time constants.
+ *
+ * Example:
+ *
+ * #define REG_FIELD_A GENMASK(6, 0)
+ * #define REG_FIELD_B BIT(7)
+ * #define REG_FIELD_C GENMASK(15, 8)
+ * #define REG_FIELD_D GENMASK(31, 16)
+ *
+ * Get:
+ * a = FIELD_GET(REG_FIELD_A, reg);
+ * b = FIELD_GET(REG_FIELD_B, reg);
+ *
+ * Set:
+ * reg = FIELD_PUT(REG_FIELD_A, 1) |
+ * FIELD_PUT(REG_FIELD_B, 0) |
+ * FIELD_PUT(REG_FIELD_C, c) |
+ * FIELD_PUT(REG_FIELD_D, 0x40);
+ *
+ * Modify:
+ * reg &= ~REG_FIELD_C;
+ * reg |= FIELD_PUT(REG_FIELD_C, c);
+ */
+
+/**
+ * FIELD_PUT() - construct a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: value to put in the field
+ *
+ * FIELD_PUT() masks and shifts up the value. The result should
+ * be combined with other fields of the bitfield using logical OR.
+ */
+#define FIELD_PUT(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u32)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+/**
+ * FIELD_GET() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: 32bit value of entire bitfield
+ *
+ * FIELD_GET() extracts the field specified by @_mask from the
+ * bitfield passed in as @_val.
+ */
+#define FIELD_GET(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u
[PATCHv6 0/2] register-field manipulation macros
Hi! This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro and have that macro compute shift at compilation time using FFS. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). I'm planning to use those macros in another driver soon, Felix may also use them when upstreaming mt76 if he chooses to do so. IMHO if accepted it would be best to push these through Kalle's wireless drivers' tree, since the only existing user is in drivers/net/wireless/. As lowly device driver developers we would greatly appreciate an Ack or a nod from the experts. v6: Do a full rename in patch 2. CC many people. v4: - add documentation in the header. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (2): add basic register-field manipulation macros mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- drivers/net/wireless/mediatek/mt7601u/dma.h | 10 +-- drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 +-- drivers/net/wireless/mediatek/mt7601u/init.c| 9 +- drivers/net/wireless/mediatek/mt7601u/mac.c | 38 - drivers/net/wireless/mediatek/mt7601u/mcu.c | 18 ++-- drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- drivers/net/wireless/mediatek/mt7601u/phy.c | 36 drivers/net/wireless/mediatek/mt7601u/tx.c | 16 ++-- drivers/net/wireless/mediatek/mt7601u/util.h| 77 - include/linux/bitfield.h| 109 include/linux/bug.h | 3 + 12 files changed, 184 insertions(+), 150 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1
Re: [PATCHv5 wl-drv-next 0/2] register-field manipulation macros
On Tue, 26 Jul 2016 16:00:04 +0300, Kalle Valo wrote: > Jakub Kicinski writes: > > > On Wed, 6 Jul 2016 17:19:35 +0100, Jakub Kicinski wrote: > >> Hi! > >> > >> I've added few lines about the compilation problems in > >> the commit message of patch 1. I would prefer the mass > >> rename of macros in mt7601u not to be part of this series > >> so patch 2 is left as it was and I'll follow up once this > >> is accepted. > > > > Hi Kalle, > > > > what's the status of this set? It's marked as 'Deferred' in > > linux-wireless patchwork. > > Like I said before, I would prefer to see an ack from someone else like > Andrew Morton or Dave Miller. I don't feel comfortable adding new files > to include/ without some sort of support from others. Or maybe Andrew > could even take these directly to his tree? OK, once the merge window is over I'll repost and TO: some important guys. Hopefully we'll catch someone's attention :) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 wl-drv-next 0/2] register-field manipulation macros
On Wed, 6 Jul 2016 17:19:35 +0100, Jakub Kicinski wrote: > Hi! > > I've added few lines about the compilation problems in > the commit message of patch 1. I would prefer the mass > rename of macros in mt7601u not to be part of this series > so patch 2 is left as it was and I'll follow up once this > is accepted. Hi Kalle, what's the status of this set? It's marked as 'Deferred' in linux-wireless patchwork. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [oss-drivers] [PATCH][V2] nfp: check idx is -ENOSPC before using it is an index
On Mon, 11 Jul 2016 16:54:20 +0100, Colin King wrote: > From: Colin Ian King > > idx can be returned as -ENOSPC, so we should check for this first > before using it as an index into nn->vxlan_usecnt[] to avoid an > out of bounds array offset read. > > Signed-off-by: Colin Ian King Acked-by: Jakub Kicinski Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv5 wl-drv-next 0/2] register-field manipulation macros
Hi! I've added few lines about the compilation problems in the commit message of patch 1. I would prefer the mass rename of macros in mt7601u not to be part of this series so patch 2 is left as it was and I'll follow up once this is accepted. == v4 blurb This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro. It is also nice to have that macro compute the shift amount based on the mask automatically. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. v3: Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). Please review and advise on improvements. If accepted I think would be best to push this through Kalle's tree, since the only existing user is in drivers/net/wireless/. v4: - add documentation in the header. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (2): add basic register-field manipulation macros mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.h | 2 - drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +- drivers/net/wireless/mediatek/mt7601u/util.h| 77 - include/linux/bitfield.h| 109 include/linux/bug.h | 3 + 5 files changed, 116 insertions(+), 80 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv5 wl-drv-next 2/2] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.h | 2 -
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +-
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
3 files changed, 4 insertions(+), 80 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..166ac71905d2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 428bd2f10b7b..5ef62e02ce66 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -15,6 +15,7 @@
#ifndef MT7601U_H
#define MT7601U_H
+#include
#include
#include
#include
@@ -24,7 +25,6 @@
#include
#include "regs.h"
-#include "util.h"
#define MT_CALIBRATE_INTERVAL (4 * HZ)
@@ -282,6 +282,9 @@ struct mt7601u_rxwi;
extern const struct ieee80211_ops mt7601u_ops;
+#define MT76_SET FIELD_PUT
+#define MT76_GET FIELD_GET
+
void mt7601u_init_debugfs(struct mt7601u_dev *dev);
u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset);
diff --git a/drivers/net/wireless/mediatek/mt7601u/util.h
b/drivers/net/wireless/mediatek/mt7601u/util.h
deleted file mode 100644
index b89140bf1210..
--- a/drivers/net/wireless/mediatek/mt7601u/util.h
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2014 Felix Fietkau
- * Copyright (C) 2004 - 2009 Ivo van Doorn
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef __MT76_UTIL_H
-#define __MT76_UTIL_H
-
-/*
- * Power of two check, this will check
- * if the mask that has been given contains and contiguous set of bits.
- * Note that we cannot use the is_power_of_2() function since this
- * check must be done at compile-time.
- */
-#define is_power_of_two(x) ( !((x) & ((x)-1)) )
-#define low_bit_mask(x)( ((x)-1) & ~(x) )
-#define is_valid_mask(x) is_power_of_two(1LU + (x) + low_bit_mask(x))
-
-/*
- * Macros to find first set bit in a variable.
- * These macros behave the same as the __ffs() functions but
- * the most important difference that this is done during
- * compile-time rather then run-time.
- */
-#define compile_ffs2(__x) \
- __builtin_choose_expr(((__x) & 0x1), 0, 1)
-
-#define compile_ffs4(__x) \
- __builtin_choose_expr(((__x) & 0x3), \
- (compile_ffs2((__x))), \
- (compile_ffs2((__x) >> 2) + 2))
-
-#define compile_ffs8(__x) \
- __builtin_choose_expr(((__x) & 0xf), \
- (compile_ffs4((__x))), \
- (compile_ffs4((__x) >> 4) + 4))
-
-#define compile_ffs16(__x) \
- __builtin_choose_expr(((__x) & 0xff), \
- (compile_ffs8((__x))), \
- (compile_ffs8((__x) >> 8) + 8))
-
-#define compile_ffs32(__x) \
- __builtin_choose_expr(((__x) & 0x), \
- (compile_ffs16((__x))), \
- (compile_ffs16((__x) >> 16) + 16))
-
-/*
- * This macro will check the requirements for the FIELD{8,16,32} macros
- * The mask should be a constant non-zero contiguous set of bits which
- * does not exceed the given typelimit.
- */
-#define FIELD_CHECK(__mask) \
- BUILD_BUG_ON(!(__mask) || !is_valid_mask(__mask))
-
-#define MT76_SET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (((u32) (_val)) << compile_ffs32(_mask)) & _mask; \
- })
-
-#define MT76_GET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (u32) (((_val) & _mask) >> compile_ffs32(_mask)); \
- })
-
-#endif
--
1.9.1
--
To unsubscribe from this list: send the
[PATCHv5 wl-drv-next 1/2] add basic register-field manipulation macros
Common approach to accessing register fields is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define REG_FIELD 0x000ff000
field = FIELD_GET(REG_FIELD, reg);
reg &= ~REG_FIELD;
reg |= FIELD_PUT(REG_FIELD, field);
FIELD_{GET,PUT} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Attempts to use static inlines instead of macros failed due
to false positive triggering of BUILD_BUG_ON()s, especially with
GCC < 6.0.
Signed-off-by: Jakub Kicinski
---
include/linux/bitfield.h | 109 +++
include/linux/bug.h | 3 ++
2 files changed, 112 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..ff9fd0af2ac7
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+#include
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _val) \
+ ({ \
+ BUILD_BUG_ON(!(_mask)); \
+ BUILD_BUG_ON(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) :\
+0);\
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << _bf_shf(_mask))); \
+ })
+
+/*
+ * Bitfield access macros
+ *
+ * This file contains macros which take as input shifted mask
+ * from which they extract the base mask and shift amount at
+ * compilation time. There are two separate sets of the macros
+ * one for 32bit registers and one for 64bit ones.
+ *
+ * Fields can be defined using GENMASK (which is usually
+ * less error-prone and easier to match with datasheets).
+ *
+ * FIELD_{GET,PUT} macros are designed to be used with masks which
+ * are compilation time constants.
+ *
+ * Example:
+ *
+ * #define REG_FIELD_A GENMASK(6, 0)
+ * #define REG_FIELD_B BIT(7)
+ * #define REG_FIELD_C GENMASK(15, 8)
+ * #define REG_FIELD_D GENMASK(31, 16)
+ *
+ * Get:
+ * a = FIELD_GET(REG_FIELD_A, reg);
+ * b = FIELD_GET(REG_FIELD_B, reg);
+ *
+ * Set:
+ * reg = FIELD_PUT(REG_FIELD_A, 1) |
+ * FIELD_PUT(REG_FIELD_B, 0) |
+ * FIELD_PUT(REG_FIELD_C, c) |
+ * FIELD_PUT(REG_FIELD_D, 0x40);
+ *
+ * Modify:
+ * reg &= ~REG_FIELD_C;
+ * reg |= FIELD_PUT(REG_FIELD_C, c);
+ */
+
+/**
+ * FIELD_PUT() - construct a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: value to put in the field
+ *
+ * FIELD_PUT() masks and shifts up the value. The result should
+ * be combined with other fields of the bitfield using logical OR.
+ */
+#define FIELD_PUT(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u32)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+/**
+ * FIELD_GET() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: 32bit value of entire bitfield
+ *
+ * FIELD_GET() extracts the field specified by @_mask from the
+ * bitfield passed in as @_val.
+ */
+#define FIELD_GET(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u32)(((
[PATCHv4 wl-drv-next 2/2] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.h | 2 -
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +-
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
3 files changed, 4 insertions(+), 80 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..166ac71905d2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 428bd2f10b7b..5ef62e02ce66 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -15,6 +15,7 @@
#ifndef MT7601U_H
#define MT7601U_H
+#include
#include
#include
#include
@@ -24,7 +25,6 @@
#include
#include "regs.h"
-#include "util.h"
#define MT_CALIBRATE_INTERVAL (4 * HZ)
@@ -282,6 +282,9 @@ struct mt7601u_rxwi;
extern const struct ieee80211_ops mt7601u_ops;
+#define MT76_SET FIELD_PUT
+#define MT76_GET FIELD_GET
+
void mt7601u_init_debugfs(struct mt7601u_dev *dev);
u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset);
diff --git a/drivers/net/wireless/mediatek/mt7601u/util.h
b/drivers/net/wireless/mediatek/mt7601u/util.h
deleted file mode 100644
index b89140bf1210..
--- a/drivers/net/wireless/mediatek/mt7601u/util.h
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2014 Felix Fietkau
- * Copyright (C) 2004 - 2009 Ivo van Doorn
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef __MT76_UTIL_H
-#define __MT76_UTIL_H
-
-/*
- * Power of two check, this will check
- * if the mask that has been given contains and contiguous set of bits.
- * Note that we cannot use the is_power_of_2() function since this
- * check must be done at compile-time.
- */
-#define is_power_of_two(x) ( !((x) & ((x)-1)) )
-#define low_bit_mask(x)( ((x)-1) & ~(x) )
-#define is_valid_mask(x) is_power_of_two(1LU + (x) + low_bit_mask(x))
-
-/*
- * Macros to find first set bit in a variable.
- * These macros behave the same as the __ffs() functions but
- * the most important difference that this is done during
- * compile-time rather then run-time.
- */
-#define compile_ffs2(__x) \
- __builtin_choose_expr(((__x) & 0x1), 0, 1)
-
-#define compile_ffs4(__x) \
- __builtin_choose_expr(((__x) & 0x3), \
- (compile_ffs2((__x))), \
- (compile_ffs2((__x) >> 2) + 2))
-
-#define compile_ffs8(__x) \
- __builtin_choose_expr(((__x) & 0xf), \
- (compile_ffs4((__x))), \
- (compile_ffs4((__x) >> 4) + 4))
-
-#define compile_ffs16(__x) \
- __builtin_choose_expr(((__x) & 0xff), \
- (compile_ffs8((__x))), \
- (compile_ffs8((__x) >> 8) + 8))
-
-#define compile_ffs32(__x) \
- __builtin_choose_expr(((__x) & 0x), \
- (compile_ffs16((__x))), \
- (compile_ffs16((__x) >> 16) + 16))
-
-/*
- * This macro will check the requirements for the FIELD{8,16,32} macros
- * The mask should be a constant non-zero contiguous set of bits which
- * does not exceed the given typelimit.
- */
-#define FIELD_CHECK(__mask) \
- BUILD_BUG_ON(!(__mask) || !is_valid_mask(__mask))
-
-#define MT76_SET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (((u32) (_val)) << compile_ffs32(_mask)) & _mask; \
- })
-
-#define MT76_GET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (u32) (((_val) & _mask) >> compile_ffs32(_mask)); \
- })
-
-#endif
--
1.9.1
--
To unsubscribe from this list: send the
[PATCHv4 wl-drv-next 1/2] add basic register-field manipulation macros
Common approach to accessing register fields is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define REG_FIELD 0x000ff000
field = FIELD_GET(REG_FIELD, reg);
reg &= ~REG_FIELD;
reg |= FIELD_PUT(REG_FIELD, field);
FIELD_{GET,PUT} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Signed-off-by: Jakub Kicinski
---
include/linux/bitfield.h | 109 +++
include/linux/bug.h | 3 ++
2 files changed, 112 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..ff9fd0af2ac7
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+#include
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _val) \
+ ({ \
+ BUILD_BUG_ON(!(_mask)); \
+ BUILD_BUG_ON(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) :\
+0);\
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << _bf_shf(_mask))); \
+ })
+
+/*
+ * Bitfield access macros
+ *
+ * This file contains macros which take as input shifted mask
+ * from which they extract the base mask and shift amount at
+ * compilation time. There are two separate sets of the macros
+ * one for 32bit registers and one for 64bit ones.
+ *
+ * Fields can be defined using GENMASK (which is usually
+ * less error-prone and easier to match with datasheets).
+ *
+ * FIELD_{GET,PUT} macros are designed to be used with masks which
+ * are compilation time constants.
+ *
+ * Example:
+ *
+ * #define REG_FIELD_A GENMASK(6, 0)
+ * #define REG_FIELD_B BIT(7)
+ * #define REG_FIELD_C GENMASK(15, 8)
+ * #define REG_FIELD_D GENMASK(31, 16)
+ *
+ * Get:
+ * a = FIELD_GET(REG_FIELD_A, reg);
+ * b = FIELD_GET(REG_FIELD_B, reg);
+ *
+ * Set:
+ * reg = FIELD_PUT(REG_FIELD_A, 1) |
+ * FIELD_PUT(REG_FIELD_B, 0) |
+ * FIELD_PUT(REG_FIELD_C, c) |
+ * FIELD_PUT(REG_FIELD_D, 0x40);
+ *
+ * Modify:
+ * reg &= ~REG_FIELD_C;
+ * reg |= FIELD_PUT(REG_FIELD_C, c);
+ */
+
+/**
+ * FIELD_PUT() - construct a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: value to put in the field
+ *
+ * FIELD_PUT() masks and shifts up the value. The result should
+ * be combined with other fields of the bitfield using logical OR.
+ */
+#define FIELD_PUT(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u32)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+/**
+ * FIELD_GET() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val: 32bit value of entire bitfield
+ *
+ * FIELD_GET() extracts the field specified by @_mask from the
+ * bitfield passed in as @_val.
+ */
+#define FIELD_GET(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u32)(((_val) & (_mask)) >> _bf_shf(_mask));\
+ })
+
+#define FIELD_PUT64(_mask, _val) \
+ ({
[PATCHv4 wl-drv-next 0/2] register-field manipulation macros
Hi! This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro. It is also nice to have that macro compute the shift amount based on the mask automatically. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. v3: Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). Please review and advise on improvements. If accepted I think would be best to push this through Kalle's tree, since the only existing user is in drivers/net/wireless/. v4: - add documentation in the header. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (2): add basic register-field manipulation macros mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.h | 2 - drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +- drivers/net/wireless/mediatek/mt7601u/util.h| 77 - include/linux/bitfield.h| 110 include/linux/bug.h | 3 + 5 files changed, 117 insertions(+), 80 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 wl-drv-next 1/2] add basic register-field manipulation macros
On Tue, 5 Jul 2016 10:56:13 +, David Laight wrote: > From: Jakub Kicinski > > Sent: 01 July 2016 22:27 > > > > C bitfields are problematic and best avoided. Developers > > interacting with hardware registers find themselves searching > > for easy-to-use alternatives. Common approach is to define > > structures or sets of macros containing mask and shift pair. > > Operations on the register are then performed as follows: > > > > field = (reg >> shift) & mask; > > > > reg &= ~(mask << shift); > > reg |= (field & mask) << shift; > > > > Defining shift and mask separately is tedious. Ivo van Doorn > > came up with an idea of computing them at compilation time > > based on a single shifted mask (later refined by Felix) which > > can be used like this: > > > > #define REG_FIELD 0x000ff000 > > > > field = FIELD_GET(REG_FIELD, reg); > > > > reg &= ~REG_FIELD; > > reg |= FIELD_PUT(REG_FIELD, field); > > My problem with these sort of 'helpers' is that they make it much harder > to read code unless you happen to know exactly what the helpers do. > Unexpected issues (like values being sign extended) can be hard to spot. > > A lot of the time you can make things simpler by not doing the shifts > (ie define shifted constants). I think creating a standard set of macros in a global header file is actually helping the problems you raise. One is much more likely to know exactly what a common macro is doing than some driver-specific ad hoc macro. Common macros are also much more likely to be well tested making "unexpected issues" less likely to appear. Defining constants is fine in 20% of cases when you have only a small set of meaningful values (e.g. what to do for a 8 bit delay or priority field?) Besides when fields are not aligned to 4 bits it's hard to tell from the shifted value what the base was. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 wl-drv-next 1/2] add basic register-field manipulation macros
C bitfields are problematic and best avoided. Developers
interacting with hardware registers find themselves searching
for easy-to-use alternatives. Common approach is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define REG_FIELD 0x000ff000
field = FIELD_GET(REG_FIELD, reg);
reg &= ~REG_FIELD;
reg |= FIELD_PUT(REG_FIELD, field);
FIELD_{GET,PUT} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Signed-off-by: Jakub Kicinski
---
include/linux/bitfield.h | 58
include/linux/bug.h | 3 +++
2 files changed, 61 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..d6a36c3c1775
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+#include
+#include
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _val) \
+ ({ \
+ BUILD_BUG_ON(!(_mask)); \
+ BUILD_BUG_ON(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) :\
+0);\
+ __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
+ (1ULL << _bf_shf(_mask))); \
+ })
+
+#define FIELD_PUT(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u32)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+#define FIELD_GET(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u32)(((_val) & (_mask)) >> _bf_shf(_mask));\
+ })
+
+#define FIELD_PUT64(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u64)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+#define FIELD_GET64(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u64)(((_val) & (_mask)) >> _bf_shf(_mask));\
+ })
+
+#endif
diff --git a/include/linux/bug.h b/include/linux/bug.h
index e51b0709e78d..bba5bdae1681 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -13,6 +13,7 @@ enum bug_trap_type {
struct pt_regs;
#ifdef __CHECKER__
+#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) (0)
#define BUILD_BUG_ON_NOT_POWER_OF_2(n) (0)
#define BUILD_BUG_ON_ZERO(e) (0)
#define BUILD_BUG_ON_NULL(e) ((void*)0)
@@ -24,6 +25,8 @@ struct pt_regs;
#else /* __CHECKER__ */
/* Force a compilation error if a constant expression is not a power of 2 */
+#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
+ BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 wl-drv-next 2/2] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.h | 2 -
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +-
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
3 files changed, 4 insertions(+), 80 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..166ac71905d2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 428bd2f10b7b..5ef62e02ce66 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -15,6 +15,7 @@
#ifndef MT7601U_H
#define MT7601U_H
+#include
#include
#include
#include
@@ -24,7 +25,6 @@
#include
#include "regs.h"
-#include "util.h"
#define MT_CALIBRATE_INTERVAL (4 * HZ)
@@ -282,6 +282,9 @@ struct mt7601u_rxwi;
extern const struct ieee80211_ops mt7601u_ops;
+#define MT76_SET FIELD_PUT
+#define MT76_GET FIELD_GET
+
void mt7601u_init_debugfs(struct mt7601u_dev *dev);
u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset);
diff --git a/drivers/net/wireless/mediatek/mt7601u/util.h
b/drivers/net/wireless/mediatek/mt7601u/util.h
deleted file mode 100644
index b89140bf1210..
--- a/drivers/net/wireless/mediatek/mt7601u/util.h
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2014 Felix Fietkau
- * Copyright (C) 2004 - 2009 Ivo van Doorn
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef __MT76_UTIL_H
-#define __MT76_UTIL_H
-
-/*
- * Power of two check, this will check
- * if the mask that has been given contains and contiguous set of bits.
- * Note that we cannot use the is_power_of_2() function since this
- * check must be done at compile-time.
- */
-#define is_power_of_two(x) ( !((x) & ((x)-1)) )
-#define low_bit_mask(x)( ((x)-1) & ~(x) )
-#define is_valid_mask(x) is_power_of_two(1LU + (x) + low_bit_mask(x))
-
-/*
- * Macros to find first set bit in a variable.
- * These macros behave the same as the __ffs() functions but
- * the most important difference that this is done during
- * compile-time rather then run-time.
- */
-#define compile_ffs2(__x) \
- __builtin_choose_expr(((__x) & 0x1), 0, 1)
-
-#define compile_ffs4(__x) \
- __builtin_choose_expr(((__x) & 0x3), \
- (compile_ffs2((__x))), \
- (compile_ffs2((__x) >> 2) + 2))
-
-#define compile_ffs8(__x) \
- __builtin_choose_expr(((__x) & 0xf), \
- (compile_ffs4((__x))), \
- (compile_ffs4((__x) >> 4) + 4))
-
-#define compile_ffs16(__x) \
- __builtin_choose_expr(((__x) & 0xff), \
- (compile_ffs8((__x))), \
- (compile_ffs8((__x) >> 8) + 8))
-
-#define compile_ffs32(__x) \
- __builtin_choose_expr(((__x) & 0x), \
- (compile_ffs16((__x))), \
- (compile_ffs16((__x) >> 16) + 16))
-
-/*
- * This macro will check the requirements for the FIELD{8,16,32} macros
- * The mask should be a constant non-zero contiguous set of bits which
- * does not exceed the given typelimit.
- */
-#define FIELD_CHECK(__mask) \
- BUILD_BUG_ON(!(__mask) || !is_valid_mask(__mask))
-
-#define MT76_SET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (((u32) (_val)) << compile_ffs32(_mask)) & _mask; \
- })
-
-#define MT76_GET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (u32) (((_val) & _mask) >> compile_ffs32(_mask)); \
- })
-
-#endif
--
1.9.1
--
To unsubscribe from this list: send the
[PATCHv3 wl-drv-next 0/2] register-field manipulation macros
Hi! This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro. It is also nice to have that macro compute the shift amount based on the mask automatically. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. v3: Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). Please review and advise on improvements. If accepted I think would be best to push this through Kalle's tree, since the only existing user is in drivers/net/wireless/. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (2): add basic register-field manipulation macros mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.h | 2 - drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +- drivers/net/wireless/mediatek/mt7601u/util.h| 77 - include/linux/bitfield.h| 58 +++ include/linux/bug.h | 3 + 5 files changed, 65 insertions(+), 80 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] add basic register-field manipulation macros
On Tue, 14 Jun 2016 20:53:28 +0200, Arend van Spriel wrote:
> On 14-06-16 13:44, Jakub Kicinski wrote:
> > +#ifndef _LINUX_BITFIELD_H
> > +#define _LINUX_BITFIELD_H
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#define _bf_shf(x) (__builtin_ffsll(x) - 1)
> > +
> > +#define _BF_FIELD_CHECK(_mask, _val)
> > \
> > + ({ \
> > + const u64 hi = (_mask) + (1ULL << _bf_shf(_mask)); \
> > + \
> > + BUILD_BUG_ON(!(_mask) || (hi && !is_power_of_2_u64(hi))); \
> > + BUILD_BUG_ON(__builtin_constant_p(_val) ? \
> > +~((_mask) >> _bf_shf(_mask)) & (_val) :\
> > +0);\
> > + })
>
> I am sceptic whether it is useful to have 64-bit used here and there is
> a price to pay on (many) 32-bit architectures for using 64-bit
> operations. Maybe it is not an issue because it is inside BUILD_BUG_ON()
> macro.
It's a trade-off between having a separate set of macros for 32-bit
machines and risking someone potentially loosing cycles when using the
macros in a non-standard way. Note that all 64 bit operations are
performed on the mask which I expect to be compile time constant.
> > +#define FIELD_PUT(_mask, _val) \
> > + ({ \
> > + _BF_FIELD_CHECK(_mask, _val); \
> > + ((u32)(_val) << _bf_shf(_mask)) & (_mask); \
> > + })
> > +
> > +#define FIELD_GET(_mask, _val) \
> > + ({ \
> > + _BF_FIELD_CHECK(_mask, 0); \
> > + (u32)(((_val) & (_mask)) >> _bf_shf(_mask));\
> > + })
> > +
> > +#define FIELD_PUT64(_mask, _val) \
> > + ({ \
> > + _BF_FIELD_CHECK(_mask, _val); \
> > + ((u64)(_val) << _bf_shf(_mask)) & (_mask); \
> > + })
> > +
> > +#define FIELD_GET64(_mask, _val) \
> > + ({ \
> > + _BF_FIELD_CHECK(_mask, 0); \
> > + (u64)(((_val) & (_mask)) >> _bf_shf(_mask));\
> > + })
>
> Is there really hardware out there that exposes 64-bit wide hardware
> registers?
I need this for encoding 64-bit wide RISC instructions [1] to be honest.
[1] http://thread.gmane.org/gmane.linux.network/414538
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 0/2] register-field manipulation macros
Hi! This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro. It is also nice to have that macro compute the shift amount based on the mask automatically. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. Please review and advise on improvements. If accepted I think would be best to push this through Kalle's tree, since the only existing user is in drivers/net/wireless/. v2: - change Felix's email address. Jakub Kicinski (2): add basic register-field manipulation macros mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.h | 2 - drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +- drivers/net/wireless/mediatek/mt7601u/util.h| 77 - include/linux/bitfield.h| 58 +++ include/linux/log2.h| 6 ++ 5 files changed, 68 insertions(+), 80 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 1/2] add basic register-field manipulation macros
C bitfields are problematic and best avoided. Developers
interacting with hardware registers find themselves searching
for easy-to-use alternatives. Common approach is to define
structures or sets of macros containing mask and shift pair.
Operations on the register are then performed as follows:
field = (reg >> shift) & mask;
reg &= ~(mask << shift);
reg |= (field & mask) << shift;
Defining shift and mask separately is tedious. Ivo van Doorn
came up with an idea of computing them at compilation time
based on a single shifted mask (later refined by Felix) which
can be used like this:
#define X_REG_FIELD 0x000ff000
field = FIELD_GET(X_REG_FIELD, reg);
reg &= ~X_REG_FIELD;
reg |= FIELD_PUT(X_REG_FIELD, field);
FIELD_{GET,PUT} macros take care of finding out what the
appropriate shift is based on compilation time ffs operation.
GENMASK can be used to define registers (which is usually
less error-prone and easier to match with datasheets).
This approach is the most convenient I've seen so to limit code
multiplication let's move the macros to a global header file.
Compared to Felix Fietkau's implementation from mt76 this one
uses standard Linux and GCC functions such as is_power_of_2()
and __builtin_ffsll().
Signed-off-by: Jakub Kicinski
---
v2:
- change Felix's email address.
include/linux/bitfield.h | 58
include/linux/log2.h | 6 +
2 files changed, 64 insertions(+)
create mode 100644 include/linux/bitfield.h
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
new file mode 100644
index ..9560d1877cbc
--- /dev/null
+++ b/include/linux/bitfield.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2014 Felix Fietkau
+ * Copyright (C) 2004 - 2009 Ivo van Doorn
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_BITFIELD_H
+#define _LINUX_BITFIELD_H
+
+#include
+#include
+#include
+
+#define _bf_shf(x) (__builtin_ffsll(x) - 1)
+
+#define _BF_FIELD_CHECK(_mask, _val) \
+ ({ \
+ const u64 hi = (_mask) + (1ULL << _bf_shf(_mask)); \
+ \
+ BUILD_BUG_ON(!(_mask) || (hi && !is_power_of_2_u64(hi))); \
+ BUILD_BUG_ON(__builtin_constant_p(_val) ? \
+~((_mask) >> _bf_shf(_mask)) & (_val) :\
+0);\
+ })
+
+#define FIELD_PUT(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u32)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+#define FIELD_GET(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u32)(((_val) & (_mask)) >> _bf_shf(_mask));\
+ })
+
+#define FIELD_PUT64(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, _val); \
+ ((u64)(_val) << _bf_shf(_mask)) & (_mask); \
+ })
+
+#define FIELD_GET64(_mask, _val) \
+ ({ \
+ _BF_FIELD_CHECK(_mask, 0); \
+ (u64)(((_val) & (_mask)) >> _bf_shf(_mask));\
+ })
+
+#endif
diff --git a/include/linux/log2.h b/include/linux/log2.h
index fd7ff3d91e6a..1b5e1f4da043 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -54,6 +54,12 @@ bool is_power_of_2(unsigned long n)
return (n != 0 && ((n & (n - 1)) == 0));
}
+static inline __attribute__((const))
+bool is_power_of_2_u64(u64 n)
+{
+ return (n != 0 && ((n & (n - 1)) == 0));
+}
+
/*
* round up to nearest power of two
*/
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 2/2] mt7601u: use linux/bitfield.h
Use the newly added linux/bitfield.h.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.h | 2 -
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 5 +-
drivers/net/wireless/mediatek/mt7601u/util.h| 77 -
3 files changed, 4 insertions(+), 80 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h
b/drivers/net/wireless/mediatek/mt7601u/dma.h
index 978e8a90b87f..166ac71905d2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.h
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.h
@@ -18,8 +18,6 @@
#include
#include
-#include "util.h"
-
#define MT_DMA_HDR_LEN 4
#define MT_RX_INFO_LEN 4
#define MT_FCE_INFO_LEN4
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 428bd2f10b7b..5ef62e02ce66 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -15,6 +15,7 @@
#ifndef MT7601U_H
#define MT7601U_H
+#include
#include
#include
#include
@@ -24,7 +25,6 @@
#include
#include "regs.h"
-#include "util.h"
#define MT_CALIBRATE_INTERVAL (4 * HZ)
@@ -282,6 +282,9 @@ struct mt7601u_rxwi;
extern const struct ieee80211_ops mt7601u_ops;
+#define MT76_SET FIELD_PUT
+#define MT76_GET FIELD_GET
+
void mt7601u_init_debugfs(struct mt7601u_dev *dev);
u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset);
diff --git a/drivers/net/wireless/mediatek/mt7601u/util.h
b/drivers/net/wireless/mediatek/mt7601u/util.h
deleted file mode 100644
index b89140bf1210..
--- a/drivers/net/wireless/mediatek/mt7601u/util.h
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2014 Felix Fietkau
- * Copyright (C) 2004 - 2009 Ivo van Doorn
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef __MT76_UTIL_H
-#define __MT76_UTIL_H
-
-/*
- * Power of two check, this will check
- * if the mask that has been given contains and contiguous set of bits.
- * Note that we cannot use the is_power_of_2() function since this
- * check must be done at compile-time.
- */
-#define is_power_of_two(x) ( !((x) & ((x)-1)) )
-#define low_bit_mask(x)( ((x)-1) & ~(x) )
-#define is_valid_mask(x) is_power_of_two(1LU + (x) + low_bit_mask(x))
-
-/*
- * Macros to find first set bit in a variable.
- * These macros behave the same as the __ffs() functions but
- * the most important difference that this is done during
- * compile-time rather then run-time.
- */
-#define compile_ffs2(__x) \
- __builtin_choose_expr(((__x) & 0x1), 0, 1)
-
-#define compile_ffs4(__x) \
- __builtin_choose_expr(((__x) & 0x3), \
- (compile_ffs2((__x))), \
- (compile_ffs2((__x) >> 2) + 2))
-
-#define compile_ffs8(__x) \
- __builtin_choose_expr(((__x) & 0xf), \
- (compile_ffs4((__x))), \
- (compile_ffs4((__x) >> 4) + 4))
-
-#define compile_ffs16(__x) \
- __builtin_choose_expr(((__x) & 0xff), \
- (compile_ffs8((__x))), \
- (compile_ffs8((__x) >> 8) + 8))
-
-#define compile_ffs32(__x) \
- __builtin_choose_expr(((__x) & 0x), \
- (compile_ffs16((__x))), \
- (compile_ffs16((__x) >> 16) + 16))
-
-/*
- * This macro will check the requirements for the FIELD{8,16,32} macros
- * The mask should be a constant non-zero contiguous set of bits which
- * does not exceed the given typelimit.
- */
-#define FIELD_CHECK(__mask) \
- BUILD_BUG_ON(!(__mask) || !is_valid_mask(__mask))
-
-#define MT76_SET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (((u32) (_val)) << compile_ffs32(_mask)) & _mask; \
- })
-
-#define MT76_GET(_mask, _val) \
- ({ \
- FIELD_CHECK(_mask); \
- (u32) (((_val) & _mask) >> compile_ffs32(_mask)); \
- })
-
-#endif
--
1.9.1
--
To unsubscribe from this list: send the
Re: [PATCH v2 23/27] rt2x00: move under ralink vendor directory
On Wed, 18 Nov 2015 16:46:02 +0200, Kalle Valo wrote: > Part of reorganising wireless drivers directory and Kconfig. > > Signed-off-by: Kalle Valo For Ralink you could probably drop the rt2x00 directory. RaLink Tech. doesn't exist any more and rt2x00 contains drivers for all of their devices. Obviously this is just a suggestion, not a show stopper. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next 2/4] mt7601u: use correct ieee80211_rx variant
From: Jakub Kicinski Rx is run inside a tasklet so ieee80211_rx() should be used instead of ieee80211_rx_ni(). Signed-off-by: Jakub Kicinski --- drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c index 7217da4f1543..fb183e369d92 100644 --- a/drivers/net/wireless/mediatek/mt7601u/dma.c +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c @@ -112,7 +112,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data, if (!skb) return; - ieee80211_rx_ni(dev->hw, skb); + ieee80211_rx(dev->hw, skb); } static u16 mt7601u_rx_next_seg_len(u8 *data, u32 data_len) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next 3/4] mt7601u: fix tx status reporting contexts
From: Jakub Kicinski
mac80211 requires that rx path does not run concurrently with
tx status reporting. Since rx path is run in driver tasklet,
tx status cannot be reported directly from interrupt context
(there would be no way to lock it out).
Add tasklet for tx and move all possible code from irq handler
there.
Note: tx tasklet is needed because workqueue is queued very
rarely and that kills TCP performance.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 30 +
drivers/net/wireless/mediatek/mt7601u/init.c| 1 +
drivers/net/wireless/mediatek/mt7601u/mac.c | 4
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 2 ++
4 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index fb183e369d92..63c485443a38 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -236,23 +236,42 @@ static void mt7601u_complete_tx(struct urb *urb)
skb = q->e[q->start].skb;
trace_mt_tx_dma_done(dev, skb);
- mt7601u_tx_status(dev, skb);
+ __skb_queue_tail(&dev->tx_skb_done, skb);
+ tasklet_schedule(&dev->tx_tasklet);
if (q->used == q->entries - q->entries / 8)
ieee80211_wake_queue(dev->hw, skb_get_queue_mapping(skb));
q->start = (q->start + 1) % q->entries;
q->used--;
+out:
+ spin_unlock_irqrestore(&dev->tx_lock, flags);
+}
- if (urb->status)
- goto out;
+static void mt7601u_tx_tasklet(unsigned long data)
+{
+ struct mt7601u_dev *dev = (struct mt7601u_dev *) data;
+ struct sk_buff_head skbs;
+ unsigned long flags;
+
+ __skb_queue_head_init(&skbs);
+
+ spin_lock_irqsave(&dev->tx_lock, flags);
set_bit(MT7601U_STATE_MORE_STATS, &dev->state);
if (!test_and_set_bit(MT7601U_STATE_READING_STATS, &dev->state))
queue_delayed_work(dev->stat_wq, &dev->stat_work,
msecs_to_jiffies(10));
-out:
+
+ skb_queue_splice_init(&dev->tx_skb_done, &skbs);
+
spin_unlock_irqrestore(&dev->tx_lock, flags);
+
+ while (!skb_queue_empty(&skbs)) {
+ struct sk_buff *skb = __skb_dequeue(&skbs);
+
+ mt7601u_tx_status(dev, skb);
+ }
}
static int mt7601u_dma_submit_tx(struct mt7601u_dev *dev,
@@ -475,6 +494,7 @@ int mt7601u_dma_init(struct mt7601u_dev *dev)
{
int ret = -ENOMEM;
+ tasklet_init(&dev->tx_tasklet, mt7601u_tx_tasklet, (unsigned long) dev);
tasklet_init(&dev->rx_tasklet, mt7601u_rx_tasklet, (unsigned long) dev);
ret = mt7601u_alloc_tx(dev);
@@ -502,4 +522,6 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
mt7601u_free_rx(dev);
mt7601u_free_tx(dev);
+
+ tasklet_kill(&dev->tx_tasklet);
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c
b/drivers/net/wireless/mediatek/mt7601u/init.c
index df3dd56199a7..38eb20ba6e58 100644
--- a/drivers/net/wireless/mediatek/mt7601u/init.c
+++ b/drivers/net/wireless/mediatek/mt7601u/init.c
@@ -456,6 +456,7 @@ struct mt7601u_dev *mt7601u_alloc_device(struct device
*pdev)
spin_lock_init(&dev->lock);
spin_lock_init(&dev->con_mon_lock);
atomic_set(&dev->avg_ampdu_len, 1);
+ skb_queue_head_init(&dev->tx_skb_done);
dev->stat_wq = alloc_workqueue("mt7601u", WQ_UNBOUND, 0);
if (!dev->stat_wq) {
diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c
b/drivers/net/wireless/mediatek/mt7601u/mac.c
index 7514bce1ac91..e3928cfa3d63 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mac.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mac.c
@@ -181,7 +181,11 @@ void mt76_send_tx_status(struct mt7601u_dev *dev, struct
mt76_tx_status *stat)
}
mt76_mac_fill_tx_status(dev, &info, stat);
+
+ local_bh_disable();
ieee80211_tx_status_noskb(dev->hw, sta, &info);
+ local_bh_enable();
+
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 6bdfc1103fcc..bc5e294feb8c 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -199,7 +199,9 @@ struct mt7601u_dev {
/* TX */
spinlock_t tx_lock;
+ struct tasklet_struct tx_tasklet;
struct mt7601u_tx_queue *tx_q;
+ struct sk_buff_head tx_skb_done;
atomic_t avg_ampdu_len;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next 1/4] mt7601u: fix dma from stack address
From: Jakub Kicinski
DMA to variables located on the stack is a bad idea.
For simplicity and to avoid frequent allocations create
a buffer inside the device structure. Protect this
buffer with vendor_req_mutex. Don't protect vendor
requests which don't use this buffer.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
drivers/net/wireless/mediatek/mt7601u/usb.c | 63 -
drivers/net/wireless/mediatek/mt7601u/usb.h | 2 +
3 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 9102be6b95cb..6bdfc1103fcc 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -146,7 +146,7 @@ enum {
* @rx_lock: protects @rx_q.
* @con_mon_lock: protects @ap_bssid, @bcn_*, @avg_rssi.
* @mutex: ensures exclusive access from mac80211 callbacks.
- * @vendor_req_mutex: ensures atomicity of vendor requests.
+ * @vendor_req_mutex: protects @vend_buf, ensures atomicity of split writes.
* @reg_atomic_mutex: ensures atomicity of indirect register accesses
* (accesses to RF and BBP).
* @hw_atomic_mutex: ensures exclusive access to HW during critical
@@ -184,6 +184,8 @@ struct mt7601u_dev {
struct mt7601u_eeprom_params *ee;
struct mutex vendor_req_mutex;
+ void *vend_buf;
+
struct mutex reg_atomic_mutex;
struct mutex hw_atomic_mutex;
diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c
b/drivers/net/wireless/mediatek/mt7601u/usb.c
index 54dba4001865..416c6045ff31 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -92,10 +92,9 @@ void mt7601u_complete_urb(struct urb *urb)
complete(cmpl);
}
-static int
-__mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
-const u8 direction, const u16 val, const u16 offset,
-void *buf, const size_t buflen)
+int mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
+ const u8 direction, const u16 val, const u16 offset,
+ void *buf, const size_t buflen)
{
int i, ret;
struct usb_device *usb_dev = mt7601u_to_usb_dev(dev);
@@ -110,6 +109,8 @@ __mt7601u_vendor_request(struct mt7601u_dev *dev, const u8
req,
trace_mt_vend_req(dev, pipe, req, req_type, val, offset,
buf, buflen, ret);
+ if (ret == -ENODEV)
+ set_bit(MT7601U_STATE_REMOVED, &dev->state);
if (ret >= 0 || ret == -ENODEV)
return ret;
@@ -122,25 +123,6 @@ __mt7601u_vendor_request(struct mt7601u_dev *dev, const u8
req,
return ret;
}
-int
-mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
- const u8 direction, const u16 val, const u16 offset,
- void *buf, const size_t buflen)
-{
- int ret;
-
- mutex_lock(&dev->vendor_req_mutex);
-
- ret = __mt7601u_vendor_request(dev, req, direction, val, offset,
- buf, buflen);
- if (ret == -ENODEV)
- set_bit(MT7601U_STATE_REMOVED, &dev->state);
-
- mutex_unlock(&dev->vendor_req_mutex);
-
- return ret;
-}
-
void mt7601u_vendor_reset(struct mt7601u_dev *dev)
{
mt7601u_vendor_request(dev, MT_VEND_DEV_MODE, USB_DIR_OUT,
@@ -150,19 +132,21 @@ void mt7601u_vendor_reset(struct mt7601u_dev *dev)
u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset)
{
int ret;
- __le32 reg;
- u32 val;
+ u32 val = ~0;
WARN_ONCE(offset > USHRT_MAX, "read high off:%08x", offset);
+ mutex_lock(&dev->vendor_req_mutex);
+
ret = mt7601u_vendor_request(dev, MT_VEND_MULTI_READ, USB_DIR_IN,
-0, offset, ®, sizeof(reg));
- val = le32_to_cpu(reg);
- if (ret > 0 && ret != sizeof(reg)) {
+0, offset, dev->vend_buf, MT_VEND_BUF);
+ if (ret == MT_VEND_BUF)
+ val = get_unaligned_le32(dev->vend_buf);
+ else if (ret > 0)
dev_err(dev->dev, "Error: wrong size read:%d off:%08x\n",
ret, offset);
- val = ~0;
- }
+
+ mutex_unlock(&dev->vendor_req_mutex);
trace_reg_read(dev, offset, val);
return val;
@@ -173,12 +157,17 @@ int mt7601u_vendor_single_wr(struct mt7601u_dev *dev,
const u8 req,
{
int ret;
+ mutex_lock(&dev->vendor_req_mutex);
+
ret = mt7601u_vendor_request(dev, req, USB_DIR_OUT,
val & 0xfff
[PATCH -next 4/4] mt7601u: lock out rx path and tx status reporting
From: Jakub Kicinski
mac80211 requires that rx path does not run concurrently with
tx status reporting. Add a spinlock which will ensure that.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 ++
drivers/net/wireless/mediatek/mt7601u/init.c| 1 +
drivers/net/wireless/mediatek/mt7601u/mac.c | 4 ++--
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +++-
drivers/net/wireless/mediatek/mt7601u/tx.c | 3 +++
5 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 63c485443a38..57a80cfa39b1 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -112,7 +112,9 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev,
u8 *data,
if (!skb)
return;
+ spin_lock(&dev->mac_lock);
ieee80211_rx(dev->hw, skb);
+ spin_unlock(&dev->mac_lock);
}
static u16 mt7601u_rx_next_seg_len(u8 *data, u32 data_len)
diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c
b/drivers/net/wireless/mediatek/mt7601u/init.c
index 38eb20ba6e58..26190fd33407 100644
--- a/drivers/net/wireless/mediatek/mt7601u/init.c
+++ b/drivers/net/wireless/mediatek/mt7601u/init.c
@@ -454,6 +454,7 @@ struct mt7601u_dev *mt7601u_alloc_device(struct device
*pdev)
spin_lock_init(&dev->tx_lock);
spin_lock_init(&dev->rx_lock);
spin_lock_init(&dev->lock);
+ spin_lock_init(&dev->mac_lock);
spin_lock_init(&dev->con_mon_lock);
atomic_set(&dev->avg_ampdu_len, 1);
skb_queue_head_init(&dev->tx_skb_done);
diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c
b/drivers/net/wireless/mediatek/mt7601u/mac.c
index e3928cfa3d63..e21c53ed09fb 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mac.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mac.c
@@ -182,9 +182,9 @@ void mt76_send_tx_status(struct mt7601u_dev *dev, struct
mt76_tx_status *stat)
mt76_mac_fill_tx_status(dev, &info, stat);
- local_bh_disable();
+ spin_lock_bh(&dev->mac_lock);
ieee80211_tx_status_noskb(dev->hw, sta, &info);
- local_bh_enable();
+ spin_unlock_bh(&dev->mac_lock);
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index bc5e294feb8c..428bd2f10b7b 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -141,8 +141,9 @@ enum {
/**
* struct mt7601u_dev - adapter structure
* @lock: protects @wcid->tx_rate.
+ * @mac_lock: locks out mac80211's tx status and rx paths.
* @tx_lock: protects @tx_q and changes of MT7601U_STATE_*_STATS
- flags in @state.
+ * flags in @state.
* @rx_lock: protects @rx_q.
* @con_mon_lock: protects @ap_bssid, @bcn_*, @avg_rssi.
* @mutex: ensures exclusive access from mac80211 callbacks.
@@ -177,6 +178,7 @@ struct mt7601u_dev {
struct mt76_wcid __rcu *wcid[N_WCIDS];
spinlock_t lock;
+ spinlock_t mac_lock;
const u16 *beacon_offsets;
diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c
b/drivers/net/wireless/mediatek/mt7601u/tx.c
index 0be2080ceab3..a0a33dc8f6bc 100644
--- a/drivers/net/wireless/mediatek/mt7601u/tx.c
+++ b/drivers/net/wireless/mediatek/mt7601u/tx.c
@@ -116,7 +116,10 @@ void mt7601u_tx_status(struct mt7601u_dev *dev, struct
sk_buff *skb)
ieee80211_tx_info_clear_status(info);
info->status.rates[0].idx = -1;
info->flags |= IEEE80211_TX_STAT_ACK;
+
+ spin_lock(&dev->mac_lock);
ieee80211_tx_status(dev->hw, skb);
+ spin_unlock(&dev->mac_lock);
}
static int mt7601u_skb_rooms(struct mt7601u_dev *dev, struct sk_buff *skb)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Driver for Mediatek MT7630E
On Wed, 29 Jul 2015 09:38:04 +0200, Linus Walleij wrote: > On Wed, Jul 29, 2015 at 1:24 AM, Xose Vazquez Perez > wrote: > > Linus Walleij wrote: > > > >> Did anything ever happen to this? > >> > >> My daughter has this device in her laptop it seems, sigh. > >> > >> I might start fiddling with it unless someone else is already > >> doing it. > > > > Jakub Kicinski was working on it at: https://github.com/kuba-moo/mt7630e > > Ah yeah that is what I have running. It's a fork of rt2x00 so it's > missing all fixes since 3.11 or so, just a question of when this > becomes unsupportable. The rt2x00 does not change that much itself so it's not a biggie. However, we are missing all the mac80211-related changes :/ > This code is a complete mashup of Linux and duct-taped Windows > NDIS-based driver code, my eyes are bleeding from looking at it. > > The defines etc patched in have obvious gaps due to other unsupported > chips such as MT7601u I suspect. I guess it's necessary to also > look at this in order to not screw things up for 7601. > https://github.com/porjo/mt7601 The problem with this hardware is that it's a something between old Ralink stuff and new AC devices which Felix is supporting in mt76, just like mt7601u. I started hacking on mt76 to add support but not sure if Felix is interested in merging support for old chips there. So the support for mt7630e could be added in three places, theoretically: (1) rt2x00, (2) mt7601u, (3) mt76. IMO they're all bad choices. Also MediaTek has no interest in supporting Open Source driver for this device. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mt7601u: don't warn about devices without per-rate power table
From: Jakub Kicinski
We expect EEPROM per-rate power table to be filled with
s6 values and warn user if values are invalid. However,
there appear to be devices which don't have this section
of EEPROM initialized. In such case we should ignore
the values and leave the driver power tables set to zero.
Note that vendor driver doesn't care about this case but
mt76x2 skips 0xff per value. We take mt76x2's approach.
Signed-off-by: Jakub Kicinski
---
Kalle, I tried my best with patchwork settings but if my name still
contains a question mark on this submission you can go ahead and
apply automatically anyway. The patch itself does not have any hairy
characters and Johannes said on IRC yesterday that pwclient ends up
doing the right thing in this case.
---
drivers/net/wireless/mediatek/mt7601u/eeprom.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
index ce3837f270f0..8d8ee0344f7b 100644
--- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
@@ -277,6 +277,10 @@ mt7601u_extra_power_over_mac(struct mt7601u_dev *dev)
static void
mt7601u_set_power_rate(struct power_per_rate *rate, s8 delta, u8 value)
{
+ /* Invalid? Note: vendor driver does not handle this */
+ if (value == 0xff)
+ return;
+
rate->raw = s6_validate(value);
rate->bw20 = s6_to_int(value);
/* Note: vendor driver does cap the value to s6 right away */
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] mt7601u: set promiscous mode based on FIF_OTHER_BSS
From: Jakub Kicinski Most drivers use FIF_OTHER_BSS to set promiscous mode. Let us follow their lead even though it doesn't match exactly the HW filter flags. Signed-off-by: Jakub Kicinski --- drivers/net/wireless/mediatek/mt7601u/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c b/drivers/net/wireless/mediatek/mt7601u/main.c index ced82abb414f..169384b48b27 100644 --- a/drivers/net/wireless/mediatek/mt7601u/main.c +++ b/drivers/net/wireless/mediatek/mt7601u/main.c @@ -119,6 +119,7 @@ mt76_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags, dev->rxfilter &= ~MT_RX_FILTR_CFG_OTHER_BSS; + MT76_FILTER(OTHER_BSS, MT_RX_FILTR_CFG_PROMISC); MT76_FILTER(FCSFAIL, MT_RX_FILTR_CFG_CRC_ERR); MT76_FILTER(PLCPFAIL, MT_RX_FILTR_CFG_PHY_ERR); MT76_FILTER(CONTROL, MT_RX_FILTR_CFG_ACK | -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] mt7601u: unify paged and non-paged RX dma paths
From: Jakub Kicinski
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 62 ++---
1 file changed, 12 insertions(+), 50 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 9c9e1288644b..16df67b2e62c 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -34,56 +34,28 @@ static unsigned int ieee80211_get_hdrlen_from_buf(const u8
*data, unsigned len)
static struct sk_buff *
mt7601u_rx_skb_from_seg(struct mt7601u_dev *dev, struct mt7601u_rxwi *rxwi,
- u8 *data, u32 seg_len)
+ void *data, u32 seg_len, u32 truesize, struct page *p)
{
struct sk_buff *skb;
u32 true_len;
+ int hdr_len, copy, frag;
- if (rxwi->rxinfo & cpu_to_le32(MT_RXINFO_L2PAD))
- seg_len -= 2;
-
- skb = alloc_skb(seg_len, GFP_ATOMIC);
- if (!skb)
- return NULL;
-
- if (rxwi->rxinfo & cpu_to_le32(MT_RXINFO_L2PAD)) {
- int hdr_len = ieee80211_get_hdrlen_from_buf(data, seg_len);
-
- memcpy(skb_put(skb, hdr_len), data, hdr_len);
- data += hdr_len + 2;
- seg_len -= hdr_len;
- }
-
- memcpy(skb_put(skb, seg_len), data, seg_len);
-
- true_len = mt76_mac_process_rx(dev, skb, skb->data, rxwi);
- skb_trim(skb, true_len);
-
- return skb;
-}
-
-static struct sk_buff *
-mt7601u_rx_skb_from_seg_paged(struct mt7601u_dev *dev,
- struct mt7601u_rxwi *rxwi, void *data,
- u32 seg_len, u32 truesize, struct page *p)
-{
- unsigned int hdr_len = ieee80211_get_hdrlen_from_buf(data, seg_len);
- unsigned int true_len, copy, frag;
- struct sk_buff *skb;
-
- skb = alloc_skb(128, GFP_ATOMIC);
+ skb = alloc_skb(p ? 128 : seg_len, GFP_ATOMIC);
if (!skb)
return NULL;
true_len = mt76_mac_process_rx(dev, skb, data, rxwi);
+ hdr_len = ieee80211_get_hdrlen_from_buf(data, true_len);
if (rxwi->rxinfo & cpu_to_le32(MT_RXINFO_L2PAD)) {
memcpy(skb_put(skb, hdr_len), data, hdr_len);
+
data += hdr_len + 2;
true_len -= hdr_len;
hdr_len = 0;
}
+ /* If not doing paged RX allocated skb will always have enough space */
copy = (true_len <= skb_tailroom(skb)) ? true_len : hdr_len + 8;
frag = true_len - copy;
@@ -100,7 +72,7 @@ mt7601u_rx_skb_from_seg_paged(struct mt7601u_dev *dev,
}
static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data,
- u32 seg_len, struct page *p, bool paged)
+ u32 seg_len, struct page *p)
{
struct sk_buff *skb;
struct mt7601u_rxwi *rxwi;
@@ -126,11 +98,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev,
u8 *data,
trace_mt_rx(dev, rxwi, fce_info);
- if (paged)
- skb = mt7601u_rx_skb_from_seg_paged(dev, rxwi, data, seg_len,
- truesize, p);
- else
- skb = mt7601u_rx_skb_from_seg(dev, rxwi, data, seg_len);
+ skb = mt7601u_rx_skb_from_seg(dev, rxwi, data, seg_len, truesize, p);
if (!skb)
return;
@@ -158,23 +126,17 @@ mt7601u_rx_process_entry(struct mt7601u_dev *dev, struct
mt7601u_dma_buf_rx *e)
u32 seg_len, data_len = e->urb->actual_length;
u8 *data = page_address(e->p);
struct page *new_p = NULL;
- bool paged = true;
int cnt = 0;
if (!test_bit(MT7601U_STATE_INITIALIZED, &dev->state))
return;
/* Copy if there is very little data in the buffer. */
- if (data_len < 512) {
- paged = false;
- } else {
+ if (data_len > 512)
new_p = dev_alloc_pages(MT_RX_ORDER);
- if (!new_p)
- paged = false;
- }
while ((seg_len = mt7601u_rx_next_seg_len(data, data_len))) {
- mt7601u_rx_process_seg(dev, data, seg_len, e->p, paged);
+ mt7601u_rx_process_seg(dev, data, seg_len, new_p ? e->p : NULL);
data_len -= seg_len;
data += seg_len;
@@ -182,9 +144,9 @@ mt7601u_rx_process_entry(struct mt7601u_dev *dev, struct
mt7601u_dma_buf_rx *e)
}
if (cnt > 1)
- trace_mt_rx_dma_aggr(dev, cnt, paged);
+ trace_mt_rx_dma_aggr(dev, cnt, !!new_p);
- if (paged) {
+ if (new_p) {
/* we have one extra ref from the allocator */
__free_pages(e->p, MT_RX_ORDER);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body
[PATCH 3/4] mt7601u: don't cleanup device second time after .resume()
From: Jakub Kicinski
Make sure .disconnect() doesn't cleanup the device if
.resume() failed. This may happen when device is removed
during suspend.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/init.c | 3 +++
drivers/net/wireless/mediatek/mt7601u/usb.c | 9 -
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c
b/drivers/net/wireless/mediatek/mt7601u/init.c
index 1fc86e865c8c..45eb0796a2e5 100644
--- a/drivers/net/wireless/mediatek/mt7601u/init.c
+++ b/drivers/net/wireless/mediatek/mt7601u/init.c
@@ -427,6 +427,9 @@ err:
void mt7601u_cleanup(struct mt7601u_dev *dev)
{
+ if (!test_and_clear_bit(MT7601U_STATE_INITIALIZED, &dev->state))
+ return;
+
mt7601u_stop_hardware(dev);
mt7601u_dma_cleanup(dev);
mt7601u_mcu_cmd_deinit(dev);
diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c
b/drivers/net/wireless/mediatek/mt7601u/usb.c
index 99e2b3997bfa..54dba4001865 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -338,8 +338,15 @@ static int mt7601u_suspend(struct usb_interface *usb_intf,
pm_message_t state)
static int mt7601u_resume(struct usb_interface *usb_intf)
{
struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
+ int ret;
+
+ ret = mt7601u_init_hardware(dev);
+ if (ret)
+ return ret;
+
+ set_bit(MT7601U_STATE_INITIALIZED, &dev->state);
- return mt7601u_init_hardware(dev);
+ return 0;
}
MODULE_DEVICE_TABLE(usb, mt7601u_device_table);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] mt7601u: watch out for invalid-length frames
From: Jakub Kicinski
Users of older Ralink devices report that received frames
sometimes have zero length. Watch out for that.
Signed-off-by: Jakub Kicinski
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 14 --
drivers/net/wireless/mediatek/mt7601u/mac.c | 8 ++--
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 16df67b2e62c..7217da4f1543 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -37,16 +37,20 @@ mt7601u_rx_skb_from_seg(struct mt7601u_dev *dev, struct
mt7601u_rxwi *rxwi,
void *data, u32 seg_len, u32 truesize, struct page *p)
{
struct sk_buff *skb;
- u32 true_len;
- int hdr_len, copy, frag;
+ u32 true_len, hdr_len = 0, copy, frag;
skb = alloc_skb(p ? 128 : seg_len, GFP_ATOMIC);
if (!skb)
return NULL;
true_len = mt76_mac_process_rx(dev, skb, data, rxwi);
+ if (!true_len || true_len > seg_len)
+ goto bad_frame;
hdr_len = ieee80211_get_hdrlen_from_buf(data, true_len);
+ if (!hdr_len)
+ goto bad_frame;
+
if (rxwi->rxinfo & cpu_to_le32(MT_RXINFO_L2PAD)) {
memcpy(skb_put(skb, hdr_len), data, hdr_len);
@@ -69,6 +73,12 @@ mt7601u_rx_skb_from_seg(struct mt7601u_dev *dev, struct
mt7601u_rxwi *rxwi,
}
return skb;
+
+bad_frame:
+ dev_err_ratelimited(dev->dev, "Error: incorrect frame len:%u hdr:%u\n",
+ true_len, hdr_len);
+ dev_kfree_skb(skb);
+ return NULL;
}
static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data,
diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c
b/drivers/net/wireless/mediatek/mt7601u/mac.c
index c161bcc6a7fa..7514bce1ac91 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mac.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mac.c
@@ -450,10 +450,14 @@ u32 mt76_mac_process_rx(struct mt7601u_dev *dev, struct
sk_buff *skb,
{
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct mt7601u_rxwi *rxwi = rxi;
- u32 ctl = le32_to_cpu(rxwi->ctl);
+ u32 len, ctl = le32_to_cpu(rxwi->ctl);
u16 rate = le16_to_cpu(rxwi->rate);
int rssi;
+ len = MT76_GET(MT_RXWI_CTL_MPDU_LEN, ctl);
+ if (len < 10)
+ return 0;
+
if (rxwi->rxinfo & cpu_to_le32(MT_RXINFO_DECRYPT)) {
status->flag |= RX_FLAG_DECRYPTED;
status->flag |= RX_FLAG_IV_STRIPPED | RX_FLAG_MMIC_STRIPPED;
@@ -474,7 +478,7 @@ u32 mt76_mac_process_rx(struct mt7601u_dev *dev, struct
sk_buff *skb,
dev->avg_rssi = (dev->avg_rssi * 15) / 16 + (rssi << 8);
spin_unlock_bh(&dev->con_mon_lock);
- return MT76_GET(MT_RXWI_CTL_MPDU_LEN, ctl);
+ return len;
}
static enum mt76_cipher_type
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mac80211: remove obsolete sentence from documentation
From: Jakub Kicinski
FIF_PROMISC_IN_BSS was removed in commit df1404650ccb
("mac80211: remove support for IFF_PROMISC").
Signed-off-by: Jakub Kicinski
---
include/net/mac80211.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2f8b7decace0..e117119927ec 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2591,8 +2591,7 @@ void ieee80211_free_txskb(struct ieee80211_hw *hw, struct
sk_buff *skb);
*
* @FIF_OTHER_BSS: pass frames destined to other BSSes
*
- * @FIF_PSPOLL: pass PS Poll frames, if PROMISC_IN_BSS is not set then only
- * those addressed to this station.
+ * @FIF_PSPOLL: pass PS Poll frames
*
* @FIF_PROBE_REQ: pass probe request frames
*/
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 2/2] add mt7601u kbuild and others
From: Jakub Kicinski Signed-off-by: Jakub Kicinski --- MAINTAINERS| 6 ++ drivers/net/wireless/Kconfig | 1 + drivers/net/wireless/Makefile | 2 ++ drivers/net/wireless/mediatek/Kconfig | 10 ++ drivers/net/wireless/mediatek/Makefile | 1 + drivers/net/wireless/mediatek/mt7601u/Kconfig | 6 ++ drivers/net/wireless/mediatek/mt7601u/Makefile | 9 + 7 files changed, 35 insertions(+) create mode 100644 drivers/net/wireless/mediatek/Kconfig create mode 100644 drivers/net/wireless/mediatek/Makefile create mode 100644 drivers/net/wireless/mediatek/mt7601u/Kconfig create mode 100644 drivers/net/wireless/mediatek/mt7601u/Makefile diff --git a/MAINTAINERS b/MAINTAINERS index df106f87a3ba..c6f4576efd56 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6365,6 +6365,12 @@ F: include/uapi/linux/meye.h F: include/uapi/linux/ivtv* F: include/uapi/linux/uvcvideo.h +MEDIATEK MT7601U WIRELESS LAN DRIVER +M: Jakub Kicinski +L: linux-wireless@vger.kernel.org +S: Maintained +F: drivers/net/wireless/mediatek/mt7601u/ + MEGARAID SCSI/SAS DRIVERS M: Kashyap Desai M: Sumit Saxena diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig index 16604bdf5197..a63ab2e83105 100644 --- a/drivers/net/wireless/Kconfig +++ b/drivers/net/wireless/Kconfig @@ -277,6 +277,7 @@ source "drivers/net/wireless/libertas/Kconfig" source "drivers/net/wireless/orinoco/Kconfig" source "drivers/net/wireless/p54/Kconfig" source "drivers/net/wireless/rt2x00/Kconfig" +source "drivers/net/wireless/mediatek/Kconfig" source "drivers/net/wireless/rtlwifi/Kconfig" source "drivers/net/wireless/ti/Kconfig" source "drivers/net/wireless/zd1211rw/Kconfig" diff --git a/drivers/net/wireless/Makefile b/drivers/net/wireless/Makefile index 0c8891686718..6b9e729dd8ac 100644 --- a/drivers/net/wireless/Makefile +++ b/drivers/net/wireless/Makefile @@ -45,6 +45,8 @@ obj-$(CONFIG_IWLWIFI) += iwlwifi/ obj-$(CONFIG_IWLEGACY) += iwlegacy/ obj-$(CONFIG_RT2X00) += rt2x00/ +obj-$(CONFIG_WL_MEDIATEK) += mediatek/ + obj-$(CONFIG_P54_COMMON) += p54/ obj-$(CONFIG_ATH_CARDS)+= ath/ diff --git a/drivers/net/wireless/mediatek/Kconfig b/drivers/net/wireless/mediatek/Kconfig new file mode 100644 index ..cba300c6b5da --- /dev/null +++ b/drivers/net/wireless/mediatek/Kconfig @@ -0,0 +1,10 @@ +menuconfig WL_MEDIATEK + bool "Mediatek Wireless LAN support" + ---help--- + Enable community drivers for MediaTek WiFi devices. + Those drivers make use of the Linux mac80211 stack. + + +if WL_MEDIATEK +source "drivers/net/wireless/mediatek/mt7601u/Kconfig" +endif # WL_MEDIATEK diff --git a/drivers/net/wireless/mediatek/Makefile b/drivers/net/wireless/mediatek/Makefile new file mode 100644 index ..9d5f182fd7fd --- /dev/null +++ b/drivers/net/wireless/mediatek/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_MT7601U) += mt7601u/ diff --git a/drivers/net/wireless/mediatek/mt7601u/Kconfig b/drivers/net/wireless/mediatek/mt7601u/Kconfig new file mode 100644 index ..f46bed92796b --- /dev/null +++ b/drivers/net/wireless/mediatek/mt7601u/Kconfig @@ -0,0 +1,6 @@ +config MT7601U + tristate "MediaTek MT7601U (USB) support" + depends on MAC80211 + depends on USB + ---help--- + This adds support for MT7601U-based wireless USB dongles. diff --git a/drivers/net/wireless/mediatek/mt7601u/Makefile b/drivers/net/wireless/mediatek/mt7601u/Makefile new file mode 100644 index ..ea9ed8a5db4d --- /dev/null +++ b/drivers/net/wireless/mediatek/mt7601u/Makefile @@ -0,0 +1,9 @@ +ccflags-y += -D__CHECK_ENDIAN__ + +obj-$(CONFIG_MT7601U) += mt7601u.o + +mt7601u-objs = \ + usb.o init.o main.o mcu.o trace.o dma.o core.o eeprom.o phy.o \ + mac.o util.o debugfs.o tx.o + +CFLAGS_trace.o := -I$(src) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 0/2] add mt7601u driver
From: Jakub Kicinski > This miniseries adds support for the simplest of MediaTek Wi-Fi > devices. MT7601U is a single stream bgn chip with no bells or whistles. > My driver is partially based on Felix's mt76 but IMHO it doesn't > make sense to merge the two right now because MT7601U is a design > somewhere between old Ralink devices and new Mediatek chips. There > wouldn't be all that much code sharing with the devices mt76 supports. > Situation may obviously change when someone decides to extend m76 with > support for the more recent USB dongles. > > The driver supports only station mode. I'm hoping to add AP support > when time allows. > > This driver sat on GitHub for quite a while and got some testing there. > If anyone is interested in full git history and such here's a link: > > http://github.com/kuba-moo/mt7601u > > I split the submission into the build things and meta data (kconfig, > Makefiles, MAINTAINERS update etc.) and the actual code to make the > reviewing a little easier. v2: - don't zero parts of just allocated skb (Johannes) - add delay to polling of MAC state (Johannes) - use paged RX (Johannes) - add more device IDs (Jose) - reduce number of RX buffers - increase max length of USB dma aggregate v3: - rebase Jakub Kicinski (2): add mt7601u driver add mt7601u kbuild and others MAINTAINERS|6 + drivers/net/wireless/Kconfig |1 + drivers/net/wireless/Makefile |2 + drivers/net/wireless/mediatek/Kconfig | 10 + drivers/net/wireless/mediatek/Makefile |1 + drivers/net/wireless/mediatek/mt7601u/Kconfig |6 + drivers/net/wireless/mediatek/mt7601u/Makefile |9 + drivers/net/wireless/mediatek/mt7601u/core.c | 78 ++ drivers/net/wireless/mediatek/mt7601u/debugfs.c| 172 +++ drivers/net/wireless/mediatek/mt7601u/dma.c| 533 + drivers/net/wireless/mediatek/mt7601u/dma.h| 127 ++ drivers/net/wireless/mediatek/mt7601u/eeprom.c | 414 +++ drivers/net/wireless/mediatek/mt7601u/eeprom.h | 151 +++ drivers/net/wireless/mediatek/mt7601u/init.c | 625 ++ drivers/net/wireless/mediatek/mt7601u/initvals.h | 164 +++ .../net/wireless/mediatek/mt7601u/initvals_phy.h | 291 + drivers/net/wireless/mediatek/mt7601u/mac.c| 569 + drivers/net/wireless/mediatek/mt7601u/mac.h| 178 +++ drivers/net/wireless/mediatek/mt7601u/main.c | 412 +++ drivers/net/wireless/mediatek/mt7601u/mcu.c| 534 + drivers/net/wireless/mediatek/mt7601u/mcu.h| 94 ++ drivers/net/wireless/mediatek/mt7601u/mt7601u.h| 390 ++ drivers/net/wireless/mediatek/mt7601u/phy.c| 1251 drivers/net/wireless/mediatek/mt7601u/regs.h | 636 ++ drivers/net/wireless/mediatek/mt7601u/trace.c | 21 + drivers/net/wireless/mediatek/mt7601u/trace.h | 400 +++ drivers/net/wireless/mediatek/mt7601u/tx.c | 319 + drivers/net/wireless/mediatek/mt7601u/usb.c| 360 ++ drivers/net/wireless/mediatek/mt7601u/usb.h| 77 ++ drivers/net/wireless/mediatek/mt7601u/util.c | 42 + drivers/net/wireless/mediatek/mt7601u/util.h | 77 ++ 31 files changed, 7950 insertions(+) create mode 100644 drivers/net/wireless/mediatek/Kconfig create mode 100644 drivers/net/wireless/mediatek/Makefile create mode 100644 drivers/net/wireless/mediatek/mt7601u/Kconfig create mode 100644 drivers/net/wireless/mediatek/mt7601u/Makefile create mode 100644 drivers/net/wireless/mediatek/mt7601u/core.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/debugfs.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/dma.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/dma.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/eeprom.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/eeprom.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/init.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/initvals.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/initvals_phy.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/mac.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/mac.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/main.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/mcu.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/mcu.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/mt7601u.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/phy.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/regs.h create mode 100644 drivers/net/wireless/mediatek/mt7601u/trace.c create mode 100644 drivers/net/wireless/mediatek/mt7601u/trace.h create mode 100644 dr
Re: [rt2x00-users] Ralink RT5390/RT5370 no longer works on more recent kernels
Hello Simon, On Sat, 7 Feb 2015 17:54:15 +0100, Simon Raffeiner (SCC) wrote: > Hello everyone, > > one of my USB wireless adapters, based on the Ralink RT5390/RT5370 > chipset and RF, no longer works on more recent kernels. It doesn't > report any networks on "iw dev wlan0 scan" and doesn't associate if told > to do so. I tested a TP-Link TL-727N v3.2 which also is: RT chipset 5390, rev 0502 detected RF chipset 5370 detected and it works fine on 3.19-rc5 (from the wireless-testing tree). Are you testing the different kernels on the same machine? Can you try bisection? kuba -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
