Re: arm64 crash in ffs on 4.9/4.12 - Internal error: Accessing user space memory with fs=KERNEL_DS

[Greg KH]

On Wed, Aug 30, 2017 at 01:27:32PM -0700, Jerry Zhang wrote:
> Hi,
> 
> I'm running into a kernel crash while using the functionfs driver on
> 4.9 and 4.12 kernels. This is reproducible by using any of the aio
> syscalls with functionfs on the above kernels. The code in question is
> in ffs_user_copy_worker, which is a work thread that copies a bit of
> data into userspace. Interestingly, a set_fs(USER_DS) before
> copy_to_iter in the work thread will prevent the crash. I'm told
> though that this shouldn't be necessary if the thread does not set_fs
> to KERNEL_DS, which it does not (nor does the rest of the driver, at
> all). There is also no problem on 4.4.
> 
> Perhaps this is related to some of the mm changes in arch/arm64 that
> went in between 4.4 and 4.9? Alternatively, it could actually be
> necessary to set_fs(USER_DS) in a work thread before accessing user
> memory? Let us know.

4.4-4.9 is a long time, with a lot of changes :)

Any chance you can run git bisect to find the issue?

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: fix coding style issue

[pierre kuo]

hi Greg

> Note, not all subsystems appreciate coding style cleanups, so be aware
> of that if you do start to do this type of work outside of
> drivers/staging/
Got it.
thanks for your friendly reminder.
--
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 3/3] arm64: dts: qcom: Collapse usb support into one node

[Stephen Boyd]

Quoting Shawn Guo (2017-08-16 23:43:49)
> Hi Stephen,
> 
> On Fri, Jul 14, 2017 at 02:40:05PM -0700, Stephen Boyd wrote:
> > We currently have three device nodes for the same USB hardware
> > block, as evident by the reuse of the same reg address multiple
> > times. Now that the chipidea driver fully supports OTG with the
> > MSM wrapper we can collapse all these nodes into one USB device
> > node, reflecting the true nature of the hardware.
> > 
> > Signed-off-by: Stephen Boyd 
> > ---
> >  arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 38 ++-
> >  arch/arm64/boot/dts/qcom/msm8916.dtsi | 62 
> > +++
> >  2 files changed, 50 insertions(+), 50 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi 
> > b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > index f326f4fb4d72..494560a1a6ef 100644
> > --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > @@ -213,24 +213,20 @@
> >   };
> >  
> >   usb@78d9000 {
> > - extcon = <_id>, <_id>;
> > + extcon = <_id>;
> 
> I'm trying to play the series on db410c, and it doesn't seem to work as
> expected.  Here is basically what I did:
> 
> - Revert ed75d6a96905, and apply the series.
> - Turn on the following options:
> CONFIG_USB_CHIPIDEA_ULPI
> CONFIG_USB_ULPI_BUS
> CONFIG_PHY_QCOM_USB_HS
> CONFIG_MUX_GPIO
> 
> The role switching doesn't happen when I connect/disconnect cable
> to/from micro-usb port.

Good. That's expected.

> But if I revert above extcon change (keep two
> usb_id phandle for extcon), the role switching happens.  However, host
> driver still fails to enumerate the usb mouse attached to Type-A port.
> 
> [   15.400074] ci_hdrc ci_hdrc.0: EHCI Host Controller
> [   15.400119] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 
> 1
> [   15.419847] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
> [   15.420700] hub 1-0:1.0: USB hub found
> [   15.425820] hub 1-0:1.0: 1 port detected
> [   15.759862] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> [   31.943942] usb 1-1: device not accepting address 2, error -110
> [   32.063938] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> [   48.071943] usb 1-1: device not accepting address 3, error -110
> [   48.191935] usb 1-1: new high-speed USB device number 4 using ci_hdrc
> [   58.823934] usb 1-1: device not accepting address 4, error -110
> [   58.943936] usb 1-1: new high-speed USB device number 5 using ci_hdrc
> [   69.575935] usb 1-1: device not accepting address 5, error -110
> [   69.576001] usb usb1-port1: unable to enumerate USB device
> 
> I must have missed something.  Can you please advice?  Thanks.

Yes. The role switch happens now by userspace writing different values
to the chipidea node sysfs file. For db410c it's located at
/sys/bus/platform/devices/ci_hdrc.0/role and by echoing 'host' or
'gadget' into that file you can change the role. Unplugging the cable
won't do anything anymore, because the micro-usb port is only indicating
vbus presence, and not the ID pin. At least that is my reading of the
schematic.

Obviously, this changes behavior from previous designs where
disconnecting the cable changed the role, but I don't know if we want to
support this method via the kernel alone. Instead, it seems simpler to
have userspace decide to change the role whenever it wants with some
policy. Do you have any suggestion on how this can be integrated into
userspace so we write this file at boot? That way, we can switch to host
mode by default on db410c and then users can decide to make a gadget if
they want to lose the host ports while the gadget is active. We could
probably have an extcon event handler in userspace as well to change the
role when the micro-usb cable is disconnected. That way, old behavior
could be maintained but it would be a pure policy decision in userspace.
--
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

arm64 crash in ffs on 4.9/4.12 - Internal error: Accessing user space memory with fs=KERNEL_DS

[Jerry Zhang]

Hi,

I'm running into a kernel crash while using the functionfs driver on
4.9 and 4.12 kernels. This is reproducible by using any of the aio
syscalls with functionfs on the above kernels. The code in question is
in ffs_user_copy_worker, which is a work thread that copies a bit of
data into userspace. Interestingly, a set_fs(USER_DS) before
copy_to_iter in the work thread will prevent the crash. I'm told
though that this shouldn't be necessary if the thread does not set_fs
to KERNEL_DS, which it does not (nor does the rest of the driver, at
all). There is also no problem on 4.4.

Perhaps this is related to some of the mm changes in arch/arm64 that
went in between 4.4 and 4.9? Alternatively, it could actually be
necessary to set_fs(USER_DS) in a work thread before accessing user
memory? Let us know.

Thanks,

Jerry

Here is the full crash log:

[  215.871963] Internal error: Accessing user space memory with
fs=KERNEL_DS: 964f [#1] PREEMPT SMP
[  215.881330] CPU: 1 PID: 1343 Comm: kworker/1:1 Tainted: G S
 4.9.41-gf48b3b5b324d #1
[  215.890314] Hardware name: HiKey960 (DT)
[  215.894319] Workqueue: events ffs_user_copy_worker
[  215.899178] task: ffc0b5ce5080 task.stack: ffc0b52bc000
[  215.905184] PC is at __arch_copy_to_user+0xa4/0x280
[  215.910128] LR is at copy_to_iter+0x7c/0x3e0
[  215.914454] pc : [] lr : []
pstate: 8105
[  215.921940] sp : ffc0b52bfca0
[  215.925295] x29: ffc0b52bfca0 x28: ffc0b5322400
[  215.930682] x27: ffc0b324fb98 x26: 0018
[  215.936069] x25: ffc0a8417a10 x24: 0018
[  215.941456] x23:  x22: 
[  215.946845] x21: ffc0b0e5a380 x20: 0018
[  215.952249] x19: ffc0a8417a50 x18: 
[  215.957686] x17:  x16: 
[  215.963121] x15:  x14: 
[  215.968557] x13:  x12: 
[  215.973991] x11: 0001 x10: 0a60
[  215.979432] x9 : ffc0b52bc000 x8 : 0011
[  215.984870] x7 : 06c445545257 x6 : 007fb78418d8
[  215.990299] x5 : 007fb78418f0 x4 : 
[  215.995728] x3 : 0010 x2 : 0018
[  216.001162] x1 : ffc0b324fb90 x0 : 007fb78418d8
[  216.006606]
[  216.006606] SP: 0xffc0b52bfc20:
[  216.011683] fc20 [  216.013469]  
 [  216.016598]  
 [  216.019722]  0018
 [  216.022846]  a8417a10
 ffc0[  216.025971]
[  216.027493] fc40 [  216.029282]  0018
 [  216.032403]  b324fb98
 ffc0[  216.035526]  b5322400
 ffc0[  216.038652]  b52bfca0
 ffc0[  216.041778]
[  216.043296] fc60 [  216.045082]  08525c10
 ff80[  216.048209]  b52bfca0
 ffc0[  216.051334]  08510fa4
 ff80[  216.054459]  8105
 [  216.057584]
[  216.059105] fc80 [  216.060889]  082000dc
 ff80[  216.064016]  b5ce5888
 ffc0[  216.067142]  
 [  216.070264]  0018
 [  216.073384]
[  216.074903] fca0 [  216.076683]  b52bfd20
 ffc0[  216.079808]  08864224
 ff80[  216.082934]  a8417a50
 ffc0[  216.086062]  a8417a00
 ffc0[  216.089185]
[  216.090703] fcc0 [  216.092487]  0018
 [  216.095612]  
 [  216.098736]  
 [  216.101859]  0018
 [  216.104981]
[  216.106500] fce0 [  216.108288]  
 [  216.111413]  
 [  216.114537]  
 [  216.117662]  b5322400
 ffc0[  216.120786]
[  216.122303] fd00 [  216.124089]  a8417a50
 ffc0[  216.127214]  a8417a00
 ffc0[  216.130339]  0018
 [  216.133464]  
 [  216.136590]
[  216.138114]
[  216.138114] X1: 0xffc0b324fb10:
[  216.143184] fb10 [  216.144969]  b324fb08
 ffc0[  216.148093]  b324fb18
 ffc0[  216.151215]  b324fb18
 ffc0[  216.154340]  b324fb28
 ffc0[  216.157464]
[  216.158985] fb30 [  216.160764]  b324fb28
 ffc0[  216.163890]  
 [  216.167016]  
 [  216.170143]  00fb
 [  216.173263]
[  216.174785] fb50 [  216.176565]  ac2ab000
 ffc0[  216.179688]  
 [  216.182818]  aff9
 [  216.185940]  
 [  216.189065]
[  216.190584] fb70 [  216.192365]  
 [  216.195487]  
 [  216.198614]  45545257
 06c4[  216.201737]  0001
 0010[  216.204858]
[  216.206378] fb90 [  216.208159]  028e
 baabada8[  216.211284]  a8417a00
 ffc0[  216.214412]  
 [  216.217540]  ae5ceb00
 ffc0[  216.220664]
[  216.222184] fbb0 [  216.223968]  08864580
 ff80[  216.227094]  b7a3b000
 007f[  216.230219]  
 [  216.233340]  0100
 dead[  216.236467]
[  216.237986] fbd0 [  216.239772]  0200
 dead[  216.242895]  
 [  216.246016]  
 [  216.249143]  
 [  216.252264]
[  216.253786] fbf0 [  216.255572]  

Re: xhci bandwidth problem with isochronous endpoints

[Curt Meyers]

On 08/28/2017 06:00 AM, Mathias Nyman wrote:

On 19.07.2017 01:39, Curt Meyers wrote:

Hello,

I have a new USB3 camera that can only operate in isochronous mode. I 
can open and stream data from the camera using guvcview. The camera 
has 3 interfaces, two show up as uvc video devices. The interface I 
open claims a maximum bandwidth dwMaxBitRate=73728000, I should be 
able to open many of these devices on a USB3 interface based on 
bandwidth.


If I try to open 2 or more devices on any hardware except one 
particular computer it fails and I get the message:


[ 6345.752365] usb 2-2: Not enough bandwidth for new device state.
[ 6345.752368] usb 2-2: Not enough bandwidth for altsetting 2

On the working computer I have run up to 5 devices from a single xhci 
controller and a hub without any problem, it ran for hours. The 
working computers belongs to someone else and has an i7-3770 
processor (3rd generation). The failing computers are all more modern 
and contain an i7-6600U, i3-7700 and an E5-2620v3 with various 
supporting chipsets, the one I am concerned with is a:


00:14.0 USB controller: Intel Corporation Sunrise Point-H USB 3.0 
xHCI Controller (rev 31)


Initially this started on a 4.4 kernel, then I upgraded to 4.8 and it 
still fails. I got the owner of the working machine to upgrade to 4.8 
and it still works. Then I fetched and built 4.13 from the "next" 
branch. I have poked around in the uvc code and in the xhci code and 
I can't really get a handle on what is going on. I tried to force 
some different alternate settings, I have modified the uvc code to 
fix other "quirky" cameras in the past but nothing seems to make any 
improvements here.


I am looking for suggestions on what I can try next. I am attaching 
the output from the usb device descriptor, this is a stereo camera 
that outputs depth data on the second interface, to the 320x480 
resolution is really the depth of two pixels packet together. It is 
this resolution that I am trying to run but all resolutions basically 
fail.




xhci configure endpoint command returned with a bandwidth, or 
secondarty bandwidth error
because xHC hardware can't fulfill the bandwidth requested in xhci 
endpoint context structure.


xhci endpoint context is filled with slightly modified values from the 
endpoint descriptor

(like wMaxPacketSize, bInterval, bMaxBurst and Mult fields)

New kernel have tracing for the endpoint context, First thing would
probably be to check that the endpoint context look sane.

xhci tracing will generate a lot of data, It can be enabled with:

mount -t debugfs none /sys/kernel/debug
echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable

Then send me the content of

/sys/kernel/debug/tracing/trace

Thanks
Mathias


Did the log file I sent make any sense?

I sent it only to you, didn't want to email it through the list server.

Curt
--
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] usbip: vhci-hcd: make vhci_hc_driver const

[Shuah Khan]

On 08/30/2017 06:52 AM, Bhumika Goyal wrote:
> Make this const as it is not modified anywhere.
> 
> Signed-off-by: Bhumika Goyal 
> ---
>  drivers/usb/usbip/vhci_hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index c747623..11b9a22 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1274,7 +1274,7 @@ static int vhci_free_streams(struct usb_hcd *hcd, 
> struct usb_device *udev,
>   return 0;
>  }
>  
> -static struct hc_driver vhci_hc_driver = {
> +static const struct hc_driver vhci_hc_driver = {
>   .description= driver_name,
>   .product_desc   = driver_desc,
>   .hcd_priv_size  = sizeof(struct vhci_hcd),
> 

Looks good to me.

Acked-by: Shuah Khan 

thanks,
-- Shuah
--
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: fix coding style issue

[Greg KH]

On Wed, Aug 30, 2017 at 09:37:26PM +0800, pierre kuo wrote:
> hi Greg:
> 
> 2017-08-16 23:46 GMT+08:00 Greg KH :
> > On Wed, Aug 16, 2017 at 11:01:20AM +0800, pierre Kuo wrote:
> >> try to fix some codeing style issue, such as "space prohibited" and "not
> >> initialise statics".
> >
> > Only do one type of thing in a single patch please.  And no, "fix all
> > coding style issues" is not a single thing.
> >
> > This needs to be multiple patches.
> so you mean
> a. patch1 for "space prohibited"
> b. patch2 for "not initalise statics"
> right?

Yes.

> >
> > If you want to do coding style cleanups, please start out in
> > drivers/staging/ so you can get the process correct.
> Got it.
> I just found some part of usb driver may need coding style cleanups
> while tracing code flow and hope can do some feedback to this mailing
> list.
> 
> So you mean we first run some typo check on drivers/staging/ and once
> patches accepted then we started for usb part?
> BTW, I found below link show people use checkpatch.pl to checking coding 
> style.
> http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
> 
> is that ok if we use "./scripts/checkpatch.pl -f
> drivers/usb/host/ehci-xxx.c" and create different patches based on
> different coding style issue?
> (like above #a and #b shows?)

Please start in the drivers/staging/ portion of the kernel tree for
coding style cleanups.  That's a "safe" place to learn how to do all of
this work.  Only after you have experience there should you venture out
to other portions of the kernel.

Note, not all subsystems appreciate coding style cleanups, so be aware
of that if you do start to do this type of work outside of
drivers/staging/

good luck!

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: fix coding style issue

[pierre kuo]

hi Greg:

2017-08-16 23:46 GMT+08:00 Greg KH :
> On Wed, Aug 16, 2017 at 11:01:20AM +0800, pierre Kuo wrote:
>> try to fix some codeing style issue, such as "space prohibited" and "not
>> initialise statics".
>
> Only do one type of thing in a single patch please.  And no, "fix all
> coding style issues" is not a single thing.
>
> This needs to be multiple patches.
so you mean
a. patch1 for "space prohibited"
b. patch2 for "not initalise statics"
right?

>
> If you want to do coding style cleanups, please start out in
> drivers/staging/ so you can get the process correct.
Got it.
I just found some part of usb driver may need coding style cleanups
while tracing code flow and hope can do some feedback to this mailing
list.

So you mean we first run some typo check on drivers/staging/ and once
patches accepted then we started for usb part?
BTW, I found below link show people use checkpatch.pl to checking coding style.
http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel

is that ok if we use "./scripts/checkpatch.pl -f
drivers/usb/host/ehci-xxx.c" and create different patches based on
different coding style issue?
(like above #a and #b shows?)

appreciate your kind help,
--
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: imx21-hcd: make imx21_hc_driver const

[Bhumika Goyal]

Make this const as it is not modified anywhere.

Signed-off-by: Bhumika Goyal 
---
 drivers/usb/host/imx21-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/imx21-hcd.c b/drivers/usb/host/imx21-hcd.c
index e25d72e..39ae7fb 100644
--- a/drivers/usb/host/imx21-hcd.c
+++ b/drivers/usb/host/imx21-hcd.c
@@ -1779,7 +1779,7 @@ static void imx21_hc_stop(struct usb_hcd *hcd)
 /* Driver glue */
 /* === */
 
-static struct hc_driver imx21_hc_driver = {
+static const struct hc_driver imx21_hc_driver = {
.description = hcd_name,
.product_desc = "IMX21 USB Host Controller",
.hcd_priv_size = sizeof(struct imx21),
-- 
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] usbip: vhci-hcd: make vhci_hc_driver const

[Bhumika Goyal]

Make this const as it is not modified anywhere.

Signed-off-by: Bhumika Goyal 
---
 drivers/usb/usbip/vhci_hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index c747623..11b9a22 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1274,7 +1274,7 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct 
usb_device *udev,
return 0;
 }
 
-static struct hc_driver vhci_hc_driver = {
+static const struct hc_driver vhci_hc_driver = {
.description= driver_name,
.product_desc   = driver_desc,
.hcd_priv_size  = sizeof(struct vhci_hcd),
-- 
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 v3 1/3] usb: xhci-mtk: add generic compatible string

[Chunfeng Yun]

The xhci-mtk driver is a generic driver for MediaTek xHCI IP, add
a generic compatible to avoid confusion when support new SoCs but
use a compatible with specific SoC's name "mt8173".

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/host/xhci-mtk.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 67d5dc7..8fb6065 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -795,6 +795,7 @@ static int __maybe_unused xhci_mtk_resume(struct device 
*dev)
 #ifdef CONFIG_OF
 static const struct of_device_id mtk_xhci_of_match[] = {
{ .compatible = "mediatek,mt8173-xhci"},
+   { .compatible = "mediatek,mtk-xhci"},
{ },
 };
 MODULE_DEVICE_TABLE(of, mtk_xhci_of_match);
-- 
1.7.9.5

--
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 v3 3/3] dt-bindings: mt8173-mtu3: add generic compatible and rename file

[Chunfeng Yun]

The mt8173-mtu3.txt actually holds the bindings for all mediatek
SoCs with usb3 DRD IP, so add a generic compatible and change the
name to mediatek,mtu3.txt.

Signed-off-by: Chunfeng Yun 
Acked-by: Rob Herring 
---
 .../usb/{mt8173-mtu3.txt => mediatek,mtu3.txt} |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/usb/{mt8173-mtu3.txt => 
mediatek,mtu3.txt} (92%)

diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt 
b/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
similarity index 92%
rename from Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
rename to Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
index b8ea394..838ae48 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
@@ -1,7 +1,11 @@
 The device node for Mediatek USB3.0 DRD controller
 
 Required properties:
- - compatible : should be "mediatek,mt8173-mtu3"
+ - compatible : should be "mediatek,-mtu3", "mediatek,mtu3",
+   soc-model is the name of SoC, such as mt8173, mt2712 etc,
+   when using "mediatek,mtu3" compatible string, you need SoC specific
+   ones in addition, one of:
+   - "mediatek,mt8173-mtu3"
  - reg : specifies physical base address and size of the registers
  - reg-names: should be "mac" for device IP and "ippc" for IP port control
  - interrupts : interrupt used by the device IP
-- 
1.7.9.5

--
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 v3 2/3] dt-bindings: mt8173-xhci: add generic compatible and rename file

[Chunfeng Yun]

The mt8173-xhci.txt actually holds the bindings for all mediatek
SoCs with xHCI controller, so add a generic compatible and change
the name to mediatek,mtk-xhci.txt to reflect that.

Signed-off-by: Chunfeng Yun 
---
 .../usb/{mt8173-xhci.txt => mediatek,mtk-xhci.txt} |   14 +++---
 .../devicetree/bindings/usb/mt8173-mtu3.txt|2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)
 rename Documentation/devicetree/bindings/usb/{mt8173-xhci.txt => 
mediatek,mtk-xhci.txt} (85%)

diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt 
b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
similarity index 85%
rename from Documentation/devicetree/bindings/usb/mt8173-xhci.txt
rename to Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
index 0acfc8a..5611a2e 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
@@ -11,7 +11,11 @@ into two parts.
 
 
 Required properties:
- - compatible : should contain "mediatek,mt8173-xhci"
+ - compatible : should be "mediatek,-xhci", "mediatek,mtk-xhci",
+   soc-model is the name of SoC, such as mt8173, mt2712 etc, when using
+   "mediatek,mtk-xhci" compatible string, you need SoC specific ones in
+   addition, one of:
+   - "mediatek,mt8173-xhci"
  - reg : specifies physical base address and size of the registers
  - reg-names: should be "mac" for xHCI MAC and "ippc" for IP port control
  - interrupts : interrupt used by the controller
@@ -68,10 +72,14 @@ usb30: usb@1127 {
 
 In the case, xhci is added as subnode to mtu3. An example and the DT binding
 details of mtu3 can be found in:
-Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
+Documentation/devicetree/bindings/usb/mediatek,mtu3.txt
 
 Required properties:
- - compatible : should contain "mediatek,mt8173-xhci"
+ - compatible : should be "mediatek,-xhci", "mediatek,mtk-xhci",
+   soc-model is the name of SoC, such as mt8173, mt2712 etc, when using
+   "mediatek,mtk-xhci" compatible string, you need SoC specific ones in
+   addition, one of:
+   - "mediatek,mt8173-xhci"
  - reg : specifies physical base address and size of the registers
  - reg-names: should be "mac" for xHCI MAC
  - interrupts : interrupt used by the host controller
diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt 
b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
index 1d7c3bc..b8ea394 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
@@ -44,7 +44,7 @@ Optional properties:
 Sub-nodes:
 The xhci should be added as subnode to mtu3 as shown in the following example
 if host mode is enabled. The DT binding details of xhci can be found in:
-Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
 
 Example:
 ssusb: usb@11271000 {
-- 
1.7.9.5

--
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: dwc3: of-simple: Add compatible for Spreadtrum SC9860 platform

[Baolin Wang]

Add compatible string to use this generic glue layer to support
Spreadtrum SC9860 platform's dwc3 controller.

Signed-off-by: Baolin Wang 
---
 drivers/usb/dwc3/dwc3-of-simple.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index fe414e7..43a7d44 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -177,6 +177,7 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
{ .compatible = "rockchip,rk3399-dwc3" },
{ .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
+   { .compatible = "sprd,sc9860-dwc3" },
{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
-- 
1.7.9.5

--
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: Possible regression between 4.9 and 4.13

[Mason]

On 30/08/2017 11:07, Ard Biesheuvel wrote:

> Please don't forget to mention that this is quirky hardware that
> depends on BROKEN because it multiplexes MMIO and config space
> accesses in the same memory window without any locking whatsoever
> (which would be difficult to do in the first place because we don't
> use accessors for MMIO in the kernel).

You're right, it was in the back of my mind, but I didn't state
it explicitly for the benefit of linux-usb readers.

> So how likely is it that you are attempting to read from the xhci BAR
> window while a config space access is in progress? Any way to
> instrument this in your driver?

I logged config space accesses here:

https://www.spinics.net/lists/arm-kernel/msg602832.html

IIRC, the config space accesses are generated by the AER ISR.
So disabling the AER driver should guarantee that no config space
accesses are occurring when the drive is unplugged.

Regards.
--
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: Possible regression between 4.9 and 4.13

[Greg Kroah-Hartman]

On Wed, Aug 30, 2017 at 10:07:59AM +0100, Ard Biesheuvel wrote:
> On 30 August 2017 at 09:55, Mason  wrote:
> > On 30/08/2017 08:02, Greg Kroah-Hartman wrote:
> >
> >> To get back to the original issue here, the hardware seems to have died,
> >> the driver stops talking to it, and all is good.  The "regression" here
> >> is that we now properly can determine that the hardware is crap.
> >
> > Before 4.12, when I unplugged my USB3 Flash drive, Linux would
> > detect a few "Uncorrected Non-Fatal errors" via AER, but it was
> > still possible to plug the drive back in.
> >
> > Since 4.12, once I unplug the drive, the whole USB3 card is marked
> > as dead (all 4 ports), and I can no longer plug anything in (not even
> > the USB2 drive that didn't have any issues, IIRC).
> >
> > It seems a bit premature to "mark as dead" something that remains
> > functional, doesn't it?
> >
> > Disclaimer, there are many variables in this setup, and I've only
> > tested a small fraction of the problem space: only one system,
> > only one USB3 board, only one USB3 Flash drive.
> >
> 
> Please don't forget to mention that this is quirky hardware that
> depends on BROKEN because it multiplexes MMIO and config space
> accesses in the same memory window without any locking whatsoever
> (which would be difficult to do in the first place because we don't
> use accessors for MMIO in the kernel).
> 
> So how likely is it that you are attempting to read from the xhci BAR
> window while a config space access is in progress? Any way to
> instrument this in your driver?

Seriously?  Ok, that's crap hardware, sorry, I don't feel bad at all
here.  You are going to have worse problems than just a single USB
controller issue if that's your hardware design, go kick some hardware
engineers for me please.

good luck, you are on your own :(

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: Possible regression between 4.9 and 4.13

[Ard Biesheuvel]

On 30 August 2017 at 09:55, Mason  wrote:
> On 30/08/2017 08:02, Greg Kroah-Hartman wrote:
>
>> To get back to the original issue here, the hardware seems to have died,
>> the driver stops talking to it, and all is good.  The "regression" here
>> is that we now properly can determine that the hardware is crap.
>
> Before 4.12, when I unplugged my USB3 Flash drive, Linux would
> detect a few "Uncorrected Non-Fatal errors" via AER, but it was
> still possible to plug the drive back in.
>
> Since 4.12, once I unplug the drive, the whole USB3 card is marked
> as dead (all 4 ports), and I can no longer plug anything in (not even
> the USB2 drive that didn't have any issues, IIRC).
>
> It seems a bit premature to "mark as dead" something that remains
> functional, doesn't it?
>
> Disclaimer, there are many variables in this setup, and I've only
> tested a small fraction of the problem space: only one system,
> only one USB3 board, only one USB3 Flash drive.
>

Please don't forget to mention that this is quirky hardware that
depends on BROKEN because it multiplexes MMIO and config space
accesses in the same memory window without any locking whatsoever
(which would be difficult to do in the first place because we don't
use accessors for MMIO in the kernel).

So how likely is it that you are attempting to read from the xhci BAR
window while a config space access is in progress? Any way to
instrument this in your driver?

>> So, how do you think we should proceed, delay a bit longer before saying
>> the device is gone?  How long is "long enough"?  How many bus errors are
>> we allowed to tolerate (hint, the PCI spec says none...)
>>
>> Maybe someone wants to get to the root problem here, why is the hardware
>> suddenly reporting all 1s?
>
> I'm afraid I won't be able to make any progress on this front,
> unless I can get my hands on a PCIe packet analyzer.
>
> Regards.
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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: Possible regression between 4.9 and 4.13

[Greg Kroah-Hartman]

On Wed, Aug 30, 2017 at 10:55:37AM +0200, Mason wrote:
> On 30/08/2017 08:02, Greg Kroah-Hartman wrote:
> 
> > To get back to the original issue here, the hardware seems to have died,
> > the driver stops talking to it, and all is good.  The "regression" here
> > is that we now properly can determine that the hardware is crap.
> 
> Before 4.12, when I unplugged my USB3 Flash drive, Linux would
> detect a few "Uncorrected Non-Fatal errors" via AER, but it was
> still possible to plug the drive back in.
> 
> Since 4.12, once I unplug the drive, the whole USB3 card is marked
> as dead (all 4 ports), and I can no longer plug anything in (not even
> the USB2 drive that didn't have any issues, IIRC).
> 
> It seems a bit premature to "mark as dead" something that remains
> functional, doesn't it?

I agree, but if the device sends all ones, it's a good indication it is
really dead, right?  Or something is wrong with it.

> Disclaimer, there are many variables in this setup, and I've only
> tested a small fraction of the problem space: only one system,
> only one USB3 board, only one USB3 Flash drive.

Did you ever happen to narrow this down to a single git commit using
'git bisect'?  I can't remember what happened in the beginning of this
thread...

> > So, how do you think we should proceed, delay a bit longer before saying
> > the device is gone?  How long is "long enough"?  How many bus errors are
> > we allowed to tolerate (hint, the PCI spec says none...)
> > 
> > Maybe someone wants to get to the root problem here, why is the hardware
> > suddenly reporting all 1s?
> 
> I'm afraid I won't be able to make any progress on this front,
> unless I can get my hands on a PCIe packet analyzer.

Odds of that happening are pretty rare, right?  I've never even seen one
of those...

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: host: make ehci_fsl_overrides const and __initconst

[Bhumika Goyal]

Make this structure const as it is not modified. And replace __initdata
with __initconst to avoid section conflict error.

Signed-off-by: Bhumika Goyal 
---
 drivers/usb/host/ehci-fsl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 4a08b70..d025cc0 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -642,7 +642,7 @@ static int ehci_start_port_reset(struct usb_hcd *hcd, 
unsigned port)
 #define ehci_start_port_reset  NULL
 #endif /* CONFIG_USB_OTG */
 
-static struct ehci_driver_overrides ehci_fsl_overrides __initdata = {
+static const struct ehci_driver_overrides ehci_fsl_overrides __initconst = {
.extra_priv_size = sizeof(struct ehci_fsl),
.reset = ehci_fsl_setup,
 };
-- 
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: Possible regression between 4.9 and 4.13

[Mason]

On 30/08/2017 08:02, Greg Kroah-Hartman wrote:

> To get back to the original issue here, the hardware seems to have died,
> the driver stops talking to it, and all is good.  The "regression" here
> is that we now properly can determine that the hardware is crap.

Before 4.12, when I unplugged my USB3 Flash drive, Linux would
detect a few "Uncorrected Non-Fatal errors" via AER, but it was
still possible to plug the drive back in.

Since 4.12, once I unplug the drive, the whole USB3 card is marked
as dead (all 4 ports), and I can no longer plug anything in (not even
the USB2 drive that didn't have any issues, IIRC).

It seems a bit premature to "mark as dead" something that remains
functional, doesn't it?

Disclaimer, there are many variables in this setup, and I've only
tested a small fraction of the problem space: only one system,
only one USB3 board, only one USB3 Flash drive.

> So, how do you think we should proceed, delay a bit longer before saying
> the device is gone?  How long is "long enough"?  How many bus errors are
> we allowed to tolerate (hint, the PCI spec says none...)
> 
> Maybe someone wants to get to the root problem here, why is the hardware
> suddenly reporting all 1s?

I'm afraid I won't be able to make any progress on this front,
unless I can get my hands on a PCIe packet analyzer.

Regards.
--
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: serial: option: simplify 3 D-Link device entries

[Johan Hovold]

On Tue, Aug 29, 2017 at 04:41:21PM +0200, Johan Hovold wrote:
> On Tue, Aug 29, 2017 at 10:45:13AM +0200, Bjørn Mork wrote:
> > All the vendor specific interfaces on these devices are serial
> > functions handled by this driver, so we can use a single class
> > match entry for each.
> > 
> >  P:  Vendor=2001 ProdID=7d01 Rev= 3.00
> >  S:  Manufacturer=D-Link,Inc
> >  S:  Product=D-Link DWM-156
> >  C:* #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
> >  A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
> >  I:* If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
> >  E:  Ad=88(I) Atr=03(Int.) MxPS=  64 Ivl=125us
> >  I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
> >  I:* If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
> >  E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=02 Prot=01 Driver=option
> >  E:  Ad=87(I) Atr=03(Int.) MxPS=  64 Ivl=500us
> >  E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  I:* If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> >  E:  Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  E:  Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  I:* If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> >  E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  I:* If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> >  E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  E:  Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  I:* If#= 6 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage
> >  E:  Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> >  E:  Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> > 
> > Signed-off-by: Bjørn Mork 
> > ---
> > 
> > Johan Hovold  writes:
> > 
> > > Do you want to submit a patch or should I do it?
> > 
> > Well, I can save you that job if this is fine with you.  Feel
> > free to add a stable cc if you think it is appropriate.  I am not
> > sure...
> 
> Thanks! Yeah, not sure about stable either. Maybe I'll just apply this
> one on top the new entry coming in, and we can defer the decision to the
> stable maintainers next time there's a backport conflict.

Now applied (without stable-CC).

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: serial: option: add support for D-Link DWM-157 C1

[Johan Hovold]

On Tue, Aug 29, 2017 at 09:50:03PM +0200, Maciej S. Szmigiero wrote:
> This commit adds support (an ID, really) for D-Link DWM-157 hardware
> version C1 USB modem to option driver.
> 
> According to manufacturer-provided Windows INF file the device has four
> serial ports:
> "D-Link HSPA+DataCard Diagnostics Interface" (interface 2; modem port),
> "D-Link HSPA+DataCard NMEA Device" (interface 3),
> "D-Link HSPA+DataCard Speech Port" (interface 4),
> "D-Link HSPA+DataCard Debug Port" (interface 5).
> 
> usb-devices output:
> T:  Bus=05 Lev=01 Prnt=01 Port=04 Cnt=01 Dev#=  3 Spd=480 MxCh= 0
> D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=2001 ProdID=7d0e Rev=03.00
> S:  Manufacturer=D-Link,Inc
> S:  Product=D-Link DWM-157
> C:  #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim
> I:  If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
> I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=02 Prot=01 Driver=option
> I:  If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> I:  If#= 4 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> I:  If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> I:  If#= 6 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage
> 
> Signed-off-by: Maciej S. Szmigiero 
> Cc: sta...@vger.kernel.org
> ---
> Changes from v1: Match the whole vendor-specific interface class instead
> of particular interface subclasses and protocols, add usb-devices output
> to the commit message.

Thanks for the v2, now applied.

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: dwc2: Error message on AMCC 405EX SoC

[Benjamin Herrenschmidt]

On Wed, 2017-08-30 at 17:21 +1000, Benjamin Herrenschmidt wrote:
> Hi !
> 
> (Adding the mailing list)
> 
> On the AMCC 405EX SoC, if I enable the OTG, I get this occasional
> message:
> 
> dwc2 ef6c.usbotg: dwc2_hc_chhltd_intr_dma: Channel 1 - ChHltd set, but 
> reason is unknown
> dwc2 ef6c.usbotg: hcint 0x0002, intsts 0x04400029
> 
> The core version is:
> 
> dwc2 ef6c.usbotg: Core Release: 2.60a (snpsid=4f54260a)
> 
> Is there any woraround missing in the code you reckon ? Any idea what
> can be causing this ?

When I enable debugging in the driver I see a lot of these too:

dwc2 ef6c.usbotg: --Host Channel 3 Interrupt: Transaction Error--
dwc2 ef6c.usbotg: --Host Channel 3 Interrupt: Transaction Error--
dwc2 ef6c.usbotg: --Host Channel 3 Interrupt: Transaction Error--
dwc2 ef6c.usbotg: --Host Channel 2 Interrupt: Transaction Error--
dwc2 ef6c.usbotg: --Host Channel 0 Interrupt: Transaction Error--
dwc2 ef6c.usbotg: --Host Channel 0 Interrupt: Transaction Error--
dwc2 ef6c.usbotg: --Host Channel 1 Interrupt: Transaction Error--

While reading data from a usb mass storage (CF reader).

Cheers,
Ben.

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

dwc2: Error message on AMCC 405EX SoC

[Benjamin Herrenschmidt]

Hi !

(Adding the mailing list)

On the AMCC 405EX SoC, if I enable the OTG, I get this occasional
message:

dwc2 ef6c.usbotg: dwc2_hc_chhltd_intr_dma: Channel 1 - ChHltd set, but 
reason is unknown
dwc2 ef6c.usbotg: hcint 0x0002, intsts 0x04400029

The core version is:

dwc2 ef6c.usbotg: Core Release: 2.60a (snpsid=4f54260a)

Is there any woraround missing in the code you reckon ? Any idea what
can be causing this ?

Cheers,
Ben.
--
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: Possible regression between 4.9 and 4.13

[Greg Kroah-Hartman]

On Wed, Aug 30, 2017 at 08:36:23AM +0200, Lukas Wunner wrote:
> On Tue, Aug 29, 2017 at 05:51:38PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Aug 29, 2017 at 05:34:56PM +0200, Lukas Wunner wrote:
> > > On Tue, Aug 29, 2017 at 04:47:25PM +0200, Greg Kroah-Hartman wrote:
> > > > On Tue, Aug 29, 2017 at 03:38:52PM +0200, Lukas Wunner wrote:
> > > Is 0x not a valid value to be stored in and read from mmio space?
> > 
> > For a specific register, doubtful
> 
> Well, "doubtful" means you don't know for sure.
> 
> It's fine to check for "all ones" as a heuristic if that's not a valid
> value for the register read, however a hotplug notification is a
> *definitive* indication the hardware is gone.
> 
> I you seem to prefer forgoing a *definitive* indication for a mere
> heuristic, that doesn't make sense from my point of view.

I still don't know what you are arguing about here.  The _driver_ knows
if a specific read allows all ones as a valid return value.  If it
isn't, then the driver knows the device is now gone.  It's that simple,
don't do that type of check if all ones is a valid read.

And that's not what is happening here anyway, so again, what is this
discussion about?

Unless there's something specific we can do here for the xhci driver, I
think this thread is dead until someone determines what is going wrong
with the hardware the original reporter posted about.

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: Possible regression between 4.9 and 4.13

[Lukas Wunner]

On Tue, Aug 29, 2017 at 05:51:38PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 29, 2017 at 05:34:56PM +0200, Lukas Wunner wrote:
> > On Tue, Aug 29, 2017 at 04:47:25PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Aug 29, 2017 at 03:38:52PM +0200, Lukas Wunner wrote:
> > Is 0x not a valid value to be stored in and read from mmio space?
> 
> For a specific register, doubtful

Well, "doubtful" means you don't know for sure.

It's fine to check for "all ones" as a heuristic if that's not a valid
value for the register read, however a hotplug notification is a
*definitive* indication the hardware is gone.

I you seem to prefer forgoing a *definitive* indication for a mere
heuristic, that doesn't make sense from my point of view.


> It's a really long time for everything to shut down and to help
> prevent the driver from going crazy, [...]
> Also, there is always a race between when that notification has been
> sent and processed on the PCIe channel and the reading of all 1s on the
> PCI bus by the driver.

Yes I know that.  In practice the interrupt signaling hot removal
comes fast enough to prevent drivers from "going crazy", as I've
mentioned in this patch:

https://patchwork.kernel.org/patch/9405255/


> For fun, try disconnecting a USB device from one of the more modern
> laptops with a USB 3.1 connection on it.  The bios treats those as a pci
> hotpluggable xhci controller on the PCI bus.  When the device is
> disconnected, the BIOS rips out the PCI device as well, but all that
> time, the xhci driver is thinking the device is still present as it
> takes a while for the BIOS to do all of the needed housekeeping.

Yes, that is the case with Thunderbolt 3 controllers on non-Macs:
The XHCI controller appears below downstream bridge 2 of the Thunderbolt
controller's PCIe switch.  Once the last device is removed, the PCIe
switch and all devices below it disappear and the controller is powered
down.  The controller is thus only visible if powered up.  On Mac this
is all native instead:  Native pciehp, native tunnel setup, native PM.


> > > > Just make pci_dev_is_disconnected() public to detect PCI hot removal.
> > > > We *know* when the device was hot removed, so I think there's no need
> > > > to guess that based on reading "all ones" from mmio (which may happen
> > > > for entirely legitimate reasons unrelated to hot removal).
> > > 
> > > No, you don't always "know" when a device is removed, don't rely on it,
> > > not all platforms support that.
> > 
> > Please explain, which platforms don't support that?  They wouldn't be
> > compliant with the spec it seems.
> 
> What about all of the non-pciehp platforms?  :)

Fair enough, those should be extended to set PCI_DEV_DISCONNECTED as well.

Thanks,

Lukas
--
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: Possible regression between 4.9 and 4.13

[Greg Kroah-Hartman]

On Wed, Aug 30, 2017 at 01:53:10AM +0200, Lukas Wunner wrote:
> On Tue, Aug 29, 2017 at 04:28:53PM +0300, Mathias Nyman wrote:
> > This tight check was originally done to detect pci hotplug removed
> > hosts as soon as possible.
> 
> In Mason's case, the parent of the XHCI controller isn't a hotplug port,
> see this lspci output:
> 
> https://www.spinics.net/lists/linux-usb/msg160010.html
> 
> Please check is_hotplug_bridge in the parent's struct pci_dev before
> assuming that the XHCI controller was unplugged.

How can you guarantee that this is set on some systems?  Will it be set
on cardbus devices?  What about on a "normal" system where I can just go
and yank out a PCI card at will?

I don't think this is a valid thing to check, and again, why are we
arguing this point?  It's been this way since the 1990's, this isn't a
new thing...

To get back to the original issue here, the hardware seems to have died,
the driver stops talking to it, and all is good.  The "regression" here
is that we now properly can determine that the hardware is crap.

So, how do you think we should proceed, delay a bit longer before saying
the device is gone?  How long is "long enough"?  How many bus errors are
we allowed to tolerate (hint, the PCI spec says none...)

Maybe someone wants to get to the root problem here, why is the hardware
suddenly reporting all 1s?

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