From: Adam Lee <adam.lee@canonical.com>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: BlueZ development <linux-bluetooth@vger.kernel.org>,
"Gustavo F. Padovan" <gustavo@padovan.org>,
Johan Hedberg <johan.hedberg@gmail.com>
Subject: Re: [PATCH] Bluetooth: ath3k: workaround the compatibility issue with xHCI controller
Date: Wed, 28 Jan 2015 15:16:36 -0500 [thread overview]
Message-ID: <20150128201636.GA2659@adam-laptop> (raw)
In-Reply-To: <0731CFAA-B45A-4DC2-8579-1A4E5E7707F3@holtmann.org>
On Wed, Jan 28, 2015 at 12:10:21PM -0800, Marcel Holtmann wrote:
> Hi Adam,
> ...
> > --- a/drivers/bluetooth/ath3k.c
> > +++ b/drivers/bluetooth/ath3k.c
> > @@ -174,6 +174,7 @@ static const struct usb_device_id ath3k_blist_tbl[] = {
> > #define USB_REQ_DFU_DNLOAD 1
> > #define BULK_SIZE 4096
> > #define FW_HDR_SIZE 20
> > +#define TIMEGAP_US 100
> >
> > static int ath3k_load_firmware(struct usb_device *udev,
> > const struct firmware *firmware)
> > @@ -205,6 +206,9 @@ static int ath3k_load_firmware(struct usb_device *udev,
> > pipe = usb_sndbulkpipe(udev, 0x02);
> >
> > while (count) {
> > + /* workaround the compatibility issue with xHCI controller*/
> > + usleep_range(TIMEGAP_US - 50, TIMEGAP_US);
> > +
>
> why are you using a usleep_range() here. Why not just sleep for 100us.
>
> Regards
>
> Marcel
Yes, I intended to do that, but timer is not that simple, check this:
https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range
- Why not msleep for (1ms - 20ms)?
Explained originally here:
http://lkml.org/lkml/2007/8/3/250
msleep(1~20) may not do what the caller intends, and
will often sleep longer (~20 ms actual sleep for any
value given in the 1~20ms range). In many cases this
is not the desired behavior.
- Why is there no "usleep" / What is a good range?
Since usleep_range is built on top of hrtimers, the
wakeup will be very precise (ish), thus a simple
usleep function would likely introduce a large number
of undesired interrupts.
With the introduction of a range, the scheduler is
free to coalesce your wakeup with any other wakeup
that may have happened for other reasons, or at the
worst case, fire an interrupt for your upper bound.
The larger a range you supply, the greater a chance
that you will not trigger an interrupt; this should
be balanced with what is an acceptable upper bound on
delay / performance for your specific code path. Exact
tolerances here are very situation specific, thus it
is left to the caller to determine a reasonable range.
--
Adam Lee
next prev parent reply other threads:[~2015-01-28 20:16 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-28 19:52 [PATCH] Bluetooth: ath3k: workaround the compatibility issue with xHCI controller Adam Lee
2015-01-28 20:10 ` Marcel Holtmann
2015-01-28 20:16 ` Adam Lee [this message]
2015-01-28 20:22 ` Marcel Holtmann
2015-01-28 20:26 ` Adam Lee
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=20150128201636.GA2659@adam-laptop \
--to=adam.lee@canonical.com \
--cc=gustavo@padovan.org \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=marcel@holtmann.org \
/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