Re: [PATCH 1/1] Avoid usb reset crashes by making tty_io cdevs truly dynamic
Hi,
Sure - sorry, my description was a little .. basic.
So, I have a client who was having problems with machines hanging in
the field. Very rare, associated with a h/w change that introduced
more cores. Kernel dumps implied that the timer list was getting
corrupted.
This configuration of machine is an SBC on a board which communicates
with the SBC (partly) via a USB CDC device, which pops up as
/dev/ttyACM0.
So one of the things we turned on was CONFIG_DEBUG_KOBJECT_RELEASE.
One of the side-effects of this is to delay kobject destruction.
When we did that, we could reproduce the crash by performing a
USB reset on the CDC device - and logs suggest that this was
happening in the field too.
When the USB reset happens, we get a bunch of complaints from the
kernel.
Some of these are to do with races on the kobjects associated with the
sysfs entries for the ttyACM0 device. They turn out not to be fatal,
and have their own patch series ('Attempt to cope with device changes
and delayed kobject deallocation' on linux-kernel).
The fatal one turns out to be an execution path that goes like this:
1 USB device declares itself to be CDC
2 tty driver fires up and allocates a cdev for the relevant tty.
3 driver-cdevs[0].kobj gets initialised as part of the cdev_alloc()
4 USB reset happens, queueing driver-cdevs[0].kobj for release.
5 The tty driver calls cdev_init(driver-cdevs[0]), which
reinitialises driver-cdevs[0].kobj with a refcount of 1.
6 tty driver starts using that new cdev, queueing an operation on it.
This causes a timer entry to be added including the kobj.
7 At this point, the release we scheduled in (4) happens and the
members of kobj are deallocated.
8 Someone allocates the newly released memory for one of the members of
cdriver-cdevs[0].kobj somewhere else and overwrites it.
9 The timer goes off.
10 Boom
My patch (ham-fistedly) fixes this by ensuring that because we
never reuse the cdev pointer, we are never fooled into reinitialising
a kobject queued for deletion.
I'm not all that familiar with how the locking should go here, and
there is a definite argument that under non CONFIG_DEBUG_KOBJECT_RELEASE
conditions, the kobject_release() would have happened by 5, and
therefore this situation should never exist for real.
.. but (a) that makes it rather hard to test kernels with
CONFIG_DEBUG_KOBJECT_RELEASE, and (b) my customer's crashes have
(allegedly) now gone away even without CONFIG_DEBUG_KOBJECT_RELEASE
set.
Does that help at all? I've attached my 0/1, just in case that
got lost somewhere.
Richard.
---BeginMessage---
Sometimes, usb buses on which CDC ACM devices sit encounter a usb reset.
When this happens, particularly when CONFIG_DEBUG_KOBJECT_RELEASE is on,
we attempt to destroy the cdev for the associated tty and then
rapidly re-initialise it. Since kobject destruction is not immediate,
this potentially leaves us with cdev_init() calling kobject_init() on a
kobject that is about to be destroyed.
This turns out not to be such a good thing and this patch solves the
problem by making the cdevs tty_operations-cdevs dynamically
allocated.
This may not be a problem in the wild (though I have some circumstantial
evidence that it is), but I submit that we might want to think about
fixing it anyway, since it makes debugging on systems with
CONFIG_DEBUG_KOBJECT_RELEASE=y and USB resets rather difficult
(guess what I have been doing lately .. ).
Patch is against e26081808edadfd257c6c9d81014e3b25e9a6118 (head of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git ).
(in fact, you will still get an oops - which is the subject of
another, more controversial, patchset ..)
Richard.
---End Message---
Re: [Xen-devel] [Patch V4 1/3] usb: Add Xen pvUSB protocol description
On Thu, Jul 23, 2015 at 12:08:01PM -0700, Greg KH wrote: Somewhere that people can refer to that describes this public-facing API that must not ever be broken or changed. If you want to put it in a documentation file, or a .h file, I don't care. It is used e.g. in SUSE's xen kernel since 2.6.18. I am very aware of the amount of Xen crap in SuSE's kernel, don't use that as an excuse for me to merge it to mainline :) :-) Wasn't meant as an excuse, just a hint why the interface can't be the same as for usbip. We have to ensure compatibility with those kernels This shouldn't be a kernel/kernel compability issue, as the api talks between Xen and the OS, not between different OSs, right? and possibly other operating systems (BSD?, Windows?) which already might be using pvUSB with a Dom0 based on the SUSE xen kernel. Are there other operating system drivers today that use this API? Is this an API in the Xen core today that we have to support? Some more background / descriptions would be nice to have. For example Xen GPLPV drivers for Windows do have PVUSB frontend driver.. -- Pasi thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ulpi: call put_device if device_register fails
On Thu, Jul 23, 2015 at 03:08:08PM -0500, Felipe Balbi wrote: I don't like fixes like this because no one now has any pressure to fix it properly. Are you doing that work? If not, who is? Heikki is author, I'd expect him to fix it up. We can also revert the fix if you prefer, I'm totally fine with that. Let's leave it alone for now and see what happens... -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On 07/23/2015 03:00 AM, Peter Chen wrote:
Thanks, it is correct. But looking the code at afunc_set_alt:
the method of calculating uac2-p_pktsize seems incorrect, it
may need to change like below:
Ok, sorry. I just read the code again an figured you're right here, this
needs fixing. It doesn't really matter for the currently configured
values of bInterval though, as p_interval will always be 1000 for both
HS and FS.
@@ -1176,15 +1188,16 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
factor = 1000;
} else {
ep_desc = hs_epin_desc;
- factor = 125;
+ factor = 8000;
}
/* pre-compute some values for iso_complete() */
uac2-p_framesize = opts-p_ssize *
num_channels(opts-p_chmask);
rate = opts-p_srate * uac2-p_framesize;
- uac2-p_interval = (1 (ep_desc-bInterval - 1)) * factor;
- uac2-p_pktsize = min_t(unsigned int, rate / uac2-p_interval,
+ uac2-p_interval = factor / (1 (ep_desc-bInterval - 1));
Your version is correct. b_interval needs to get larger when bInterval
decreases of course.
+ uac2-p_pktsize = min_t(unsigned int, DIV_ROUND_UP(rate,
+ uac2-p_interval),
prm-max_psize);
This change, however, is not needed. uac2-p_pktsize needs to be rounded
down, so an extra frame can be added when the residue accumulator
overflows. The reason is simply that we can only send packets that
contain full sample frames, so we have to evenly distribute those
left-over samples that accumulate in one go once we have enough to fill
a complete frame.
Could you put the above change in an extra patch, as it's not directly
related to your wMaxPacketSize change?
Thanks,
Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] USB: Fix persist resume of some USB 2.0 devices
Please tell your email client to wrap lines of text after 72 columns or so. On Thu, 23 Jul 2015, Vasudevan, Krishna PrasathX K wrote: Hi, This mail is for RFC regarding the persist resume of some USB 2.0 devices, Problem Summary: Problem has been observed for some USB 2.0 devices while resuming from sleep. When the USB �persist� feature is enabled through sysfs it is expected to retain its previous mount point across sleep and resume states, it works fine for most of the USB 2.0 mass storage devices, but for some USB 2.0 pendrives such as (Transcend Jet flash 16 GB Alcor micro corporation) it seems to fail and re-enumeration happens resulting in creation of new device data structure. This problem seems to be similar to the problem faced with USB 3.0 devices which is mentioned in below link http://marc.info/?l=linux-usbm=140566728011240w=2 In the above mentioned link the problem is observed with USB 3.0 devices, but in this case it is observed with USB 2.0 mass storage devices.While resuming from sleep, a USB disconnect message and re enumeration messages are seen resulting in the reset of �persist � variable in sysfs. The link above refers to a problem involving link training. USB-2.0 devices don't perform link training in the same way as SuperSpeed devices, so they don't suffer from the same problem. In the above mentioned link a fix is proposed and that patch was merged in mainline kernel, but the patch restricts the timeout only to USB 3.0 devices. I am not sure why it was restricted to USB 3.0 devices, but the same issue seems to appear for USB 2.0 devices too. No, it isn't the same problem. It may be a similar problem, but it can't be the same, for the reason mentioned above. The below patch removes the restriction for USB 3.0 devices and makes the time out applicable to all USB devices. Signed-off-by: vasudevan,krishna prasath krishna.prasathx.k.vasude...@intel.com --- drivers/usb/core/hub.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d2bd9d7..b2b709d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3320,7 +3320,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) clear_bit(port1, hub-busy_bits); - if (udev-persist_enabled hub_is_superspeed(hub-hdev)) + if (udev-persist_enabled) status = wait_for_ss_port_enable(udev, hub, port1, portchange, portstatus); If you are going to do this then you also need to rename the wait_for_ss_port_enable function and update the comment preceding the function's definition. It now applies to all ports, not just to SuperSpeed (SS) ports. RESULT: This patch has been tested with USB 2.0 devices(Transcend Alcor micro corp.) that were facing this issue and after applying this patch, both the USB 2.0 devices seems to work fine without any issues for all tested sleep-resume cycles . Please make this changes before you resubmit the patch. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Several races in usbnet module (kernel 4.1.x)
23.07.2015 12:43, Oliver Neukum пишет: On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote: [Race #5] Race on dev-rx_urb_size. I reproduced it a similar way as the races #2 and #3 (changing MTU while downloading files). dev-rx_urb_size is written to here: #0 usbnet_change_mtu (usbnet.c:392) dev-rx_urb_size = dev-hard_mtu; Here is the conflicting read from dev-rx_urb_size: * rx_submit (usbnet.c:467) size_t size = dev-rx_urb_size; Yes, but what is the actual bad race? I mean, if you change the MTU in operation, there will be a race. That is just unavoidable. Do we generate errors? No, I have observed no problems due to this race so far. Regards, Eugene -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: pl2303: Rewrite pl2303_encode_baud_rate_divisor
On Wed, Jul 15, 2015 at 01:35:58PM +0200, Michał Pecio wrote:
This commit fixes the following issues:
1. The 9th bit of buf was believed to be the LSB of divisor's
exponent, but the hardware interprets it as MSB (9th bit) of the
mantissa. The exponent is actually one bit shorter and applies
to base 4, not 2 as previously believed.
2. Loop iterations doubled the exponent instead of incrementing.
3. The exponent wasn't checked for overflow.
4. The function returned requested rate instead of actual rate.
Due to issue #2, the old code deviated from the wrong formula
described in #1 and actually yielded correct rates when divisor
was lower than 4096 by using exponents of 0, 2 or 4 base-2,
interpreted as 0, 1, 2 base-4 with the 9th mantissa bit clear.
However, at 93.75 kbaud or less the rate turned out too slow
due to #2 or too fast due to #2 and #3.
I tested this patch by sending and validating 0x00,0x01,..,0xff
to an FTDI dongle at 234, 987, 2401, 9601, 31415, 115199, 250k,
500k, 750k, 1M, 1.5M, 3M+1 baud. All rates passed.
I also used pv to check speed at some rates unsupported by FTDI:
45 (the lowest possible), 2M, 4M, 5M and 6M-1. Looked sane.
Signed-off-by: Michal Pecio michal.pe...@gmail.com
I took another look at this one and discovered an issue that should be
addressed.
Pointing out two style issues I had fixed locally as well.
---
drivers/usb/serial/pl2303.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index f5257af..3f9a808 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -362,22 +362,36 @@ static speed_t pl2303_encode_baud_rate_direct(unsigned
char buf[4],
static speed_t pl2303_encode_baud_rate_divisor(unsigned char buf[4],
speed_t baud)
{
- unsigned int tmp;
+ u32 baseline, mantissa, exponent;
Please keep these as unsigned int.
/*
* Apparently the formula is:
- * baudrate = 12M * 32 / (2^buf[1]) / buf[0]
+ * baudrate = 12M * 32 / (mantissa * 4^exponent)
+ * where
+ * mantissa = buf[8:0]
+ * exponent = buf[11:9]
*/
- tmp = 1200 * 32 / baud;
+ baseline = 1200 * 32;
+ mantissa = baseline / baud;
+ exponent = 0;
+ while (mantissa = 512) {
+ if (exponent 7) {
+ mantissa = 2; /* divide by 4 */
+ exponent++;
+ } else {
+ /* Exponent is maxed. Trim mantissa and leave. */
+ mantissa = 511;
+ break;
+ }
+ }
+
buf[3] = 0x80;
buf[2] = 0;
- buf[1] = (tmp = 256);
- while (tmp = 256) {
- tmp = 2;
- buf[1] = 1;
- }
- buf[0] = tmp;
+ buf[1] = exponent 1 | mantissa 8;
+ buf[0] = mantissa 0xff;
+ /* Calculate and return the exact baud rate. */
+ baud = (baseline / mantissa) (exponent 1);
You should handle division by zero here. It cannot currently happen as
we cap the baudrate, but you should handle it here nonetheless. You can
still assume a non-zero baudrate, though.
Please add a newline before returning as well.
return baud;
}
Thanks,
Johan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH v3]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()
Dear Greg
Sorry for your inconvience.
This patch doesn't apply to my tree, can you please refresh the patch
agasint 4.2-rc3 and resend it so that I can apply it?
I will resend a new patch against 4.2-rc3 version.
Also, please try to fix your email client to not send base64 patches,
that made it really hard for me to try to figure out what was wrong with
it...
I will opt out a new email client. It will take some time because of some
issues at my end.
Please accept the new patch with my old email client format.
Thanks Regards,
Aman Deep
--- Original Message ---
Sender : Greg KHg...@kroah.com
Date : Jul 23, 2015 03:52 (GMT+06:00)
Title : Re: [PATCH v3]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle
race condition between usb_kill_urb() and finish_unlinks()
On Sat, Jul 11, 2015 at 12:04:26PM +, AMAN DEEP wrote:
There is a race condition between
finish_unlinks-finish_urb() function and
usb_kill_urb() in ohci controller case. The finish_urb
calls spin_unlock(ohci-lock) before
usb_hcd_giveback_urb() function call, then if during
this time, usb_kill_urb is called for another endpoint,
then new ed will be added to ed_rm_list at beginning
for unlink. and ed_rm_list will point to newly added
ed.
When finish_urb() is completed in finish_unlinks() and
ed-td_list becomes empty as in below code (in finish_unlinks() function)
if (list_empty(ed-td_list)) {
*last = ed-ed_next;
ed-ed_next = NULL;
} else if (ohci-rh_state == OHCI_RH_RUNNING) {
*last = ed-ed_next;
ed-ed_next = NULL;
ed_schedule(ohci, ed);
}
*last = ed-ed_next will make ed_rm_list to point to ed-ed_next and
previously added ed by usb_kill_urb will be left unreferenced by
ed_rm_list. This causes usb_kill_urb() hang forever waiting for
finish_unlink to remove added ed from ed_rm_list.
The main reason for hang in this race condtion is addition and removal
of ed from ed_rm_list in the beginning during usb_kill_urb and later last*
is modified in finish_unlinks().
As suggested by Alan Stern, the solution for proper handling of
ohci-ed_rm_list is to remove ed from the ed_rm_list before finishing
any URBs. Then at the end, we can add ed back to the list if necessary.
This properly handle the updated ohci-ed_rm_list in
usb_kill_urb().
Fixes: 977dcfdc6031 (USB: OHCI: don't lose track of EDs when a controller
dies)
Acked-by: Alan Stern
Signed-off-by: Aman Deep
CC:
---
drivers/usb/host/ohci-q.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index f7d561e..496658b 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -1023,6 +1023,8 @@ ed_idle:
* have modified this list. normally it's just prepending
* entries (which we'd ignore), but paranoia won't hurt.
*/
+ *last = ed-ed_next;
+ ed-ed_next = NULL;
modified = 0;
/* unlink urbs as requested, but rescan the list after
@@ -1081,20 +1083,21 @@ rescan_this:
goto rescan_this;
/*
- * If no TDs are queued, take ED off the ed_rm_list.
+ * If no TDs are queued, ED is now idle.
* Otherwise, if the HC is running, reschedule.
- * If not, leave it on the list for further dequeues.
+ * If the HC isn't running, add ED back to the
+ * start of the list for later processing.
*/
if (list_empty(ed-td_list)) {
- *last = ed-ed_next;
- ed-ed_next = NULL;
list_del(ed-in_use_list);
} else if (ohci-rh_state == OHCI_RH_RUNNING) {
- *last = ed-ed_next;
- ed-ed_next = NULL;
ed_schedule(ohci, ed);
} else {
- last = ed-ed_next;
+ ed-ed_next = ohci-ed_rm_list;
+ ohci-ed_rm_list = ed;
+ /* Don't loop on the same ED */
+ if (last == ohci-ed_rm_list)
+ last = ed-ed_next;
}
if (modified)
--
This patch doesn't apply to my tree, can you please refresh the patch
agasint 4.2-rc3 and resend it so that I can apply it?
Also, please try to fix your email client to not send base64 patches,
that made it really hard for me to try to figure out what was wrong with
it...
thanks,
greg k-h
Thanks Regards,
Aman
DeepN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ºÆâžØ^n‡r¡ö¦zË�ëh™¨èÚ¢ø ®G«�éh®(階ŠÝ¢j�ú¶m§ÿï�êäz¹Þ–Šàþf£¢·hšˆ§~ˆmš
[PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00 2001
From: Du, Changbin changbin...@intel.com
Date: Thu, 23 Jul 2015 20:08:04 +0800
Subject: [PATCH] usb/gadget: make composite gadget meet usb compliance for
vbus draw
USB-IF compliance requirement limits the vbus current according to
current state. USB2 Spec requires that un-configured current must
be = 100mA, for USB3 is 150mA, OTG is 2.5mA. So set corresponding
vbus draw current according to device mode.
Signed-off-by: Du, Changbin changbin...@intel.com
---
drivers/usb/gadget/composite.c | 39 ---
include/linux/usb/composite.h | 8
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 4e3447b..fdfd625 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -610,6 +610,21 @@ static void device_qual(struct usb_composite_dev *cdev)
qual-bRESERVED = 0;
}
+static unsigned unconfigured_vbus_draw(struct usb_composite_dev *cdev)
+{
+ struct usb_gadget *g = cdev-gadget;
+ unsigned power;
+
+ if (gadget_is_otg(g))
+ power = USB_OTG_VBUS_DRAW_UNCONF;
+ else if (g-speed == USB_SPEED_SUPER)
+ power = USB3_VBUS_DRAW_UNCONF;
+ else
+ power = USB2_VBUS_DRAW_UNCONF;
+
+ return power;
+}
+
/*-*/
static void reset_config(struct usb_composite_dev *cdev)
@@ -634,7 +649,7 @@ static int set_config(struct usb_composite_dev *cdev,
struct usb_gadget *gadget = cdev-gadget;
struct usb_configuration *c = NULL;
int result = -EINVAL;
- unsignedpower = gadget_is_otg(gadget) ? 8 : 100;
+ unsignedpower = unconfigured_vbus_draw(cdev);
int tmp;
if (number) {
@@ -1829,6 +1844,15 @@ done:
return value;
}
+static void composite_reset(struct usb_gadget *gadget)
+{
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+
+ DBG(cdev, reset\n);
+ usb_gadget_vbus_draw(gadget, unconfigured_vbus_draw(cdev));
+ composite_disconnect(gadget);
+}
+
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget *gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
void composite_resume(struct usb_gadget *gadget)
@@ -2117,10 +2141,11 @@ void composite_resume(struct usb_gadget *gadget)
}
maxpower = cdev-config-MaxPower;
-
- usb_gadget_vbus_draw(gadget, maxpower ?
- maxpower : CONFIG_USB_GADGET_VBUS_DRAW);
- }
+ if (!maxpower)
+ maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+ } else
+ maxpower = unconfigured_vbus_draw(cdev);
+ usb_gadget_vbus_draw(gadget, maxpower);
cdev-suspended = 0;
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
.suspend= composite_suspend,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2511469..90b434d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -333,6 +333,14 @@ enum {
USB_GADGET_FIRST_AVAIL_IDX,
};
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
+
/**
* struct usb_composite_driver - groups configurations into a gadget
* @name: For diagnostics, identifies the driver.
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
@@ -1176,15 +1188,16 @@ afunc_set_alt(struct usb_function *fn, unsigned
intf, unsigned alt)
factor = 1000;
} else {
ep_desc = hs_epin_desc;
- factor = 125;
+ factor = 8000;
}
/* pre-compute some values for iso_complete() */
uac2-p_framesize = opts-p_ssize *
num_channels(opts-p_chmask);
rate = opts-p_srate * uac2-p_framesize;
- uac2-p_interval = (1 (ep_desc-bInterval - 1)) * factor;
- uac2-p_pktsize = min_t(unsigned int, rate /
uac2-p_interval,
+ uac2-p_interval = factor / (1 (ep_desc-bInterval
+ - 1));
Your version is correct. b_interval needs to get larger when bInterval
decreases
of course.
+ uac2-p_pktsize = min_t(unsigned int, DIV_ROUND_UP(rate,
+ uac2-p_interval),
prm-max_psize);
This change, however, is not needed. uac2-p_pktsize needs to be rounded
down, so an extra frame can be added when the residue accumulator
overflows. The reason is simply that we can only send packets that contain
full
sample frames, so we have to evenly distribute those left-over samples that
accumulate in one go once we have enough to fill a complete frame.
Could you put the above change in an extra patch, as it's not directly
related to
your wMaxPacketSize change?
Ok, I will.
Peter
RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Thanks, Pietrasiewicz.
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
*gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of magic numbers.
This change does make sense. As you know, when device is reset, it is in a
'unconfigured'
state. Compliance Test equipment may also measure vbus current at this moment.
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar template is in drivers/usb/gadget/configfs.c. Shouldn't the
reset
method be changed there as well?
Yes, it also need to change. I will change it as well.
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Sorry, I didn't get your idea. Why separate these macros definition?
Thanks,
AP
Regards
Changbin
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net v2 2/3] r8152: fix wakeup settings
Avoid the driver to enable WOL if the device doesn't support it.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 28 ++--
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e3a0110..d537c30 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2378,6 +2378,13 @@ static void r8153_power_cut_en(struct r8152 *tp, bool
enable)
ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
}
+static bool rtl_can_wakeup(struct r8152 *tp)
+{
+ struct usb_device *udev = tp-udev;
+
+ return (udev-actconfig-desc.bmAttributes USB_CONFIG_ATT_WAKEUP);
+}
+
static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
{
if (enable) {
@@ -3417,12 +3424,15 @@ static void rtl8152_get_wol(struct net_device *dev,
struct ethtool_wolinfo *wol)
if (usb_autopm_get_interface(tp-intf) 0)
return;
- mutex_lock(tp-control);
-
- wol-supported = WAKE_ANY;
- wol-wolopts = __rtl_get_wol(tp);
-
- mutex_unlock(tp-control);
+ if (!rtl_can_wakeup(tp)) {
+ wol-supported = 0;
+ wol-wolopts = 0;
+ } else {
+ mutex_lock(tp-control);
+ wol-supported = WAKE_ANY;
+ wol-wolopts = __rtl_get_wol(tp);
+ mutex_unlock(tp-control);
+ }
usb_autopm_put_interface(tp-intf);
}
@@ -3432,6 +3442,9 @@ static int rtl8152_set_wol(struct net_device *dev, struct
ethtool_wolinfo *wol)
struct r8152 *tp = netdev_priv(dev);
int ret;
+ if (!rtl_can_wakeup(tp))
+ return -EOPNOTSUPP;
+
ret = usb_autopm_get_interface(tp-intf);
if (ret 0)
goto out_set_wol;
@@ -4073,6 +4086,9 @@ static int rtl8152_probe(struct usb_interface *intf,
goto out1;
}
+ if (!rtl_can_wakeup(tp))
+ __rtl_set_wol(tp, 0);
+
tp-saved_wolopts = __rtl_get_wol(tp);
if (tp-saved_wolopts)
device_set_wakeup_enable(udev-dev, true);
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net v2 0/3] r8152: issues fix
v2: Replace patch #2 with r8152: fix wakeup settings. v1: These patches are used to fix issues. Hayes Wang (3): r8152: fix the issue about U1/U2 - r8152: fix remote wakeup + r8152: fix wakeup settings r8152: don't enable napi before rx ready drivers/net/usb/r8152.c | 103 1 file changed, 60 insertions(+), 43 deletions(-) -- 2.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net v2 1/3] r8152: fix the issue about U1/U2
- Disable U1/U2 during initialization.
- Disable lpm when linking is on, and enable it when linking is off.
- Disable U1/U2 when enabling runtime suspend.
It is possible to let hw stop working, if the U1/U2 request occurs
during some situations. The patch is used to avoid it.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 94 -
1 file changed, 54 insertions(+), 40 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7f6419e..e3a0110 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2166,6 +2166,7 @@ static int rtl8153_enable(struct r8152 *tp)
if (test_bit(RTL8152_UNPLUG, tp-flags))
return -ENODEV;
+ usb_disable_lpm(tp-udev);
set_tx_qlen(tp);
rtl_set_eee_plus(tp);
r8153_set_rx_early_timeout(tp);
@@ -2337,11 +2338,54 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
device_set_wakeup_enable(tp-udev-dev, false);
}
+static void r8153_u1u2en(struct r8152 *tp, bool enable)
+{
+ u8 u1u2[8];
+
+ if (enable)
+ memset(u1u2, 0xff, sizeof(u1u2));
+ else
+ memset(u1u2, 0x00, sizeof(u1u2));
+
+ usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
+}
+
+static void r8153_u2p3en(struct r8152 *tp, bool enable)
+{
+ u32 ocp_data;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
+ if (enable tp-version != RTL_VER_03 tp-version != RTL_VER_04)
+ ocp_data |= U2P3_ENABLE;
+ else
+ ocp_data = ~U2P3_ENABLE;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
+}
+
+static void r8153_power_cut_en(struct r8152 *tp, bool enable)
+{
+ u32 ocp_data;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
+ if (enable)
+ ocp_data |= PWR_EN | PHASE2_EN;
+ else
+ ocp_data = ~(PWR_EN | PHASE2_EN);
+ ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
+ ocp_data = ~PCUT_STATUS;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
+}
+
static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
{
if (enable) {
u32 ocp_data;
+ r8153_u1u2en(tp, false);
+ r8153_u2p3en(tp, false);
+
__rtl_set_wol(tp, WAKE_ANY);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
@@ -2353,6 +2397,8 @@ static void rtl_runtime_suspend_enable(struct r8152 *tp,
bool enable)
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
} else {
__rtl_set_wol(tp, tp-saved_wolopts);
+ r8153_u2p3en(tp, true);
+ r8153_u1u2en(tp, true);
}
}
@@ -2599,46 +2645,6 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
set_bit(PHY_RESET, tp-flags);
}
-static void r8153_u1u2en(struct r8152 *tp, bool enable)
-{
- u8 u1u2[8];
-
- if (enable)
- memset(u1u2, 0xff, sizeof(u1u2));
- else
- memset(u1u2, 0x00, sizeof(u1u2));
-
- usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
-}
-
-static void r8153_u2p3en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
- if (enable)
- ocp_data |= U2P3_ENABLE;
- else
- ocp_data = ~U2P3_ENABLE;
- ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
-}
-
-static void r8153_power_cut_en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
- if (enable)
- ocp_data |= PWR_EN | PHASE2_EN;
- else
- ocp_data = ~(PWR_EN | PHASE2_EN);
- ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
- ocp_data = ~PCUT_STATUS;
- ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
-}
-
static void r8153_first_init(struct r8152 *tp)
{
u32 ocp_data;
@@ -2781,6 +2787,7 @@ static void rtl8153_disable(struct r8152 *tp)
r8153_disable_aldps(tp);
rtl_disable(tp);
r8153_enable_aldps(tp);
+ usb_enable_lpm(tp-udev);
}
static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8
duplex)
@@ -2901,9 +2908,13 @@ static void rtl8153_up(struct r8152 *tp)
if (test_bit(RTL8152_UNPLUG, tp-flags))
return;
+ r8153_u1u2en(tp, false);
r8153_disable_aldps(tp);
r8153_first_init(tp);
r8153_enable_aldps(tp);
+ r8153_u2p3en(tp, true);
+ r8153_u1u2en(tp, true);
+ usb_enable_lpm(tp-udev);
}
static void rtl8153_down(struct r8152 *tp)
@@ -2914,6 +2925,7 @@ static
[PATCH net v2 3/3] r8152: don't enable napi before rx ready
Adjust napi_disable() and napi_enable() to avoid r8152_poll() start
working before rx ready. Otherwise, it may have race condition for
rx_agg.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index d537c30..144dc64 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2075,7 +2075,6 @@ static int rtl_start_rx(struct r8152 *tp)
{
int i, ret = 0;
- napi_disable(tp-napi);
INIT_LIST_HEAD(tp-rx_done);
for (i = 0; i RTL8152_MAX_RX; i++) {
INIT_LIST_HEAD(tp-rx_info[i].list);
@@ -2083,7 +2082,6 @@ static int rtl_start_rx(struct r8152 *tp)
if (ret)
break;
}
- napi_enable(tp-napi);
if (ret ++i RTL8152_MAX_RX) {
struct list_head rx_queue;
@@ -2951,8 +2949,10 @@ static void set_carrier(struct r8152 *tp)
if (!netif_carrier_ok(netdev)) {
tp-rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, tp-flags);
+ napi_disable(tp-napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
+ napi_enable(tp-napi);
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3395,9 +3395,11 @@ static int rtl8152_resume(struct usb_interface *intf)
if (test_bit(SELECTIVE_SUSPEND, tp-flags)) {
rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, tp-flags);
+ napi_disable(tp-napi);
set_bit(WORK_ENABLE, tp-flags);
if (netif_carrier_ok(tp-netdev))
rtl_start_rx(tp);
+ napi_enable(tp-napi);
} else {
tp-rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V4 1/3] usb: Add Xen pvUSB protocol description
On 07/23/2015 09:08 PM, Greg KH wrote: On Thu, Jul 23, 2015 at 08:46:17AM +0200, Juergen Gross wrote: On 07/23/2015 06:36 AM, Greg KH wrote: On Thu, Jul 23, 2015 at 06:04:39AM +0200, Juergen Gross wrote: On 07/23/2015 01:46 AM, Greg KH wrote: On Tue, Jun 23, 2015 at 08:53:23AM +0200, Juergen Gross wrote: Add the definition of pvUSB protocol used between the pvUSB frontend in a Xen domU and the pvUSB backend in a Xen driver domain (usually Dom0). This header was originally provided by Fujitsu for Xen based on Linux 2.6.18. Changes are: - adapt to Linux style guide Signed-off-by: Juergen Gross jgr...@suse.com --- include/xen/interface/io/usbif.h | 252 +++ Why is this a different interface than the existing ones we have today (i.e. usbip?) Where is it documented? Do the Xen developers / The interface definition is living in the Xen git repository for several years now: git://xenbits.xen.org/xen.git - xen/include/public/io/usbif.h That's header file, not a document describing the api here. I suppose you want to tell me I should add something like: Documentation/DocBook/usb/API-struct-urb.html Somewhere that people can refer to that describes this public-facing API that must not ever be broken or changed. If you want to put it in a documentation file, or a .h file, I don't care. It is used e.g. in SUSE's xen kernel since 2.6.18. I am very aware of the amount of Xen crap in SuSE's kernel, don't use that as an excuse for me to merge it to mainline :) :-) Wasn't meant as an excuse, just a hint why the interface can't be the same as for usbip. We have to ensure compatibility with those kernels This shouldn't be a kernel/kernel compability issue, as the api talks between Xen and the OS, not between different OSs, right? Depends on where the backend is living. It's the backend the frontend is talking to. There is a backend in SUSE's kernels up to SLE12. So compatibility is to be maintained to those kernels. Looks as if in future there will be one in qemu. and possibly other operating systems (BSD?, Windows?) which already might be using pvUSB with a Dom0 based on the SUSE xen kernel. Are there other operating system drivers today that use this API? Is this an API in the Xen core today that we have to support? Yes. Some more background / descriptions would be nice to have. I guess a documentation file giving a brief explanation about the interfaces of Xen wouldn't be a bad idea. This could avoid discussions like this. It shouldn't define each interface, but the classes of interfaces which are existing (between kernel and hypervisor, frontends and backends) and the stability requirements. Headers like the one we are discussing here could then refer to this document. Juergen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
On Thu, 23 Jul 2015, Petr Cvek wrote:
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It's hard to understand your question. Are you asking whether this
code is still present in the current version of the kernel? You can
find out for yourself easily enough.
It seems that it allows using a BULK endpoint for requested INT
endpoint. For my PXA27x machine the original code returns BULK EP
even with valid INT endpoint definition (because BULK EPs are defined
earlier than INT EPs).
Yes, it does allow a bulk endpoint to be used when an interrupt
endpoint was requested. However, it won't return a bulk endpoint if
all the bulk endpoints are already in use.
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported.
Does anyone know if the INT endpoints works now?
What makes you think they might not work?
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/4] usb: gadget: udc: bdc: Fix a driver crash on disconnect
On Thu, Jul 23, 2015 at 03:23:26PM -0400, Alan Cooper wrote: On Wed, Jul 22, 2015 at 10:03 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Wed, Jul 22, 2015 at 06:27:29PM -0400, Alan Cooper wrote: On Wed, Jul 22, 2015 at 5:29 PM, Felipe Balbi ba...@ti.com wrote: On Wed, Jul 22, 2015 at 04:26:57PM -0500, Felipe Balbi wrote: On Wed, Jul 22, 2015 at 04:58:07PM -0400, Al Cooper wrote: V2 - Fix a compiler bug that happend when the config options CONFIG_USB_GADGET_DEBUG and CONFIG_USB_GADGET_VERBOSE were enabled. ep_dequeue() in bdc_ep.c was capturing the hw dequeue pointer incorrectly by reading the wrong register for the upper 32 bits. The header file defining the registers was incorrect. btw, the header file was really incorrect as long as you passed 0 to the argument :-p in fact, the minimal fix for this bug would be the one below: diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c index b04980cf6dc4..1efa61265d8d 100644 --- a/drivers/usb/gadget/udc/bdc/bdc_ep.c +++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c @@ -779,7 +779,7 @@ static int ep_dequeue(struct bdc_ep *ep, struct bdc_req *req) /* The current hw dequeue pointer */ tmp_32 = bdc_readl(bdc-regs, BDC_EPSTS0(0)); deq_ptr_64 = tmp_32; - tmp_32 = bdc_readl(bdc-regs, BDC_EPSTS0(1)); + tmp_32 = bdc_readl(bdc-regs, BDC_EPSTS1(0)); deq_ptr_64 |= ((u64)tmp_32 32); /* we have the dma addr of next bd that will be fetched by hardware */ And $subject becomes a cleanup patch for v4.3. Can you make these changes, please ? Yes, I can make these changes, but first let me explain why I did it this way. The 8 End Point Status registers are 32 bit consecutive registers, so defining them as: #define BDC_EPSTS0(n) (0x60 + (n * 0x10)) #define BDC_EPSTS1(n) (0x64 + (n * 0x10)) #define BDC_EPSTS2(n) (0x68 + (n * 0x10)) #define BDC_EPSTS3(n) (0x6c + (n * 0x10)) #define BDC_EPSTS4(n) (0x70 + (n * 0x10)) #define BDC_EPSTS5(n) (0x74 + (n * 0x10)) #define BDC_EPSTS6(n) (0x78 + (n * 0x10)) #define BDC_EPSTS7(n) (0x7c + (n * 0x10)) seems misleading, does not reflect the hardware and using anything other than (0) would get you to some other unexpected register and should be considered a coding error. it sure is, but the minimal patch for -rc is what I sent above :-) As long as you pass 0 as parameters, all your offsets are correct, so removing the parameter (which must be always zero) is, actually, refactoring happening. OK, I think I understand. You're asking me to split the bug fix patch into two patches, one is a minimal bug fix patch for -rc and the other is a cleanup patch (fix the header file) that will go into 4.3. Is that correct? correct. I think the original hardware spec had each End Point Status as a block of registers but the first silicon had them as single registers and they are expected to stay that way. I believe they wanted to have: #define BDC_EPSTS(n)(0x60 + (n * 0x4)) and that way you can access each EP by passing the correct argument to that macro. Actually, the 8 EPSTS registers are all different and together contain the info for 1 end point. The original plan was to have 4 32bit registers per end point and enough sets of these registers for all endpoints, but this turned out to be too expensive in silicon so they changed it to 1 set of 8 registers that are updated when an end point is stopped. oh ok. thanks -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On Thu, 23 Jul 2015, Peter Chen wrote: Assume the interval for each packet is 2ms, the rate is 192 kbytes for both FS HS: uac2-p_interval = 2000; uac2-p_pktsize = 192 kbytes / 2000 = 96 bytes It's important to keep track of the units. Intervals are measures of time, and they use units like seconds or microseconds. In this case, one packet gets sent every 2000 us, so the value is 2000 us/packet. The data rate is 192 KB/s. Let's agree to use units of microseconds and bytes. So converting from KB to bytes and seconds to microseconds, the data rate is: (192 KB/s) * (1000 bytes/KB) * (1/100 s/us) = (0.192 bytes/us). Therefore to compute the number of bytes per packet (the packet size), you have to _multiply_ the values -- not _divide_: (0.192 bytes/us) * (2000 us/packet) = (384 bytes/packet). Therefore the packet size should be 384 bytes, not 96. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ulpi: call put_device if device_register fails
Hi,
On Thu, Jul 23, 2015 at 11:00:29AM -0700, Greg Kroah-Hartman wrote:
On Thu, Jul 23, 2015 at 12:02:40AM -0500, Felipe Balbi wrote:
Hi,
On Wed, Jul 22, 2015 at 08:14:46PM -0700, Greg Kroah-Hartman wrote:
On Wed, Jul 22, 2015 at 09:04:40PM -0500, Felipe Balbi wrote:
On Wed, Jul 22, 2015 at 02:39:34PM -0700, Greg Kroah-Hartman wrote:
On Tue, Jun 23, 2015 at 01:57:38PM +0300, Heikki Krogerus wrote:
On Fri, Jun 19, 2015 at 01:12:36AM +0800, ChengYi He wrote:
put_device is required to release the last reference to the
device.
Signed-off-by: ChengYi He chengyihetai...@gmail.com
---
drivers/usb/common/ulpi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..bd25bdb 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -184,8 +184,10 @@ static int ulpi_register(struct device *dev,
struct ulpi *ulpi)
request_module(ulpi:v%04xp%04x, ulpi-id.vendor,
ulpi-id.product);
ret = device_register(ulpi-dev);
- if (ret)
+ if (ret) {
+ put_device(ulpi-dev);
If device_register returns failure, put_device has already been
called. Check device_add in drivers/base/core.c.
Yes, please read the function, which says:
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up your
* reference instead.
But, the problem is that the ulpi core doesn't own that struct
device.
It comes from elsewhere. It comes from somewhere deep down in the dw3
core, which is where I lost the path. Something needs to be fixed in
dwc3_probe() to properly clean up the device if it fails, which is not
happening right now.
So this patch would actually cause much bigger problems than fixing
anything, so it's wrong, but for a different reason than you are
talking
about here.
And ugh, the ulpi and dwc code binding together, what a mess,
horrid...
any suggestions ? DWC *is* the one implementing the bus. If there's a
better way, we can certainly shuffle code around.
As dwc is the only thing using the bus, why is it drivers/usb/core/ ?
musb also has a SW-accessible ULPI bus. And, IIRC, so does DWC2 ;-)
But they aren't calling ulpi_register(), so how can they be using this
code?
the thing was just added :-) It didn't exist before.
And the error path here is broken, the bus should be creating the device
(i.e. no subsystem should ever be registering a device it did not
create), so that it can properly clean things up when stuff goes wrong.
The whole subsys_init() is also a bad feeling that it's not architected
correctly, that shouldn't be needed, which is why I never took that
patch. Just noticed it came in through yours, I wanted it broken so
it would be fixed properly and not papered over like this.
I just felt it would be better to 'fix' it for the -rc until it can be
fixed *properly*. A follow up fix should incur no visible changes to
drivers anyway.
I don't like fixes like this because no one now has any pressure to fix
it properly. Are you doing that work? If not, who is?
Heikki is author, I'd expect him to fix it up. We can also revert the
fix if you prefer, I'm totally fine with that.
--
balbi
signature.asc
Description: Digital signature
Re: GPIO support for Silicon Labs cp210x USB serial
On Thu, 2015-07-23 at 18:00 +0200, Johan Hovold wrote: The short answer is that we do not want custom driver ioctls. Any gpio implementation needs to based on gpiolib, which provides a standardised interface. I understand. We will evaluate and decide if the work is worth it for us to do. thanks much, John. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple drives on JMS56x-based sata-usb docking station.
On Thu, 23 Jul 2015, Giulio Bernardi wrote: Here is the output of usbmon - collected with usbmon program. The output seems the same of cat /sys/kernel/debug/usb/usbmon/0u with slightly more readable timestamps and shortened addresses so I used that. This trace shows the uas driver was used. Can you post a similar trace using usb-storage instead? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Avoid usb reset crashes by making tty_io cdevs truly dynamic
On Tue, May 19, 2015 at 04:06:53PM +0100, Richard Watts wrote:
Avoid usb reset crashes by making tty_io cdevs truly dynamic
What USB reset crashes are you referring to here?
Signed-off-by: Richard Watts r...@kynesim.co.uk
Reported-by: Duncan Mackintosh dmackint...@cbnl.com
---
drivers/tty/tty_io.c | 24
include/linux/tty_driver.h | 2 +-
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e569546..699cf20 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3168,9 +3168,12 @@ static int tty_cdev_add(struct tty_driver *driver,
dev_t dev,
unsigned int index, unsigned int count)
{
/* init here, since reused cdevs cause crashes */
- cdev_init(driver-cdevs[index], tty_fops);
- driver-cdevs[index].owner = driver-owner;
- return cdev_add(driver-cdevs[index], dev, count);
+ driver-cdevs[index] = cdev_alloc();
+ if (!driver-cdevs[index])
+ return -ENOMEM;
+ cdev_init(driver-cdevs[index], tty_fops);
+ driver-cdevs[index]-owner = driver-owner;
+ return cdev_add(driver-cdevs[index], dev, count);
}
/**
@@ -3276,8 +3279,10 @@ struct device *tty_register_device_attr(struct
tty_driver *driver,
error:
put_device(dev);
- if (cdev)
- cdev_del(driver-cdevs[index]);
+ if (cdev) {
+ cdev_del(driver-cdevs[index]);
+ driver-cdevs[index] = NULL;
+ }
return ERR_PTR(retval);
}
EXPORT_SYMBOL_GPL(tty_register_device_attr);
@@ -3297,8 +3302,10 @@ void tty_unregister_device(struct tty_driver *driver,
unsigned index)
{
device_destroy(tty_class,
MKDEV(driver-major, driver-minor_start) + index);
- if (!(driver-flags TTY_DRIVER_DYNAMIC_ALLOC))
- cdev_del(driver-cdevs[index]);
+ if (!(driver-flags TTY_DRIVER_DYNAMIC_ALLOC)) {
+ cdev_del(driver-cdevs[index]);
+ driver-cdevs[index] = NULL;
+ }
}
EXPORT_SYMBOL(tty_unregister_device);
@@ -3363,6 +3370,7 @@ err_free_all:
kfree(driver-ports);
kfree(driver-ttys);
kfree(driver-termios);
+ kfree(driver-cdevs);
kfree(driver);
return ERR_PTR(err);
}
@@ -3391,7 +3399,7 @@ static void destruct_tty_driver(struct kref *kref)
}
proc_tty_unregister_driver(driver);
if (driver-flags TTY_DRIVER_DYNAMIC_ALLOC)
- cdev_del(driver-cdevs[0]);
+ cdev_del(driver-cdevs[0]);
}
kfree(driver-cdevs);
kfree(driver-ports);
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 92e337c..1610524 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -296,7 +296,7 @@ struct tty_operations {
struct tty_driver {
int magic; /* magic number for this structure */
struct kref kref; /* Reference management */
- struct cdev *cdevs;
+ struct cdev **cdevs;
struct module *owner;
const char *driver_name;
const char *name;
I don't understand what bug this patch is trying to solve, care to help
describe it better?
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Hi Changbin,
(I assume I address your name properly, if not please excuse)
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00 2001
snip
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget *gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of magic numbers.
void composite_resume(struct usb_gadget *gadget)
@@ -2117,10 +2141,11 @@ void composite_resume(struct usb_gadget *gadget)
}
maxpower = cdev-config-MaxPower;
-
- usb_gadget_vbus_draw(gadget, maxpower ?
- maxpower : CONFIG_USB_GADGET_VBUS_DRAW);
- }
+ if (!maxpower)
+ maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+ } else
+ maxpower = unconfigured_vbus_draw(cdev);
+ usb_gadget_vbus_draw(gadget, maxpower);
cdev-suspended = 0;
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar template is in drivers/usb/gadget/configfs.c. Shouldn't the reset
method be changed there as well?
.suspend= composite_suspend,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2511469..90b434d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -333,6 +333,14 @@ enum {
USB_GADGET_FIRST_AVAIL_IDX,
};
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Thanks,
AP
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: pl2303: Rewrite pl2303_encode_baud_rate_divisor
+ u32 baseline, mantissa, exponent; Please keep these as unsigned int. What's the reason for that? u32 is the exact width needed to perform these computations, while unsigned int is something a bit unspecified. + /* Calculate and return the exact baud rate. */ + baud = (baseline / mantissa) (exponent 1); You should handle division by zero here. It cannot currently happen as we cap the baudrate, but you should handle it here nonetheless. You can still assume a non-zero baudrate, though. How about that? mantissa = baseline / baud; if (mantissa == 0) mantissa = 1; Writing zero to the chip as it's currently done probably doesn't make much sense anyway... -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 42/46] usb: gadget: move ep_matches() from epautoconf to udc-core
Hi,
On Thu, Jul 23, 2015 at 06:40:40AM +0200, Petr Cvek wrote:
Hello,
Is this:
case USB_ENDPOINT_XFER_INT:
/* Bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if (!ep-caps.type_int !ep-caps.type_bulk)
return 0;
... or original:
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* bulk endpoints handle interrupt transfers,
* except the toggle-quirky iso-synch kind
*/
if ('s' == tmp[2]) {// == -iso
return 0;
code still valid?
It seems that it allows using a BULK endpoint for requested INT
endpoint. For my PXA27x machine the original code returns BULK EP even
with valid INT endpoint definition (because BULK EPs are defined
earlier than INT EPs).
This part of the code is from pre git era
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
before pxa27x driver was written and only few chips was supported.
Does anyone know if the INT endpoints works now?
it's very difficult to read your reply when you remove all context.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 2/3] drivers: usb: dwc3: Add adjust_frame_length_quirk
Hi,
On Thu, Jul 23, 2015 at 03:41:35PM +0530, Nikhil Badola wrote:
Add adjust_frame_length_quirk for writing to fladj register
which adjusts (micro)frame length to value provided by
snps,configure-fladj property thus avoiding USB 2.0 devices
to time-out over a longer run
Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com
just like the other patch, I won't take this without a glue layer making
use of it.
---
drivers/usb/dwc3/core.c | 12
drivers/usb/dwc3/core.h | 7 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5c110d8..72ba025 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -779,6 +779,7 @@ static int dwc3_probe(struct platform_device *pdev)
u8 lpm_nyet_threshold;
u8 tx_de_emphasis;
u8 hird_threshold;
+ u32 fladj_value;
int ret;
@@ -886,6 +887,12 @@ static int dwc3_probe(struct platform_device *pdev)
tx_de_emphasis);
of_property_read_string(node, snps,hsphy_interface,
dwc-hsphy_interface);
+ ret = of_property_read_u32(node, snps,configure-fladj,
This is not the correct name for the property, it should be something
like 'snps,quirk-frame-length-adjustment'. Quirk because this is only
needed when the 30MHz sideband signal is invalid for some reason.
This is also something that's only needed for host side operation, so
consider what would you do if you weren't using dwc3, if all you had was
XHCI.
I hope freescale will fix this silicon bug.
One extra thing: everything that can be done via DT, should be available
for pdata users as well.
+fladj_value);
+ if (!ret)
+ dwc-adjust_frame_length_quirk = 1;
+ else
+ dwc-adjust_frame_length_quirk = 0;
this flag is unnecessary. Just initialize fladj_value to 0 and check for
that:
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
dwc-has_lpm_erratum = pdata-has_lpm_erratum;
@@ -957,6 +964,11 @@ static int dwc3_probe(struct platform_device *pdev)
goto err1;
}
+ /* Adjust Frame Length */
+ if (dwc-adjust_frame_length_quirk)
if (fladj) {
u32 reg;
u32 dft;
reg = dwc3_readl(dwc-regs, DWC3_GFLADJ);
dft = reg 0x3f; /* needs a mask macro */
if (!dev_WARN_ONCE(dwc-dev, dft == fladj,
request value same as default, ignoring\n)) {
reg = ~0x3f; /* needs a mask macro */
reg |= DWC3_GFLADJ_30MHZ_SDBND_SEL |
DWC3_GFLADJ_30MHZ(fladj_value);
dwc3_writel(dwc-regs, DWC3_GFLADJ, reg);
}
}
+ dwc3_writel(dwc-regs, DWC3_GFLADJ, GFLADJ_30MHZ_REG_SEL |
+ GFLADJ_30MHZ(fladj_value));
+
if (IS_ENABLED(CONFIG_USB_DWC3_HOST))
dwc-dr_mode = USB_DR_MODE_HOST;
else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0447788..b7a5119 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -124,6 +124,7 @@
#define DWC3_GEVNTCOUNT(n) (0xc40c + (n * 0x10))
#define DWC3_GHWPARAMS8 0xc600
+#define DWC3_GFLADJ 0xc630
/* Device Registers */
#define DWC3_DCFG0xc700
@@ -234,6 +235,10 @@
/* Global HWPARAMS6 Register */
#define DWC3_GHWPARAMS6_EN_FPGA (1 7)
+/* Global Frame Length Adjustment Register */
+#define GFLADJ_30MHZ_REG_SEL (1 7)
always prepend with DWC3_ like *all* other macros in this file.
Also, match docs to ease grepping. This should be called
DWC3_GFLADJ_30MHZ_SDBND_SEL
+#define GFLADJ_30MHZ(n) ((n) 0x3f)
+
/* Device Configuration Register */
#define DWC3_DCFG_DEVADDR(addr) ((addr) 3)
#define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
@@ -712,6 +717,7 @@ struct dwc3_scratchpad_array {
* @rx_detect_poll_quirk: set if we enable rx_detect to polling lfps quirk
* @dis_u3_susphy_quirk: set if we disable usb3 suspend phy
* @dis_u2_susphy_quirk: set if we disable usb2 suspend phy
+ * @adjust_frame_length_quirk: enables post-silicon frame length adjustment
* @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
* @tx_de_emphasis: Tx de-emphasis value
* 0 - -6dB de-emphasis
@@ -841,6 +847,7 @@ struct dwc3 {
unsignedrx_detect_poll_quirk:1;
unsigneddis_u3_susphy_quirk:1;
unsigneddis_u2_susphy_quirk:1;
+ unsigned
[PATCH 0/2] hid printer fixes
Users should not be able to create more function instances than minor numbers allocated for the functions. The problem is in hid and printer functions. This short series fixes it by enforcing the maximum number of function directories created. Andrzej Pietrasiewicz (2): usb: gadget: f_hid: actually limit the number of instances usb: gadget: f_printer: actually limit the number of instances drivers/usb/gadget/function/f_hid.c | 5 + drivers/usb/gadget/function/f_printer.c | 10 +- 2 files changed, 14 insertions(+), 1 deletion(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: gadget: f_hid: actually limit the number of instances
There is a predefined maximum number of hid instances, currently 4.
A chrdev region is allocated accordingly, but with configfs the user
can create as many hid function directories as they like. To make
the number of hid instances consistent with the number of allocated minors,
the limit is enforced at directory creation time.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
drivers/usb/gadget/function/f_hid.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/gadget/function/f_hid.c
b/drivers/usb/gadget/function/f_hid.c
index f7f35a3..b02be06 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -700,6 +700,11 @@ static inline int hidg_get_minor(void)
ret = ida_simple_get(hidg_ida, 0, 0, GFP_KERNEL);
+ if (ret = HIDG_MINORS) {
+ ida_simple_remove(hidg_ida, ret);
+ ret = -ENODEV;
+ }
+
return ret;
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: gadget: f_printer: actually limit the number of instances
There is a predefined maximum number of printer instances, currently 4.
A chrdev region is allocated accordingly, but with configfs the user
can create as many printer function directories as they like. To make the
number of printer instances consistent with the number of allocated
minors, the limit is enforced at directory creation time.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
drivers/usb/gadget/function/f_printer.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_printer.c
b/drivers/usb/gadget/function/f_printer.c
index 44173df..357f63f 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1248,7 +1248,15 @@ static struct config_item_type printer_func_type = {
static inline int gprinter_get_minor(void)
{
- return ida_simple_get(printer_ida, 0, 0, GFP_KERNEL);
+ int ret;
+
+ ret = ida_simple_get(printer_ida, 0, 0, GFP_KERNEL);
+ if (ret = PRINTER_MINORS) {
+ ida_simple_remove(printer_ida, ret);
+ ret = -ENODEV;
+ }
+
+ return ret;
}
static inline void gprinter_put_minor(int minor)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Multiple drives on JMS56x-based sata-usb docking station.
Here is the output of usbmon - collected with usbmon program. The output seems the same of cat /sys/kernel/debug/usb/usbmon/0u with slightly more readable timestamps and shortened addresses so I used that. Giulio 8--- 343a9e40 0.721002 C Ii:2:001:1 0:2048 2 = 0400 343a9e40 0.721022 S Ii:2:001:1 -:2048 4 04b6d840 0.721173 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.721198 C Ci:2:001:0 0 4 = 01050100 04b6d840 0.721205 S Co:2:001:0 s 23 01 0010 0002 0 04b6d840 0.721213 C Co:2:001:0 0 0 04b6d840 0.721219 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.721225 C Ci:2:001:0 0 4 = 0105 04b6d840 0.746309 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.746328 C Ci:2:001:0 0 4 = 0105 04b6d840 0.772385 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.772416 C Ci:2:001:0 0 4 = 0105 04b6d840 0.798387 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.798412 C Ci:2:001:0 0 4 = 0105 04b6d840 0.824296 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.824314 C Ci:2:001:0 0 4 = 0105 04b6d840 0.824337 S Co:2:001:0 s 23 03 0004 0002 0 04b6d840 0.824352 C Co:2:001:0 0 0 04b6d840 0.875396 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 0.875658 C Ci:2:001:0 0 4 = 03051000 04b6d840 0.926516 S Co:2:001:0 s 23 01 0014 0002 0 04b6d840 0.926541 C Co:2:001:0 0 0 04b6d840 0.926573 S Ci:2:000:0 s 80 06 0100 0040 64 04b6d840 0.965570 C Ci:2:000:0 0 18 = 12011002 0040 2d156715 01010102 0501 . . . . . . . @ - . g . . . . . . . 04b6d840 0.965626 S Co:2:001:0 s 23 03 0004 0002 0 04b6d840 0.965648 C Co:2:001:0 0 0 04b6d840 1.016304 S Ci:2:001:0 s a3 00 0002 0004 4 04b6d840 1.016530 C Ci:2:001:0 0 4 = 03051000 04b6d840 1.067304 S Co:2:001:0 s 23 01 0014 0002 0 04b6d840 1.067323 C Co:2:001:0 0 0 04b6d840 1.067334 S Co:2:000:0 s 00 05 0003 0 04b6d840 1.109927 C Co:2:000:0 0 0 04b6dcc0 1.121308 S Ci:2:003:0 s 80 06 0100 0012 18 04b6dcc0 1.121523 C Ci:2:003:0 0 18 = 12011002 0040 2d156715 01010102 0501 . . . . . . . @ - . g . . . . . . . 04b6dcc0 1.121591 S Ci:2:003:0 s 80 06 0f00 0005 5 04b6dcc0 1.121782 C Ci:2:003:0 0 5 = 050f1600 02 04b6dcc0 1.121821 S Ci:2:003:0 s 80 06 0f00 0016 22 04b6dcc0 1.122017 C Ci:2:003:0 0 22 = 050f1600 02071002 0e0f 0a100300 0e00010a 2000 . . . . . . . . . . . . . . . . . . . .. 04b6dcc0 1.122049 S Ci:2:003:0 s 80 06 0200 0009 9 04b6dcc0 1.122179 C Ci:2:003:0 0 9 = 09025500 010104c0 01 . . U . . . . . . 04b6dcc0 1.122192 S Ci:2:003:0 s 80 06 0200 0055 85 04b6dcc0 1.122518 C Ci:2:003:0 0 85 = 09025500 010104c0 01090400 00020806 50060705 81020002 00070502 02000200 . . U . . . . . . . . . . . . . P . . . . . . . . . . . . . . . 04b6d480 1.122550 S Ci:2:003:0 s 80 06 0300 00ff 255 04b6d480 1.122642 C Ci:2:003:0 0 4 = 04030904 04b6d480 1.122653 S Ci:2:003:0 s 80 06 0302 0409 00ff 255 04b6d480 1.122891 C Ci:2:003:0 0 28 = 1c034a00 4d005300 35003600 78002000 53006500 72006900 65007300 . . J . M . S . 5 . 6 . x . . S . e . r . i . e . s . 04b6d480 1.122915 S Ci:2:003:0 s 80 06 0301 0409 00ff 255 04b6d480 1.123037 C Ci:2:003:0 0 16 = 10034a00 4d006900 63007200 6f006e00 . . J . M . i . c . r . o . n . 04b6d480 1.123050 S Ci:2:003:0 s 80 06 0305 0409 00ff 255 04b6d480 1.123276 C Ci:2:003:0 0 32 = 20034400 42003900 38003700 36003500 34003300 32003100 31003100 36003600 . D . B . 9 . 8 . 7 . 6 . 5 . 4 . 3 . 2 . 1 . 1 . 1 . 6 . 6 . 04b6dc00 1.123579 S Co:2:003:0 s 00 09 0001 0 04b6dc00 1.123773 C Co:2:003:0 0 0 04b6dc00 1.123817 S Ci:2:003:0 s 80 06 0304 0409 00ff 255 04b6dc00 1.124017 C Ci:2:003:0 0 34 = 22035500 53004200 20004d00 61007300 73002000 53007400 6f007200 61006700 . U . S . B .. M . a . s . s . . S . t . o . r . a . g . 04b6d300 1.124067 S Ci:2:003:0 s 80 06 0306 0409 00ff 255 04b6d300 1.124280 C Ci:2:003:0 0 46 = 2e034d00 53004300 20004200 75006c00 6b002d00 4f006e00 6c007900 20005400 . . M . S . C .. B . u . l . k . - . O . n . l . y .. T . 04a21480 1.238864 S Co:2:003:0 s 01 0b 0001 0 04a21480 1.239531 C Co:2:003:0 0 0 04a23540 1.239625 S Ci:2:003:0 s 80 06 030a 0409 00ff 255 04a23540 1.239898 C Ci:2:003:0 0 42 = 2a034d00 53004300 20004200 4f005400 2f005500 41005300 20005400 72006100 * . M . S . C .. B . O . T . / . U . A . S .. T . r . a . 04a21000 1.240585 S Bi:2:003:2 - 112 04a216c0 1.240594 S Bo:2:003:1 - 32 = 0101 1200 2400 . . . . . . . . . . . . . . . . . . . . $ . . . . . . . . . . . 04a216c0 1.240641 C Bo:2:003:1 0 32 04a21000 1.240889 C Bi:2:003:2 0 4 = 0601 04a23f00 1.240896 S Bi:2:003:2 - 112 04a21a80 1.240899 S Bi:2:003:3 - 36 04a21a80 1.241014 C Bi:2:003:3 0 36 = 0612
Re: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On 07/23/2015 03:00 AM, Peter Chen wrote:
That detail is merely about completeness. The code that calculates the
value of wMaxPacketSize should take into account what is configured in
bInterval of the endpoint, so if users change one thing, they don't have
to tweak the other as well.
bInterval denotes how many packets an endpoint can serve per second, and
wMaxPacketSize defines how large each packet can be. So in an
application that knows how many bytes/s are to be transferred,
wMaxPacketSize depends on bInterval.
On HS endpoints, we have 8 microframes per USB frame, so the divisor is
8000, not 1000. However, I just figured the descriptors in f_uac2 set
.bInterval to 4, which means a period of 8 (2^(4-1)), and that
compensates the factor again.
So, to conclude - your calculation indeed comes up with the correct
value, but it should still take the configured endpoint details into
account so the code makes clear how the numbers are determined.
Something like the following should work:
/* for FS */
div = 1000 / (1 (fs_epout_desc-bInterval - 1));
/* for HS */
div = 8000 / (1 (hs_epout_desc-bInterval - 1));
c_max_packet_size = uac2_opts-c_chmask * uac2_opts-c_ssize
* DIV_ROUND_UP(uac2_opts-c_srate, div);
Makes sense?
Thanks, it is correct. But looking the code at afunc_set_alt:
the method of calculating uac2-p_pktsize seems incorrect, it
may need to change like below:
@@ -1176,15 +1188,16 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
factor = 1000;
} else {
ep_desc = hs_epin_desc;
- factor = 125;
+ factor = 8000;
}
/* pre-compute some values for iso_complete() */
uac2-p_framesize = opts-p_ssize *
num_channels(opts-p_chmask);
rate = opts-p_srate * uac2-p_framesize;
- uac2-p_interval = (1 (ep_desc-bInterval - 1)) * factor;
- uac2-p_pktsize = min_t(unsigned int, rate / uac2-p_interval,
+ uac2-p_interval = factor / (1 (ep_desc-bInterval - 1));
+ uac2-p_pktsize = min_t(unsigned int, DIV_ROUND_UP(rate,
+ uac2-p_interval),
prm-max_psize);
Your p_interval calculation is equivalent in both cases:
FS: 1 * 1000 == 1000 / 1
HS: 8 * 125 == 8000 / 8
And no, p_pktsize is intentionally set to the minimum packet size that a
packet will usually have. The actual size might be higher due to the
accumulated residue (see below).
Two more questions:
1. If the wMaxPacketSize is calculated correctly at afunc_bind, could we use
it
directly at afunc_set_alt?
All code that sets up runtime parameters should live in afunc_set_alt(),
while code that cares for preparation of descriptor parameters remains
in afunc_setup(). At least in theory, the driver could support multiple
alternate settings which operate on different parameters.
2. If we use DIV_ROUND_UP to calculate packet size, do we still need
p_pktsize_residue?
The packets the audio driver sends can only contain full sample frames,
and the residue cares about accumulated left-overs that are smaller than
such frames. Setting p_pktsize with DIV_ROUND_UP() would cause every
packet to be slightly too large in certain setups, which will make the
audio run slightly too fast. So yes, we do need the residue logic in
order to provide exact timing.
Note that this is different from the wMaxPacketSize calculation, which
is a value that's stored in the descriptors, transfered to the host and
cached there, so it and cannot be changed at runtime. Hence, it has to
prepare for the 'worst' case, while the determination of actual packet
sizes at runtime might come up with smaller values than the maximum.
Thanks,
Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: GPIO support for Silicon Labs cp210x USB serial
On Thu, Jul 23, 2015 at 08:39:41AM -0700, John D. Blair wrote: Silicon Labs provides GPIOs on the CP210x USB serial hardware. Silicon Labs also supplies a version of the cp210x driver which provides ioctl() support allowing control of these GPIOs. Support of the ioctl() requires adding some additional code to read the specific cp210x chip version, as GPIO support varies across the product line. My question is: do you know why this GPIO support hasn't been integrated into the mainline kernel? Is it just Silicon Labs being lazy, or is there a technical reason this vendor-specific extension has been left out? The short answer is that we do not want custom driver ioctls. Any gpio implementation needs to based on gpiolib, which provides a standardised interface. Our application uses one of these GPIOs and we prefer to submit all our required changes upstream to simplify future maintenance. Patches are always welcome if you're willing to work on this. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: pl2303: Rewrite pl2303_encode_baud_rate_divisor
On Thu, Jul 23, 2015 at 04:21:36PM +0200, Michał Pecio wrote: + u32 baseline, mantissa, exponent; Please keep these as unsigned int. What's the reason for that? u32 is the exact width needed to perform these computations, while unsigned int is something a bit unspecified. Because you don't need a specific size (e.g. for marshalling). You can safely assume an int is at least 32 bits. + /* Calculate and return the exact baud rate. */ + baud = (baseline / mantissa) (exponent 1); You should handle division by zero here. It cannot currently happen as we cap the baudrate, but you should handle it here nonetheless. You can still assume a non-zero baudrate, though. How about that? mantissa = baseline / baud; if (mantissa == 0) mantissa = 1; Writing zero to the chip as it's currently done probably doesn't make much sense anyway... Sure, that works. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] drivers: usb: dwc3: Add adjust_frame_length_quirk
Hi again,
On Thu, Jul 23, 2015 at 09:55:32AM -0500, Felipe Balbi wrote:
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0447788..b7a5119 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -124,6 +124,7 @@
#define DWC3_GEVNTCOUNT(n) (0xc40c + (n * 0x10))
#define DWC3_GHWPARAMS80xc600
+#define DWC3_GFLADJ0xc630
/* Device Registers */
#define DWC3_DCFG 0xc700
@@ -234,6 +235,10 @@
/* Global HWPARAMS6 Register */
#define DWC3_GHWPARAMS6_EN_FPGA(1 7)
+/* Global Frame Length Adjustment Register */
+#define GFLADJ_30MHZ_REG_SEL (1 7)
always prepend with DWC3_ like *all* other macros in this file.
Also, match docs to ease grepping. This should be called
DWC3_GFLADJ_30MHZ_SDBND_SEL
yet another problem is that this register doesn't exist in *all*
versions of DWC3. It was introduced in version 2.50a so the branch I
typed above needs one extra check, and since it's getting so large, it
deserves be factored out into its own function.
static int dwc3_frame_length_adjustment(struct dwc3 *dwc, u32 fladj)
{
u32 reg;
u32 dft;
if (dwc-revision = DWC3_REVISION_250A)
return 0;
if (fladj == 0)
return 0;
reg = dwc3_readl(dwc-regs, DWC3_GFLADJ);
dft = reg 0x3f; /* needs a mask macro */
if (!dev_WARN_ONCE(dwc-dev, dft == fladj,
requested value same as default, ignoring\n)) {
reg = ~0x3f; /* needs a mask macro */
reg |= DWC3_GFLADJ_30MHZ_SDBND_SEL |
DWC3_GFLADJ_30MHZ(fladj_value);
dwc3_writel(dwc-regs, DWC3_GFLADJ, reg);
}
}
you really *MUST* check this sort of this out when writing patches. It's
not only about *your* SoC. You gotta remember we have a ton of different
users and those a prone to major grumpyness should a completely
unrelated patch break their use case.
You have access to the IP's documentation, and that contains the entire
history of the IP itself, so it's easy to figure all of this out with a
simple search in the documentation.
One extra detail is that you were very careless when writing to the
GFLADJ register too. You simply wrote your 30MHz sideband value,
potentially clearing other bits which shouldn't be touched. That alone
can add regressions.
When resending, make sure all 3 patches reach linux-usb. I still can't
find patch 3/3.
--
balbi
signature.asc
Description: Digital signature
GPIO support for Silicon Labs cp210x USB serial
Dear USB devs, I have a question regarding GPIO support for the Silicon Labs cp210x USB serial driver. Silicon Labs provides GPIOs on the CP210x USB serial hardware. Silicon Labs also supplies a version of the cp210x driver which provides ioctl() support allowing control of these GPIOs. Support of the ioctl() requires adding some additional code to read the specific cp210x chip version, as GPIO support varies across the product line. My question is: do you know why this GPIO support hasn't been integrated into the mainline kernel? Is it just Silicon Labs being lazy, or is there a technical reason this vendor-specific extension has been left out? Our application uses one of these GPIOs and we prefer to submit all our required changes upstream to simplify future maintenance. If there is no technical or stylistic objection that you know of I will prepare a patch based on Silicon Lab's implementation. The Silicon Labs driver is available here: https://www.silabs.com/products/mcu/Pages/USBtoUARTBridgeVCPDrivers.aspx best, John. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: GPIO support for Silicon Labs cp210x USB serial
On Thu, Jul 23, 2015 at 08:39:41AM -0700, John D. Blair wrote: Dear USB devs, I have a question regarding GPIO support for the Silicon Labs cp210x USB serial driver. Silicon Labs provides GPIOs on the CP210x USB serial hardware. Silicon Labs also supplies a version of the cp210x driver which provides ioctl() support allowing control of these GPIOs. Support of the ioctl() requires adding some additional code to read the specific cp210x chip version, as GPIO support varies across the product line. My question is: do you know why this GPIO support hasn't been integrated into the mainline kernel? Is it just Silicon Labs being lazy, or is there a technical reason this vendor-specific extension has been left out? I don't think anyone has ever submitted the code, and yes, vendor-specific ioctls will not be accepted. Integrating into the kernel's GPIO subsystem is the best thing to do to get this done properly. hope this helps, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net 2/3] r8152: fix remote wakeup
Oliver Neukum [mailto:oneu...@suse.com] [...] If the device does not support remote wakeup and the driver enables it, runtime power management will be switched off. That is the current state and it means that devices which don't support remote wakeup cannot do runtime power management at all. But the driver is correct. The only time a device that doesn't support remote wakeup can do runtime power managent is when no packets can be received that is while the interface is down. If you want to allow that you must not set needs_remote_wakeup in probe(), but you must set it in open() because it is necessary for runtime power management as I explained above. Sorry for the length of this mail, but I wanted to make sure I am absolutely clear this time. Thanks for you explanation. I focus on the wrong point. You mean this would cause the problem for runtime suspend. I would rethink my method and correct it. Best Regards, Hayes
[PATCH] da8xx musb: add device tree support
Trying this again as plain text... sorry about that.
Attached is a patch that adds device tree support to the da8xx musb driver.
The current driver expects a board file to setup the platform device and
perform the initialization. With this patch all of the setup is done through
the device tree.
diffstat for this patch is:
Documentation/devicetree/bindings/usb/da8xx-usb.txt | 18 ++
drivers/usb/musb/Kconfig|
1
drivers/usb/musb/da8xx.c |
139 ++--
3 files changed, 119 insertions(+), 39 deletions(-)
To apply this patch, in the root of a kernel tree use:
patch -p1 da8xx-musb.patch
Please let me know any feedback you have on this patch.
Thanks
Andrew Holcomb
Software Engineer
RELM Wireless
Signed-off-by: Andrew T Holcomb aholc...@relmbk.com
-
diff -pruN linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
--- linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
1969-12-31 18:00:00.0 -0600
+++ linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
2015-07-23 10:49:35.926160500 -0500
@@ -0,0 +1,18 @@
+DA8XX MUSB
+
+Required properties:
+ - compatible : Should be ti,da8xx-musb
+ - reg: Offset and length of registers
+ - interrupts : Interrupt number
+ - mode : Dual-role; either host mode host, peripheral mode
peripheral
+or both otg
+
+Example:
+
+musb@1e0 {
+ compatible = ti,da8xx-musb;
+ reg = 0x01e0 0x1;
+ interrupts = 58;
+ interrupt-names = mc;
+ mode = peripheral;
+};
diff -pruN linux-4.1/drivers/usb/musb/da8xx.c
linux-4.1.musb/drivers/usb/musb/da8xx.c
--- linux-4.1/drivers/usb/musb/da8xx.c 2015-06-22 00:05:43.0 -0500
+++ linux-4.1.musb/drivers/usb/musb/da8xx.c 2015-07-23 10:49:35.926160500
-0500
@@ -89,6 +89,36 @@ struct da8xx_glue {
struct clk *clk;
};
+static struct musb_hdrc_eps_bits musb_eps[] = {
+{ ep1_tx, 8, },
+{ ep1_rx, 8, },
+{ ep2_tx, 8, },
+{ ep2_rx, 8, },
+{ ep3_tx, 5, },
+{ ep3_rx, 5, },
+{ ep4_tx, 5, },
+{ ep4_rx, 5, },
+};
+
+static struct musb_hdrc_config musb_config = {
+.multipoint = true,
+.dyn_fifo = true,
+.soft_con = true,
+.dma= true,
+
+.num_eps= 5,
+.dma_channels = 8,
+.ram_bits = 10,
+.eps_bits = musb_eps,
+};
+
+static struct musb_hdrc_platform_data usb_data = {
+/* OTG requires a Mini-AB connector */
+.mode = MUSB_PERIPHERAL,
+.clock = usb20,
+.config = musb_config,
+};
+
/*
* REVISIT (PM): we should be able to keep the PHY in low power mode most
* of the time (24 MHz oscillator and PLL off, etc.) by setting POWER.D0
@@ -105,6 +135,9 @@ static inline void phy_on(void)
*/
cfgchip2 = ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN);
cfgchip2 |= CFGCHIP2_PHY_PLLON;
+ /* USB2.0 PHY reference clock is 24 MHz */
+ cfgchip2 = ~CFGCHIP2_REFFREQ;
+ cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
__raw_writel(cfgchip2, CFGCHIP2);
pr_info(Waiting for USB PHY clock good...\n);
@@ -480,44 +513,68 @@ static const struct platform_device_info
static int da8xx_probe(struct platform_device *pdev)
{
- struct resource musb_resources[2];
+ struct resource musb_resources[2];
struct musb_hdrc_platform_data *pdata = dev_get_platdata(pdev-dev);
+ struct device_node *np = pdev-dev.of_node;
struct platform_device *musb;
struct da8xx_glue *glue;
- struct platform_device_info pinfo;
struct clk *clk;
-
int ret = -ENOMEM;
+ const char *mode;
+ int strlen;
- glue = kzalloc(sizeof(*glue), GFP_KERNEL);
- if (!glue) {
- dev_err(pdev-dev, failed to allocate glue context\n);
+ glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
+ if (!glue)
+ goto err0;
+
+ musb = platform_device_alloc(musb-hdrc, PLATFORM_DEVID_AUTO);
+ if (!musb) {
+ dev_err(pdev-dev, failed to allocate musb device\n);
goto err0;
}
- clk = clk_get(pdev-dev, usb20);
+ clk = devm_clk_get(pdev-dev, usb20);
if (IS_ERR(clk)) {
dev_err(pdev-dev, failed to get clock\n);
ret = PTR_ERR(clk);
- goto err3;
+ goto err1;
}
- ret =
Re: [PATCH v2] USB: sierra: add 1199:68AB device ID
On Thu, Jul 23, 2015 at 02:58:03PM +0200, Dirk Behme wrote:
On 23.07.2015 12:36, Johan Hovold wrote:
On Thu, Jul 23, 2015 at 12:31:55PM +0200, Dirk Behme wrote:
Hi Johan,
On 20.07.2015 11:24, Johan Hovold wrote:
[ +CC: Bjørn and Dan ]
On Mon, Jul 20, 2015 at 08:14:19AM +0200, Dirk Behme wrote:
Add support for the Sierra Wireless AR8550 device with
USB descriptor 0x1199, 0x68AB. For this, lsusb reports:
Thanks for the patch. This modem business is a bit of a mess and it's
not always apparent what driver a device id belongs to.
Bus 001 Device 004: ID 1199:68ab Sierra Wireless, Inc.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x1199 Sierra Wireless, Inc.
idProduct 0x68ab
bcdDevice0.06
iManufacturer 3 Sierra Wireless, Incorporated
iProduct2 AR8550
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 198
bNumInterfaces 7
[...]
Signed-off-by: Dirk Behme dirk.be...@de.bosch.com
---
Changes in v2: Improve the commit message.
drivers/usb/serial/sierra.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 46179a0..4122e4f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -289,6 +289,9 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x1199, 0x68AA, 0xFF, 0xFF,
0xFF),
.driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
},
+{ USB_DEVICE(0x1199, 0x68AB), /* Sierra Wireless Direct IP
modems */
+ .driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
+},
This device has seven interfaces (0..6) so why do you use the direct-ip
interface blacklisting, which only blacklists interfaces = 7?
Using it the same way like the quite similar ID 0x1199, 0x68AA above
'just works for us'. Not being the experts on this, do you like to
propose anything else you like us to try?
Also do you notice any delays when connecting the device (which could
indicate that sierra is not the right driver)?
As above, do you like us to try anything else?
Could you post a log from when connecting the device with you patch
applied?
Are you looking for something like this?
[ 102.948830] usb 1-1: new high-speed USB device number 4 using ci_hdrc
[ 103.101172] usb 1-1: New USB device found, idVendor=1199, idProduct=68ab
[ 103.101239] usb 1-1: New USB device strings: Mfr=3, Product=2,
SerialNumber=0
[ 103.101270] usb 1-1: Product: AR8550
[ 103.101297] usb 1-1: Manufacturer: Sierra Wireless, Incorporated
[ 103.195766] usbcore: registered new interface driver usbserial
[ 103.204471] usbcore: registered new interface driver sierra
[ 103.204692] usbserial: USB Serial support registered for Sierra USB modem
[ 103.204861] sierra 1-1:1.0: Sierra USB modem converter detected
[ 103.218385] usb 1-1: Sierra USB modem converter now attached to ttyUSB0
[ 103.220304] sierra 1-1:1.1: Sierra USB modem converter detected
[ 103.227186] usb 1-1: Sierra USB modem converter now attached to ttyUSB1
[ 103.227721] sierra 1-1:1.2: Sierra USB modem converter detected
[ 103.234287] usb 1-1: Sierra USB modem converter now attached to ttyUSB2
[ 103.234533] sierra 1-1:1.3: Sierra USB modem converter detected
[ 103.236469] usb 1-1: Sierra USB modem converter now attached to ttyUSB3
[ 103.236663] sierra 1-1:1.4: Sierra USB modem converter detected
[ 103.237712] usb 1-1: Sierra USB modem converter now attached to ttyUSB4
[ 103.237873] sierra 1-1:1.5: Sierra USB modem converter detected
[ 103.247026] usb 1-1: Sierra USB modem converter now attached to ttyUSB5
[ 103.247327] sierra 1-1:1.6: Sierra USB modem converter detected
[ 103.251378] usb 1-1: Sierra USB modem converter now attached to ttyUSB6
Yes, we've seen lengthy timeouts during probe for some Sierra devices
when incorrectly driven by the sierra driver, but that's not the case
here.
This is probably the right driver, but I'm confused over the DirectIP
bits. Those interfaces are lacking AFAICT and still your comment claims
it's a DirectIP modem (that would also require it to be added not the
network driver).
My guess is that you should just drop the black_listing (and trim the
comment), but I'd like to wait a bit and see what the modem people says
about this. Bjørn, Dan?
Thanks,
Johan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to
[PATCH v3]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()
There is a race condition between
finish_unlinks-finish_urb() function and
usb_kill_urb() in ohci controller case. The finish_urb
calls spin_unlock(ohci-lock) before
usb_hcd_giveback_urb() function call, then if during
this time, usb_kill_urb is called for another endpoint,
then new ed will be added to ed_rm_list at beginning
for unlink. and ed_rm_list will point to newly added
ed.
When finish_urb() is completed in finish_unlinks() and
ed-td_list becomes empty as in below code (in finish_unlinks() function)
if (list_empty(ed-td_list)) {
*last = ed-ed_next;
ed-ed_next = NULL;
} else if (ohci-rh_state == OHCI_RH_RUNNING) {
*last = ed-ed_next;
ed-ed_next = NULL;
ed_schedule(ohci, ed);
}
*last = ed-ed_next will make ed_rm_list to point to ed-ed_next and
previously added ed by usb_kill_urb will be left unreferenced by
ed_rm_list. This causes usb_kill_urb() hang forever waiting for
finish_unlink to remove added ed from ed_rm_list.
The main reason for hang in this race condtion is addition and removal
of ed from ed_rm_list in the beginning during usb_kill_urb and later last*
is modified in finish_unlinks().
As suggested by Alan Stern, the solution for proper handling of
ohci-ed_rm_list is to remove ed from the ed_rm_list before finishing
any URBs. Then at the end, we can add ed back to the list if necessary.
This properly handle the updated ohci-ed_rm_list in
usb_kill_urb().
Fixes:977dcfdc6031(USB:OHCI:don't lose track of EDs when a
controller dies)
Acked-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Aman Deep aman.d...@samsung.com
CC: sta...@vger.kernel.org
---
drivers/usb/host/ohci-q.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index f7d561e..496658b 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -1023,6 +1023,8 @@ ed_idle:
* have modified this list. normally it's just prepending
* entries (which we'd ignore), but paranoia won't hurt.
*/
+ *last = ed-ed_next;
+ ed-ed_next = NULL;
modified = 0;
/* unlink urbs as requested, but rescan the list after
@@ -1081,20 +1083,21 @@ rescan_this:
goto rescan_this;
/*
-* If no TDs are queued, take ED off the ed_rm_list.
+* If no TDs are queued, ED is now idle.
* Otherwise, if the HC is running, reschedule.
-* If not, leave it on the list for further dequeues.
+* If the HC isn't running, add ED back to the
+* start of the list for later processing.
*/
if (list_empty(ed-td_list)) {
- *last = ed-ed_next;
- ed-ed_next = NULL;
list_del(ed-in_use_list);
} else if (ohci-rh_state == OHCI_RH_RUNNING) {
- *last = ed-ed_next;
- ed-ed_next = NULL;
ed_schedule(ohci, ed);
} else {
- last = ed-ed_next;
+ ed-ed_next = ohci-ed_rm_list;
+ ohci-ed_rm_list = ed;
+ /* Don't loop on the same ED */
+ if (last == ohci-ed_rm_list)
+ last = ed-ed_next;
}
if (modified)
--
1.7.9.5
Re: [PATCH v2] USB: sierra: add 1199:68AB device ID
On 23.07.2015 12:36, Johan Hovold wrote:
On Thu, Jul 23, 2015 at 12:31:55PM +0200, Dirk Behme wrote:
Hi Johan,
On 20.07.2015 11:24, Johan Hovold wrote:
[ +CC: Bjørn and Dan ]
On Mon, Jul 20, 2015 at 08:14:19AM +0200, Dirk Behme wrote:
Add support for the Sierra Wireless AR8550 device with
USB descriptor 0x1199, 0x68AB. For this, lsusb reports:
Thanks for the patch. This modem business is a bit of a mess and it's
not always apparent what driver a device id belongs to.
Bus 001 Device 004: ID 1199:68ab Sierra Wireless, Inc.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x1199 Sierra Wireless, Inc.
idProduct 0x68ab
bcdDevice0.06
iManufacturer 3 Sierra Wireless, Incorporated
iProduct2 AR8550
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 198
bNumInterfaces 7
[...]
Signed-off-by: Dirk Behme dirk.be...@de.bosch.com
---
Changes in v2: Improve the commit message.
drivers/usb/serial/sierra.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 46179a0..4122e4f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -289,6 +289,9 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x1199, 0x68AA, 0xFF, 0xFF, 0xFF),
.driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
},
+ { USB_DEVICE(0x1199, 0x68AB), /* Sierra Wireless Direct IP modems */
+ .driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
+ },
This device has seven interfaces (0..6) so why do you use the direct-ip
interface blacklisting, which only blacklists interfaces = 7?
Using it the same way like the quite similar ID 0x1199, 0x68AA above
'just works for us'. Not being the experts on this, do you like to
propose anything else you like us to try?
Also do you notice any delays when connecting the device (which could
indicate that sierra is not the right driver)?
As above, do you like us to try anything else?
Could you post a log from when connecting the device with you patch
applied?
Are you looking for something like this?
[ 102.948830] usb 1-1: new high-speed USB device number 4 using ci_hdrc
[ 103.101172] usb 1-1: New USB device found, idVendor=1199, idProduct=68ab
[ 103.101239] usb 1-1: New USB device strings: Mfr=3, Product=2,
SerialNumber=0
[ 103.101270] usb 1-1: Product: AR8550
[ 103.101297] usb 1-1: Manufacturer: Sierra Wireless, Incorporated
[ 103.195766] usbcore: registered new interface driver usbserial
[ 103.204471] usbcore: registered new interface driver sierra
[ 103.204692] usbserial: USB Serial support registered for Sierra USB modem
[ 103.204861] sierra 1-1:1.0: Sierra USB modem converter detected
[ 103.218385] usb 1-1: Sierra USB modem converter now attached to ttyUSB0
[ 103.220304] sierra 1-1:1.1: Sierra USB modem converter detected
[ 103.227186] usb 1-1: Sierra USB modem converter now attached to ttyUSB1
[ 103.227721] sierra 1-1:1.2: Sierra USB modem converter detected
[ 103.234287] usb 1-1: Sierra USB modem converter now attached to ttyUSB2
[ 103.234533] sierra 1-1:1.3: Sierra USB modem converter detected
[ 103.236469] usb 1-1: Sierra USB modem converter now attached to ttyUSB3
[ 103.236663] sierra 1-1:1.4: Sierra USB modem converter detected
[ 103.237712] usb 1-1: Sierra USB modem converter now attached to ttyUSB4
[ 103.237873] sierra 1-1:1.5: Sierra USB modem converter detected
[ 103.247026] usb 1-1: Sierra USB modem converter now attached to ttyUSB5
[ 103.247327] sierra 1-1:1.6: Sierra USB modem converter detected
[ 103.251378] usb 1-1: Sierra USB modem converter now attached to ttyUSB6
Best regards
Dirk
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V4 1/3] usb: Add Xen pvUSB protocol description
On Thu, Jul 23, 2015 at 08:46:17AM +0200, Juergen Gross wrote: On 07/23/2015 06:36 AM, Greg KH wrote: On Thu, Jul 23, 2015 at 06:04:39AM +0200, Juergen Gross wrote: On 07/23/2015 01:46 AM, Greg KH wrote: On Tue, Jun 23, 2015 at 08:53:23AM +0200, Juergen Gross wrote: Add the definition of pvUSB protocol used between the pvUSB frontend in a Xen domU and the pvUSB backend in a Xen driver domain (usually Dom0). This header was originally provided by Fujitsu for Xen based on Linux 2.6.18. Changes are: - adapt to Linux style guide Signed-off-by: Juergen Gross jgr...@suse.com --- include/xen/interface/io/usbif.h | 252 +++ Why is this a different interface than the existing ones we have today (i.e. usbip?) Where is it documented? Do the Xen developers / The interface definition is living in the Xen git repository for several years now: git://xenbits.xen.org/xen.git - xen/include/public/io/usbif.h That's header file, not a document describing the api here. I suppose you want to tell me I should add something like: Documentation/DocBook/usb/API-struct-urb.html Somewhere that people can refer to that describes this public-facing API that must not ever be broken or changed. If you want to put it in a documentation file, or a .h file, I don't care. It is used e.g. in SUSE's xen kernel since 2.6.18. I am very aware of the amount of Xen crap in SuSE's kernel, don't use that as an excuse for me to merge it to mainline :) :-) Wasn't meant as an excuse, just a hint why the interface can't be the same as for usbip. We have to ensure compatibility with those kernels This shouldn't be a kernel/kernel compability issue, as the api talks between Xen and the OS, not between different OSs, right? and possibly other operating systems (BSD?, Windows?) which already might be using pvUSB with a Dom0 based on the SUSE xen kernel. Are there other operating system drivers today that use this API? Is this an API in the Xen core today that we have to support? Some more background / descriptions would be nice to have. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] da8xx musb: add device tree support
Hello.
I'm the author of the da8xx.c file you're patching. :-)
On 07/23/2015 07:43 PM, Andrew Holcomb wrote:
Trying this again as plain text... sorry about that.
Please place such comments under the --- tear line (that goes after
signoff), else they end up in the patch change log and a maintainer will have
to remove them by hand.
Attached is a patch
It's not really attached.
that adds device tree support to the da8xx musb driver.
I prefer to call it glue layer, not a driver.
The current driver expects a board file to setup the platform device and
perform the initialization.
With this patch all of the setup is done through the device tree.
Please wrap your change log at 80 columns (or even less).
diffstat for this patch is:
You can have diffstat under --- without such comments.
Documentation/devicetree/bindings/usb/da8xx-usb.txt | 18 ++
drivers/usb/musb/Kconfig|
1
drivers/usb/musb/da8xx.c |
139 ++--
3 files changed, 119 insertions(+), 39 deletions(-)
To apply this patch, in the root of a kernel tree use:
patch -p1 da8xx-musb.patch
No need to tell that, -p1 is a default.
Please let me know any feedback you have on this patch.
Thanks
Andrew Holcomb
Software Engineer
RELM Wireless
The patch change log is not a place for your signature.
It's better to completely omit that.
Signed-off-by: Andrew T Holcomb aholc...@relmbk.com
-
No need for such separator.
diff -pruN linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
--- linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
1969-12-31 18:00:00.0 -0600
+++ linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
2015-07-23 10:49:35.926160500 -0500
@@ -0,0 +1,18 @@
+DA8XX MUSB
I'd prefer to call it DA8xx/OMAP-L1x.
+
+Required properties:
+ - compatible : Should be ti,da8xx-musb
+ - reg: Offset and length of registers
+ - interrupts : Interrupt number
+ - mode : Dual-role; either host mode host, peripheral mode
peripheral
+or both otg
Isn't there a standardized property called dr_mode for that?
And shouldn't be also e.g. clocks property?
+
+Example:
+
+musb@1e0 {
Just usb@1e0 please, to conform to the ePAPR standard.
+ compatible = ti,da8xx-musb;
+ reg = 0x01e0 0x1;
+ interrupts = 58;
+ interrupt-names = mc;
You didn't mention it above.
+ mode = peripheral;
+};
diff -pruN linux-4.1/drivers/usb/musb/da8xx.c
linux-4.1.musb/drivers/usb/musb/da8xx.c
--- linux-4.1/drivers/usb/musb/da8xx.c 2015-06-22 00:05:43.0 -0500
+++ linux-4.1.musb/drivers/usb/musb/da8xx.c 2015-07-23 10:49:35.926160500
-0500
@@ -89,6 +89,36 @@ struct da8xx_glue {
[...]
+static struct musb_hdrc_platform_data usb_data = {
+/* OTG requires a Mini-AB connector */
+.mode = MUSB_PERIPHERAL,
Hm, why are you defaulting to gadget?
[...]
@@ -105,6 +135,9 @@ static inline void phy_on(void)
*/
cfgchip2 = ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN);
cfgchip2 |= CFGCHIP2_PHY_PLLON;
+ /* USB2.0 PHY reference clock is 24 MHz */
+ cfgchip2 = ~CFGCHIP2_REFFREQ;
+ cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
I think this depends on the board.
[...]
@@ -480,44 +513,68 @@ static const struct platform_device_info
static int da8xx_probe(struct platform_device *pdev)
{
- struct resource musb_resources[2];
+ struct resource musb_resources[2];
struct musb_hdrc_platform_data *pdata = dev_get_platdata(pdev-dev);
+ struct device_node *np = pdev-dev.of_node;
struct platform_device *musb;
struct da8xx_glue *glue;
- struct platform_device_info pinfo;
struct clk *clk;
-
int ret = -ENOMEM;
+ const char *mode;
+ int strlen;
Why not just 'len'?
- glue = kzalloc(sizeof(*glue), GFP_KERNEL);
- if (!glue) {
- dev_err(pdev-dev, failed to allocate glue context\n);
+ glue = devm_kzalloc(pdev-dev, sizeof(*glue), GFP_KERNEL);
+ if (!glue)
+ goto err0;
+
This change is not related to device tree support.
+ musb = platform_device_alloc(musb-hdrc, PLATFORM_DEVID_AUTO);
+ if (!musb) {
+ dev_err(pdev-dev, failed to allocate musb device\n);
goto err0;
}
- clk = clk_get(pdev-dev, usb20);
+ clk = devm_clk_get(pdev-dev, usb20);
Re: [PATCH V2 1/4] usb: gadget: udc: bdc: Fix a driver crash on disconnect
On Wed, Jul 22, 2015 at 10:03 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Wed, Jul 22, 2015 at 06:27:29PM -0400, Alan Cooper wrote: On Wed, Jul 22, 2015 at 5:29 PM, Felipe Balbi ba...@ti.com wrote: On Wed, Jul 22, 2015 at 04:26:57PM -0500, Felipe Balbi wrote: On Wed, Jul 22, 2015 at 04:58:07PM -0400, Al Cooper wrote: V2 - Fix a compiler bug that happend when the config options CONFIG_USB_GADGET_DEBUG and CONFIG_USB_GADGET_VERBOSE were enabled. ep_dequeue() in bdc_ep.c was capturing the hw dequeue pointer incorrectly by reading the wrong register for the upper 32 bits. The header file defining the registers was incorrect. btw, the header file was really incorrect as long as you passed 0 to the argument :-p in fact, the minimal fix for this bug would be the one below: diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c index b04980cf6dc4..1efa61265d8d 100644 --- a/drivers/usb/gadget/udc/bdc/bdc_ep.c +++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c @@ -779,7 +779,7 @@ static int ep_dequeue(struct bdc_ep *ep, struct bdc_req *req) /* The current hw dequeue pointer */ tmp_32 = bdc_readl(bdc-regs, BDC_EPSTS0(0)); deq_ptr_64 = tmp_32; - tmp_32 = bdc_readl(bdc-regs, BDC_EPSTS0(1)); + tmp_32 = bdc_readl(bdc-regs, BDC_EPSTS1(0)); deq_ptr_64 |= ((u64)tmp_32 32); /* we have the dma addr of next bd that will be fetched by hardware */ And $subject becomes a cleanup patch for v4.3. Can you make these changes, please ? Yes, I can make these changes, but first let me explain why I did it this way. The 8 End Point Status registers are 32 bit consecutive registers, so defining them as: #define BDC_EPSTS0(n) (0x60 + (n * 0x10)) #define BDC_EPSTS1(n) (0x64 + (n * 0x10)) #define BDC_EPSTS2(n) (0x68 + (n * 0x10)) #define BDC_EPSTS3(n) (0x6c + (n * 0x10)) #define BDC_EPSTS4(n) (0x70 + (n * 0x10)) #define BDC_EPSTS5(n) (0x74 + (n * 0x10)) #define BDC_EPSTS6(n) (0x78 + (n * 0x10)) #define BDC_EPSTS7(n) (0x7c + (n * 0x10)) seems misleading, does not reflect the hardware and using anything other than (0) would get you to some other unexpected register and should be considered a coding error. it sure is, but the minimal patch for -rc is what I sent above :-) As long as you pass 0 as parameters, all your offsets are correct, so removing the parameter (which must be always zero) is, actually, refactoring happening. OK, I think I understand. You're asking me to split the bug fix patch into two patches, one is a minimal bug fix patch for -rc and the other is a cleanup patch (fix the header file) that will go into 4.3. Is that correct? I think the original hardware spec had each End Point Status as a block of registers but the first silicon had them as single registers and they are expected to stay that way. I believe they wanted to have: #define BDC_EPSTS(n)(0x60 + (n * 0x4)) and that way you can access each EP by passing the correct argument to that macro. Actually, the 8 EPSTS registers are all different and together contain the info for 1 end point. The original plan was to have 4 32bit registers per end point and enough sets of these registers for all endpoints, but this turned out to be too expensive in silicon so they changed it to 1 set of 8 registers that are updated when an end point is stopped. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: f_hid: actually limit the number of instances
Hello.
On 07/23/2015 05:11 PM, Andrzej Pietrasiewicz wrote:
There is a predefined maximum number of hid instances, currently 4.
A chrdev region is allocated accordingly, but with configfs the user
can create as many hid function directories as they like. To make
the number of hid instances consistent with the number of allocated minors,
the limit is enforced at directory creation time.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
drivers/usb/gadget/function/f_hid.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/gadget/function/f_hid.c
b/drivers/usb/gadget/function/f_hid.c
index f7f35a3..b02be06 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -700,6 +700,11 @@ static inline int hidg_get_minor(void)
ret = ida_simple_get(hidg_ida, 0, 0, GFP_KERNEL);
I don't think empty line is needed here.
+ if (ret = HIDG_MINORS) {
+ ida_simple_remove(hidg_ida, ret);
+ ret = -ENODEV;
+ }
+
return ret;
}
MBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] USB: Fix persist resume of some USB 2.0 devices
On Thu, Jul 23, 2015 at 10:17:38AM +, Vasudevan, Krishna PrasathX K wrote: Hi, This mail is for RFC regarding the persist resume of some USB 2.0 devices, Problem Summary: Problem has been observed for some USB 2.0 devices while resuming from sleep. When the USB “persist” feature is enabled through sysfs it is expected to retain its previous mount point across sleep and resume states, it works fine for most of the USB 2.0 mass storage devices, but for some USB 2.0 pendrives such as (Transcend Jet flash 16 GB Alcor micro corporation) it seems to fail and re-enumeration happens resulting in creation of new device data structure. This problem seems to be similar to the problem faced with USB 3.0 devices which is mentioned in below link http://marc.info/?l=linux-usbm=140566728011240w=2 In the above mentioned link the problem is observed with USB 3.0 devices, but in this case it is observed with USB 2.0 mass storage devices.While resuming from sleep, a USB disconnect message and re enumeration messages are seen resulting in the reset of “persist ” variable in sysfs. In the above mentioned link a fix is proposed and that patch was merged in mainline kernel, but the patch restricts the timeout only to USB 3.0 devices. I am not sure why it was restricted to USB 3.0 devices, but the same issue seems to appear for USB 2.0 devices too. The below patch removes the restriction for USB 3.0 devices and makes the time out applicable to all USB devices. Signed-off-by: vasudevan,krishna prasath krishna.prasathx.k.vasude...@intel.com --- drivers/usb/core/hub.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d2bd9d7..b2b709d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3320,7 +3320,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) clear_bit(port1, hub-busy_bits); - if (udev-persist_enabled hub_is_superspeed(hub-hdev)) + if (udev-persist_enabled) status = wait_for_ss_port_enable(udev, hub, port1, portchange, portstatus); -- This patch is totally corrupted and can't be applied even for testing :( Please fix up your email client and try again. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: mass_storage: Use static array for luns
This patch replace dynamicly allocated luns array with static one.
This simplifies the code of mass storage function and modules.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_mass_storage.c | 127 ++-
drivers/usb/gadget/function/f_mass_storage.h | 4 -
drivers/usb/gadget/legacy/acm_ms.c | 6 --
drivers/usb/gadget/legacy/mass_storage.c | 6 --
drivers/usb/gadget/legacy/multi.c| 6 --
drivers/usb/gadget/legacy/nokia.c| 14 +--
6 files changed, 49 insertions(+), 114 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 9e88e2b..5fcd9a0 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -279,9 +279,8 @@ struct fsg_common {
int cmnd_size;
u8 cmnd[MAX_COMMAND_SIZE];
- unsigned intnluns;
unsigned intlun;
- struct fsg_lun **luns;
+ struct fsg_lun *luns[FSG_MAX_LUNS];
struct fsg_lun *curlun;
unsigned intbulk_out_maxpacket;
@@ -490,6 +489,16 @@ static void bulk_out_complete(struct usb_ep *ep, struct
usb_request *req)
spin_unlock(common-lock);
}
+static int _fsg_common_get_max_lun(struct fsg_common *common)
+{
+ int i = ARRAY_SIZE(common-luns) - 1;
+
+ while (i = 0 !common-luns[i])
+ --i;
+
+ return i;
+}
+
static int fsg_setup(struct usb_function *f,
const struct usb_ctrlrequest *ctrl)
{
@@ -533,7 +542,7 @@ static int fsg_setup(struct usb_function *f,
w_length != 1)
return -EDOM;
VDBG(fsg, get max LUN\n);
- *(u8 *)req-buf = fsg-common-nluns - 1;
+ *(u8 *)req-buf = _fsg_common_get_max_lun(fsg-common);
/* Respond with data/status */
req-length = min((u16)1, w_length);
@@ -2131,8 +2140,9 @@ static int received_cbw(struct fsg_dev *fsg, struct
fsg_buffhd *bh)
}
/* Is the CBW meaningful? */
- if (cbw-Lun = FSG_MAX_LUNS || cbw-Flags ~US_BULK_FLAG_IN ||
- cbw-Length = 0 || cbw-Length MAX_COMMAND_SIZE) {
+ if (cbw-Lun = ARRAY_SIZE(common-luns) ||
+ cbw-Flags ~US_BULK_FLAG_IN || cbw-Length = 0 ||
+ cbw-Length MAX_COMMAND_SIZE) {
DBG(fsg, non-meaningful CBW: lun = %u, flags = 0x%x,
cmdlen %u\n,
cbw-Lun, cbw-Flags, cbw-Length);
@@ -2159,7 +2169,7 @@ static int received_cbw(struct fsg_dev *fsg, struct
fsg_buffhd *bh)
if (common-data_size == 0)
common-data_dir = DATA_DIR_NONE;
common-lun = cbw-Lun;
- if (common-lun common-nluns)
+ if (common-lun ARRAY_SIZE(common-luns))
common-curlun = common-luns[common-lun];
else
common-curlun = NULL;
@@ -2307,7 +2317,7 @@ reset:
}
common-running = 1;
- for (i = 0; i common-nluns; ++i)
+ for (i = 0; i ARRAY_SIZE(common-luns); ++i)
if (common-luns[i])
common-luns[i]-unit_attention_data =
SS_RESET_OCCURRED;
@@ -2409,7 +2419,7 @@ static void handle_exception(struct fsg_common *common)
if (old_state == FSG_STATE_ABORT_BULK_OUT)
common-state = FSG_STATE_STATUS_PHASE;
else {
- for (i = 0; i common-nluns; ++i) {
+ for (i = 0; i ARRAY_SIZE(common-luns); ++i) {
curlun = common-luns[i];
if (!curlun)
continue;
@@ -2453,7 +2463,7 @@ static void handle_exception(struct fsg_common *common)
* a waste of time. Ditto for the INTERFACE_CHANGE and
* CONFIG_CHANGE cases.
*/
- /* for (i = 0; i common-nluns; ++i) */
+ /* for (i = 0; i common-ARRAY_SIZE(common-luns); ++i) */
/* if (common-luns[i]) */
/* common-luns[i]-unit_attention_data = */
/* SS_RESET_OCCURRED; */
@@ -2552,12 +2562,11 @@ static int fsg_main_thread(void *common_)
if (!common-ops || !common-ops-thread_exits
|| common-ops-thread_exits(common) 0) {
- struct fsg_lun **curlun_it = common-luns;
- unsigned i = common-nluns;
+ int i;
down_write(common-filesem);
- for (; i--; ++curlun_it) {
- struct fsg_lun *curlun = *curlun_it;
+ for (i = 0; i ARRAY_SIZE(common-luns); --i) {
+ struct fsg_lun *curlun =
Re: [PATCH] usb: ulpi: call put_device if device_register fails
On Thu, Jul 23, 2015 at 12:02:40AM -0500, Felipe Balbi wrote:
Hi,
On Wed, Jul 22, 2015 at 08:14:46PM -0700, Greg Kroah-Hartman wrote:
On Wed, Jul 22, 2015 at 09:04:40PM -0500, Felipe Balbi wrote:
On Wed, Jul 22, 2015 at 02:39:34PM -0700, Greg Kroah-Hartman wrote:
On Tue, Jun 23, 2015 at 01:57:38PM +0300, Heikki Krogerus wrote:
On Fri, Jun 19, 2015 at 01:12:36AM +0800, ChengYi He wrote:
put_device is required to release the last reference to the device.
Signed-off-by: ChengYi He chengyihetai...@gmail.com
---
drivers/usb/common/ulpi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..bd25bdb 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -184,8 +184,10 @@ static int ulpi_register(struct device *dev,
struct ulpi *ulpi)
request_module(ulpi:v%04xp%04x, ulpi-id.vendor,
ulpi-id.product);
ret = device_register(ulpi-dev);
- if (ret)
+ if (ret) {
+ put_device(ulpi-dev);
If device_register returns failure, put_device has already been
called. Check device_add in drivers/base/core.c.
Yes, please read the function, which says:
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up your
* reference instead.
But, the problem is that the ulpi core doesn't own that struct device.
It comes from elsewhere. It comes from somewhere deep down in the dw3
core, which is where I lost the path. Something needs to be fixed in
dwc3_probe() to properly clean up the device if it fails, which is not
happening right now.
So this patch would actually cause much bigger problems than fixing
anything, so it's wrong, but for a different reason than you are talking
about here.
And ugh, the ulpi and dwc code binding together, what a mess, horrid...
any suggestions ? DWC *is* the one implementing the bus. If there's a
better way, we can certainly shuffle code around.
As dwc is the only thing using the bus, why is it drivers/usb/core/ ?
musb also has a SW-accessible ULPI bus. And, IIRC, so does DWC2 ;-)
But they aren't calling ulpi_register(), so how can they be using this
code?
And the error path here is broken, the bus should be creating the device
(i.e. no subsystem should ever be registering a device it did not
create), so that it can properly clean things up when stuff goes wrong.
The whole subsys_init() is also a bad feeling that it's not architected
correctly, that shouldn't be needed, which is why I never took that
patch. Just noticed it came in through yours, I wanted it broken so
it would be fixed properly and not papered over like this.
I just felt it would be better to 'fix' it for the -rc until it can be
fixed *properly*. A follow up fix should incur no visible changes to
drivers anyway.
I don't like fixes like this because no one now has any pressure to fix
it properly. Are you doing that work? If not, who is?
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] da8xx musb: add device tree support
Hi,
On Thu, Jul 23, 2015 at 04:19:06PM +, Andrew Holcomb wrote:
Attached is a patch that adds device tree support to the da8xx musb driver.
The current driver expects a board file to setup the platform device and
perform the initialization. With this patch all of the setup is done through
the device tree.
diffstat for this patch is:
Documentation/devicetree/bindings/usb/da8xx-usb.txt | 18 ++
drivers/usb/musb/Kconfig|1
drivers/usb/musb/da8xx.c| 139
++--
3 files changed, 119 insertions(+), 39 deletions(-)
To apply this patch, in the root of a kernel tree use:
patch -p1 da8xx-musb.patch
Please let me know any feedback you have on this patch.
Thanks
Andrew Holcomb
Software Engineer
RELM Wireless
Signed-off-by: Andrew T Holcomb
aholc...@relmbk.commailto:aholc...@relmbk.com
diff -pruN linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
--- linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt 1969-12-31
18:00:00.0 -0600
+++ linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
2015-07-23 10:49:35.926160500 -0500
@@ -0,0 +1,18 @@
+DA8XX MUSB
+
+Required properties:
+ - compatible : Should be ti,da8xx-musb
+ - reg: Offset and length of registers
+ - interrupts : Interrupt number
+ - mode : Dual-role; either host mode host, peripheral mode
peripheral
+or both otg
+
+Example:
+
+musb@1e0mailto:+musb@1e0 {
+ compatible = ti,da8xx-musb;
+ reg = 0x01e0 0x1;
+ interrupts = 58;
+ interrupt-names = mc;
+ mode = peripheral;
+};
diff -pruN linux-4.1/drivers/usb/musb/da8xx.c
linux-4.1.musb/drivers/usb/musb/da8xx.c
--- linux-4.1/drivers/usb/musb/da8xx.c 2015-06-22 00:05:43.0 -0500
+++ linux-4.1.musb/drivers/usb/musb/da8xx.c 2015-07-23 10:49:35.926160500
-0500
@@ -89,6 +89,36 @@ struct da8xx_glue {
struct clk *clk;
};
+static struct musb_hdrc_eps_bits musb_eps[] = {
+{ ep1_tx, 8, },
patch's mangled. Sorry, can't apply.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] da8xx musb: add device tree support
On Thu, Jul 23, 2015 at 04:43:55PM +, Andrew Holcomb wrote:
Trying this again as plain text... sorry about that.
Attached is a patch that adds device tree support to the da8xx musb driver.
The current driver expects a board file to setup the platform device and
perform the initialization. With this patch all of the setup is done through
the device tree.
diffstat for this patch is:
Documentation/devicetree/bindings/usb/da8xx-usb.txt | 18 ++
drivers/usb/musb/Kconfig|
1
drivers/usb/musb/da8xx.c
| 139 ++--
3 files changed, 119 insertions(+), 39 deletions(-)
To apply this patch, in the root of a kernel tree use:
patch -p1 da8xx-musb.patch
Please let me know any feedback you have on this patch.
Thanks
Andrew Holcomb
Software Engineer
RELM Wireless
Signed-off-by: Andrew T Holcomb aholc...@relmbk.com
-
diff -pruN linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
--- linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt
1969-12-31 18:00:00.0 -0600
+++ linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
2015-07-23 10:49:35.926160500 -0500
@@ -0,0 +1,18 @@
+DA8XX MUSB
+
+Required properties:
+ - compatible : Should be ti,da8xx-musb
+ - reg: Offset and length of registers
+ - interrupts : Interrupt number
+ - mode : Dual-role; either host mode host, peripheral mode
peripheral
+or both otg
+
+Example:
+
+musb@1e0 {
+ compatible = ti,da8xx-musb;
+ reg = 0x01e0 0x1;
+ interrupts = 58;
+ interrupt-names = mc;
+ mode = peripheral;
+};
diff -pruN linux-4.1/drivers/usb/musb/da8xx.c
linux-4.1.musb/drivers/usb/musb/da8xx.c
--- linux-4.1/drivers/usb/musb/da8xx.c2015-06-22 00:05:43.0
-0500
+++ linux-4.1.musb/drivers/usb/musb/da8xx.c 2015-07-23 10:49:35.926160500
-0500
@@ -89,6 +89,36 @@ struct da8xx_glue {
struct clk *clk;
};
+static struct musb_hdrc_eps_bits musb_eps[] = {
+{ ep1_tx, 8, },
+{ ep1_rx, 8, },
+{ ep2_tx, 8, },
+{ ep2_rx, 8, },
+{ ep3_tx, 5, },
+{ ep3_rx, 5, },
+{ ep4_tx, 5, },
+{ ep4_rx, 5, },
tabs are still converted to spaces, still can't apply.
--
balbi
signature.asc
Description: Digital signature
Re: [Patch V4 1/3] usb: Add Xen pvUSB protocol description
On 07/23/2015 06:36 AM, Greg KH wrote: On Thu, Jul 23, 2015 at 06:04:39AM +0200, Juergen Gross wrote: On 07/23/2015 01:46 AM, Greg KH wrote: On Tue, Jun 23, 2015 at 08:53:23AM +0200, Juergen Gross wrote: Add the definition of pvUSB protocol used between the pvUSB frontend in a Xen domU and the pvUSB backend in a Xen driver domain (usually Dom0). This header was originally provided by Fujitsu for Xen based on Linux 2.6.18. Changes are: - adapt to Linux style guide Signed-off-by: Juergen Gross jgr...@suse.com --- include/xen/interface/io/usbif.h | 252 +++ Why is this a different interface than the existing ones we have today (i.e. usbip?) Where is it documented? Do the Xen developers / The interface definition is living in the Xen git repository for several years now: git://xenbits.xen.org/xen.git - xen/include/public/io/usbif.h That's header file, not a document describing the api here. I suppose you want to tell me I should add something like: Documentation/DocBook/usb/API-struct-urb.html I can do this, of course. It is used e.g. in SUSE's xen kernel since 2.6.18. I am very aware of the amount of Xen crap in SuSE's kernel, don't use that as an excuse for me to merge it to mainline :) :-) Wasn't meant as an excuse, just a hint why the interface can't be the same as for usbip. We have to ensure compatibility with those kernels and possibly other operating systems (BSD?, Windows?) which already might be using pvUSB with a Dom0 based on the SUSE xen kernel. Juergen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 3/3] r8152: don't enable napi before rx ready
Adjust napi_disable() and napi_enable() to avoid r8152_poll() start
working before rx ready. Otherwise, it may have race condition for
rx_agg.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index eff1f25..fe2cd8a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2075,7 +2075,6 @@ static int rtl_start_rx(struct r8152 *tp)
{
int i, ret = 0;
- napi_disable(tp-napi);
INIT_LIST_HEAD(tp-rx_done);
for (i = 0; i RTL8152_MAX_RX; i++) {
INIT_LIST_HEAD(tp-rx_info[i].list);
@@ -2083,7 +2082,6 @@ static int rtl_start_rx(struct r8152 *tp)
if (ret)
break;
}
- napi_enable(tp-napi);
if (ret ++i RTL8152_MAX_RX) {
struct list_head rx_queue;
@@ -2944,8 +2942,10 @@ static void set_carrier(struct r8152 *tp)
if (!netif_carrier_ok(netdev)) {
tp-rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, tp-flags);
+ napi_disable(tp-napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
+ napi_enable(tp-napi);
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3388,9 +3388,11 @@ static int rtl8152_resume(struct usb_interface *intf)
if (test_bit(SELECTIVE_SUSPEND, tp-flags)) {
rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, tp-flags);
+ napi_disable(tp-napi);
set_bit(WORK_ENABLE, tp-flags);
if (netif_carrier_ok(tp-netdev))
rtl_start_rx(tp);
+ napi_enable(tp-napi);
} else {
tp-rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 1/3] r8152: fix the issue about U1/U2
- Disable U1/U2 during initialization.
- Disable lpm when linking is on, and enable it when linking is off.
- Disable U1/U2 when enabling runtime suspend.
It is possible to let hw stop working, if the U1/U2 request occurs
during some situations. The patch is used to avoid it.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 94 -
1 file changed, 54 insertions(+), 40 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7f6419e..e3a0110 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2166,6 +2166,7 @@ static int rtl8153_enable(struct r8152 *tp)
if (test_bit(RTL8152_UNPLUG, tp-flags))
return -ENODEV;
+ usb_disable_lpm(tp-udev);
set_tx_qlen(tp);
rtl_set_eee_plus(tp);
r8153_set_rx_early_timeout(tp);
@@ -2337,11 +2338,54 @@ static void __rtl_set_wol(struct r8152 *tp, u32 wolopts)
device_set_wakeup_enable(tp-udev-dev, false);
}
+static void r8153_u1u2en(struct r8152 *tp, bool enable)
+{
+ u8 u1u2[8];
+
+ if (enable)
+ memset(u1u2, 0xff, sizeof(u1u2));
+ else
+ memset(u1u2, 0x00, sizeof(u1u2));
+
+ usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
+}
+
+static void r8153_u2p3en(struct r8152 *tp, bool enable)
+{
+ u32 ocp_data;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
+ if (enable tp-version != RTL_VER_03 tp-version != RTL_VER_04)
+ ocp_data |= U2P3_ENABLE;
+ else
+ ocp_data = ~U2P3_ENABLE;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
+}
+
+static void r8153_power_cut_en(struct r8152 *tp, bool enable)
+{
+ u32 ocp_data;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
+ if (enable)
+ ocp_data |= PWR_EN | PHASE2_EN;
+ else
+ ocp_data = ~(PWR_EN | PHASE2_EN);
+ ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
+ ocp_data = ~PCUT_STATUS;
+ ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
+}
+
static void rtl_runtime_suspend_enable(struct r8152 *tp, bool enable)
{
if (enable) {
u32 ocp_data;
+ r8153_u1u2en(tp, false);
+ r8153_u2p3en(tp, false);
+
__rtl_set_wol(tp, WAKE_ANY);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);
@@ -2353,6 +2397,8 @@ static void rtl_runtime_suspend_enable(struct r8152 *tp,
bool enable)
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);
} else {
__rtl_set_wol(tp, tp-saved_wolopts);
+ r8153_u2p3en(tp, true);
+ r8153_u1u2en(tp, true);
}
}
@@ -2599,46 +2645,6 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
set_bit(PHY_RESET, tp-flags);
}
-static void r8153_u1u2en(struct r8152 *tp, bool enable)
-{
- u8 u1u2[8];
-
- if (enable)
- memset(u1u2, 0xff, sizeof(u1u2));
- else
- memset(u1u2, 0x00, sizeof(u1u2));
-
- usb_ocp_write(tp, USB_TOLERANCE, BYTE_EN_SIX_BYTES, sizeof(u1u2), u1u2);
-}
-
-static void r8153_u2p3en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL);
- if (enable)
- ocp_data |= U2P3_ENABLE;
- else
- ocp_data = ~U2P3_ENABLE;
- ocp_write_word(tp, MCU_TYPE_USB, USB_U2P3_CTRL, ocp_data);
-}
-
-static void r8153_power_cut_en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_POWER_CUT);
- if (enable)
- ocp_data |= PWR_EN | PHASE2_EN;
- else
- ocp_data = ~(PWR_EN | PHASE2_EN);
- ocp_write_word(tp, MCU_TYPE_USB, USB_POWER_CUT, ocp_data);
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
- ocp_data = ~PCUT_STATUS;
- ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
-}
-
static void r8153_first_init(struct r8152 *tp)
{
u32 ocp_data;
@@ -2781,6 +2787,7 @@ static void rtl8153_disable(struct r8152 *tp)
r8153_disable_aldps(tp);
rtl_disable(tp);
r8153_enable_aldps(tp);
+ usb_enable_lpm(tp-udev);
}
static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u16 speed, u8
duplex)
@@ -2901,9 +2908,13 @@ static void rtl8153_up(struct r8152 *tp)
if (test_bit(RTL8152_UNPLUG, tp-flags))
return;
+ r8153_u1u2en(tp, false);
r8153_disable_aldps(tp);
r8153_first_init(tp);
r8153_enable_aldps(tp);
+ r8153_u2p3en(tp, true);
+ r8153_u1u2en(tp, true);
+ usb_enable_lpm(tp-udev);
}
static void rtl8153_down(struct r8152 *tp)
@@ -2914,6 +2925,7 @@ static
[PATCH net 2/3] r8152: fix remote wakeup
Set needs_remote_wakeup only when the device supports it. Signed-off-by: Hayes Wang hayesw...@realtek.com --- drivers/net/usb/r8152.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index e3a0110..eff1f25 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -4059,7 +4059,8 @@ static int rtl8152_probe(struct usb_interface *intf, break; } - intf-needs_remote_wakeup = 1; + if (udev-actconfig-desc.bmAttributes USB_CONFIG_ATT_WAKEUP) + intf-needs_remote_wakeup = 1; tp-rtl_ops.init(tp); set_ethernet_addr(tp); -- 2.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 0/3] r8152: issues fix
These patches are used to fix issues. Hayes Wang (3): r8152: fix the issue about U1/U2 r8152: fix remote wakeup r8152: don't enable napi before rx ready drivers/net/usb/r8152.c | 103 1 file changed, 60 insertions(+), 43 deletions(-) -- 2.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On Thu, Jul 23, 2015 at 08:11:11AM +0200, Daniel Mack wrote:
On 07/23/2015 03:00 AM, Peter Chen wrote:
That detail is merely about completeness. The code that calculates the
value of wMaxPacketSize should take into account what is configured in
bInterval of the endpoint, so if users change one thing, they don't have
to tweak the other as well.
bInterval denotes how many packets an endpoint can serve per second, and
wMaxPacketSize defines how large each packet can be. So in an
application that knows how many bytes/s are to be transferred,
wMaxPacketSize depends on bInterval.
On HS endpoints, we have 8 microframes per USB frame, so the divisor is
8000, not 1000. However, I just figured the descriptors in f_uac2 set
.bInterval to 4, which means a period of 8 (2^(4-1)), and that
compensates the factor again.
So, to conclude - your calculation indeed comes up with the correct
value, but it should still take the configured endpoint details into
account so the code makes clear how the numbers are determined.
Something like the following should work:
/* for FS */
div = 1000 / (1 (fs_epout_desc-bInterval - 1));
/* for HS */
div = 8000 / (1 (hs_epout_desc-bInterval - 1));
c_max_packet_size = uac2_opts-c_chmask * uac2_opts-c_ssize
* DIV_ROUND_UP(uac2_opts-c_srate, div);
Makes sense?
Thanks, it is correct. But looking the code at afunc_set_alt:
the method of calculating uac2-p_pktsize seems incorrect, it
may need to change like below:
@@ -1176,15 +1188,16 @@ afunc_set_alt(struct usb_function *fn, unsigned
intf, unsigned alt)
factor = 1000;
} else {
ep_desc = hs_epin_desc;
- factor = 125;
+ factor = 8000;
}
/* pre-compute some values for iso_complete() */
uac2-p_framesize = opts-p_ssize *
num_channels(opts-p_chmask);
rate = opts-p_srate * uac2-p_framesize;
- uac2-p_interval = (1 (ep_desc-bInterval - 1)) * factor;
- uac2-p_pktsize = min_t(unsigned int, rate /
uac2-p_interval,
+ uac2-p_interval = factor / (1 (ep_desc-bInterval -
1));
+ uac2-p_pktsize = min_t(unsigned int, DIV_ROUND_UP(rate,
+ uac2-p_interval),
prm-max_psize);
Your p_interval calculation is equivalent in both cases:
FS: 1 * 1000 == 1000 / 1
HS: 8 * 125 == 8000 / 8
And no, p_pktsize is intentionally set to the minimum packet size that a
packet will usually have. The actual size might be higher due to the
accumulated residue (see below).
Assume the interval for each packet is 2ms, the rate is 192 kbytes
for both FS HS:
uac2-p_interval = 2000;
uac2-p_pktsize = 192 kbytes / 2000 = 96 bytes
And the uac2-p_pktsize is real usb packet length, it means hardware
would expect there are 96 bytes per 2ms, but the real frame rate is 192k,
and it needs to 192 * 2 bytes per 2ms in the bus, am I missing
something?
Another think I am not understand is why playback uses IN endpoint?
Don't this playback mean the data is from host, and play at device side?
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V4 2/3] usb: Introduce Xen pvUSB frontend (xen hcd)
On 23/06/15 07:53, Juergen Gross wrote: Introduces the Xen pvUSB frontend. With pvUSB it is possible for a Xen domU to communicate with a USB device assigned to that domU. The communication is all done via the pvUSB backend in a driver domain (usually Dom0) which is owner of the physical device. The pvUSB frontend is a USB hcd for a virtual USB host connector. For the Xen-related parts: Reveiwed-by: David Vrabel david.vra...@citrix.com David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net 2/3] r8152: fix remote wakeup
Hi, this is most likely wrong. Usbcore does check for a device's ability to do remote wakeup and will block a runtime suspend if it detects that a remote wakeup would be required but the device cannot deliver. (static int autosuspend_check()) So by removing the flag in the probe() method means that devices will suspend during operations without remote wakeup requested. Thus an incoming packet cannot wake them up. If you remove setting the flag on probe() you need to set it at open() [and reset on close()], as devices which cannot do remote wakeup must only be suspended when they are down. Hi, I don't think I understand your description clearly. My idea is that if the device doesn't support wakeup, we don't need set needs_remote_wakeup. We allow the device could be suspended and couldn't be waked up by incoming packet. The system could be waked up by other methods except by the device. I don't understand why I have to set it at open() and reset it at close(). And why must the device only be suspended when it is down? Best Regards, Hayes
[PATCH 2/3] drivers: usb: dwc3: Add adjust_frame_length_quirk
Add adjust_frame_length_quirk for writing to fladj register
which adjusts (micro)frame length to value provided by
snps,configure-fladj property thus avoiding USB 2.0 devices
to time-out over a longer run
Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com
---
drivers/usb/dwc3/core.c | 12
drivers/usb/dwc3/core.h | 7 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5c110d8..72ba025 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -779,6 +779,7 @@ static int dwc3_probe(struct platform_device *pdev)
u8 lpm_nyet_threshold;
u8 tx_de_emphasis;
u8 hird_threshold;
+ u32 fladj_value;
int ret;
@@ -886,6 +887,12 @@ static int dwc3_probe(struct platform_device *pdev)
tx_de_emphasis);
of_property_read_string(node, snps,hsphy_interface,
dwc-hsphy_interface);
+ ret = of_property_read_u32(node, snps,configure-fladj,
+ fladj_value);
+ if (!ret)
+ dwc-adjust_frame_length_quirk = 1;
+ else
+ dwc-adjust_frame_length_quirk = 0;
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
dwc-has_lpm_erratum = pdata-has_lpm_erratum;
@@ -957,6 +964,11 @@ static int dwc3_probe(struct platform_device *pdev)
goto err1;
}
+ /* Adjust Frame Length */
+ if (dwc-adjust_frame_length_quirk)
+ dwc3_writel(dwc-regs, DWC3_GFLADJ, GFLADJ_30MHZ_REG_SEL |
+ GFLADJ_30MHZ(fladj_value));
+
if (IS_ENABLED(CONFIG_USB_DWC3_HOST))
dwc-dr_mode = USB_DR_MODE_HOST;
else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0447788..b7a5119 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -124,6 +124,7 @@
#define DWC3_GEVNTCOUNT(n) (0xc40c + (n * 0x10))
#define DWC3_GHWPARAMS80xc600
+#define DWC3_GFLADJ0xc630
/* Device Registers */
#define DWC3_DCFG 0xc700
@@ -234,6 +235,10 @@
/* Global HWPARAMS6 Register */
#define DWC3_GHWPARAMS6_EN_FPGA(1 7)
+/* Global Frame Length Adjustment Register */
+#define GFLADJ_30MHZ_REG_SEL (1 7)
+#define GFLADJ_30MHZ(n)((n) 0x3f)
+
/* Device Configuration Register */
#define DWC3_DCFG_DEVADDR(addr)((addr) 3)
#define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
@@ -712,6 +717,7 @@ struct dwc3_scratchpad_array {
* @rx_detect_poll_quirk: set if we enable rx_detect to polling lfps quirk
* @dis_u3_susphy_quirk: set if we disable usb3 suspend phy
* @dis_u2_susphy_quirk: set if we disable usb2 suspend phy
+ * @adjust_frame_length_quirk: enables post-silicon frame length adjustment
* @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
* @tx_de_emphasis: Tx de-emphasis value
* 0 - -6dB de-emphasis
@@ -841,6 +847,7 @@ struct dwc3 {
unsignedrx_detect_poll_quirk:1;
unsigneddis_u3_susphy_quirk:1;
unsigneddis_u2_susphy_quirk:1;
+ unsignedadjust_frame_length_quirk:1;
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth
On 07/23/2015 10:35 AM, Peter Chen wrote:
Assume the interval for each packet is 2ms, the rate is 192 kbytes
for both FS HS:
uac2-p_interval = 2000;
uac2-p_pktsize = 192 kbytes / 2000 = 96 bytes
And the uac2-p_pktsize is real usb packet length, it means hardware
would expect there are 96 bytes per 2ms, but the real frame rate is 192k,
and it needs to 192 * 2 bytes per 2ms in the bus, am I missing
something?
Hmm? The numerator in that division ('rate') includes the actual frame size:
rate = opts-p_srate * uac2-p_framesize;
uac2-p_pktsize = rate / uac2-p_interval;
Which means for 192KHz, stereo 16bit, rate is 768000. In this case,
bInterval = 4 doesn't work, as the packets would become 512 bytes, so
the bandwidth wouldn't suffice.
Another think I am not understand is why playback uses IN endpoint?
Don't this playback mean the data is from host, and play at device side?
That's a little confusing on first sight, but actually quite logical:
The ALSA capture stream is the one that allows applications on the
gadget to record audio, which is data that is sent *from* the host
(playback on their side) to the device, using OUT tokens.
The ALSA playback stream is the one that allows applications on the
gadget to playback audio, which is data that is sent *to* the host
(capture on their side) by the device, using IN tokens.
Thanks,
Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] USB: Fix persist resume of some USB 2.0 devices
Hi, This mail is for RFC regarding the persist resume of some USB 2.0 devices, Problem Summary: Problem has been observed for some USB 2.0 devices while resuming from sleep. When the USB “persist” feature is enabled through sysfs it is expected to retain its previous mount point across sleep and resume states, it works fine for most of the USB 2.0 mass storage devices, but for some USB 2.0 pendrives such as (Transcend Jet flash 16 GB Alcor micro corporation) it seems to fail and re-enumeration happens resulting in creation of new device data structure. This problem seems to be similar to the problem faced with USB 3.0 devices which is mentioned in below link http://marc.info/?l=linux-usbm=140566728011240w=2 In the above mentioned link the problem is observed with USB 3.0 devices, but in this case it is observed with USB 2.0 mass storage devices.While resuming from sleep, a USB disconnect message and re enumeration messages are seen resulting in the reset of “persist ” variable in sysfs. In the above mentioned link a fix is proposed and that patch was merged in mainline kernel, but the patch restricts the timeout only to USB 3.0 devices. I am not sure why it was restricted to USB 3.0 devices, but the same issue seems to appear for USB 2.0 devices too. The below patch removes the restriction for USB 3.0 devices and makes the time out applicable to all USB devices. Signed-off-by: vasudevan,krishna prasath krishna.prasathx.k.vasude...@intel.com --- drivers/usb/core/hub.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d2bd9d7..b2b709d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3320,7 +3320,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) clear_bit(port1, hub-busy_bits); - if (udev-persist_enabled hub_is_superspeed(hub-hdev)) + if (udev-persist_enabled) status = wait_for_ss_port_enable(udev, hub, port1, portchange, portstatus); -- 1.7.10.4 RESULT: This patch has been tested with USB 2.0 devices(Transcend Alcor micro corp.) that were facing this issue and after applying this patch, both the USB 2.0 devices seems to work fine without any issues for all tested sleep-resume cycles . Thanks Regards, Krishna -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Several races in usbnet module (kernel 4.1.x)
On Wed, 2015-07-22 at 21:33 +0300, Eugene Shatokhin wrote: The following part is not necessary, I think. usbnet_bh() does not touch EVENT_NO_RUNTIME_PM bit explicitly and these bit operations are atomic w.r.t. each other. + mpn |= !test_and_clear_bit(EVENT_NO_RUNTIME_PM, dev-flags); + /* in case the bh reset a flag */ Yes, they are atomic w.r.t. each other. And that limitation worries me. I am considering architectures which do atomic operations with spinlocks. And this code mixes another operation into it. Can this happen? CPU A CPU B take lock read old value set value to 0 clear bit write back changed value release lock Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [Patch V4 1/3] usb: Add Xen pvUSB protocol description
On 23/07/15 07:46, Juergen Gross wrote: On 07/23/2015 06:36 AM, Greg KH wrote: On Thu, Jul 23, 2015 at 06:04:39AM +0200, Juergen Gross wrote: On 07/23/2015 01:46 AM, Greg KH wrote: On Tue, Jun 23, 2015 at 08:53:23AM +0200, Juergen Gross wrote: Add the definition of pvUSB protocol used between the pvUSB frontend in a Xen domU and the pvUSB backend in a Xen driver domain (usually Dom0). This header was originally provided by Fujitsu for Xen based on Linux 2.6.18. Changes are: - adapt to Linux style guide Signed-off-by: Juergen Gross jgr...@suse.com --- include/xen/interface/io/usbif.h | 252 +++ Why is this a different interface than the existing ones we have today (i.e. usbip?) Where is it documented? Do the Xen developers / The interface definition is living in the Xen git repository for several years now: git://xenbits.xen.org/xen.git - xen/include/public/io/usbif.h That's header file, not a document describing the api here. This is the format that Xen uses to document the interface to frontend drivers. This is also the style used for netfront, blkfront etc. The documentation in the header could be expanded if necessary but... I suppose you want to tell me I should add something like: Documentation/DocBook/usb/API-struct-urb.html ... we certainly don't want a second location for the interface description in the Linux source. David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [Patch V4 1/3] usb: Add Xen pvUSB protocol description
On 23/06/15 07:53, Juergen Gross wrote: Add the definition of pvUSB protocol used between the pvUSB frontend in a Xen domU and the pvUSB backend in a Xen driver domain (usually Dom0). This header was originally provided by Fujitsu for Xen based on Linux 2.6.18. Changes are: - adapt to Linux style guide Acked-by: David Vrabel david.vra...@citrix.com David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Several races in usbnet module (kernel 4.1.x)
On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote: [Race #5] Race on dev-rx_urb_size. I reproduced it a similar way as the races #2 and #3 (changing MTU while downloading files). dev-rx_urb_size is written to here: #0 usbnet_change_mtu (usbnet.c:392) dev-rx_urb_size = dev-hard_mtu; Here is the conflicting read from dev-rx_urb_size: * rx_submit (usbnet.c:467) size_t size = dev-rx_urb_size; Yes, but what is the actual bad race? I mean, if you change the MTU in operation, there will be a race. That is just unavoidable. Do we generate errors? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 2/3] r8152: fix remote wakeup
On Thu, 2015-07-23 at 15:09 +0800, Hayes Wang wrote: Set needs_remote_wakeup only when the device supports it. Signed-off-by: Hayes Wang hayesw...@realtek.com --- drivers/net/usb/r8152.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index e3a0110..eff1f25 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -4059,7 +4059,8 @@ static int rtl8152_probe(struct usb_interface *intf, break; } - intf-needs_remote_wakeup = 1; + if (udev-actconfig-desc.bmAttributes USB_CONFIG_ATT_WAKEUP) + intf-needs_remote_wakeup = 1; Hi, this is most likely wrong. Usbcore does check for a device's ability to do remote wakeup and will block a runtime suspend if it detects that a remote wakeup would be required but the device cannot deliver. (static int autosuspend_check()) So by removing the flag in the probe() method means that devices will suspend during operations without remote wakeup requested. Thus an incoming packet cannot wake them up. If you remove setting the flag on probe() you need to set it at open() [and reset on close()], as devices which cannot do remote wakeup must only be suspended when they are down. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 2/3] r8152: fix remote wakeup
On Thu, 2015-07-23 at 09:55 +, Hayes Wang wrote:
Hi,
this is most likely wrong. Usbcore does check for a device's ability to
do remote wakeup and will block a runtime suspend if it detects that
a remote wakeup would be required but the device cannot deliver.
(static int autosuspend_check())
So by removing the flag in the probe() method means that devices will
suspend during operations without remote wakeup requested. Thus an
incoming packet cannot wake them up.
If you remove setting the flag on probe() you need to set it at open()
[and reset on close()], as devices which cannot do remote wakeup must
only be suspended when they are down.
Hi,
I don't think I understand your description clearly. My idea is that if the
Sorry, I will try to be clearer.
device doesn't support wakeup, we don't need set needs_remote_wakeup.
The algorithm for power saving needs remote wakeup.
We allow the device could be suspended and couldn't be waked up by
incoming packet. The system could be waked up by other methods except
by the device.
The system is not the problem. I was talking about device level power
management at runtime.
I don't understand why I have to set it at open() and reset it at close(). And
why must the device only be suspended when it is down?
OK,
the driver right now requests that runtime power management be done:
static struct usb_driver rtl8152_driver = {
.name = MODULENAME,
.id_table = rtl8152_table,
.probe =rtl8152_probe,
.disconnect = rtl8152_disconnect,
.suspend = rtl8152_suspend,
.resume = rtl8152_resume,
.reset_resume = rtl8152_resume,
.supports_autosuspend = 1,
^ the crucial flag
.disable_hub_initiated_lpm = 1,
};
It implements an advanced form of runtime power management which
requires remote wakeup.
For the device to work correctly it must not be suspended while
a - a setting is changed
b - a packet is transmitted
c - a packet is received
Cases a and b are covered by the driver on its own.
Case a see for example rtl8152_set_mac_address()
Case b is handled in rtl8152_start_xmit()
if (test_bit(SELECTIVE_SUSPEND, tp-flags)) {
set_bit(SCHEDULE_NAPI, tp-flags);
schedule_delayed_work(tp-schedule, 0);
} else {
usb_mark_last_busy(tp-udev);
The scheduled work will resume the device if necessary.
Case c is handled in rtl8152_resume()
if (netif_running(tp-netdev)) {
if (test_bit(SELECTIVE_SUSPEND, tp-flags)) {
rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, tp-flags);
set_bit(WORK_ENABLE, tp-flags);
if (netif_carrier_ok(tp-netdev))
rtl_start_rx(tp);
But that code path only runs if remote wakeup is enabled.
So to operate with runtime power management is is necessary
that the device support remote wakeup and the driver enable it.
If the device does not support remote wakeup and the driver
enables it, runtime power management will be switched off.
That is the current state and it means that devices which
don't support remote wakeup cannot do runtime power management
at all. But the driver is correct.
The only time a device that doesn't support remote wakeup can
do runtime power managent is when no packets can be received
that is while the interface is down. If you want to allow that
you must not set needs_remote_wakeup in probe(), but you must
set it in open() because it is necessary for runtime power
management as I explained above.
Sorry for the length of this mail, but I wanted to make sure
I am absolutely clear this time.
HTH
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: sierra: add 1199:68AB device ID
Hi Johan,
On 20.07.2015 11:24, Johan Hovold wrote:
[ +CC: Bjørn and Dan ]
On Mon, Jul 20, 2015 at 08:14:19AM +0200, Dirk Behme wrote:
Add support for the Sierra Wireless AR8550 device with
USB descriptor 0x1199, 0x68AB. For this, lsusb reports:
Thanks for the patch. This modem business is a bit of a mess and it's
not always apparent what driver a device id belongs to.
Bus 001 Device 004: ID 1199:68ab Sierra Wireless, Inc.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x1199 Sierra Wireless, Inc.
idProduct 0x68ab
bcdDevice0.06
iManufacturer 3 Sierra Wireless, Incorporated
iProduct2 AR8550
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 198
bNumInterfaces 7
[...]
Signed-off-by: Dirk Behme dirk.be...@de.bosch.com
---
Changes in v2: Improve the commit message.
drivers/usb/serial/sierra.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 46179a0..4122e4f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -289,6 +289,9 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x1199, 0x68AA, 0xFF, 0xFF, 0xFF),
.driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
},
+ { USB_DEVICE(0x1199, 0x68AB), /* Sierra Wireless Direct IP modems */
+ .driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
+ },
This device has seven interfaces (0..6) so why do you use the direct-ip
interface blacklisting, which only blacklists interfaces = 7?
Using it the same way like the quite similar ID 0x1199, 0x68AA above
'just works for us'. Not being the experts on this, do you like to
propose anything else you like us to try?
Also do you notice any delays when connecting the device (which could
indicate that sierra is not the right driver)?
As above, do you like us to try anything else?
Best regards
Dirk
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: sierra: add 1199:68AB device ID
On Thu, Jul 23, 2015 at 12:31:55PM +0200, Dirk Behme wrote:
Hi Johan,
On 20.07.2015 11:24, Johan Hovold wrote:
[ +CC: Bjørn and Dan ]
On Mon, Jul 20, 2015 at 08:14:19AM +0200, Dirk Behme wrote:
Add support for the Sierra Wireless AR8550 device with
USB descriptor 0x1199, 0x68AB. For this, lsusb reports:
Thanks for the patch. This modem business is a bit of a mess and it's
not always apparent what driver a device id belongs to.
Bus 001 Device 004: ID 1199:68ab Sierra Wireless, Inc.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x1199 Sierra Wireless, Inc.
idProduct 0x68ab
bcdDevice0.06
iManufacturer 3 Sierra Wireless, Incorporated
iProduct2 AR8550
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 198
bNumInterfaces 7
[...]
Signed-off-by: Dirk Behme dirk.be...@de.bosch.com
---
Changes in v2: Improve the commit message.
drivers/usb/serial/sierra.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 46179a0..4122e4f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -289,6 +289,9 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x1199, 0x68AA, 0xFF, 0xFF, 0xFF),
.driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
},
+ { USB_DEVICE(0x1199, 0x68AB), /* Sierra Wireless Direct IP modems */
+.driver_info = (kernel_ulong_t)direct_ip_interface_blacklist
+ },
This device has seven interfaces (0..6) so why do you use the direct-ip
interface blacklisting, which only blacklists interfaces = 7?
Using it the same way like the quite similar ID 0x1199, 0x68AA above
'just works for us'. Not being the experts on this, do you like to
propose anything else you like us to try?
Also do you notice any delays when connecting the device (which could
indicate that sierra is not the right driver)?
As above, do you like us to try anything else?
Could you post a log from when connecting the device with you patch
applied?
Thanks,
Johan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 3/4] USB: io_ti: Add firmware image sanity checks
Hello.
On 7/22/2015 9:56 PM, Peter E. Berger wrote:
From: Peter E. Berger pber...@brimson.com
Do what we can to verify that the driver's firmware image is valid
(before attempting to download it to the Edgeport) by adding a new
function, check_fw_sanity(), and a call to it in in download_fw().
Note: It looks like some Edgeports (models like the EP/416 with on-board
E2PROM) may be able to function even if the on-disk firmware image is
bad or missing, iff their local E2PROM versions are valid. But most
Edgeport models (I've tried EP/1 and EP/8) do not appear to have this
capability and they always rely on the on-disk firmware image.
I tested an implementation that calls the new check_fw_sanity()
function at the top of download_fw() and, rather than simply returning
an error if the firmware image is bad or missing, it saves the result
and defers the decision until later when it may find that it is running
on a E2PROM-equipped device with a valid image. But I think this is
messier than it is worth (adding still more messiness to the already
very messy download_fw()) for such a marginal possible benefit. So, at
least for now, I have chosen the much simpler approach of returning an
error whenever edge_startup() fails to load an on-disk firmware image, or
check_fw_sanity() indicates that it is unusable.
Signed-off-by: Peter E. Berger pber...@brimson.com
---
drivers/usb/serial/io_ti.c | 40
1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 6cff12c..a73d242 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -928,6 +928,41 @@ static int ti_cpu_rev(struct edge_ti_manuf_descriptor
*desc)
return TI_GET_CPU_REVISION(desc-CpuRev_BoardRev);
}
+static int check_fw_sanity(struct edgeport_serial *serial,
+ const struct firmware *fw)
+{
+ u16 length_total;
+ int checksum = 0;
+ int pos;
+ struct device *dev = serial-serial-interface-dev;
+ struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw-data;
+
+ if (fw-size sizeof(struct edgeport_fw_hdr)) {
+ dev_err(dev, Incomplete fw header\n);
+ return -EINVAL;
+ }
+
+ length_total = le16_to_cpu(fw_hdr-length) +
+ sizeof(struct edgeport_fw_hdr);
+
+ if (fw-size != length_total) {
+ dev_err(dev, Bad fw size (Expected: %u, Got: %zu)\n,
I would not capitalize the latter 2 words.
+ length_total, fw-size);
+ return -EINVAL;
+ }
+
+ for (pos = sizeof(struct edgeport_fw_hdr); pos fw-size; ++pos)
+ checksum = (checksum + fw-data[pos]) 0xFF;
Why not make 'checksum' 's8' or 'u8' instead of *int*?
+
+ if (checksum != fw_hdr-checksum) {
+ dev_err(dev, Bad fw checksum (Expected: 0x%x, Got: 0x%x)\n,
+ fw_hdr-checksum, checksum);
I would not capitalize the latter 2 words.
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
