From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Cc: Felix Fietkau <nbd@nbd.name>,
Stefan Wahren <stefan.wahren@i2se.com>,
Alan Stern <stern@rowland.harvard.edu>,
Doug Anderson <dianders@chromium.org>,
Minas Harutyunyan <hminas@synopsys.com>,
USB list <linux-usb@vger.kernel.org>,
linux-wireless <linux-wireless@vger.kernel.org>
Subject: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
Date: Wed, 20 Feb 2019 17:14:16 +0100 [thread overview]
Message-ID: <20190220161415.GA14165@redhat.com> (raw)
On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > >> >> The way I see it, we have two choices.
> > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > >> >
> > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > >> > if this is not a bug on other platforms as well.
> > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > >> any further improvements later on.
> > > >
> > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > SG should be red flag.
> > > I think we're simply dealing with multiple issues here, only some of
> > > which are fixed by Lorenzo's patches.
> > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > since the Linux USB API supports non-aligned transfer buffers and it
> > > should be up to the controller driver to deal with that.
> >
> > Agree.
> >
> > > dwc2 tries to do that, but that has limitations which I already pointed
> > > out and which are properly dealt with by Lorenzo's patches.
> >
> > I planed to just accept current solution, but I started to work on patch
> > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > related code is now tangled.
> >
> > Would be ok to send this patch with proper changelog as fix for RPI
> > against wireless-drivers and cc:stable (assuming it works and really
> > fix things on RPI) and revert Lorenzo's patches in -next ?
>
> Hi Stanislaw,
>
> what is the advantage of doing so?
To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
and have simpler code.
> You have duplicated most of the fields that are
> already in the urb data structure and you use transfer_buffer (no SG I/O).
URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern
this can be optimized by packing num_sgs, len, done into fields variable.
> Moreover I have ready a series that removes 99% of the dual allocation code and
> maintain it in the control path (instead of the datapath one).
> I need just to rebase it ontop of your series. I will post it soon.
So I would ask what is the point to adding bunch of code and remove it in very
next patch?
Stanislaw
WARNING: multiple messages have this Message-ID (diff)
From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Cc: Felix Fietkau <nbd@nbd.name>,
Stefan Wahren <stefan.wahren@i2se.com>,
Alan Stern <stern@rowland.harvard.edu>,
Doug Anderson <dianders@chromium.org>,
Minas Harutyunyan <hminas@synopsys.com>,
USB list <linux-usb@vger.kernel.org>,
linux-wireless <linux-wireless@vger.kernel.org>
Subject: Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+
Date: Wed, 20 Feb 2019 17:14:16 +0100 [thread overview]
Message-ID: <20190220161415.GA14165@redhat.com> (raw)
In-Reply-To: <20190220132206.GF2626@localhost.localdomain>
On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > >> >> The way I see it, we have two choices.
> > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL case
> > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > >> >
> > > >> > I agree, if this is only needed for dwc2. Though I would investigate
> > > >> > if this is not a bug on other platforms as well.
> > > >> >From what I can see, using Lorenzo's patches seems to be the better
> > > >> solution, since they avoid these corner cases in dwc2 (and maybe other
> > > >> drivers as well). I will apply them and then we'll see if we need to do
> > > >> any further improvements later on.
> > > >
> > > > They work on rpi dwc2, but they do not address root of the problem.
> > > > There is clearly something wrong how mt76usb handle SG, what is not
> > > > fixed. And adding disable_usb_sg module parameter for hcd's supporting
> > > > SG should be red flag.
> > > I think we're simply dealing with multiple issues here, only some of
> > > which are fixed by Lorenzo's patches.
> > > I'm pretty sure it's still wrong for mt76 to try to align its buffers,
> > > since the Linux USB API supports non-aligned transfer buffers and it
> > > should be up to the controller driver to deal with that.
> >
> > Agree.
> >
> > > dwc2 tries to do that, but that has limitations which I already pointed
> > > out and which are properly dealt with by Lorenzo's patches.
> >
> > I planed to just accept current solution, but I started to work on patch
> > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized how
> > related code is now tangled.
> >
> > Would be ok to send this patch with proper changelog as fix for RPI
> > against wireless-drivers and cc:stable (assuming it works and really
> > fix things on RPI) and revert Lorenzo's patches in -next ?
>
> Hi Stanislaw,
>
> what is the advantage of doing so?
To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
and have simpler code.
> You have duplicated most of the fields that are
> already in the urb data structure and you use transfer_buffer (no SG I/O).
URB has plenty of fields, I duplicated 2. If size of mt76u_buf is a concern
this can be optimized by packing num_sgs, len, done into fields variable.
> Moreover I have ready a series that removes 99% of the dual allocation code and
> maintain it in the control path (instead of the datapath one).
> I need just to rebase it ontop of your series. I will post it soon.
So I would ask what is the point to adding bunch of code and remove it in very
next patch?
Stanislaw
next reply other threads:[~2019-02-20 16:14 UTC|newest]
Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-20 16:14 Stanislaw Gruszka [this message]
2019-02-20 16:14 ` [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+ Stanislaw Gruszka
-- strict thread matches above, loose matches on Subject: below --
2019-03-03 21:16 Stefan Wahren
2019-03-03 21:16 ` Stefan Wahren
2019-02-20 16:36 Lorenzo Bianconi
2019-02-20 16:36 ` Lorenzo Bianconi
2019-02-20 16:32 Stanislaw Gruszka
2019-02-20 16:32 ` Stanislaw Gruszka
2019-02-20 16:22 Lorenzo Bianconi
2019-02-20 16:22 ` Lorenzo Bianconi
2019-02-20 15:25 Alan Stern
2019-02-20 15:25 ` Alan Stern
2019-02-20 13:22 Lorenzo Bianconi
2019-02-20 13:22 ` Lorenzo Bianconi
2019-02-20 13:00 Stanislaw Gruszka
2019-02-20 13:00 ` Stanislaw Gruszka
2019-02-20 10:20 Stanislaw Gruszka
2019-02-20 10:20 ` Stanislaw Gruszka
2019-02-19 17:02 Stefan Wahren
2019-02-19 17:02 ` Stefan Wahren
2019-02-19 15:40 Alan Stern
2019-02-19 15:40 ` Alan Stern
2019-02-19 12:19 Felix Fietkau
2019-02-19 12:19 ` Felix Fietkau
2019-02-19 12:11 Felix Fietkau
2019-02-19 12:11 ` Felix Fietkau
2019-02-19 10:59 Stanislaw Gruszka
2019-02-19 10:59 ` Stanislaw Gruszka
2019-02-19 10:42 Stanislaw Gruszka
2019-02-19 10:42 ` Stanislaw Gruszka
2019-02-18 22:19 Stefan Wahren
2019-02-18 22:19 ` Stefan Wahren
2019-02-18 18:52 Felix Fietkau
2019-02-18 18:52 ` Felix Fietkau
2019-02-18 15:03 Stanislaw Gruszka
2019-02-18 15:03 ` Stanislaw Gruszka
2019-02-18 14:47 Stanislaw Gruszka
2019-02-18 14:47 ` Stanislaw Gruszka
2019-02-18 14:43 Felix Fietkau
2019-02-18 14:43 ` Felix Fietkau
2019-02-18 14:25 Lorenzo Bianconi
2019-02-18 14:25 ` Lorenzo Bianconi
2019-02-18 13:52 Stanislaw Gruszka
2019-02-18 13:52 ` Stanislaw Gruszka
2019-02-16 19:17 Stefan Wahren
2019-02-16 19:17 ` Stefan Wahren
2019-02-16 14:07 Stanislaw Gruszka
2019-02-16 14:07 ` Stanislaw Gruszka
2019-02-16 11:05 Stefan Wahren
2019-02-16 11:05 ` Stefan Wahren
2019-02-15 7:12 Stanislaw Gruszka
2019-02-15 7:12 ` Stanislaw Gruszka
2019-02-14 9:54 Stanislaw Gruszka
2019-02-14 9:54 ` Stanislaw Gruszka
2019-02-14 9:48 Stefan Wahren
2019-02-14 9:48 ` Stefan Wahren
2019-02-14 9:25 Stanislaw Gruszka
2019-02-14 9:25 ` Stanislaw Gruszka
2019-02-14 6:49 Stefan Wahren
2019-02-14 6:49 ` Stefan Wahren
2019-02-13 7:05 Stefan Wahren
2019-02-13 7:05 ` Stefan Wahren
2019-02-12 15:27 Alan Stern
2019-02-12 15:27 ` Alan Stern
2019-02-12 13:15 Stanislaw Gruszka
2019-02-12 13:15 ` Stanislaw Gruszka
2019-02-12 11:58 Lorenzo Bianconi
2019-02-12 11:58 ` Lorenzo Bianconi
2019-02-12 9:30 Stanislaw Gruszka
2019-02-12 9:30 ` Stanislaw Gruszka
2019-02-12 0:06 Lorenzo Bianconi
2019-02-12 0:06 ` Lorenzo Bianconi
2019-02-11 17:49 Alan Stern
2019-02-11 17:33 Stanislaw Gruszka
2019-02-11 17:22 Stanislaw Gruszka
2019-02-11 17:22 ` Stanislaw Gruszka
2019-02-11 15:57 Lorenzo Bianconi
2019-02-11 15:57 ` Lorenzo Bianconi
2019-02-11 15:27 Stefan Wahren
2019-02-11 15:27 ` Stefan Wahren
2019-02-11 15:12 Alan Stern
2019-02-11 15:10 Lorenzo Bianconi
2019-02-11 15:10 ` Lorenzo Bianconi
2019-02-11 14:04 Stefan Wahren
2019-02-11 14:04 ` Stefan Wahren
2019-02-11 11:06 Lorenzo Bianconi
2019-02-11 11:06 ` Lorenzo Bianconi
2019-02-11 10:33 Stefan Wahren
2019-02-11 10:33 ` Stefan Wahren
2019-02-11 10:04 Lorenzo Bianconi
2019-02-11 10:04 ` Lorenzo Bianconi
2019-02-11 7:44 Stanislaw Gruszka
2019-02-11 7:44 ` Stanislaw Gruszka
2019-02-10 17:39 Lorenzo Bianconi
2019-02-10 17:39 ` Lorenzo Bianconi
2019-02-10 10:22 Lorenzo Bianconi
2019-02-10 10:22 ` Lorenzo Bianconi
2019-02-10 9:41 Stanislaw Gruszka
2019-02-10 9:41 ` Stanislaw Gruszka
2019-02-09 12:08 Stefan Wahren
2019-02-09 18:46 ` Lorenzo Bianconi
2019-02-09 20:29 ` Stefan Wahren
2019-02-09 20:33 ` Lorenzo Bianconi
2019-02-09 22:47 ` Stefan Wahren
2019-02-10 9:29 ` Stanislaw Gruszka
2019-02-10 16:38 ` Stefan Wahren
2019-02-10 16:52 ` Lorenzo Bianconi
2019-02-11 7:50 ` Stanislaw Gruszka
2019-02-11 8:08 ` Stefan Wahren
2019-02-11 9:52 ` Lorenzo Bianconi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190220161415.GA14165@redhat.com \
--to=sgruszka@redhat.com \
--cc=dianders@chromium.org \
--cc=hminas@synopsys.com \
--cc=linux-usb@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lorenzo.bianconi@redhat.com \
--cc=nbd@nbd.name \
--cc=stefan.wahren@i2se.com \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.