date:20190923

Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted

2019-09-23 Thread alex zheng

Hi, balbi,

Thank you for your reply~

Felipe Balbi  于2019年9月23日周一 下午1:36写道：
>
>
> Hi,
>
> (it helps when you Cc correct maintainers ;-)
>
> alex zheng  writes:
>
> > Hi all,
> >
> > I am a user of dwc3 USB host controller, I found there are some
> > confused behavior of trb event on this controller.
> > When I run a raw USB data transfer(run bulk in&out transfer with
> > libusb) and iperf3(over rndis) at the same time,
> > there are some strange interrupts occurs and make the driver report
> > error(ERROR DMA transfer).
> > And:
>
> So dwc3 is workingo n host mode. Which platform is this?

This is our self-design platform (ARM v7 cpu core  with synopsys DWC
USB3.0 controller).
version info: Linux localhost 4.9.130-645692-g6ecde01-dirty #394 SMP
PREEMPT Sun Sep 22 15:10:51 CST 2019 armv7l

>
> > 1. this problem only hapened in USB SS mode
> > 2. this problem seems not hapen when I run same test case with other
> > xhci controller(such as asmedia/intel pcie xhci controller) on PC.
> > 3. the kernel version is 4.9.130
>
> Have you tried a more recent kernel? 4.9 is really ancient. Please try
> v5.3.

Our platform only support 4.9 kernel now, and it may take a lot of
work to do to support the recent kernel.
Are there any causes may lead the link TRB trigger a interrupt when
the IOC bit is not setted?

>
> > I think this may be a hw bug of DWC3 USB controller, could anyone
> > please give me some help to debug this problem？
> >
> > The detail log see as below:
> > [  131.074102] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto: xHCI
> > handle event, 8000
> > 27630 [  131.074109] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > process trans event : ep_index = 11, event_dma = 1eb13e90
> > 27631 [  131.074117] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > inc_deq, start trb dma = 1eb13e90, dequeue_p = e482ce90, trb_free num
> > = 1871, ring type = 2
> > 27632 [  131.074123] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > inc_deq 111, start trb dma = 1eb13ea0, dequeue_p = e482cea0, trb_free
> > num = 1872, ring type = 2
> > 27633 [  131.074130] c0 3 (ksoftirqd/0) xhci-hcd xh[  133.057617] c0 3
> > (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto: ERROR Transfer event TRB DMA
> > ptr not part of current TD ep_index 16 comp_code 1
> > 27634 [  133.059312] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > Looking for event-dma 1eb0fff0 trb-start 1eb1
> > trb-end 1eb1 seg-start 1eb1 seg-end
> > 1eb10ff0
> > 27635 [  133.066215] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
> > comp_code 1
> > 27636 [  133.067908] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > Looking for event-dma 1eb1 trb-start 1eb10230
> > trb-end 1eb10230 seg-start 1eb1 seg-end
> > 1eb10ff0
> > 27637 [  133.070572] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
> > comp_code 1
> > 27638 [  133.072260] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > Looking for event-dma 1eb10010 trb-start 1eb10230
> > trb-end 1eb10230 seg-start 1eb1 seg-end
> > 1eb10ff0
> > 27639 [  133.075052] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
> > comp_code 1
> > 27640 [  133.076739] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > Looking for event-dma 1eb10020 trb-start 1eb10230
> > trb-end 1eb10230 seg-start 1eb1 seg-end
> > 1eb10ff0
> > 27641 [  133.079472] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
> > comp_code 1
> > 27642 [  133.081159] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > Looking for event-dma 1eb10030 trb-start 1eb10230
> > trb-end 1eb10230 seg-start 1eb1 seg-end
> > 1eb10ff0
> > 27643 [  133.083896] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
> > comp_code 1
> > 27644 [  133.085584] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > Looking for event-dma 1eb10040 trb-start 1eb10230
> > trb-end 1eb10230 seg-start 1eb1 seg-end
> > 1eb10ff0
> > 27645 [  133.088328] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
> > ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
> > comp_code 1
> >
> > 1. According these logs above the link trb whose address is 0x1eb0fff0
> > occurs a transfer event, but this DMA address is not in the trb ring,
> > then the driver report an error(and followed a few error logs witch
> > invalid DMA address).
> > 2. I dump the data of the address(0x1eb0fff0) and find the IOC bit is
> > not set, see as below:
> > # dump_reg.sh 0x1eb0fff0 4
> > 0x1eb0fff0:0x1EB1 0x 0x 0x1800
>
> --

Re: Ryzen7 3700U xhci fails on resume from sleep

2019-09-23 Thread Daniel Drake

On Wed, Aug 28, 2019 at 4:43 PM Rafael J. Wysocki  wrote:
> > I'll report it to the vendor,
>
> Yes, please.  At least try to get the information on what the exact
> platform expectations with respect to the OS are.  Quite evidently,
> they aren't just "do the usual thing".

AMD's response was:
 > It’s modern stanby system, we don’t have any resource to debug
Linux issue on it.

I imagine there are people in AMD that do care, but we don't have the
right contacts here, not sure if you happen to have anyone you could
forward this thread to just in case.

Anyway, I looked again and found some more interesting points and a
likely workaround, if you can help me nail down the details.

I checked again the experiment of runtime-suspending and resuming the
USB controller. As before, the problematic step is waking up. In
pci_raw_set_power_state() the pmcsr is first read as value 103, then
written as 0, then it msleeps for 10ms (in pci_dev_d3_sleep()), then
reads back value 3.

What I hadn't spotted before is that even though it failed to change
the power state bits, bit 8 did get successfully unset, indicating
that the device is not completely dead.

I then increased the msleep delay to 20ms and now it resumes fine &
USB devices work.

Unfortunately it's not quite as simple as quirking d3_delay though,
because the problem still happens upon resume from s2idle. In that
case, pci_dev_d3_sleep() is not called at all.

if (state == PCI_D3hot || dev->current_state == PCI_D3hot)
pci_dev_d3_sleep(dev);

In the runtime resume case, dev->current_state == PCI_D3hot here (even
though pci_set_power_state had been called to put it into D3cold), so
the msleep happens.
But in the system sleep (s2idle) case, dev->current_state ==
PCI_D3cold here, so no sleep happens.
That is strangely inconsistent - is it a bug?

I also noticed that there is a 100ms d3cold_delay, but that seems to
happen before pcmsr is accessed at all, and doesn't have take any
effect here. However, I did also notice that there is no d3cold_delay
done during wakeup from s2idle, it only happens on wakeup from runtime
suspend. The code does seem to be written that way (runtime_d3cold
flag) but I wonder if that is correct. From the standpoint of the ACPI
PM specs, is there a difference between runtime suspend and s2idle
suspend? Since there is no firmware-based system suspend happening I
wonder if the d3cold_delay should apply in both cases.

I compared behaviour to another system with Amd Ryzen5 3500U. It's not
quite the same SoC but the XHCI controllers have the same PCI IDs. On
that platform, I was able to reproduce the failure to runtime resume,
but then it succeeds with a d3_delay of 20ms. On system
suspend/resume, this other platform uses S3, and the XHCI controller
is already in D0 upon resume (looks like the firmware turns on the USB
controllers for us, so Linux avoids any difficulties there).

That seems to agree that quirking these XHCI controllers (based on PCI
ID) to have a d3_delay of 20ms seems sane, but first we need to nail
down why that delay is not applied at all during resume from s2idle.

Daniel

[PATCH 1/1] usb: serial: option: add Telit FN980 compositions

2019-09-23 Thread Daniele Palmas

This patch adds the following Telit FN980 compositions:

0x1050: tty, adb, rmnet, tty, tty, tty, tty
0x1051: tty, adb, mbim, tty, tty, tty, tty
0x1052: rndis, tty, adb, tty, tty, tty, tty
0x1053: tty, adb, ecm, tty, tty, tty, tty
---
Hi Johan,

following the output of usb-devices for the compositions

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#= 10 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1050 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 7 Cfg#= 1 Atr=80 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#= 11 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1051 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 2 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim
I:  If#= 3 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 7 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#=  9 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1052 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=ef(misc ) Sub=04 Prot=01 Driver=rndis_host
I:  If#= 1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=rndis_host
I:  If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 7 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#= 12 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1053 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 2 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether
I:  If#= 3 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 7 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

Thanks,
Daniele
---
 drivers/usb/serial/option.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 38e920ac7f82..10ac3610d922 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1154,6 +1154,14 @@ static const struct usb_device_id option_ids[] = {
  .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) | RSVD(3) },
{ USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 
TELIT_PRODUCT_LE922_USBCFG5, 0xff),
  .driver_info = RSVD(0) | RSVD(1) | NCTRL(2) | RSVD(3) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1050, 0xff),/* 
Telit FN980 (rmnet) */
+ .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1051, 0xff),/* 
Telit FN980 (MBIM) */
+ .driver_info = NCTRL(0) | RSVD(1) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1052, 0xff),/* 
Telit FN980 (RNDIS) */
+ .driver_info = NCTRL(2) | RSVD(3) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1053, 0xff),/* 
Telit FN980 (ECM) */
+ .driver_info = NCTRL(0) | RSVD(1) },
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_ME910),
  .driver_info = NCTRL(0) | RSVD(1) | RSVD(3) },
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_ME910_DUAL_MODEM),
-- 
2.17.1

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

2019-09-23 Thread Charles Yeh

Johan Hovold  於 2019年9月20日 週五 下午3:56寫道：
> Yes, that's better, but you're mixing register addresses, bit values and
> masks above. Perhaps things would be more clear if you but a _REG suffix
> on the register defines and order things as follows:
>
> #define PL2303_HXN__REG  0xX1
> #define PL2303_HXN___MASK 0xY1
> #define PL2303_HXN___  0xZ1
>
> #define PL2303_HXN__REG  0xX2
> #define PL2303_HXN___MASK 0xY2
> #define PL2303_HXN___  0xZ2
>
> The idea is simply to keep related defines together and not mix
> register address, masks and value defines.
>
> Keep registers sorted by address, and bit masks and values by bit order
> (e.g. MSB first).

Thank you for your reply

Charles Ans:
The new define is follows

#define PL2303_READ_TYPE_HX_STATUS0x8080

#define PL2303_HXN_FLOWCTRL_REG0x0A
#define PL2303_HXN_FLOWCTRL_MASK0x1C
#define PL2303_HXN_FLOWCTRL_NONE0x1C
#define PL2303_HXN_FLOWCTRL_RTS_CTS0x18
#define PL2303_HXN_FLOWCTRL_XON_XOFF0x0C

#define PL2303_HXN_RESET_REG0x07
#define PL2303_HXN_RESET_UPSTREAM_PIPE0x02
#define PL2303_HXN_RESET_DOWNSTREAM_PIPE0x01


> Yes, but that doesn't imply that you need to read back the old value.
>
> I'm assuming it would either always read back as 0, or you would read
> back the previous value written, which means you could end up resetting
> something you did not intend.
>
> Either way, you should not read back the current value when resetting
> the data pipes.
>

Thank you for your reply

Charles Ans:
The new code is follows

pl2303_vendor_write(serial,
PL2303_HXN_RESET_REG,
PL2303_HXN_RESET_UPSTREAM_PIPE |
PL2303_HXN_RESET_DOWNSTREAM_PIPE);


Please confirm the above new define & code..
If there is no problem.. I will write a new Patch file.

Charles.

Re: [PATCH 1/1] usb: serial: option: add Telit FN980 compositions

2019-09-23 Thread Greg KH

On Mon, Sep 23, 2019 at 11:51:42AM +0200, Daniele Palmas wrote:
> This patch adds the following Telit FN980 compositions:
> 
> 0x1050: tty, adb, rmnet, tty, tty, tty, tty
> 0x1051: tty, adb, mbim, tty, tty, tty, tty
> 0x1052: rndis, tty, adb, tty, tty, tty, tty
> 0x1053: tty, adb, ecm, tty, tty, tty, tty
> ---

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/SubmittingPatches and resend it after
  adding that line.  Note, the line needs to be in the body of the
  email, before the patch, not at the bottom of the patch or in the
  email signature.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted

2019-09-23 Thread Mathias Nyman


On 23.9.2019 10.08, alex zheng wrote:

Hi, balbi,

Thank you for your reply~

Felipe Balbi  于2019年9月23日周一 下午1:36写道：



Hi,

(it helps when you Cc correct maintainers ;-)

alex zheng  writes:


Hi all,

I am a user of dwc3 USB host controller, I found there are some
confused behavior of trb event on this controller.
When I run a raw USB data transfer(run bulk in&out transfer with
libusb) and iperf3(over rndis) at the same time,
there are some strange interrupts occurs and make the driver report
error(ERROR DMA transfer).
And:


So dwc3 is workingo n host mode. Which platform is this?


This is our self-design platform (ARM v7 cpu core  with synopsys DWC
USB3.0 controller).
version info: Linux localhost 4.9.130-645692-g6ecde01-dirty #394 SMP
PREEMPT Sun Sep 22 15:10:51 CST 2019 armv7l




1. this problem only hapened in USB SS mode
2. this problem seems not hapen when I run same test case with other
xhci controller(such as asmedia/intel pcie xhci controller) on PC.
3. the kernel version is 4.9.130


Have you tried a more recent kernel? 4.9 is really ancient. Please try
v5.3.


Our platform only support 4.9 kernel now, and it may take a lot of
work to do to support the recent kernel.
Are there any causes may lead the link TRB trigger a interrupt when
the IOC bit is not setted?




I think this may be a hw bug of DWC3 USB controller, could anyone
please give me some help to debug this problem？

The detail log see as below:
[  131.074102] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto: xHCI
handle event, 8000
27630 [  131.074109] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
process trans event : ep_index = 11, event_dma = 1eb13e90
27631 [  131.074117] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
inc_deq, start trb dma = 1eb13e90, dequeue_p = e482ce90, trb_free num
= 1871, ring type = 2
27632 [  131.074123] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
inc_deq 111, start trb dma = 1eb13ea0, dequeue_p = e482cea0, trb_free
num = 1872, ring type = 2
27633 [  131.074130] c0 3 (ksoftirqd/0) xhci-hcd xh[  133.057617] c0 3
(ksoftirqd/0) xhci-hcd xhci-hcd.0.auto: ERROR Transfer event TRB DMA
ptr not part of current TD ep_index 16 comp_code 1
27634 [  133.059312] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
Looking for event-dma 1eb0fff0 trb-start 1eb1
trb-end 1eb1 seg-start 1eb1 seg-end
1eb10ff0
27635 [  133.066215] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
comp_code 1
27636 [  133.067908] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
Looking for event-dma 1eb1 trb-start 1eb10230
trb-end 1eb10230 seg-start 1eb1 seg-end
1eb10ff0
27637 [  133.070572] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
comp_code 1
27638 [  133.072260] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
Looking for event-dma 1eb10010 trb-start 1eb10230
trb-end 1eb10230 seg-start 1eb1 seg-end
1eb10ff0
27639 [  133.075052] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
comp_code 1
27640 [  133.076739] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
Looking for event-dma 1eb10020 trb-start 1eb10230
trb-end 1eb10230 seg-start 1eb1 seg-end
1eb10ff0
27641 [  133.079472] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
comp_code 1
27642 [  133.081159] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
Looking for event-dma 1eb10030 trb-start 1eb10230
trb-end 1eb10230 seg-start 1eb1 seg-end
1eb10ff0
27643 [  133.083896] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
comp_code 1
27644 [  133.085584] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
Looking for event-dma 1eb10040 trb-start 1eb10230
trb-end 1eb10230 seg-start 1eb1 seg-end
1eb10ff0
27645 [  133.088328] c0 3 (ksoftirqd/0) xhci-hcd xhci-hcd.0.auto:
ERROR Transfer event TRB DMA ptr not part of current TD ep_index 16
comp_code 1

1. According these logs above the link trb whose address is 0x1eb0fff0
occurs a transfer event, but this DMA address is not in the trb ring,
then the driver report an error(and followed a few error logs witch
invalid DMA address).


To me it looks like the controller creates an extra successful transfer event
for the Link TRB.

The link TRB DMA that the event is pointing to is part of the ring, but not part
of next Transfer Descriptor (TD) xhci driver expects to handle.
The link TRB is the last TRB of the previous ring segment, The TD the xhci 
driver
expects is on the next segment.


2. I dump the data of the address(0x1eb0fff0) and find the IOC bit is
not set, see as below:
# dump_reg.sh 0x1eb0

Re: Linux Keyspan USB serial driver ignoring XOFF

2019-09-23 Thread Johan Hovold

On Fri, Sep 20, 2019 at 02:49:51PM -0500, John Goerzen wrote:
> Hello,
> 
> I have narrowed down the issue I'm about to describe to keyspan.c; a
> Digi Edgeport/1 with identical configuration works fine.
> 
> I am configuring a Raspberry Pi running 4.19.66 (though keyspan.c hasn't
> changed since 2017) to talk to a real-live vt420.  Configuring agetty
> with systemd worked easy enough, but I found that XON/XOFF wasn't
> working.  stty -a shows ixon and ixoff as appropriate, but sending
> Ctrl-S (tested from multiple ways of sending) had no effect on output in
> bash, or scrolling output.  (Emacs, though, recognized it as the start
> of a search, so I knew it was getting down the line.)
> 
> 
> After a great deal of head-scratching on this, I went to look at the
> kernel source and found that keyspan.c does not appear to be honoring
> XOFF.  I also have a Digi Edgeport/1 on hand (which uses io_ti.c), and
> when I swapped to that, everything worked fine - Ctrl-S caused the
> expected pause.
> 
> As far as I can tell, keyspan.c simply never implemented handling of
> XOFF, but you guys are the experts there.

That's correct, the driver does not support software flow control even
if the hardware seems to have some support for assisted XON/XOFF.

Would you mind testing the below patch which may be enough to turn
sw-flow control always on. If that works, I can probably find time to
cook up a proper patch to make this configurable later this week.

Johan


>From 55b46d78fe63f182923e4674659fa18f4624d6b8 Mon Sep 17 00:00:00 2001
From: Johan Hovold 
Date: Mon, 23 Sep 2019 12:14:56 +0200
Subject: [PATCH] USB: serial: keyspan: enable XON flow control

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/keyspan.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index d34779fe4a8d..934430cbdfc4 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -2118,7 +2118,7 @@ static int keyspan_usa26_send_setup(struct usb_serial 
*serial,
msg.setLcr = 0xff;
 
msg.ctsFlowControl = (p_priv->flow_control == flow_cts);
-   msg.xonFlowControl = 0;
+   msg.xonFlowControl = 1;
msg.setFlowControl = 0xff;
msg.forwardingLength = 16;
msg.xonChar = 17;
@@ -2234,7 +2234,7 @@ static int keyspan_usa28_send_setup(struct usb_serial 
*serial,
msg.parity = 0; /* XXX for now */
 
msg.ctsFlowControl = (p_priv->flow_control == flow_cts);
-   msg.xonFlowControl = 0;
+   msg.xonFlowControl = 1;
 
/* Do handshaking outputs, DTR is inverted relative to RTS */
msg.rts = p_priv->rts_state;
@@ -2388,7 +2388,7 @@ static int keyspan_usa49_send_setup(struct usb_serial 
*serial,
msg.setLcr = 0xff;
 
msg.ctsFlowControl = (p_priv->flow_control == flow_cts);
-   msg.xonFlowControl = 0;
+   msg.xonFlowControl = 1;
msg.setFlowControl = 0xff;
 
msg.forwardingLength = 16;
@@ -2690,7 +2690,7 @@ static int keyspan_usa67_send_setup(struct usb_serial 
*serial,
msg.setLcr = 0xff;
 
msg.ctsFlowControl = (p_priv->flow_control == flow_cts);
-   msg.xonFlowControl = 0;
+   msg.xonFlowControl = 1;
msg.setFlowControl = 0xff;
msg.forwardingLength = 16;
msg.xonChar = 17;
-- 
2.23.0

[PATCH v2 1/1] usb: serial: option: add Telit FN980 compositions

2019-09-23 Thread Daniele Palmas

This patch adds the following Telit FN980 compositions:

0x1050: tty, adb, rmnet, tty, tty, tty, tty
0x1051: tty, adb, mbim, tty, tty, tty, tty
0x1052: rndis, tty, adb, tty, tty, tty, tty
0x1053: tty, adb, ecm, tty, tty, tty, tty

Signed-off-by: Daniele Palmas 
---
Sorry, forgot to add Signed-off-by tag...

v2: added Signed-off-by tag

Following the output of usb-devices for the compositions

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#= 10 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1050 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 7 Cfg#= 1 Atr=80 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#= 11 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1051 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 2 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim
I:  If#= 3 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 7 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#=  9 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1052 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=ef(misc ) Sub=04 Prot=01 Driver=rndis_host
I:  If#= 1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=rndis_host
I:  If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 7 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

T:  Bus=03 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#= 12 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=1053 Rev=04.14
S:  Manufacturer=Telit Wireless Solutions
S:  Product=FN980m
S:  SerialNumber=270b8241
C:  #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
I:  If#= 2 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether
I:  If#= 3 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 7 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option

---
 drivers/usb/serial/option.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 38e920ac7f82..10ac3610d922 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1154,6 +1154,14 @@ static const struct usb_device_id option_ids[] = {
  .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) | RSVD(3) },
{ USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 
TELIT_PRODUCT_LE922_USBCFG5, 0xff),
  .driver_info = RSVD(0) | RSVD(1) | NCTRL(2) | RSVD(3) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1050, 0xff),/* 
Telit FN980 (rmnet) */
+ .driver_info = NCTRL(0) | RSVD(1) | RSVD(2) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1051, 0xff),/* 
Telit FN980 (MBIM) */
+ .driver_info = NCTRL(0) | RSVD(1) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1052, 0xff),/* 
Telit FN980 (RNDIS) */
+ .driver_info = NCTRL(2) | RSVD(3) },
+   { USB_DEVICE_INTERFACE_CLASS(TELIT_VENDOR_ID, 0x1053, 0xff),/* 
Telit FN980 (ECM) */
+ .driver_info = NCTRL(0) | RSVD(1) },
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_ME910),
  .driver_info = NCTRL(0) | RSVD(1) | RSVD(3) },
{ USB_

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

2019-09-23 Thread Johan Hovold

On Mon, Sep 23, 2019 at 05:53:34PM +0800, Charles Yeh wrote:
> Johan Hovold  於 2019年9月20日 週五 下午3:56寫道：
> > Yes, that's better, but you're mixing register addresses, bit values and
> > masks above. Perhaps things would be more clear if you but a _REG suffix
> > on the register defines and order things as follows:
> >
> > #define PL2303_HXN__REG  0xX1
> > #define PL2303_HXN___MASK 0xY1
> > #define PL2303_HXN___  0xZ1
> >
> > #define PL2303_HXN__REG  0xX2
> > #define PL2303_HXN___MASK 0xY2
> > #define PL2303_HXN___  0xZ2
> >
> > The idea is simply to keep related defines together and not mix
> > register address, masks and value defines.
> >
> > Keep registers sorted by address, and bit masks and values by bit order
> > (e.g. MSB first).
> 
> Thank you for your reply
> 
> Charles Ans:
> The new define is follows
> 
> #define PL2303_READ_TYPE_HX_STATUS0x8080
> 
> #define PL2303_HXN_FLOWCTRL_REG0x0A
> #define PL2303_HXN_FLOWCTRL_MASK0x1C
> #define PL2303_HXN_FLOWCTRL_NONE0x1C
> #define PL2303_HXN_FLOWCTRL_RTS_CTS0x18
> #define PL2303_HXN_FLOWCTRL_XON_XOFF0x0C
> 
> #define PL2303_HXN_RESET_REG0x07
> #define PL2303_HXN_RESET_UPSTREAM_PIPE0x02
> #define PL2303_HXN_RESET_DOWNSTREAM_PIPE0x01

That looks much better. But please move the reset defines above the
flow control ones to keep the registers sorted by address (0x7 < 0xa).

> > Yes, but that doesn't imply that you need to read back the old value.
> >
> > I'm assuming it would either always read back as 0, or you would read
> > back the previous value written, which means you could end up resetting
> > something you did not intend.
> >
> > Either way, you should not read back the current value when resetting
> > the data pipes.
> >
> 
> Thank you for your reply
> 
> Charles Ans:
> The new code is follows
> 
> pl2303_vendor_write(serial,
> PL2303_HXN_RESET_REG,
> PL2303_HXN_RESET_UPSTREAM_PIPE |
> PL2303_HXN_RESET_DOWNSTREAM_PIPE);
> 
> 
> Please confirm the above new define & code..
> If there is no problem.. I will write a new Patch file.

Also looks good, thanks. Just move the reset define block as mentioned
above.

Johan

Re: [PATCH] ARM: imx25: provide a fixed regulator for usb phys

2019-09-23 Thread Uwe Kleine-König

On Wed, Jul 24, 2019 at 03:09:39PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Thu, Jun 27, 2019 at 03:15:10AM +, Peter Chen wrote:
> >  
> > > On 19-06-26 02:40, Peter Chen wrote:
> > > >
> > > > > Subject: [PATCH] ARM: imx25: provide a fixed regulator for usb phys
> > > > >
> > > > > The usb phys are internal to the SoC and so it their 5V supply. With
> > > > > this regulator added explicitly the following (harmless) boot 
> > > > > messages go away:
> > > > >
> > > > >   usb_phy_generic usbphy:usb-phy@0: usbphy:usb-phy@0 supply vcc 
> > > > > not found, using dummy regulator
> > > > >   usb_phy_generic usbphy:usb-phy@1: usbphy:usb-phy@1 supply vcc 
> > > > > not found, using dummy regulator
> > > > >
> > > >
> > > > To eliminate the warning message, I suggest doing below changes, as
> > > > vcc supply is not mandatory.
> > > >
> > > > diff --git a/drivers/usb/phy/phy-generic.c
> > > > b/drivers/usb/phy/phy-generic.c index a53b89be5324..01a5ff1a0515
> > > > 100644
> > > > --- a/drivers/usb/phy/phy-generic.c
> > > > +++ b/drivers/usb/phy/phy-generic.c
> > > > @@ -275,7 +275,7 @@ int usb_phy_gen_create_phy(struct device *dev, 
> > > > struct usb_phy_generic *nop,
> > > > }
> > > > }
> > > >
> > > > -   nop->vcc = devm_regulator_get(dev, "vcc");
> > > > +   nop->vcc = devm_regulator_get_optional(dev, "vcc");
> > > 
> > > Is the regulator optional? IMHO this shouldn't be the fix. I think the 
> > > right fix is Uwe's
> > > approach.
> > > 
> > 
> > Add Felipe.
> > 
> > Some USB PHY's power are from the core system's power (eg, DDR), and some 
> > are
> > fixed at the board and no switch for it. So, it is transparent for software 
> > at some cases.
> 
> It's not clear to me how to proceed. There are two opposing opinions and
> I don't know enough about USB on mx25 to judge myself.
> 
> Felipe?

This thread is still open in my inbox. Felipe, how can I lure you into
giving your opinion?

My original suggestion can be seen at
https://lore.kernel.org/linux-usb/20190625100412.11815-1-u.kleine-koe...@pengutronix.de/,
Peter's alternative is still in the quotes above. Which is the
right/better one?

Best regards and thanks,
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

2019-09-23 Thread Charles Yeh

Johan Hovold  於 2019年9月23日 週一 下午6:24寫道：
> That looks much better. But please move the reset defines above the
> flow control ones to keep the registers sorted by address (0x7 < 0xa).

Thank you for your reply

Charles Ans:
The new define is follows

#define PL2303_READ_TYPE_HX_STATUS0x8080

#define PL2303_HXN_RESET_REG0x07
#define PL2303_HXN_RESET_UPSTREAM_PIPE0x02
#define PL2303_HXN_RESET_DOWNSTREAM_PIPE0x01

#define PL2303_HXN_FLOWCTRL_REG0x0A
#define PL2303_HXN_FLOWCTRL_MASK0x1C
#define PL2303_HXN_FLOWCTRL_NONE0x1C
#define PL2303_HXN_FLOWCTRL_RTS_CTS0x18
#define PL2303_HXN_FLOWCTRL_XON_XOFF0x0C


> Also looks good, thanks. Just move the reset define block as mentioned
> above.

Thank you for your reply


Please confirm the above new define
If there is no problem.. I will write a new Patch file.

Charles.

Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted

2019-09-23 Thread Felipe Balbi

Hi Alex,

alex zheng  writes:
>> > I am a user of dwc3 USB host controller, I found there are some
>> > confused behavior of trb event on this controller.
>> > When I run a raw USB data transfer(run bulk in&out transfer with
>> > libusb) and iperf3(over rndis) at the same time,
>> > there are some strange interrupts occurs and make the driver report
>> > error(ERROR DMA transfer).
>> > And:
>>
>> So dwc3 is workingo n host mode. Which platform is this?
>
> This is our self-design platform (ARM v7 cpu core  with synopsys DWC
> USB3.0 controller).
> version info: Linux localhost 4.9.130-645692-g6ecde01-dirty #394 SMP
> PREEMPT Sun Sep 22 15:10:51 CST 2019 armv7l

This is a brand new design and you're waking it up on v4.9? Could've
tracked upstream more closely, IMHO.

>> > 1. this problem only hapened in USB SS mode
>> > 2. this problem seems not hapen when I run same test case with other
>> > xhci controller(such as asmedia/intel pcie xhci controller) on PC.
>> > 3. the kernel version is 4.9.130
>>
>> Have you tried a more recent kernel? 4.9 is really ancient. Please try
>> v5.3.
>
> Our platform only support 4.9 kernel now, and it may take a lot of
> work to do to support the recent kernel.

In that case, I'm afraid you're on your own. Have a look at known
synopsys errata.

On a side-node, getting a cortex-A7 to boot with upstream kernel should
be only about adding a DeviceTree nowadays. Remember that for Linux to
boot, all you need is a system timer and UART. If you're using ARM IP
for interrupts, timers, etc, it should be really straight forward to
boot on v5.3

> Are there any causes may lead the link TRB trigger a interrupt when
> the IOC bit is not setted?

No idea, perhaps you should have a deeper look at both Synopsys databook
and xHCI specification.

In any case, v4.9 is really old.

Good luck

-- 
balbi

Re: Event ring is full when do iozone test on UAS storage

2019-09-23 Thread Mathias Nyman


On 19.9.2019 16.59, Suwan Kim wrote:

On Thu, Sep 19, 2019 at 05:54:25PM +0800, Peter Chen wrote:

On 17.9.2019 12.55, Peter Chen wrote:


I met "event ring full error" like below, this error is met when
I do iozone test on UAS storage at v4.19.35 kernel, but not meet
this error at linux-next tree (08/24). The same host and test
UAS storage device are used. This issue is due to xhci_handle_event
does not return 0 long time, maybe the xHC speed is fast enough
at that time. If I force the xhci_handle_event only run 100 times
before update ERST dequene pointer, it will not occur this error.
I did not  see any changes for xhci_handle_event at the latest code,
so in theory, it should have this issue too. Do you think if we need
to improve xhci_handle_event to avoid event ring?



The root cause is UAS protocol is very fast
protocol, the
other threads at non-CPU0 will add TRBs during we are handling event, so if
hardware (xHC) has always consumed TD the non-CPU0s are adding,
the ERST dequene pointer never get change to update, then this
"event ring full" error will occur.

The one reason why v4.19 has this issue is the max request length is larger
than the latest kernel. At v4.19, it is 512KB, At the latest kernel,
it is 256 KB.
see /sys/block/sda/queue/max_sectors_kb.
When I change max_sectors_kb as smaller value, the test will be more smooth.
Besides, At v4.19, the UAS completion handler seems take more time
compares to the latest kernel.

I suggest adding threshold flag for event ring when it closes to full
since we can't
avoid USB3 use cases when the throughput is high, but the system is a
little slow.
Do you agree?


I agree that it makes sense to force a ERDP write after handling some amount
of events, it can solve some event ring full issues, but not the fact that
we spend a lot of time in the interrupt handler.


Ok, I will proposal one patch to fix event ring full issue.


Great





Your logs show that you have TDs containing up to 128 TRBs.
When a TD like that finishes the driver will increase the sw dequeue pointer of 
the
transfer ring one by one until we reach the end of the TD.

This means we call inc_deq() function 128 times in interrupt context, and each 
time
do a few comparisons. According to traces this takes ~120us. There might be some
tracing overhead but this could anyway be done in a saner way.

I'll look into this



Since we use hard irq for xHCI, for high performance protocol, it may hard to
reduce interrupt context time since we have lots of request handling,
cache operation,
and completion are interrupt context.


I'm working on one improvement at the moment, it would be great if you could 
test
it out once i get it done.




Other things to consider in addition to writing ERDP and fixing the inc_dec() 
loop is
increasing event ring size (adding more than current 1 segment), and support 
giving
back URBs in tasklet by adding the HCD_BH flag to xhci hcd.



So, Mathias, which solutions do you prefer:
- One way to quit xhci_handle_event if there are enough events have handled
- Change giveback as bottom half




I think it makes sense to add all patches.

Two that reduces time spent in interrupt handler, and thus makes it less likely 
to
trigger the event ring full issue. This would be the "giving back urb in 
tasklet"
patch by Suwan Kim, and my loop removal patch. These make sense anyway even 
without
the event ring full issue.

And that one Peter suggested which writes back the ERDP if some limit of 
consecutive
event were handled, this will prevent most of event ring full issues, not 
related to
time spent in interrupt handler. There is probably no need to exit interrupt 
handler
with this solution, just make sure the ERDP gets written back every now and 
then.


Hi Peter and Mathias,

Please consider the patch that deals with the bottom half of
xhci.

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

I think it is better to use tasklet as bottom half in xhci because
HCD layer supports tasklet bottom half and other host controller
dirvers use this feature to defer URB processing.

Moreover, some devices (1Gbit USB ethernet controller) can show
improved performance with tasklet bottom half in xhci.



Yes, this is the patch I have in mind, I have it in a internal
branch where it gets some testing exposure. So far it hasn't triggered any new 
issues.

-Mathias

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

2019-09-23 Thread Johan Hovold

On Mon, Sep 23, 2019 at 06:35:19PM +0800, Charles Yeh wrote:
> Johan Hovold  於 2019年9月23日 週一 下午6:24寫道：
> > That looks much better. But please move the reset defines above the
> > flow control ones to keep the registers sorted by address (0x7 < 0xa).
> 
> Thank you for your reply
> 
> Charles Ans:
> The new define is follows
> 
> #define PL2303_READ_TYPE_HX_STATUS0x8080
> 
> #define PL2303_HXN_RESET_REG0x07
> #define PL2303_HXN_RESET_UPSTREAM_PIPE0x02
> #define PL2303_HXN_RESET_DOWNSTREAM_PIPE0x01
> 
> #define PL2303_HXN_FLOWCTRL_REG0x0A
> #define PL2303_HXN_FLOWCTRL_MASK0x1C
> #define PL2303_HXN_FLOWCTRL_NONE0x1C
> #define PL2303_HXN_FLOWCTRL_RTS_CTS0x18
> #define PL2303_HXN_FLOWCTRL_XON_XOFF0x0C
> 
> 
> > Also looks good, thanks. Just move the reset define block as mentioned
> > above.
> 
> Thank you for your reply
> 
> 
> Please confirm the above new define
> If there is no problem.. I will write a new Patch file.

Yes, the above looks good.

Johan

[RFC PATCH] usb: typec: ucsi: ccg: Remove run_isr flag

2019-09-23 Thread Heikki Krogerus

There is no need to try to prevent the extra ucsi_notify()
that runtime resuming the device will cause.

This fixes potential deadlock. Both ccg_read() and
ccg_write() are called with the mutex already taken at least
from ccg_send_command(). In ccg_read() and ccg_write, the
mutex is only acquired so that run_isr flag can be set.

Signed-off-by: Heikki Krogerus 
---
Hi Ajay,

Before going forward with this I would like to get confirmation from
you that it is OK, and that I'm not missing anything. I did not see
any real purpose for that run_isr flag. The only thing that I can see
it preventing is an extra ucsi_notify() call caused by the waking of
the controller, but that should not be a problem. Is there any other
reason why the flag is there?

If the driver works fine without the flag, then let's just drop it.
The deadlock will need to be fixed in any case.

thanks,

---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 40 ++-
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c 
b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 907e20e1a71e..167cb6367198 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -195,7 +195,6 @@ struct ucsi_ccg {
 
/* fw build with vendor information */
u16 fw_build;
-   bool run_isr; /* flag to call ISR routine during resume */
struct work_struct pm_work;
 };
 
@@ -224,18 +223,6 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 
*data, u32 len)
if (quirks && quirks->max_read_len)
max_read_len = quirks->max_read_len;
 
-   if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
-   uc->fw_version <= CCG_OLD_FW_VERSION) {
-   mutex_lock(&uc->lock);
-   /*
-* Do not schedule pm_work to run ISR in
-* ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
-* since we are already in ISR path.
-*/
-   uc->run_isr = false;
-   mutex_unlock(&uc->lock);
-   }
-
pm_runtime_get_sync(uc->dev);
while (rem_len > 0) {
msgs[1].buf = &data[len - rem_len];
@@ -278,18 +265,6 @@ static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 
*data, u32 len)
msgs[0].len = len + sizeof(rab);
msgs[0].buf = buf;
 
-   if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
-   uc->fw_version <= CCG_OLD_FW_VERSION) {
-   mutex_lock(&uc->lock);
-   /*
-* Do not schedule pm_work to run ISR in
-* ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
-* since we are already in ISR path.
-*/
-   uc->run_isr = false;
-   mutex_unlock(&uc->lock);
-   }
-
pm_runtime_get_sync(uc->dev);
status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
if (status < 0) {
@@ -1130,7 +1105,6 @@ static int ucsi_ccg_probe(struct i2c_client *client,
uc->ppm.sync = ucsi_ccg_sync;
uc->dev = dev;
uc->client = client;
-   uc->run_isr = true;
mutex_init(&uc->lock);
INIT_WORK(&uc->work, ccg_update_firmware);
INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
@@ -1229,7 +1203,6 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct ucsi_ccg *uc = i2c_get_clientdata(client);
-   bool schedule = true;
 
/*
 * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue
@@ -1237,17 +1210,8 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
 * Schedule a work to call ISR as a workaround.
 */
if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
-   uc->fw_version <= CCG_OLD_FW_VERSION) {
-   mutex_lock(&uc->lock);
-   if (!uc->run_isr) {
-   uc->run_isr = true;
-   schedule = false;
-   }
-   mutex_unlock(&uc->lock);
-
-   if (schedule)
-   schedule_work(&uc->pm_work);
-   }
+   uc->fw_version <= CCG_OLD_FW_VERSION)
+   schedule_work(&uc->pm_work);
 
return 0;
 }
-- 
2.23.0

Re: Event ring is full when do iozone test on UAS storage

2019-09-23 Thread Mathias Nyman


On 23.9.2019 14.19, Mathias Nyman wrote:

On 19.9.2019 16.59, Suwan Kim wrote:

On Thu, Sep 19, 2019 at 05:54:25PM +0800, Peter Chen wrote:

On 17.9.2019 12.55, Peter Chen wrote:


I met "event ring full error" like below, this error is met when
I do iozone test on UAS storage at v4.19.35 kernel, but not meet
this error at linux-next tree (08/24). The same host and test
UAS storage device are used. This issue is due to xhci_handle_event
does not return 0 long time, maybe the xHC speed is fast enough
at that time. If I force the xhci_handle_event only run 100 times
before update ERST dequene pointer, it will not occur this error.
I did not  see any changes for xhci_handle_event at the latest code,
so in theory, it should have this issue too. Do you think if we need
to improve xhci_handle_event to avoid event ring?



The root cause is UAS protocol is very fast
protocol, the
other threads at non-CPU0 will add TRBs during we are handling event, so if
hardware (xHC) has always consumed TD the non-CPU0s are adding,
the ERST dequene pointer never get change to update, then this
"event ring full" error will occur.

The one reason why v4.19 has this issue is the max request length is larger
than the latest kernel. At v4.19, it is 512KB, At the latest kernel,
it is 256 KB.
see /sys/block/sda/queue/max_sectors_kb.
When I change max_sectors_kb as smaller value, the test will be more smooth.
Besides, At v4.19, the UAS completion handler seems take more time
compares to the latest kernel.

I suggest adding threshold flag for event ring when it closes to full
since we can't
avoid USB3 use cases when the throughput is high, but the system is a
little slow.
Do you agree?


I agree that it makes sense to force a ERDP write after handling some amount
of events, it can solve some event ring full issues, but not the fact that
we spend a lot of time in the interrupt handler.


Ok, I will proposal one patch to fix event ring full issue.


Great





Your logs show that you have TDs containing up to 128 TRBs.
When a TD like that finishes the driver will increase the sw dequeue pointer of 
the
transfer ring one by one until we reach the end of the TD.

This means we call inc_deq() function 128 times in interrupt context, and each 
time
do a few comparisons. According to traces this takes ~120us. There might be some
tracing overhead but this could anyway be done in a saner way.

I'll look into this



Since we use hard irq for xHCI, for high performance protocol, it may hard to
reduce interrupt context time since we have lots of request handling,
cache operation,
and completion are interrupt context.


I'm working on one improvement at the moment, it would be great if you could 
test
it out once i get it done.


Got something  done on top of 5.3.
It's in my tree in the irqoff_optimization branch

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git  
irqoff_optimization

Does it help at all in your case?

-Mathias

[PATCH v2] xhci: Prevent deadlock when xhci adapter breaks during init

2019-09-23 Thread Bill Kuzeja

The system can hit a deadlock if an xhci adapter breaks while initializing.
The deadlock is between two threads: thread 1 is tearing down the
adapter and is stuck in usb_unlocked_disable_lpm waiting to lock the
hcd->handwidth_mutex. Thread 2 is holding this mutex (while still trying
to add a usb device), but is stuck in xhci_endpoint_reset waiting for a
stop or config command to complete. A reboot is required to resolve.

It turns out when calling xhci_queue_stop_endpoint and
xhci_queue_configure_endpoint in xhci_endpoint_reset, the return code is
not checked for errors. If the timing is right and the adapter dies just
before either of these commands get issued, we hang indefinitely waiting
for a completion on a command that didn't get issued.

This wasn't a problem before the following fix because we didn't send
commands in xhci_endpoint_reset:

commit f5249461b504 ("xhci: Clear the host side toggle manually when
endpoint is soft reset")

With the patch I am submitting, a duration test which breaks adapters
during initialization (and which deadlocks with the standard kernel) runs
without issue.

Fixes: f5249461b504 ("xhci: Clear the host side toggle manually when
endpoint is soft reset")

Cc: Mathias Nyman 
Cc: Torez Smith 

Signed-off-by: Bill Kuzeja 
Signed-off-by: Torez Smith 
---
 drivers/usb/host/xhci.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5008659..ed44ec2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3083,6 +3083,7 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
unsigned int ep_index;
unsigned long flags;
u32 ep_flag;
+   int err;
 
xhci = hcd_to_xhci(hcd);
if (!host_ep->hcpriv)
@@ -3142,7 +3143,17 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
xhci_free_command(xhci, cfg_cmd);
goto cleanup;
}
-   xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id, ep_index, 0);
+
+   err = xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id,
+   ep_index, 0);
+   if (err < 0) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
+   xhci_free_command(xhci, cfg_cmd);
+   xhci_dbg(xhci, "%s: Failed to queue stop ep command, %d ",
+   __func__, err);
+   goto cleanup;
+   }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -3156,8 +3167,16 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
   ctrl_ctx, ep_flag, ep_flag);
xhci_endpoint_copy(xhci, cfg_cmd->in_ctx, vdev->out_ctx, ep_index);
 
-   xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
+   err = xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
  udev->slot_id, false);
+   if (err < 0) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
+   xhci_free_command(xhci, cfg_cmd);
+   xhci_dbg(xhci, "%s: Failed to queue config ep command, %d ",
+   __func__, err);
+   goto cleanup;
+   }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
-- 
1.8.3.1

Re: [PATCH v2] xhci: Prevent deadlock when xhci adapter breaks during init

2019-09-23 Thread Mathias Nyman


On 23.9.2019 16.58, Bill Kuzeja wrote:

The system can hit a deadlock if an xhci adapter breaks while initializing.
The deadlock is between two threads: thread 1 is tearing down the
adapter and is stuck in usb_unlocked_disable_lpm waiting to lock the
hcd->handwidth_mutex. Thread 2 is holding this mutex (while still trying
to add a usb device), but is stuck in xhci_endpoint_reset waiting for a
stop or config command to complete. A reboot is required to resolve.

It turns out when calling xhci_queue_stop_endpoint and
xhci_queue_configure_endpoint in xhci_endpoint_reset, the return code is
not checked for errors. If the timing is right and the adapter dies just
before either of these commands get issued, we hang indefinitely waiting
for a completion on a command that didn't get issued.

This wasn't a problem before the following fix because we didn't send
commands in xhci_endpoint_reset:

commit f5249461b504 ("xhci: Clear the host side toggle manually when
 endpoint is soft reset")

With the patch I am submitting, a duration test which breaks adapters
during initialization (and which deadlocks with the standard kernel) runs
without issue.

Fixes: f5249461b504 ("xhci: Clear the host side toggle manually when
 endpoint is soft reset")

Cc: Mathias Nyman 
Cc: Torez Smith 

Signed-off-by: Bill Kuzeja 
Signed-off-by: Torez Smith 
---


Thanks, adding to queue

-Mathias

Re: [PATCH] Check for changed device descriptors when a connection-change occurs before validating the connection.

2019-09-23 Thread Alan Stern

On Fri, 20 Sep 2019, David Heinzelmann wrote:

> On Fri, Sep 20, 2019 at 02:15:38PM +0200, Greg KH wrote:
> > On Fri, Sep 20, 2019 at 03:17:26PM +0200, David Heinzelmann wrote:
> > > Hi,
> > > 
> > > sorry for the wrong patch format.
> > 
> > No problem, that's normal.  But please do not top-post on linux mailing
> > lists.
> > 
> > > I am trying to detect a change. At the moment I think the change could be 
> > > ignored if a
> > > port connection-change occurs and the port status has again the 
> > > 'PORT_CONNECTION' bit set. 
> > > 
> > > I have a fx3 device which does a re-enumeration after a firmware 
> > > download. This is working
> > > as expected and I am seeing a 'remove event' and a 'add event' monitoring 
> > > via udevadm. But
> > > if I connect multiple devices at the same time via an usb hub I am 
> > > sometimes not receiving
> > > a 'remove event' and 'add event' for a single device.
> > 
> > Sounds like a broken hub :)
> > 
> 
> I tried different hubs but I forgot to mention that it is also possible to 
> trigger the issue
> without a hub if I reboot the devices via software at the same time. 
> 
> > > I think the problem could be that when a device disconnects and the port 
> > > connection-change
> > > occurs and before the 'PORT_CONNECTION' bit is checked the device could 
> > > already be
> > > reconnected and the 'PORT_CONNECTION' bit is set. Therefore I think it is 
> > > not correct to
> > > resuscitate the exisiting device.
> > 
> > Does your patch actually fix the issue?  When a fx3 device downloads
> > firmware and re-enumerates, it should come back as a totally different
> > device, which will fail this check, right?  So I don't see how this
> > fixes the issues with your devices.
> > 
> 
> With the patch I do not have the issue anymore. After re-enumerate the device 
> comes back with the same
> VID/PID but with a different device descriptor. Therefore the check will fail 
> and hub_port_connect is
> called which initiates a device disconnect and connect. Without this 
> 'reconnect' lsusb still shows me 
> the old device descriptor and I am not able to communicate with the device. 
> 
> > Unless all of the devices reset themselves at the same time and the hub
> > doesn't like that and can't notice that it happened?
> > 
> > If you use a different hub, does that work properly?
> > 
> 
> There is no difference if an other hub is used. It also happens without a hub 
> when the devices are
> rebooted via software. My thoughts on this is that when the device 
> re-enumerates and the device
> descriptor has changed a device disconnect and connect should be initiated 
> instead of doing nothing?
> 
> If I understand it correctly the resuscitation is used for handling port 
> enable-changes occured by EMI.
> But when the device is doing a re-enumeration there should be no 
> resuscitation.

I really don't understand this.

Your patch involves the case where there was a connect-change event but 
the port is still enabled.  Now maybe I've forgotten about one of the 
pathways, but it seems like that combination should never occur.

Certainly it shouldn't occur in your case.  The device disconnects and 
then reconnects with a new set of descriptors.  The disconnect should 
cause the port to be disabled, and the port should remain disabled 
after the reconnect occurs.  So how can your new code run in the first 
place?

Alan Stern

Re: KASAN: use-after-free Read in appledisplay_bl_get_brightness

2019-09-23 Thread Alexander Theißen

Hello,

just by looking at appledisplay_probe() I would say that the problem here is 
that usb_submit_urb() is called while the probe function can still fail. If it 
fails after submitting the urb the work item could already been scheduled from 
appledisplay_complete() and therefore operates on freed data.

Regards,
Alexander


> On 23. Sep 2019, at 16:31, syzbot 
>  wrote:
> 
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:e0bd8d79 usb-fuzzer: main usb gadget fuzzer driver
> git tree:   https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=13714ad960
> kernel config:  https://syzkaller.appspot.com/x/.config?x=8847e5384a16f66a
> dashboard link: https://syzkaller.appspot.com/bug?extid=495dab1f175edc9c2f13
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=1653d94560
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1517694560
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+495dab1f175edc9c2...@syzkaller.appspotmail.com
> 
> ==
> BUG: KASAN: use-after-free in appledisplay_bl_get_brightness+0x1ac/0x1c0 
> drivers/usb/misc/appledisplay.c:167
> Read of size 8 at addr 8881cfc576a0 by task kworker/1:0/17
> 
> CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.3.0+ #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS 
> Google 01/01/2011
> Workqueue: events appledisplay_work
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0xca/0x13e lib/dump_stack.c:113
> print_address_description+0x6a/0x32c mm/kasan/report.c:351
> __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
> kasan_report+0xe/0x12 mm/kasan/common.c:618
> appledisplay_bl_get_brightness+0x1ac/0x1c0 drivers/usb/misc/appledisplay.c:167
> appledisplay_work+0x36/0x160 drivers/usb/misc/appledisplay.c:187
> process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
> worker_thread+0x96/0xe20 kernel/workqueue.c:2415
> kthread+0x318/0x420 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> 
> Allocated by task 83:
> save_stack+0x1b/0x80 mm/kasan/common.c:69
> set_track mm/kasan/common.c:77 [inline]
> __kasan_kmalloc mm/kasan/common.c:493 [inline]
> __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:466
> kmalloc include/linux/slab.h:552 [inline]
> kzalloc include/linux/slab.h:748 [inline]
> appledisplay_probe+0x15a/0xb37 drivers/usb/misc/appledisplay.c:218
> usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
> really_probe+0x281/0x6d0 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
> bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:430
> __device_attach+0x217/0x360 drivers/base/dd.c:894
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:490
> device_add+0xae6/0x16f0 drivers/base/core.c:2201
> usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
> generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
> usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
> really_probe+0x281/0x6d0 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
> bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:430
> __device_attach+0x217/0x360 drivers/base/dd.c:894
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:490
> device_add+0xae6/0x16f0 drivers/base/core.c:2201
> usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
> hub_port_connect drivers/usb/core/hub.c:5098 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
> port_event drivers/usb/core/hub.c:5359 [inline]
> hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
> process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
> worker_thread+0x96/0xe20 kernel/workqueue.c:2415
> kthread+0x318/0x420 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> 
> Freed by task 83:
> save_stack+0x1b/0x80 mm/kasan/common.c:69
> set_track mm/kasan/common.c:77 [inline]
> __kasan_slab_free+0x130/0x180 mm/kasan/common.c:455
> slab_free_hook mm/slub.c:1423 [inline]
> slab_free_freelist_hook mm/slub.c:1474 [inline]
> slab_free mm/slub.c:3016 [inline]
> kfree+0xe4/0x2f0 mm/slub.c:3957
> appledisplay_probe+0x772/0xb37 drivers/usb/misc/appledisplay.c:312
> usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
> really_probe+0x281/0x6d0 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
> bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:430
> __device_attach+0x217/0x360 drivers/base/dd.c:894
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:490
> device_add+0xae6/0x16f0 drivers/base/core.c:2201
> usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
> gene

Re: [PATCH v3] usb: host: xhci: wait CNR when doing xhci resume

2019-09-23 Thread Mathias Nyman


On 18.9.2019 12.41, Rick Tseng wrote:

NVIDIA 3.1 xHCI card would lose power when moving power state into D3Cold.
Thus we need to wait CNR bit to clear when xhci resmue as xhci init.

Signed-off-by: Rick Tseng 


Thanks, added to queue.
Did minor changes to commit message and comment

-Mathias

Re: Linux Keyspan USB serial driver ignoring XOFF

2019-09-23 Thread John Goerzen

On Mon, Sep 23 2019, Johan Hovold wrote:

> That's correct, the driver does not support software flow control even
> if the hardware seems to have some support for assisted XON/XOFF.
>
> Would you mind testing the below patch which may be enough to turn
> sw-flow control always on. If that works, I can probably find time to
> cook up a proper patch to make this configurable later this week.

Hi Johan,

I gave it a try, but unfortunately that patch made no difference to the
behavior I was seeing.   I'm happy to do whatever debugging may be
helpful.

Thanks!

- John

[PATCH] USB: rio500: Remove Rio 500 kernel driver

2019-09-23 Thread Bastien Nocera

The Rio500 kernel driver has not been used by Rio500 owners since 2001
not long after the rio500 project added support for a user-space USB stack
through the very first versions of usbdevfs and then libusb.

Support for the kernel driver was removed from the upstream utilities
in 2008:
https://gitlab.freedesktop.org/hadess/rio500/commit/943f624ab721eb8281c287650fcc9e2026f6f5db

Cc: Cesar Miquel 
Signed-off-by: Bastien Nocera 
---
 Documentation/usb/rio.rst  | 109 --
 MAINTAINERS|   7 -
 arch/arm/configs/badge4_defconfig  |   1 -
 arch/arm/configs/corgi_defconfig   |   1 -
 arch/arm/configs/pxa_defconfig |   1 -
 arch/arm/configs/s3c2410_defconfig |   1 -
 arch/arm/configs/spitz_defconfig   |   1 -
 arch/mips/configs/mtx1_defconfig   |   1 -
 arch/mips/configs/rm200_defconfig  |   1 -
 drivers/usb/misc/Kconfig   |  10 -
 drivers/usb/misc/Makefile  |   1 -
 drivers/usb/misc/rio500.c  | 554 -
 drivers/usb/misc/rio500_usb.h  |  20 --
 13 files changed, 708 deletions(-)
 delete mode 100644 Documentation/usb/rio.rst
 delete mode 100644 drivers/usb/misc/rio500.c
 delete mode 100644 drivers/usb/misc/rio500_usb.h

diff --git a/Documentation/usb/rio.rst b/Documentation/usb/rio.rst
deleted file mode 100644
index ea73475471db..
--- a/Documentation/usb/rio.rst
+++ /dev/null
@@ -1,109 +0,0 @@
-
-Diamonds Rio
-
-
-Copyright (C) 1999, 2000 Bruce Tenison
-
-Portions Copyright (C) 1999, 2000 David Nelson
-
-Thanks to David Nelson for guidance and the usage of the scanner.txt
-and scanner.c files to model our driver and this informative file.
-
-Mar. 2, 2000
-
-Changes
-===
-
-- Initial Revision
-
-
-Overview
-
-
-This README will address issues regarding how to configure the kernel
-to access a RIO 500 mp3 player.
-Before I explain how to use this to access the Rio500 please be warned:
-
-.. warning::
-
-   Please note that this software is still under development.  The authors
-   are in no way responsible for any damage that may occur, no matter how
-   inconsequential.
-
-It seems that the Rio has a problem when sending .mp3 with low batteries.
-I suggest when the batteries are low and you want to transfer stuff that you
-replace it with a fresh one. In my case, what happened is I lost two 16kb
-blocks (they are no longer usable to store information to it). But I don't
-know if that's normal or not; it could simply be a problem with the flash
-memory.
-
-In an extreme case, I left my Rio playing overnight and the batteries wore
-down to nothing and appear to have corrupted the flash memory. My RIO
-needed to be replaced as a result.  Diamond tech support is aware of the
-problem.  Do NOT allow your batteries to wear down to nothing before
-changing them.  It appears RIO 500 firmware does not handle low battery
-power well at all.
-
-On systems with OHCI controllers, the kernel OHCI code appears to have
-power on problems with some chipsets.  If you are having problems
-connecting to your RIO 500, try turning it on first and then plugging it
-into the USB cable.
-
-Contact Information

-
-   The main page for the project is hosted at sourceforge.net in the following
-   URL: ;. You can also go to the project's
-   sourceforge home page at: ;.
-   There is also a mailing list: rio500-us...@lists.sourceforge.net
-
-Authors

-
-Most of the code was written by Cesar Miquel . Keith
-Clayton  is incharge of the PPC port and making sure
-things work there. Bruce Tenison  is adding support
-for .fon files and also does testing. The program will mostly sure be
-re-written and Pete Ikusz along with the rest will re-design it. I would
-also like to thank Tri Nguyen  who provided use
-with some important information regarding the communication with the Rio.
-
-Additional Information and userspace tools
-
-   http://rio500.sourceforge.net/
-
-
-Requirements
-
-
-A host with a USB port running a Linux kernel with RIO 500 support enabled.
-
-The driver is a module called rio500, which should be automatically loaded
-as you plug in your device. If that fails you can manually load it with
-
-  modprobe rio500
-
-Udev should automatically create a device node as soon as plug in your device.
-If that fails, you can manually add a device for the USB rio500::
-
-  mknod /dev/usb/rio500 c 180 64
-
-In that case, set appropriate permissions for /dev/usb/rio500 (don't forget
-about group and world permissions).  Both read and write permissions are
-required for proper operation.
-
-That's it.  The Rio500 Utils at: http://rio500.sourceforge.net should
-be able to access the rio500.
-
-Limits
-==
-
-You can use only a single rio500 device at a time with your computer.
-
-Bugs
-
-
-If you encounter any problems feel free to drop me an email.
-
-Bruce Tenison
-bteni...@dibbs.net
diff -

Re: musb: Could not flush host TX2 fifo: csr: 2403

2019-09-23 Thread Tony Lindgren

* Yegor Yefremov  [190821 01:24]:
> I'm moving our systems to the recent kernel and have encountered an
> older musb issue [1] that occurs with some 3G/4G modems. In this case
> it is  SIMCom 7600G-H. After this dump occurs the modem is still
> working though.
> 
> [   45.585644] [ cut here ]
> [   45.590785] WARNING: CPU: 0 PID: 242 at
> drivers/usb/musb/musb_host.c:115 musb_h_tx_flush_fifo+0x118/0x138
> [   45.600421] musb-hdrc musb-hdrc.0: Could not flush host TX2 fifo: csr: 2403
> [   45.607433] Modules linked in: 8021q garp stp mrp llc wl18xx wlcore
> mac80211 libarc4 sha256_generic cfg80211 wlcore_sdio c_can_platform
> omap_rng rng_core c_can can_dev at24 gpio_pca953x omap_wdt watchdog
> rtc_omap leds_gpio led_class
> [   45.628258] CPU: 0 PID: 242 Comm: ModemManager Not tainted 5.3.0-rc5 #2
> [   45.634920] Hardware name: Generic AM33XX (Flattened Device Tree)
> [   45.641092] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [   45.648910] [] (show_stack) from []
> (dump_stack+0xd8/0x110)
> [   45.656291] [] (dump_stack) from [] (__warn+0xe0/0x10c)
> [   45.663314] [] (__warn) from []
> (warn_slowpath_fmt+0x44/0x6c)
> [   45.670862] [] (warn_slowpath_fmt) from []
> (musb_h_tx_flush_fifo+0x118/0x138)
> [   45.679806] [] (musb_h_tx_flush_fifo) from []
> (musb_cleanup_urb+0x128/0x204)
> [   45.688660] [] (musb_cleanup_urb) from []
> (musb_urb_dequeue+0x14c/0x234)
> [   45.697165] [] (musb_urb_dequeue) from []
> (usb_hcd_unlink_urb+0x68/0x84)
> [   45.705667] [] (usb_hcd_unlink_urb) from []
> (usb_wwan_write+0x64/0x1f0)
> [   45.714090] [] (usb_wwan_write) from []
> (serial_write+0x34/0x60)
> [   45.721898] [] (serial_write) from []
> (n_tty_write+0x360/0x460)
> [   45.729626] [] (n_tty_write) from []
> (tty_write+0x1c0/0x36c)
> [   45.737092] [] (tty_write) from []
> (__vfs_write+0x28/0x1c4)
> [   45.744465] [] (__vfs_write) from []
> (vfs_write+0xa0/0x184)
> [   45.751835] [] (vfs_write) from [] 
> (ksys_write+0x98/0xdc)
> [   45.759033] [] (ksys_write) from []
> (ret_fast_syscall+0x0/0x28)
> [   45.766743] Exception stack(0xcb911fa8 to 0xcb911ff0)
> [   45.771845] 1fa0:   00160170 bea0fc14 0009
> 00141be9 0001 bea0fc14
> [   45.780086] 1fc0: 00160170 bea0fc14  0004 00141be9
> 0001 0001 00141be9
> [   45.788322] 1fe0:  bea0fbc8 b68d992b b68d9934
> [   45.793416] irq event stamp: 109442
> [   45.796945] hardirqs last  enabled at (109441): []
> __irq_svc+0x74/0x98
> [   45.804406] hardirqs last disabled at (109442): []
> _raw_spin_lock_irqsave+0x1c/0x4c
> [   45.812992] softirqs last  enabled at (109180): []
> __do_softirq+0x360/0x524
> [   45.820880] softirqs last disabled at (109157): []
> irq_exit+0x12c/0x17c
> [   45.828416] ---[ end trace eecf6f3aa6209643 ]---
> 
> [1] https://patchwork.kernel.org/patch/7389591/

Hmm so is this with am335x SoC glue or something else?

Regards,

Tony

RE: [RFC PATCH] usb: typec: ucsi: ccg: Remove run_isr flag

2019-09-23 Thread Ajay Gupta

Hi Heikki,

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org 
> On Behalf Of Heikki Krogerus
> Sent: Monday, September 23, 2019 6:31 AM
> To: Ajay Gupta 
> Cc: linux-usb@vger.kernel.org
> Subject: [RFC PATCH] usb: typec: ucsi: ccg: Remove run_isr flag
> 
> There is no need to try to prevent the extra ucsi_notify() that runtime
> resuming the device will cause.
> 
> This fixes potential deadlock. Both ccg_read() and
> ccg_write() are called with the mutex already taken at least from
> ccg_send_command(). In ccg_read() and ccg_write, the mutex is only acquired
> so that run_isr flag can be set.
> 
> Signed-off-by: Heikki Krogerus 
> ---
> Hi Ajay,
> 
> Before going forward with this I would like to get confirmation from you that 
> it
> is OK, and that I'm not missing anything. 
Thanks for this. I mixed up firmware flashing and IO path by using same lock.

>I did not see any real purpose for that run_isr flag. 
> The only thing that I can see it preventing is an extra ucsi_notify()
> call caused by the waking of the controller, but that should not be a problem.
> Is there any other reason why the flag is there?
ucsi_ccg_runtime_resume() will get called after pm_runtime_get_sync(uc->dev);
in ccg_read() and ccg_write(). If we allow extra ucsi_notify() then ccg_read() 
and
ccg_write() will get called again from ucsi_notfiy() through ucsi_sync(). This 
will
keep on happening in a loop and the controller will remain in D0 always and 
runtime pm will never be done.

> 
> If the driver works fine without the flag, then let's just drop it.
> The deadlock will need to be fixed in any case.

We need to protect read/write of run_isr flag from ccg_read()/ccg_write() and
ucsi_ccg_runtime_resume() since they can get called from interrupt and runtime
pm threads.

I propose to add new "uc->pm_lock" for this purpose and not use "uc->lock".
Please see the change below for this. I tested both FW flashing and runtime PM
and they both work with new pm_lock.

diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c 
b/drivers/usb/typec/ucsi/ucsi_ccg.c
index ed727b2..a79a475 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -206,6 +206,7 @@ struct ucsi_ccg {
/* fw build with vendor information */
u16 fw_build;
bool run_isr; /* flag to call ISR routine during resume */
+   struct mutex pm_lock; /* to sync between io and system pm thread */
struct work_struct pm_work;

bool has_multiple_dp;
@@ -240,14 +241,14 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 
*data, u32 len)

if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
uc->fw_version <= CCG_OLD_FW_VERSION) {
-   mutex_lock(&uc->lock);
+   mutex_lock(&uc->pm_lock);
/*
 * Do not schedule pm_work to run ISR in
 * ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
 * since we are already in ISR path.
 */
uc->run_isr = false;
-   mutex_unlock(&uc->lock);
+   mutex_unlock(&uc->pm_lock);
}

pm_runtime_get_sync(uc->dev);
@@ -294,14 +295,14 @@ static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 
*data, u32 len)

if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
uc->fw_version <= CCG_OLD_FW_VERSION) {
-   mutex_lock(&uc->lock);
+   mutex_lock(&uc->pm_lock);
/*
 * Do not schedule pm_work to run ISR in
 * ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
 * since we are already in ISR path.
 */
uc->run_isr = false;
-   mutex_unlock(&uc->lock);
+   mutex_unlock(&uc->pm_lock);
}

pm_runtime_get_sync(uc->dev);
@@ -1323,6 +1324,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
uc->client = client;
uc->run_isr = true;
mutex_init(&uc->lock);
+   mutex_init(&uc->pm_lock);
INIT_WORK(&uc->work, ccg_update_firmware);
INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
@@ -1434,12 +1436,12 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
 */
if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
uc->fw_version <= CCG_OLD_FW_VERSION) {
-   mutex_lock(&uc->lock);
+   mutex_lock(&uc->pm_lock);
if (!uc->run_isr) {
uc->run_isr = true;
schedule = false;
}
-   mutex_unlock(&uc->lock);
+   mutex_unlock(&uc->pm_lock);

if (schedule)
schedule_work(&uc->pm_work);


Thanks
> nvpublic
> thanks,
> 
> ---
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 40 ++-
>  1 file changed, 2 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index 907e2

Re: Moschip 7703 USB to serial free to a good home

2019-09-23 Thread Bjørn Mork

Aaron Thompson  writes:

> I have a Moschip 7703 USB to single serial port adapter that I'm not
> using primarily because it doesn't have an in-tree driver, so I'd be
> happy to send it to anyone who would like to add support for it. It
> looks like it should be easy to add to the existing mos7720 driver.
> Anyone interested?

Sorry, not interested.  But did you try the obvious fix, even mentioned
here under "Moschip MCS7720, MCS7715 driver"?:
https://www.kernel.org/doc/Documentation/usb/usb-serial.txt

Just add the VID/PID to the device table and see what happens.  Just
post any error messages or other stuff here, along with the patch you
are testing, and you will get help interpreting it.

Note that you'll probably have to do a minor change in the
mos77xx_calc_num_ports() too, since it hardcodes 2 ports. But if you're
lucky then that's it.

Bjørn

Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted

Re: Ryzen7 3700U xhci fails on resume from sleep

[PATCH 1/1] usb: serial: option: add Telit FN980 compositions

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

Re: [PATCH 1/1] usb: serial: option: add Telit FN980 compositions

Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted

Re: Linux Keyspan USB serial driver ignoring XOFF

[PATCH v2 1/1] usb: serial: option: add Telit FN980 compositions

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

Re: [PATCH] ARM: imx25: provide a fixed regulator for usb phys

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted

Re: Event ring is full when do iozone test on UAS storage

Re: [PATCH] [PATCH v7] USB: serial: pl2303: Add new PID to support PL2303HXN (TYPE_HXN)

[RFC PATCH] usb: typec: ucsi: ccg: Remove run_isr flag

Re: Event ring is full when do iozone test on UAS storage

[PATCH v2] xhci: Prevent deadlock when xhci adapter breaks during init

Re: [PATCH v2] xhci: Prevent deadlock when xhci adapter breaks during init

Re: [PATCH] Check for changed device descriptors when a connection-change occurs before validating the connection.

Re: KASAN: use-after-free Read in appledisplay_bl_get_brightness

Re: [PATCH v3] usb: host: xhci: wait CNR when doing xhci resume

Re: Linux Keyspan USB serial driver ignoring XOFF

[PATCH] USB: rio500: Remove Rio 500 kernel driver

Re: musb: Could not flush host TX2 fifo: csr: 2403

RE: [RFC PATCH] usb: typec: ucsi: ccg: Remove run_isr flag

Re: Moschip 7703 USB to serial free to a good home

26 matches

Site Navigation

Mail list logo

Footer information