public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/7] usb: ehci: Move interrupt packet length check to create_int_queue
Date: Sun, 21 Sep 2014 19:53:35 +0200	[thread overview]
Message-ID: <541F109F.9050707@redhat.com> (raw)
In-Reply-To: <CAOf5uwnaAh5VpnP4eW2ZxGMvHqNVHQ5DPHMNzv7x9iAw8gQRRA@mail.gmail.com>

Hi,

On 09/20/2014 07:42 PM, Michael Trimarchi wrote:
> Hi
> 
> On Sat, Sep 20, 2014 at 5:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Preperation patch to use create_int_queue outside of ehci-hcd.c .
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/host/ehci-hcd.c | 36 +++++++++++++++++++-----------------
>>  1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> index 20830d7..cf3e3c0 100644
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>> @@ -1163,6 +1163,23 @@ create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize,
>>         struct int_queue *result = NULL;
>>         int i;
>>
>> +       /*
>> +        * Interrupt transfers requiring several transactions are not supported
>> +        * because bInterval is ignored.
>> +        *
>> +        * Also, ehci_submit_async() relies on wMaxPacketSize being a power of 2
>> +        * <= PKT_ALIGN if several qTDs are required, while the USB
>> +        * specification does not constrain this for interrupt transfers. That
>> +        * means that ehci_submit_async() would support interrupt transfers
>> +        * requiring several transactions only as long as the transfer size does
>> +        * not require more than a single qTD.
>> +        */
>> +       if (elementsize > usb_maxpacket(dev, pipe)) {
>> +               printf("%s: xfers requiring several transactions are not supported.\n",
>> +                      __func__);
>> +               return NULL;
>> +       }
>> +
>>         debug("Enter create_int_queue\n");
>>         if (usb_pipetype(pipe) != PIPE_INTERRUPT) {
>>                 debug("non-interrupt pipe (type=%lu)", usb_pipetype(pipe));
>> @@ -1384,24 +1401,9 @@ submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
>>         debug("dev=%p, pipe=%lu, buffer=%p, length=%d, interval=%d",
>>               dev, pipe, buffer, length, interval);
>>
>> -       /*
>> -        * Interrupt transfers requiring several transactions are not supported
>> -        * because bInterval is ignored.
>> -        *
>> -        * Also, ehci_submit_async() relies on wMaxPacketSize being a power of 2
>> -        * <= PKT_ALIGN if several qTDs are required, while the USB
>> -        * specification does not constrain this for interrupt transfers. That
>> -        * means that ehci_submit_async() would support interrupt transfers
>> -        * requiring several transactions only as long as the transfer size does
>> -        * not require more than a single qTD.
>> -        */
>> -       if (length > usb_maxpacket(dev, pipe)) {
>> -               printf("%s: Interrupt transfers requiring several "
>> -                       "transactions are not supported.\n", __func__);
>> -               return -1;
>> -       }
>> -
>>         queue = create_int_queue(dev, pipe, 1, length, buffer);
>> +       if (!queue)
>> +               return -1;
> 
> Can you return a more consistent error code?

I'm just moving code around, and returning the same error code as before. Surely
changing the error code belongs in another patch ?

Regards,

Hans

  reply	other threads:[~2014-09-21 17:53 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-20 15:01 [U-Boot] [PATCH 0/7] Add a better USB keyboard polling method Hans de Goede
2014-09-20 15:01 ` [U-Boot] [PATCH 1/7] usb: ehci: Do not disable an already disabled periodic schedule Hans de Goede
2014-09-20 15:01 ` [U-Boot] [PATCH 2/7] usb: ehci: Move interrupt packet length check to create_int_queue Hans de Goede
2014-09-20 17:42   ` Michael Trimarchi
2014-09-21 17:53     ` Hans de Goede [this message]
2014-09-21 19:36       ` Marek Vasut
2014-09-21 20:00         ` Michael Trimarchi
2014-09-21 20:01           ` Marek Vasut
2014-09-20 15:01 ` [U-Boot] [PATCH 3/7] usb: ehci: Move cache invalidation to poll_int_queue Hans de Goede
2014-09-20 15:01 ` [U-Boot] [PATCH 4/7] usb: Make pollable int support available outside of ehci-hcd.c Hans de Goede
2014-09-20 15:01 ` [U-Boot] [PATCH 5/7] usb: kbd: Remove unused usb_kbd_generic_poll function Hans de Goede
2014-09-20 15:01 ` [U-Boot] [PATCH 6/7] usb: kbd: Cache pipe, interval and packetsize Hans de Goede
2014-09-20 17:53   ` Michael Trimarchi
2014-09-21 17:54     ` Hans de Goede
2014-09-20 15:01 ` [U-Boot] [PATCH 7/7] usb: kbd: Add (optional) support for using an interrupt queue for polling Hans de Goede
2014-09-20 15:18   ` Hans de Goede
2014-09-21 10:45 ` [U-Boot] [PATCH 0/7] Add a better USB keyboard polling method Marek Vasut

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=541F109F.9050707@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=u-boot@lists.denx.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox