[U-Boot] [PATCH v2] fastboot: OUT transaction length must be aligned to wMaxPacketSize

[Lukasz Majewski]

Hi Steve, Marek, Sam

> On Apr 5, 2016 3:07 PM, "Marek Vasut" <marex at denx.de> wrote:
> >
> > On 04/05/2016 08:31 PM, Steve Rae wrote:
> > > commit 9e4b510 fastboot: OUT transaction length must be aligned to
> wMaxPacketSize
> > > breaks some boards...
> > >
> > > Therefore add a conditional Kconfig to optionally enable this
> > > feature.
> >
> > Did you drill into it to figure out why this is needed ?
> >
> 
> Marek,
> Let me clarify....
> All my boards work with the original code (before the commit which
> aligned the size to the wMaxPacketSize).... Since that commit, all my
> boards are broken.
> And you will notice in this patch, that none of my boards define this
> CONFIG_ ...

Guys, please correct me if I'm wrong, but the problem _is_ be caused
by supporting different fastboot protocol versions.

Unfortunately, there is no way to specify "version" in the protocol so
different versions of fastboot application handle transmission
differently.

I think that Sam and Steve were trying to test the code by using their
fastboot apps, but without any conclusion. Am I right there? Has
something changed?

I'm OK with Kconfig flag approach, if we don't have any reliable way to
distinct protocol versions.

One fastboot user my try it with this option enabled or disabled.

What I would love to see is a proper entry into ./doc READMEs to
clarify this issue (not necessarily with this patch series). It would
help other users in the future.


> 
> So I think you are asking the wrong person to drill down into this
> issue.... Sorry, Steve
> 
> > > Signed-off-by: Steve Rae <srae at broadcom.com>
> > > ---
> > >
> > > Changes in v2:
> > > - ammendment to the original patch
> > >
> > >  drivers/usb/gadget/Kconfig      | 7 +++++++
> > >  drivers/usb/gadget/f_fastboot.c | 4 +++-
> > >  2 files changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/gadget/Kconfig
> > > b/drivers/usb/gadget/Kconfig index f4698f4..ab1c605 100644
> > > --- a/drivers/usb/gadget/Kconfig
> > > +++ b/drivers/usb/gadget/Kconfig
> > > @@ -33,3 +33,10 @@ menuconfig USB_GADGET
> > >          a USB peripheral device.  Configure one hardware driver
> > > for
> your
> > >          peripheral/device side bus controller, and a "gadget
> > > driver"
> for
> > >          your peripheral protocol.
> > > +
> > > +config USB_GADGET_FASTBOOT_DOWNLOAD_ALIGNMENT_REQUIRED
> > > +     bool "fastboot download requires alignment with
> > > wMaxPacketSize"
> > > +     help
> > > +        By default, the fastboot download OUT transactions are
> > > aligned
> > > +        to "ep->maxpacket". This option causes the fastboot
> > > download
> OUT
> > > +        transactions to be aligned with "wMaxPacketSize".
> > > diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c
> > > index 2e87fee..130b5d0 100644
> > > --- a/drivers/usb/gadget/f_fastboot.c
> > > +++ b/drivers/usb/gadget/f_fastboot.c
> > > @@ -430,17 +430,19 @@ static void cb_getvar(struct usb_ep *ep,
> > > struct
> usb_request *req)
> > >  static unsigned int rx_bytes_expected(unsigned int maxpacket)
> > >  {
> > >       int rx_remain = download_size - download_bytes;
> > > -     int rem = 0;
> > > +     int __maybe_unused rem = 0;
> > >       if (rx_remain < 0)
> > >               return 0;
> > >       if (rx_remain > EP_BUFFER_SIZE)
> > >               return EP_BUFFER_SIZE;
> > > +#ifdef CONFIG_USB_GADGET_FASTBOOT_DOWNLOAD_ALIGNMENT_REQUIRED
> > >       if (rx_remain < maxpacket) {
> > >               rx_remain = maxpacket;
> > >       } else if (rx_remain % maxpacket != 0) {
> > >               rem = rx_remain % maxpacket;
> > >               rx_remain = rx_remain + (maxpacket - rem);
> > >       }
> > > +#endif
> > >       return rx_remain;
> > >  }
> > >
> > >
> >
> >
> > --
> > Best regards,
> > Marek Vasut



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group