On Sun, Apr 13, 2025 at 11:14:07AM +0200, Marek Vasut wrote:
> On 4/11/25 4:21 PM, Francesco Dolcini wrote:
> > On Tue, Apr 08, 2025 at 04:51:22PM +0200, Marek Vasut wrote:
> > > On 4/8/25 11:06 AM, Francesco Dolcini wrote:
> > > > On Mon, Mar 24, 2025 at 03:36:52PM +0100, Marek Vasut wrote:
> > > > > On 3/24/25 3:16 PM, Francesco Dolcini wrote:
> > > > > > On Mon, Mar 24, 2025 at 02:53:23PM +0100, Marek Vasut wrote:
> > > > > > > On 3/24/25 1:30 PM, Francesco Dolcini wrote:
> > > > > > > > On Mon, Mar 24, 2025 at 09:26:03AM +0100, Mattijs Korpershoek
> > > > > > > > wrote:
> > > > > > > > > Hi Francesco,
> > > > > > > > >
> > > > > > > > > On lun., mars 24, 2025 at 09:03, Francesco Dolcini
> > > > > > > > > <france...@dolcini.it> wrote:
> > > > > > > > >
> > > > > > > > > > Hello Mattijs, Marek
> > > > > > > > > >
> > > > > > > > > > On Thu, Mar 20, 2025 at 10:47:02AM +0100, Mattijs
> > > > > > > > > > Korpershoek wrote:
> > > > > > > > > > > On mer., mars 19, 2025 at 23:07, Marek Vasut
> > > > > > > > > > > <ma...@denx.de> wrote:
> > > > > > > > > > >
> > > > > > > > > > > > The UUU tool excepts the interrupt-in endpoint to be
> > > > > > > > > > > > ep1in, otherwise
> > > > > > > > > > > > it crashes. This is a result of the previous hard-coded
> > > > > > > > > > > > EP setup in
> > > > > > > > > > > > drivers/usb/gadget/epautoconf.c which did special-case
> > > > > > > > > > > > EP allocation
> > > > > > > > > > > > for SPL builds, and which was since converted to this
> > > > > > > > > > > > callback, but
> > > > > > > > > > > > without the special-case EP allocation in SPL part.
> > > > > > > > > > > >
> > > > > > > > > > > > This reinstates the SPL part in an isolated manner,
> > > > > > > > > > > > only for NXP iMX
> > > > > > > > > > > > SoCs, only for SPL builds, and only for the ep1in
> > > > > > > > > > > > interrupt-in endpoint.
> > > > > > > > > >
> > > > > > > > > > UUU can (and in our case is) used also on non-NXP i.MX
> > > > > > > > > > platforms.
> > > > > > > > > > What should we do?
> > > > > > > > >
> > > > > > > > > Do reproduce the problem (UUU tool crashes) on those
> > > > > > > > > platforms with
> > > > > > > > > recent U-Boot versions (v2024.10+) ?
> > > > > > > >
> > > > > > > > Not tested, my comment is purely based on the code and the
> > > > > > > > commit message.
> > > > > > > > Older U-Boot versions (up to v2024.04, included) are working
> > > > > > > > fine, with UUU used
> > > > > > > > with TI K3 SoCs (AM69, AM62, AM62P).
> > > > > > > Are you talking about the NXP UUU ?
> > > > > >
> > > > > > yes, it works just fine on not-NXP SoC.
> > > > > Then please test if it still works, and if not, this patch needs to be
> > > > > expanded to cover TI ... or apply unconditionally in SPL (sigh).
> > > >
> > > > I just found the time to check some logs from our CI in which we
> > > > execute such a
> > > > workflow. We do use UUU only from U-Boot proper, so we are not going
> > > > to be
> > > > affected by this SPL specific change.
> > > >
> > > > However I found this error in the logs, on both a TI AM62 and an i.MX8MP
> > > >
> > > > ```
> > > > Starting download of 40524455 bytes
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > ..........................................................................
> > > > ..........................................................................
> > > > ..........................................................................
> > > > ..........................................................................
> > > > .............
> > > > downloading of 40524455 bytes finished
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > Starting download of 1675 bytes
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > downloading of 1675 bytes finished
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > Starting download of 82 bytes
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > downloading of 82 bytes finished
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > dwc3-generic-peripheral usb@31000000: request 0000000099f219c0 was not
> > > > queued to ep1in-bulk
> > > > ```
> > > >
> > > > the download is successful however, despite those error messages. Any
> > > > idea? Is
> > > > this related to this topic?
> > > I have a feeling it is the UUU which should be fixed to not depend on the
> > > ep1-in , really.
> > >
> > > You could however quickly try and apply the change in this patch not only
> > > to
> > > SPL, but to U-Boot on your board as well, and see if those warnings
> > > disappear.
> >
> > So, I did a quick test and the issue is still there.
> >
> > And looking better at the error now, this is about ep1in-bulk, that is
> > not affected by this change at all.
> >
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 477ecd020985..55b248505de5 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -1632,23 +1632,7 @@ usb_ep *dwc3_gadget_match_ep(struct usb_gadget
> > *gadget,
> > if (usb_endpoint_is_bulk_out(desc))
> > return dwc3_find_ep(gadget, "ep2out");
> > if (usb_endpoint_is_int_in(desc)) {
> > - /*
> > - * Special workaround for NXP UUU tool in SPL.
> > - *
> > - * The tool excepts the interrupt-in endpoint to be ep1in,
> > - * otherwise it crashes. This is a result of the previous
> > - * hard-coded EP setup in drivers/usb/gadget/epautoconf.c
> > - * which did special-case EP allocation for SPL builds,
> > - * and which was since converted to this callback, but
> > - * without the special-case EP allocation in SPL part.
> > - *
> > - * This reinstates the SPL part in an isolated manner,
> > - * only for NXP iMX SoCs, only for SPL builds, and only
> > - * for the ep1in interrupt-in endpoint.
> > - */
> > - if (IS_ENABLED(CONFIG_MACH_IMX) &&
> > IS_ENABLED(CONFIG_XPL_BUILD))
> > - return dwc3_find_ep(gadget, "ep1in");
> > - return dwc3_find_ep(gadget, "ep3in");
> > + return dwc3_find_ep(gadget, "ep1in");
> > }
> This should really be fixed in the UUU tool, not worked around in U-Boot.
> Can you try and see if the UUU tool can be fixed and not hard-code ep1 ?
I do not have time to look into that at the moment, I created an issue
https://github.com/nxp-imx/mfgtools/issues/472, hopefully NXP will have a look.
Francesco
