Re: [PATCH v1] usb: dwc3: gadget: sanity check for usb request complete function in ep_enqueue and giveback function.

[Felipe Balbi]

hi,

Tang Jianqiang  writes:
> From: Jianqiang Tang 
>
> Do sanity check for usb request complete function as we hit random
> null pointer kernel panic in giveback function.
>
> From the call trace, show the complete function should be null.
> So we add the sanity check before every usb request queue to dwc3
> also before dwc3 giveback the usb request.
>
> Logs:
> BUG: unable to handle kernel NULL pointer dereference at   (null)
> IP: [<  (null)>]   (null)
> Call Trace:
>   ? dwc3_gadget_giveback+0xa5/0x130
>   ? vsnprintf+0x166/0x3d0
>   dwc3_remove_requests+0x57/0x70
>   __dwc3_gadget_ep_disable+0x18/0x80
>   dwc3_gadget_ep_disable+0x79/0x1a0
>   linkwatch_fire_event+0x4c/0x90
>   gether_disconnect+0x45/0x1b0
>   ? wake_up_klogd+0x49/0x70
>   console_unlock+0x295/0x4c0
>   rndis_disable+0x3d/0x90
>   preempt_count_add+0x55/0xa0
>   reset_config+0x3b/0x90
>   _raw_spin_lock_irqsave+0x25/0x30
>   composite_disconnect+0x2f/0x50
>   dwc3_gadget_disconnect_interrupt+0x62/0x90
>
> Signed-off-by: Jianqiang Tang 

well, this is a gadget driver bug, not a dwc3 bug. This gadget driver
deserves to oops so we fix it. Care to provide information on how to
reproduce this ? Which kernel did you use ? Which platform ? Are you
using a vanilla kernel from Linus ?

cheers

-- 
balbi


signature.asc
Description: PGP signature

Re: Freezable workqueue blocks non-freezable workqueue during the system resume process

[Peter Chen]

On Thu, Feb 25, 2016 at 05:01:12PM -0500, Tejun Heo wrote:
> Hello, Peter.
> 
> On Wed, Feb 24, 2016 at 03:24:30PM +0800, Peter Chen wrote:
> > > You might want to complain to the block-layer people about this.  I 
> > > don't know if anything can be done to fix it.
> > > 
> > > Or maybe flush_work and flush_delayed_work can be changed to avoid 
> > > blocking if the workqueue is frozen.  Tejun?
> > > 
> > 
> > I have a patch to show the root cause of this issue.
> > 
> > http://www.spinics.net/lists/linux-usb/msg136815.html
> 
> I don't get it.  Why would it deadlock?  Shouldn't things get rolling
> once the workqueues are thawed?
> 

Hi Tejun,

The workqueue writeback can't be thawed due to driver's resume
(dpm_complete) is lock nested, and can't be finished.

-- 

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 RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Peter Chen]

On Thu, Feb 25, 2016 at 02:08:44PM -0300, Fabio Estevam wrote:
> On Wed, Feb 24, 2016 at 10:20 PM, Peter Chen  wrote:
> 
> > Would you get any chances to test i.mx28 evk with an external HUB?
> > I just would like to know it is not a HUB issue which can't be
> > suspended.
> 
> Just connected an external USB hub on a mx28evk and did not see any issue.

So, it may be USB HUB's problem, it is so strange.

Would you please have a test just toggle portsc.suspendM at issued
board? Surely, you need to add bootargs to let it work beforehand.

The address for portsc is $USB_BASE + 0x184, suspendM is bit 7.

-- 

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 2/5 v7] usb: musb: core: added helper functions for parsing DT

[Bin Liu]

Hi,

On Wed, Feb 24, 2016 at 04:27:15PM +0100, Petr Kulhavy wrote:
> This adds two functions to get DT properties "mentor,power" and "dr_mode":
> musb_get_power() and musb_get_mode()

Sorry for my late comments, but I don't see the value of adding
musb_get_power() into musb core. It is just a wraper, I don't think
other glues will use it.

Regards,
-Bin.

> 
> Signed-off-by: Petr Kulhavy 
> ---
> v4: 
>  - created musb_get_dr_mode()
> 
> v5:
>  - musb_get_dr_mode() renamed to musb_get_mode()
>  - added musb_get_power()
> 
> v6:
>  - musb_get_power() : added missing boundary check for the maximum value 510mA
>  - formatting
>  - added empty implementation of musb_get_power()
> 
> v7:
>  - removed empty implementation of musb_get_power()
>  - musb_get_mode() returns MUSB_OTG if the property is not found
> 
>  drivers/usb/musb/musb_core.c | 37 +
>  drivers/usb/musb/musb_core.h | 15 +++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index c3791a0..ff7f5d0 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -100,6 +100,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "musb_core.h"
> 
> @@ -129,6 +130,42 @@ static inline struct musb *dev_to_musb(struct device 
> *dev)
>   return dev_get_drvdata(dev);
>  }
> 
> +enum musb_mode musb_get_mode(struct device *dev)
> +{
> + enum usb_dr_mode mode;
> +
> + mode = usb_get_dr_mode(dev);
> + switch (mode) {
> + case USB_DR_MODE_HOST:
> + return MUSB_HOST;
> + case USB_DR_MODE_PERIPHERAL:
> + return MUSB_PERIPHERAL;
> + case USB_DR_MODE_OTG:
> + case USB_DR_MODE_UNKNOWN:
> + default:
> + return MUSB_OTG;
> + }
> +}
> +EXPORT_SYMBOL_GPL(musb_get_mode);
> +
> +#ifdef CONFIG_OF
> +u8 musb_get_power(struct device *dev)
> +{
> + int ret;
> + unsigned power_ma;
> +
> + /* the "mentor,power" value is in milliamperes, whereas
> +  * the mentor configuration is in 2mA units
> +  */
> + ret = of_property_read_u32(dev->of_node, "mentor,power", _ma);
> + if (ret)
> + return 0;
> +
> + return power_ma > 510 ? 255 : power_ma / 2;
> +}
> +EXPORT_SYMBOL_GPL(musb_get_power);
> +#endif
> +
>  /*-*/
> 
>  #ifndef CONFIG_BLACKFIN
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index fd215fb..c2ee702 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -614,4 +614,19 @@ static inline void 
> musb_platform_post_root_reset_end(struct musb *musb)
>   musb->ops->post_root_reset_end(musb);
>  }
> 
> +/* gets the "dr_mode" property from DT and converts it into musb_mode
> + * if the property is not found or not recognized returns MUSB_OTG
> + */
> +extern enum musb_mode musb_get_mode(struct device *dev);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +/* gets the "mentor,power" property from DT
> + * and converts it from mA to 2mA units for the "power" parameter
> + * in struct musb_hdrc_platform_data
> + *
> + * in case the property is not found returns 0
> + */
> +extern u8 musb_get_power(struct device *dev);
> +#endif
> +
>  #endif   /* __MUSB_CORE_H__ */
> --
> 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
--
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/5 v7] usb: musb: core: added helper functions for parsing DT

[Bin Liu]

Hi,

On Thu, Feb 25, 2016 at 01:04:13PM +0100, Petr Kulhavy wrote:
> On 25.02.2016 12:53, Sergei Shtylyov wrote:
> >
> >>>   I then fail to understand the logic behind hard coding.
> >>
> >>And I fail to understand your plans with the other drivers.
> >>Are you trying to say that the "mentor,power" (or "power" in OMAP2430)
> >>property should be completely abandoned and the values hard
> >>coded in _all_
> >>drivers?
> >
> >   Contrariwise, I don't think it should be hard-coded anywhere.
> 
> Well, so we're still at the same point - there is a fundamental
> mismatch in the different developers' view how the "power" parameter
> should be represented.
> There already 3 opinions at the moment:
> 1) hard code - Felipe, Rob
> 2) use the "mentor,power" - Sergei, Petr
> 3) use a regulator - Rob
> 
> So unless this conflict is resolved it is slightly difficult to
> submit a patch that would get accepted.
> How can we resolve this conflict ?

This power property is used by core to control the hub port power
budget, which is sourced by vbus. But vbus is not coming from musb, but
a board power rail. So hardcode it does not make sense.

Regards,
-Bin.

> 
> Regards
> Petr
> --
> 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
--
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 v1] usb: dwc3: gadget: sanity check for usb request complete function in ep_enqueue and giveback function.

[Tang Jianqiang]

From: Jianqiang Tang 

Do sanity check for usb request complete function as we hit random
null pointer kernel panic in giveback function.

>From the call trace, show the complete function should be null.
So we add the sanity check before every usb request queue to dwc3
also before dwc3 giveback the usb request.

Logs:
BUG: unable to handle kernel NULL pointer dereference at   (null)
IP: [<  (null)>]   (null)
Call Trace:
? dwc3_gadget_giveback+0xa5/0x130
? vsnprintf+0x166/0x3d0
dwc3_remove_requests+0x57/0x70
__dwc3_gadget_ep_disable+0x18/0x80
dwc3_gadget_ep_disable+0x79/0x1a0
linkwatch_fire_event+0x4c/0x90
gether_disconnect+0x45/0x1b0
? wake_up_klogd+0x49/0x70
console_unlock+0x295/0x4c0
rndis_disable+0x3d/0x90
preempt_count_add+0x55/0xa0
reset_config+0x3b/0x90
_raw_spin_lock_irqsave+0x25/0x30
composite_disconnect+0x2f/0x50
dwc3_gadget_disconnect_interrupt+0x62/0x90

Signed-off-by: Jianqiang Tang 
---
 drivers/usb/dwc3/gadget.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2363bad..3c1ac95 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,8 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct 
dwc3_request *req,
trace_dwc3_gadget_giveback(req);
 
spin_unlock(>lock);
-   usb_gadget_giveback_request(>endpoint, >request);
+   if (req->request.complete)
+   usb_gadget_giveback_request(>endpoint, >request);
spin_lock(>lock);
 }
 
@@ -1233,6 +1234,16 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, 
struct usb_request *request,
int ret;
 
spin_lock_irqsave(>lock, flags);
+
+   if (WARN(!request->complete, "request %p complete function is NULL\n",
+   request)) {
+   dwc3_trace(trace_dwc3_gadget,
+   "request %p complete function is NULL\n",
+   request);
+   spin_unlock_irqrestore(>lock, flags);
+   return -EINVAL;
+   }
+
ret = __dwc3_gadget_ep_queue(dep, req);
 
/*
-- 
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: System hangs with weak USB cable

[Radosław Warowny]

> I agree, but where was a crash?  I don't have any context here, was
> there a kernel oops somewhere?  I thought the hub just reset itself.  I
> have a hub like that here, it resets itself a few times a day, but
> there's no kernel crash because of it.

Yes, there were kernel oopses, you can find details at syslog output
attached to the bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=112311. I don't cite it
here since it's too large. I provided two versions of logs - with and
without  apparmor to exclude it as a cause of problem since there is
some apparmor call at the first call trace.

Regards,
Radek
--
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: Freezable workqueue blocks non-freezable workqueue during the system resume process

[Tejun Heo]

Hello, Peter.

On Wed, Feb 24, 2016 at 03:24:30PM +0800, Peter Chen wrote:
> > You might want to complain to the block-layer people about this.  I 
> > don't know if anything can be done to fix it.
> > 
> > Or maybe flush_work and flush_delayed_work can be changed to avoid 
> > blocking if the workqueue is frozen.  Tejun?
> > 
> 
> I have a patch to show the root cause of this issue.
> 
> http://www.spinics.net/lists/linux-usb/msg136815.html

I don't get it.  Why would it deadlock?  Shouldn't things get rolling
once the workqueues are thawed?

Thanks.

-- 
tejun
--
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

asynchronous IO for chaoskey

[Oliver Neukum]

Hi,

could you test this? It ought to give you more
throughput through asynchronousity.

Regards
Oliver

From b32c2f840c1ff0d0e93b627eb07b2fdc11f37bb5 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 25 Feb 2016 12:20:12 +0100
Subject: [PATCH 1/4] chaoskey: O_NONBLOCK in concurrent reads

This changes the locking in chaoskey_read() to correctly
handle O_NONBLOCK in the case of concurrent readers.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/misc/chaoskey.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index 76350e4..f7e76a3 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -438,9 +438,19 @@ static ssize_t chaoskey_read(struct file *file,
 			goto bail;
 		mutex_unlock(>rng_lock);
 
-		result = mutex_lock_interruptible(>lock);
-		if (result)
-			goto bail;
+		if (file->f_flags & O_NONBLOCK) {
+			result = mutex_trylock(>lock);
+			if (result == 0) {
+result = -EAGAIN;
+goto bail;
+			} else {
+result = 0;
+			}
+		} else {
+			result = mutex_lock_interruptible(>lock);
+			if (result)
+goto bail;
+		}
 		if (dev->valid == dev->used) {
 			result = _chaoskey_fill(dev);
 			if (result < 0) {
-- 
2.1.4

From 8d6c9c8ce3adf192a2fe19fa247208d91ee5e900 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 25 Feb 2016 14:20:22 +0100
Subject: [PATCH 2/4] chaoskey: introduce asynchronous reads

This divides requesting IO and waiting for IO from each other.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/misc/chaoskey.c | 37 ++---
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index f7e76a3..f47e5a7 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -351,14 +351,12 @@ static void chaos_read_callback(struct urb *urb)
 	wake_up(>wait_q);
 }
 
-/* Fill the buffer. Called with dev->lock held
- */
-static int _chaoskey_fill(struct chaoskey *dev)
+static int chaoskey_request_fill(struct chaoskey *dev)
 {
 	DEFINE_WAIT(wait);
 	int result;
 
-	usb_dbg(dev->interface, "fill");
+	usb_dbg(dev->interface, "request fill");
 
 	/* Return immediately if someone called before the buffer was
 	 * empty */
@@ -389,6 +387,22 @@ static int _chaoskey_fill(struct chaoskey *dev)
 		goto out;
 	}
 
+	/*
+	 * powering down while a read is under way
+	 * is blocked in suspend()
+	 */
+	usb_autopm_put_interface(dev->interface);
+	return 0;
+out:
+	usb_autopm_put_interface(dev->interface);
+	return result;
+}
+
+static int chaoskey_wait_fill(struct chaoskey *dev)
+{
+	DEFINE_WAIT(wait);
+	int result;
+
 	result = wait_event_interruptible_timeout(
 		dev->wait_q,
 		!dev->reading,
@@ -403,7 +417,6 @@ static int _chaoskey_fill(struct chaoskey *dev)
 		result = dev->valid;
 out:
 	/* Let the device go back to sleep eventually */
-	usb_autopm_put_interface(dev->interface);
 
 	usb_dbg(dev->interface, "read %d bytes", dev->valid);
 
@@ -452,7 +465,12 @@ static ssize_t chaoskey_read(struct file *file,
 goto bail;
 		}
 		if (dev->valid == dev->used) {
-			result = _chaoskey_fill(dev);
+			result = chaoskey_request_fill(dev);
+			if (result < 0) {
+mutex_unlock(>lock);
+goto bail;
+			}
+			result = chaoskey_wait_fill(dev);
 			if (result < 0) {
 mutex_unlock(>lock);
 goto bail;
@@ -520,7 +538,7 @@ static int chaoskey_rng_read(struct hwrng *rng, void *data,
 	 * the buffer will still be empty
 	 */
 	if (dev->valid == dev->used)
-		(void) _chaoskey_fill(dev);
+		(void) chaoskey_request_fill(dev);
 
 	this_time = dev->valid - dev->used;
 	if (this_time > max)
@@ -540,6 +558,11 @@ static int chaoskey_rng_read(struct hwrng *rng, void *data,
 static int chaoskey_suspend(struct usb_interface *interface,
 			pm_message_t message)
 {
+	struct chaoskey *dev = usb_get_intfdata(interface);
+
+	if (dev->reading && PMSG_IS_AUTO(message))
+		return -EBUSY;
+
 	usb_dbg(interface, "suspend");
 	return 0;
 }
-- 
2.1.4

From 877a5c63bf1b8b379971016f9537063557dda81a Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 25 Feb 2016 14:42:04 +0100
Subject: [PATCH 3/4] chaoskey: make read() obey O_NONBLOCK

This skips waiting for a read if O_NONBLOCK is set.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/misc/chaoskey.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index f47e5a7..19dfbb7 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -470,10 +470,14 @@ static ssize_t chaoskey_read(struct file *file,
 mutex_unlock(>lock);
 goto bail;
 			}
-			result = chaoskey_wait_fill(dev);
-			if (result < 0) {
-mutex_unlock(>lock);
-goto bail;
+			if (!(file->f_flags & O_NONBLOCK)) {
+result = 

Re: [PATCH] Support HP lt4114 LTE Module (Huawei ME206V-561)

[Dan Williams]

On Thu, 2016-02-25 at 18:15 +0100, Bjørn Mork wrote:
> Dan Williams  writes:
> > On Mon, 2016-02-22 at 18:03 +0700, Lars Melin wrote:
> > > On 2016-02-21 10:09, Daniel Johnson wrote:
> > > > On Fri, Feb 19, 2016 at 12:27 AM, Bjørn Mork 
> > > > wrote:
> > > > > One of them is likely a QCDM port if this is really a
> > > > > Qualcomm
> > > > > based
> > > > > device.  The other might be an inactive NMEA port.  Serial
> > > > > doesn't
> > > > > necessarily imply AT commands...
> > > > 
> > > > I found the FCC ID for the device.
> > > > QISME206V-561
> > > > 
> > > > Searching for this at the FCC website revealed internal photos.
> > > > Looks
> > > > like it's Intel based. The significant non intel parts seem to
> > > > be
> > > > DRAM, and a radio amplifier.
> > > > https://www.fcc.gov/general/fcc-id-search-page
> > > > --
> > > > 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.h
> > > > tml
> > > > 
> > > 
> > > The serial interfaces should be handled by the option serial
> > > driver 
> > > which btw already has support for it.
> > > 
> > > qcserial will fight with option for ownership of the interfaces
> > > so
> > > the 
> > > currently defined DEVICE_HWI(0x03f0, 0x581d) should be taken out
> > > of
> > > qcserial. Bjorn?
> > 
> > If it's an Intel device, which it certainly looks like from the FCC
> > photos, then it should not be in qcserial at all.  The Windows
> > driver
> > config and the usb-devices dumps indicate that instead of the
> > normal
> > Intel cdc-acm/mbim configuration that this device uses a Huawei-
> > specific layout like their Qualcomm-based devices.  So this device
> > should probably go into 'option' for Cfg#1 (Jungo) and Cfg#2
> > (cdc_ether) and it'll certainly need locking on specific
> > Class/SubClass/Protocol to ensure that option only claims the
> > serial
> > interfaces and not the ethernet/ncm ones.
> 
> Agreed.  The DEVICE_HWI stuff in qcserial got a bit messy.  There are
> either too many or too few high speed serial drivers based on
> usb_wwan
> :)
> 
> But I guess the rule should be:  Does it speak QCDM on any of the
> serial
> ports, then it goes in qcserial.  Anything else goes in option.  Does
> that match the original qcserial intention?

No.  There are plenty of things that speak QCDM that are driven by
'option' or 'qcaux'.  The original intention of qcserial was to
facilitate the firmware download with gobi_loader that is more or less
obsolete these days, since the Gobi devices usually carry their own
firmware onboard.  But G1K, G2K, and G3K devices still require the
loader.

If devices don't need the loader, or their firmware can be upgraded
through that special QMI mode then great, they don't need qcserial.  Or
if we figure out how to support whatever special magic snowflake-ness
qcserial does in option, great.

> Regarding the entries in option: It is unfortunate that HP (and other
> laptop vendors) insist on using their own vendor IDs for vendor
> specifc
> OEM products like these mdoem. The Huawei vendor matching in option
> works very well, but when the modules are programmed with a HP ID,
> then
> we are back to adding entries per device ID.  Which is a game we
> cannot
> win wrt rarely seen laptop accessories.  How many Linux users will
> care
> about the average HP LTE modem?  My bet is less than one.
> 
> What do you think are the chances that HP will be stupid enough to
> ever
> create a USB device with a ff/05/xx interface which is *not* a Huawei
> modem?  How about just duplicating every Huawei ff/05/xx entry in
> option
> with the HP vendor ID?  Seems less error prone than the alternatives
> to
> me.

That makes me both concerned (because I think the chance of this is
non-zero) and sad (for the same reason).  For now I think we should
just add the few specific class/subclass/protocol lines for this modem
for the specific HP USB IDs.  I'm not sure there's any way around this.

Dan
--
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

我的交友主页是

[我的交友主页是]

你的老朋友邀你来Q群:343257759

Re: [PATCH] Support HP lt4114 LTE Module (Huawei ME206V-561)

[Bjørn Mork]

Dan Williams  writes:
> On Mon, 2016-02-22 at 18:03 +0700, Lars Melin wrote:
>> On 2016-02-21 10:09, Daniel Johnson wrote:
>> > On Fri, Feb 19, 2016 at 12:27 AM, Bjørn Mork  wrote:
>> > > One of them is likely a QCDM port if this is really a Qualcomm
>> > > based
>> > > device.  The other might be an inactive NMEA port.  Serial
>> > > doesn't
>> > > necessarily imply AT commands...
>> > 
>> > I found the FCC ID for the device.
>> > QISME206V-561
>> > 
>> > Searching for this at the FCC website revealed internal photos.
>> > Looks
>> > like it's Intel based. The significant non intel parts seem to be
>> > DRAM, and a radio amplifier.
>> > https://www.fcc.gov/general/fcc-id-search-page
>> > --
>> > 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
>> > 
>> 
>> The serial interfaces should be handled by the option serial driver 
>> which btw already has support for it.
>> 
>> qcserial will fight with option for ownership of the interfaces so
>> the 
>> currently defined DEVICE_HWI(0x03f0, 0x581d) should be taken out of
>> qcserial. Bjorn?
>
> If it's an Intel device, which it certainly looks like from the FCC
> photos, then it should not be in qcserial at all.  The Windows driver
> config and the usb-devices dumps indicate that instead of the normal
> Intel cdc-acm/mbim configuration that this device uses a Huawei-
> specific layout like their Qualcomm-based devices.  So this device
> should probably go into 'option' for Cfg#1 (Jungo) and Cfg#2
> (cdc_ether) and it'll certainly need locking on specific
> Class/SubClass/Protocol to ensure that option only claims the serial
> interfaces and not the ethernet/ncm ones.

Agreed.  The DEVICE_HWI stuff in qcserial got a bit messy.  There are
either too many or too few high speed serial drivers based on usb_wwan
:)

But I guess the rule should be:  Does it speak QCDM on any of the serial
ports, then it goes in qcserial.  Anything else goes in option.  Does
that match the original qcserial intention?

Regarding the entries in option: It is unfortunate that HP (and other
laptop vendors) insist on using their own vendor IDs for vendor specifc
OEM products like these mdoem. The Huawei vendor matching in option
works very well, but when the modules are programmed with a HP ID, then
we are back to adding entries per device ID.  Which is a game we cannot
win wrt rarely seen laptop accessories.  How many Linux users will care
about the average HP LTE modem?  My bet is less than one.

What do you think are the chances that HP will be stupid enough to ever
create a USB device with a ff/05/xx interface which is *not* a Huawei
modem?  How about just duplicating every Huawei ff/05/xx entry in option
with the HP vendor ID?  Seems less error prone than the alternatives to
me.



Bjørn
--
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 RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Wed, Feb 24, 2016 at 10:20 PM, Peter Chen  wrote:

> Would you get any chances to test i.mx28 evk with an external HUB?
> I just would like to know it is not a HUB issue which can't be
> suspended.

Just connected an external USB hub on a mx28evk and did not see any issue.
--
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

[BUG] USB ethernet on XHCI dies under load

[Chris Bainbridge]

USB ethernet devices stop responding when plugged in to USB3 XHCI ports
and flooded with incoming traffic. The usbnet layer seems to get -EPROTO
from the xhci driver. Nothing is usually logged in kernel when this
occurs, but with dyndebug on there are errors like:

ax88179_178a 4-1.1:1.0 eth0: tx throttle -71
intr status -71

Sometimes:

xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with disabled ep 
88024ea1adc8

To reproduce just flood the adaptor with gigabit traffic from another
computer for a couple of minutes with any of:

ping -f $host
iperf3 -c $host -t 1000 -P 128
iperf3 -c $host -u -b $bandwidth
(for iperf tests run 'iperf3 -s' on the test platform)

iperf3 shows many errors/retries. If slub_debug is on (specifically
slub_debug=U) there are more retries and it fails faster,  which
suggests some kind of memory corruption. Other slub_debug options do not
appear to have any effect. Another symptom of memory corruption is that
other USB devices plugged in to different ports will disconnect and
reconnect. The problem is not these devices though, as the USB ethernet
still fails when all other devices are unplugged.

I have reproduced this with an AX88179 USB3 gigabit ethernet adaptor and
a dm9601 USB1 10/100 adaptor, on both a laptop with Intel XHCI chipset
and a desktop with VIA XHCI chipset. With the dm9601 adaptor just
running "iperf3 -c z  -t 10 -u -b 12m" (12mbit UDP stream) is enough to
quickly kill it.

I have verified that the bug does not occur with USB2 chipset of the
desktop, only USB3.

This bug appears to have been around a long time. There are some
relevant bug threads, several refer specifically to disconnects on
flooding with AX88179:

http://www.spinics.net/lists/linux-usb/msg113358.html
https://bugzilla.kernel.org/show_bug.cgi?id=75381
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1371233
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1269883

I bisected this with the AX88179 but it just led back to
v3.11-rc1-224-g452c447a497d  ("USBNET: increase max rx/tx qlen for
improving USB3 thoughtput") which is probably not the actual cause of
the bug.
--
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/5 v7] usb: musb: core: added helper functions for parsing DT

[Petr Kulhavy]

On 25.02.2016 12:53, Sergei Shtylyov wrote:



   I then fail to understand the logic behind hard coding.


And I fail to understand your plans with the other drivers.
Are you trying to say that the "mentor,power" (or "power" in OMAP2430)
property should be completely abandoned and the values hard coded in 
_all_

drivers?


   Contrariwise, I don't think it should be hard-coded anywhere.


Well, so we're still at the same point - there is a fundamental mismatch 
in the different developers' view how the "power" parameter should be 
represented.

There already 3 opinions at the moment:
1) hard code - Felipe, Rob
2) use the "mentor,power" - Sergei, Petr
3) use a regulator - Rob

So unless this conflict is resolved it is slightly difficult to submit a 
patch that would get accepted.

How can we resolve this conflict ?

Regards
Petr
--
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/5 v7] usb: musb: core: added helper functions for parsing DT

[Sergei Shtylyov]

Hello.

On 2/25/2016 11:55 AM, Petr Kulhavy wrote:


+#if IS_ENABLED(CONFIG_OF)

+/* gets the "mentor,power" property from DT
+ * and converts it from mA to 2mA units for the "power" parameter
+ * in struct musb_hdrc_platform_data
+ *
+ * in case the property is not found returns 0
+ */
+extern u8 musb_get_power(struct device *dev);
+#endif
+


   Now that you hard-coded the value, you don't need this function any more.


I have left it there so it can be used by the other drivers.


   I then fail to understand the logic behind hard coding.


And I fail to understand your plans with the other drivers.
Are you trying to say that the "mentor,power" (or "power" in OMAP2430)
property should be completely abandoned and the values hard coded in _all_
drivers?


   Contrariwise, I don't think it should be hard-coded anywhere.


Petr


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: [PATCH 2/5 v7] usb: musb: core: added helper functions for parsing DT

[Petr Kulhavy]

On 24.02.2016 18:30, Sergei Shtylyov wrote:

On 02/24/2016 08:10 PM, Petr Kulhavy wrote:


+#if IS_ENABLED(CONFIG_OF)

+/* gets the "mentor,power" property from DT
+ * and converts it from mA to 2mA units for the "power" parameter
+ * in struct musb_hdrc_platform_data
+ *
+ * in case the property is not found returns 0
+ */
+extern u8 musb_get_power(struct device *dev);
+#endif
+


   Now that you hard-coded the value, you don't need this function 
any more.



I have left it there so it can be used by the other drivers.


   I then fail to understand the logic behind hard coding.



And I fail to understand your plans with the other drivers.
Are you trying to say that the "mentor,power" (or "power" in OMAP2430) 
property should be completely abandoned and the values hard coded in 
_all_ drivers?



Petr
--
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