Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CALAqxLUMRaNxwTUi9QS7-Cy-Ve4+vteBm8-jW4yzZg_QTJVChA@mail.gmail.com>
 <7caebeb2-ea96-2276-3078-1e53f09ce227@collabora.com> <CALAqxLUfJYUtmQDC_aDMxW7KcPUawGoRq-PNUfmzQuNKh97FmQ@mail.gmail.com>
In-Reply-To: <CALAqxLUfJYUtmQDC_aDMxW7KcPUawGoRq-PNUfmzQuNKh97FmQ@mail.gmail.com>
From:   John Stultz <john.stultz@linaro.org>
Date:   Thu, 9 May 2019 14:23:44 -0700
Message-ID: <CALAqxLVUFfrPVVjR74V3PhhtcCytfp=cUYjo=BcJ14D1fkVXTw@mail.gmail.com>
Subject: Re: [REGRESSION] usb: gadget: f_fs: Allow scatter-gather buffers
To:     Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Cc:     Felipe Balbi <balbi@kernel.org>, "Yang, Fei" <fei.yang@intel.com>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Chen Yu <chenyu56@huawei.com>,
        lkml <linux-kernel@vger.kernel.org>,
        Linux USB List <linux-usb@vger.kernel.org>,
        Amit Pundir <amit.pundir@linaro.org>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        "kernel@collabora.com" <kernel@collabora.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 9, 2019 at 11:25 AM John Stultz <john.stultz@linaro.org> wrote:
>
> On Thu, May 9, 2019 at 7:02 AM Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
> >
> > Hi John,
> > W dniu 08.05.2019 o 04:18, John Stultz pisze:
> > > Since commit 772a7a724f69 ("usb: gadget: f_fs: Allow scatter-gather
> > > buffers"), I've been seeing trouble with adb transfers in Android on
> > > HiKey960, HiKey and now Dragonboard 845c.
> > >
> > > Sometimes things crash, but often the transfers just stop w/o any
> > > obvious error messages.
> > >
> > > Initially I thought it was an issue with the HiKey960 dwc3 usb patches
> > > being upstreamed, and was using the following hack workaround:
> > >    https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-5.1&id=dcdadaaec9db7a7b78ea9b838dd1453359a2f388
> > >
> > > Then dwc2 added sg support, and I ended up having to revert it to get
> > > by on HiKey:
> > >    https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-5.1&id=6e91b4c7bd1e94bdd835263403c53e85a677b848
> > >
> > > (See thread here: https://lkml.org/lkml/2019/3/8/765)
> >
> > So the thread says there are problems at boot, but here you mention about
> > adb transfers, which must obviously be happening after the board has booted.
> > Do you experience problems at boot or not?
> >
> > If a crash happens, what is in the log?
>
> So, yes.  Sorry, I am maybe muddling two issues (though they both seem
> to be tied to f_fs sg).  On dwc2, with the current code, we often (but
> not always) crash as soon as adb starts up in the boot process. Thus
> I'm running with a revert of "usb: dwc2: gadget: Add scatter-gather
> mode" to get by.
>
> As for example crashes, there is a crash in the thread linked above
> (https://lkml.org/lkml/2019/3/8/765) and also the one I sent yesterday
> when testing with your zlp patch. Let me know if you're looking for
> something more specific.
>
> One thing I didn't do, but I should is run w/ the zlp + your
> memset/kzalloc patch. See if that helps get dwc2 further along at
> least.  I'll test that shortly here and get back to you.

Ok. Apologies for earlier confusion.

So the kzalloc/memset fix you sent for f_fs.c does seem to avoid the
crash on bootup I was seeing w/  HiKey/dwc2 (previously I had only
tested it on HiKey960/dwc3).

However with that patch, I still see tranfer problems with adb, unless
I comment out setting sg_supported in dwc2/gadget.c (in the same
fashion I have to with HiKey960/dwc3).

The dwc2 zlp patch doesn't seem to affect things much either way in my
testing. But maybe I'm just not tripping on that issue yet.

So yes, the kzalloc/memset patch is a clear improvement, as it avoids
the bootup crash on dwc2, and seems like it should go in.

However, there is still the outstanding issue w/  functionfs sg
support stalling on larger transfers.

thanks
-john