Re: new driver Fresco Logic

2017-07-20 Thread PePa
Just to follow up on 
https://www.mail-archive.com/linux-usb@vger.kernel.org/msg42874.html 
about USB to VGA devices.


FL released a limited driver that decodes the USB to display, but 
doesn't interact with X in any way:


https://github.com/fresco-fl2000/fl2000
(Since last month!)
Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] usb: dwc3: Don't reinitialize core during host bus-suspend/resume

2017-07-20 Thread Baolin Wang
Hi Manu,

On 20 July 2017 at 19:12, Manu Gautam  wrote:
> Hi,
>
>
> On 7/13/2017 1:13 PM, Baolin Wang wrote:
>> Hi,
>>
>> On 13 July 2017 at 15:26, Manu Gautam  wrote:
>>>
>>> On 7/13/2017 11:33 AM, Baolin Wang wrote:
 On 12 July 2017 at 16:58, Manu Gautam  wrote:
> On 7/12/2017 2:06 PM, Baolin Wang wrote:
>> Hi,
>>
>> On 12 July 2017 at 15:25, Manu Gautam  wrote:
>>> On 7/12/2017 12:19 PM, Baolin Wang wrote:
 Hi,

 On 3 July 2017 at 19:25, Manu Gautam  wrote:
> Driver powers-off PHYs and reinitializes DWC3 core and gadget on
> resume. While this works fine for gadget mode but in host
> mode there is not re-initialization of host stack. Also, resetting
> bus as part of bus_suspend/resume is not correct which could affect
> (or disconnect) connected devices.
> Fix this by not reinitializing core on suspend/resume in host mode
> for HOST only and OTG/drd configurations.
 But for some mobile devices, they also need suspend/resume in host
 mode which will power off dwc3 controller in glue layer. If we remove
 dwc3_core_init() for host mode, I am afraid it can not work well in
 host mode after resuming dwc3.
>>> Can you point me to a glue driver doing that?
>> Yes, now there are no glue driver doing that, but for many vendor
>> trees they will do that. (Our platform will power off dwc3 when
>> suspending dwc3, but have not upstreamed yet)
> Fine, but glue driver still need to take care of re-initialization on 
> resume
> with current design.
 Yes.

>>> If dwc3 is powered off on suspend then Xhci level initialization also
>>> needed
>>> after
>>> performing dwc3_core_init on resume which is currently not present. So
>>> this
>>> seems
>>> to me broken anyway.
>> Since we've add runtime PM for xhci-plat driver [1], which means we
>> can runtime suspend/resume xhci device from dwc3 [2].
>> [1] https://www.spinics.net/lists/linux-usb/msg156077.html
>> [2] https://patchwork.kernel.org/patch/9471869/
> Sure. We can discuss this patch once you re-submit.
> One concern I have with the patch is marking ignore_children for dwc3 and 
> as
> part of
> dwc3 suspend/resume invoking
 Suppose we will suspend xhci device as dwc3's child, but now dwc3
 device is not suspend yet, which will block to suspend xhci device.
 Thus we should set ignore children flag to remove the parent
 dependency.
>>> I may not have understood the issue. In any case children must be suspended 
>>> first.
>>> Example sequence is:
>>> external hub/device (if any) suspend -> root hub  -> xhci -> dwc3 core -> 
>>> dwc3 glue.
>>> xhci device's runtime/pm suspend should happen before dwc3 gets suspended.
>> Since the [2] patch will get/put xhci usage counter to avoid affecting
>> other controller's runtime PM, then we need ignore child. But now
>> xhci-plat driver has done this (by issuing pm_runtime_forbid()
>> default), we can remove the get/put counter in dwc3, then we do not
>> need ignore child flag any more.
>>
> runtime pm on xhci. Ideally pm core should handle it seamlessly i.e.
> suspending dwc3
> once xhci gets suspended and pm_stay_awake/relax can be used to block
> pm_suspend
> until runtime suspend of glue driver happens.
>
>> But [2] has not applied yet now, I will try to upstream it again. Then
>> we can suspend xhci device--> suspend dwc3 host---> glue layer can
>> power off, and it is similar when resuming in host mode.
> Do you see any issues with this patch?
> I am able to get runtime suspend/resume happening fine with XHCI on my
> platform.
> It involves runtime suspend/resume of USB HUB and dwc3 glue driver without
> any other
> change.
 Great, I will try this patch again.
>
> Baolin, just checking if you were able to test this?

Yes, I will test this and resubmit again next week.

-- 
Baolin.wang
Best 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: xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

2017-07-20 Thread Yaroslav Isakov
Yes, I can definitely test the patch

2017-07-20 19:16 GMT+03:00 Mathias Nyman :
> On 20.07.2017 18:07, Yaroslav Isakov wrote:
>>
>> Here it is
>>
>> 2017-07-20 18:06 GMT+03:00 Mathias Nyman :
>>>
>>> On 20.07.2017 17:43, Yaroslav Isakov wrote:


 Hello everyone! I saw this thread some months ago, but do not know how
 to properly reply to it. I have the same problem, and it's just not
 few messages - it was about 40k messages in 10 minutes. No functional
 of my USB device (which is CCID USB token) is broken, just enormous
 amount of spam in the logs. I'm on 4.12 kernel, but I've seen this
 error on 4.11 and 4.9. Could this problem be caused by a bug in e.g.
 libusb, not a device itself?
 --
>>>
>>>
>>>
>>> I'd start by looking at xhci, can you take xhci traces of this?
>>>
>>> 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
>>>
>>> and then send me the content of
>>> /sys/kernel/debug/tracing/trace
>>>
>>> I'll be away for a couple of weeks, so next response might take some time
>>>
>>> -Mathias
>>>
>>>
>
> Think I found something.   (details mostly for myself in to remember this)
> A short transfer response on a bulk in transfer which is not the last TRB of
> a TD
>
> According to specs xhci will issue short transfer events both on the short
> TRB, _and_
> on the last TRB of the TD in case any previous TRB in the TD was short
> (see xhci 4.11.3.1)
>
> After the first short transfer event the driver fast forward past this TD.
> (and the last TRB)
> when we get the second short transfer event there are no TRBs queued and you
> see the
> warning.
>
> Warning should be harmless in this case, but annoying.
> If I write a patch can you try it out?
>
> As a reference, a snippet when log when a 65556 byte transfer is queued,
> split into 5 pieces (TRBS),
> and we get short transfer events for first and last TRB
>
> * A URB asking for 65556 bytes is queues:
> 2369.450362: xhci_urb_enqueue: ep1in-bulk: urb 8801398f6540 pipe
> 3221258880 slot 1 length 0/65556 sgs 5/5 stream 0 flags 00040200
> * split into 5 pieces ( TRBs at 0x0002169cb520, ..530, ..540, ..550 and
> ..560)
> 2369.450363: xhci_queue_trb: BULK: Buffer 00010a7dc000 length 16384 TD
> size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:C
> 2369.450363: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb530(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 508 bounce
> 6\
> 2369.450363: xhci_queue_trb: BULK: Buffer 00011e97 length 16384 TD
> size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> 2369.450363: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb540(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 507 bounce
> 6\
> 2369.450363: xhci_queue_trb: BULK: Buffer 0001b63ac000 length 16384 TD
> size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> 2369.450363: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb550(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 506 bounce
> 6\
> 2369.450364: xhci_queue_trb: BULK: Buffer 00010a7ac000 length 16384 TD
> size 1 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> 2369.450364: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb560(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 505 bounce
> 6\
> 2369.450364: xhci_queue_trb: BULK: Buffer 0001a26adf40 length 20 TD size
> 0 intr 0 type 'Normal' flags b:i:I:c:s:I:e:c
> 2369.450364: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb570(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 504 bounce
> 6\
> * short packet event for first TRB at ..520
> 2369.450505: xhci_handle_event: EVENT: TRB 0002169cb520 status 'Short
> Packet' len 16334 slot 1 ep 3 type 'Transfer Event' flags e:c
> 2369.450506: xhci_handle_transfer: BULK: Buffer 00010a7dc000 length
> 16384 TD size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> * fast forward past this TD, move dequeue to ..570)
> 2369.450506: xhci_inc_deq: BULK 88021726c780: enq
> 0x0002169cb570(0x0002169cb000) deq
> 0x0002169cb530(0x0002169cb000) segs 2 stream 0 free_trbs 505 bounce
> 6\
> 2369.450506: xhci_inc_deq: BULK 88021726c780: enq
> 0x0002169cb570(0x0002169cb000) deq
> 0x0002169cb540(0x0002169cb000) segs 2 stream 0 free_trbs 506 bounce
> 6\
> 2369.450506: xhci_inc_deq: BULK 88021726c780: enq
> 0x0002169cb570(0x0002169cb000) deq
> 0x0002169cb550(0x0002169cb000) segs 2 stream 0 free_trbs 507 bounce
> 6\
> 2369.450506: xhci_inc_deq: BULK 88021726c780: enq
> 0x0002169cb570(0x0002169cb000) deq
> 0x0002169cb560(0x0002169cb000) segs 2 stream 0 free_trbs 508 bounce
> 6\
> 2369.450506: xhci_inc_deq: BULK 88021726c780: enq
> 0x000

Re: xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

2017-07-20 Thread Mathias Nyman

On 20.07.2017 18:07, Yaroslav Isakov wrote:

Here it is

2017-07-20 18:06 GMT+03:00 Mathias Nyman :

On 20.07.2017 17:43, Yaroslav Isakov wrote:


Hello everyone! I saw this thread some months ago, but do not know how
to properly reply to it. I have the same problem, and it's just not
few messages - it was about 40k messages in 10 minutes. No functional
of my USB device (which is CCID USB token) is broken, just enormous
amount of spam in the logs. I'm on 4.12 kernel, but I've seen this
error on 4.11 and 4.9. Could this problem be caused by a bug in e.g.
libusb, not a device itself?
--



I'd start by looking at xhci, can you take xhci traces of this?

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

and then send me the content of
/sys/kernel/debug/tracing/trace

I'll be away for a couple of weeks, so next response might take some time

-Mathias




Think I found something.   (details mostly for myself in to remember this)
A short transfer response on a bulk in transfer which is not the last TRB of a 
TD

According to specs xhci will issue short transfer events both on the short TRB, 
_and_
on the last TRB of the TD in case any previous TRB in the TD was short
(see xhci 4.11.3.1)

After the first short transfer event the driver fast forward past this TD. (and 
the last TRB)
when we get the second short transfer event there are no TRBs queued and you 
see the
warning.

Warning should be harmless in this case, but annoying.
If I write a patch can you try it out?

As a reference, a snippet when log when a 65556 byte transfer is queued, split 
into 5 pieces (TRBS),
and we get short transfer events for first and last TRB

* A URB asking for 65556 bytes is queues:
2369.450362: xhci_urb_enqueue: ep1in-bulk: urb 8801398f6540 pipe 3221258880 
slot 1 length 0/65556 sgs 5/5 stream 0 flags 00040200
* split into 5 pieces ( TRBs at 0x0002169cb520, ..530, ..540, ..550 and 
..560)
2369.450363: xhci_queue_trb: BULK: Buffer 00010a7dc000 length 16384 TD size 
31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:C
2369.450363: xhci_inc_enq: BULK 88021726c780: enq 
0x0002169cb530(0x0002169cb000) deq 
0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 508 bounce 6\
2369.450363: xhci_queue_trb: BULK: Buffer 00011e97 length 16384 TD size 
31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
2369.450363: xhci_inc_enq: BULK 88021726c780: enq 
0x0002169cb540(0x0002169cb000) deq 
0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 507 bounce 6\
2369.450363: xhci_queue_trb: BULK: Buffer 0001b63ac000 length 16384 TD size 
31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
2369.450363: xhci_inc_enq: BULK 88021726c780: enq 
0x0002169cb550(0x0002169cb000) deq 
0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 506 bounce 6\
2369.450364: xhci_queue_trb: BULK: Buffer 00010a7ac000 length 16384 TD size 
1 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
2369.450364: xhci_inc_enq: BULK 88021726c780: enq 
0x0002169cb560(0x0002169cb000) deq 
0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 505 bounce 6\
2369.450364: xhci_queue_trb: BULK: Buffer 0001a26adf40 length 20 TD size 0 
intr 0 type 'Normal' flags b:i:I:c:s:I:e:c
2369.450364: xhci_inc_enq: BULK 88021726c780: enq 
0x0002169cb570(0x0002169cb000) deq 
0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 504 bounce 6\
* short packet event for first TRB at ..520
2369.450505: xhci_handle_event: EVENT: TRB 0002169cb520 status 'Short 
Packet' len 16334 slot 1 ep 3 type 'Transfer Event' flags e:c
2369.450506: xhci_handle_transfer: BULK: Buffer 00010a7dc000 length 16384 
TD size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
* fast forward past this TD, move dequeue to ..570)
2369.450506: xhci_inc_deq: BULK 88021726c780: enq 
0x0002169cb570(0x0002169cb000) deq 
0x0002169cb530(0x0002169cb000) segs 2 stream 0 free_trbs 505 bounce 6\
2369.450506: xhci_inc_deq: BULK 88021726c780: enq 
0x0002169cb570(0x0002169cb000) deq 
0x0002169cb540(0x0002169cb000) segs 2 stream 0 free_trbs 506 bounce 6\
2369.450506: xhci_inc_deq: BULK 88021726c780: enq 
0x0002169cb570(0x0002169cb000) deq 
0x0002169cb550(0x0002169cb000) segs 2 stream 0 free_trbs 507 bounce 6\
2369.450506: xhci_inc_deq: BULK 88021726c780: enq 
0x0002169cb570(0x0002169cb000) deq 
0x0002169cb560(0x0002169cb000) segs 2 stream 0 free_trbs 508 bounce 6\
2369.450506: xhci_inc_deq: BULK 88021726c780: enq 
0x0002169cb570(0x0002169cb000) deq 
0x0002169cb570(0x0002169cb000) segs 2 stream 0 free_trbs 509 bounce 6\
2369.450506: xhci_urb_giveback: ep1in-bulk: urb 8801398f6540 pipe 
3221258880 slot 1 length 50/65556 sgs 5/5 stream 0 flags 00040200
* short transfer event for last TRB in TD at ..560
2369.450507: xhci_handle

Re: [PATCH 2/2] usb: dwc3: pci: Runtime resume child device from wq

2017-07-20 Thread Manu Gautam
Hi,


On 7/3/2017 4:55 PM, Manu Gautam wrote:
> Driver currently resumes and increments pm usage_count
> of its child device (dwc3 main) from its runtime_resume
> handler. This requires dwc3 runtime_resume to perform
> pm_runtime_put to decrement the pm usage_count. However
> runtime_put from dwc3 happens for non pci drivers
> (e.g. dwc3-if-simple.c) as well which results in dwc3
> pm usage_count becoming negative after couple of
> runtime suspend resume iterations. Fix this by
> performing runtime_get/put from dwc3-pci driver only
> using workqueue.

Felipe, I don't have any platform that uses dwc3-pci, hence couldn't
test. However, with this change I don't see issue of pm usage_count
becoming -ve on my platform that uses dwc3-of-simple.c.
Let me know if this looks fine.

>
> Signed-off-by: Manu Gautam 
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index fc556a4..87dedb9 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1378,7 +1378,6 @@ static int dwc3_runtime_resume(struct device *dev)
>   }
>  
>   pm_runtime_mark_last_busy(dev);
> - pm_runtime_put(dev);
>  
>   return 0;
>  }
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 84a2ceb..e354acc 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -61,6 +62,7 @@ struct dwc3_pci {
>   u8 uuid[16];
>  
>   unsigned int has_dsm_for_pm:1;
> + struct work_struct wakeup_work;
>  };
>  
>  static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> @@ -174,6 +176,22 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
>   return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static void dwc3_pci_resume_work(struct work_struct *work)
> +{
> + struct dwc3_pci *dwc = container_of(work, struct dwc3_pci, wakeup_work);
> + struct platform_device *dwc3 = dwc->dwc3;
> + int ret;
> +
> + ret = pm_runtime_get_sync(&dwc3->dev);
> + if (ret)
> + return;
> +
> + pm_runtime_mark_last_busy(&dwc3->dev);
> + pm_runtime_put_sync_autosuspend(&dwc3->dev);
> +}
> +#endif
> +
>  static int dwc3_pci_probe(struct pci_dev *pci,
>   const struct pci_device_id *id)
>  {
> @@ -233,6 +251,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
>   device_set_run_wake(dev, true);
>   pci_set_drvdata(pci, dwc);
>   pm_runtime_put(dev);
> +#ifdef CONFIG_PM
> + INIT_WORK(&dwc->wakeup_work, dwc3_pci_resume_work);
> +#endif
>  
>   return 0;
>  err:
> @@ -244,6 +265,9 @@ static void dwc3_pci_remove(struct pci_dev *pci)
>  {
>   struct dwc3_pci *dwc = pci_get_drvdata(pci);
>  
> +#ifdef CONFIG_PM
> + cancel_work_sync(&dwc->wakeup_work);
> +#endif
>   device_init_wakeup(&pci->dev, false);
>   pm_runtime_get(&pci->dev);
>   platform_device_unregister(dwc->dwc3);
> @@ -319,14 +343,15 @@ static int dwc3_pci_runtime_suspend(struct device *dev)
>  static int dwc3_pci_runtime_resume(struct device *dev)
>  {
>   struct dwc3_pci *dwc = dev_get_drvdata(dev);
> - struct platform_device  *dwc3 = dwc->dwc3;
>   int ret;
>  
>   ret = dwc3_pci_dsm(dwc, PCI_INTEL_BXT_STATE_D0);
>   if (ret)
>   return ret;
>  
> - return pm_runtime_get(&dwc3->dev);
> + queue_work(pm_wq, &dwc->wakeup_work);
> +
> + return 0;
>  }
>  #endif /* CONFIG_PM */
>  

--
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: xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

2017-07-20 Thread Yaroslav Isakov
Here it is

2017-07-20 18:06 GMT+03:00 Mathias Nyman :
> On 20.07.2017 17:43, Yaroslav Isakov wrote:
>>
>> Hello everyone! I saw this thread some months ago, but do not know how
>> to properly reply to it. I have the same problem, and it's just not
>> few messages - it was about 40k messages in 10 minutes. No functional
>> of my USB device (which is CCID USB token) is broken, just enormous
>> amount of spam in the logs. I'm on 4.12 kernel, but I've seen this
>> error on 4.11 and 4.9. Could this problem be caused by a bug in e.g.
>> libusb, not a device itself?
>> --
>
>
> I'd start by looking at xhci, can you take xhci traces of this?
>
> 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
>
> and then send me the content of
> /sys/kernel/debug/tracing/trace
>
> I'll be away for a couple of weeks, so next response might take some time
>
> -Mathias
>
>


usb.trace.gz
Description: GNU Zip compressed data


Re: xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

2017-07-20 Thread Mathias Nyman

On 20.07.2017 17:43, Yaroslav Isakov wrote:

Hello everyone! I saw this thread some months ago, but do not know how
to properly reply to it. I have the same problem, and it's just not
few messages - it was about 40k messages in 10 minutes. No functional
of my USB device (which is CCID USB token) is broken, just enormous
amount of spam in the logs. I'm on 4.12 kernel, but I've seen this
error on 4.11 and 4.9. Could this problem be caused by a bug in e.g.
libusb, not a device itself?
--


I'd start by looking at xhci, can you take xhci traces of this?

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

and then send me the content of
/sys/kernel/debug/tracing/trace

I'll be away for a couple of weeks, so next response might take some time

-Mathias


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


xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

2017-07-20 Thread Yaroslav Isakov
Hello everyone! I saw this thread some months ago, but do not know how
to properly reply to it. I have the same problem, and it's just not
few messages - it was about 40k messages in 10 minutes. No functional
of my USB device (which is CCID USB token) is broken, just enormous
amount of spam in the logs. I'm on 4.12 kernel, but I've seen this
error on 4.11 and 4.9. Could this problem be caused by a bug in e.g.
libusb, not a device itself?
--
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] powerpc/asm/cacheflush: Cleanup cacheflush function params

2017-07-20 Thread Michael Ellerman
Geert Uytterhoeven  writes:

> On Thu, Jul 20, 2017 at 1:43 PM, Michael Ellerman  wrote:
>> Matt Brown  writes:
>>> The cacheflush prototypes currently use start and stop values and each
>>> call requires typecasting the address to an unsigned long.
>>> This patch changes the cacheflush prototypes to follow the x86 style of
>>> using a base and size values, with base being a void pointer.
>>>
>>> All callers of the cacheflush functions, including drivers, have been
>>> modified to conform to the new prototypes.
>>>
>>> The 64 bit cacheflush functions which were implemented in assembly code
>>> (flush_dcache_range, flush_inval_dcache_range) have been translated into
>>> C for readability and coherence.
>
>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>> @@ -51,13 +51,13 @@ static inline void __flush_dcache_icache_phys(unsigned 
>>> long physaddr)
>>>   * Write any modified data cache blocks out to memory and invalidate them.
>>>   * Does not invalidate the corresponding instruction cache blocks.
>>>   */
>>> -static inline void flush_dcache_range(unsigned long start, unsigned long 
>>> stop)
>>> +static inline void flush_dcache_range(void *start, unsigned long size)
>>>  {
>>> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
>>> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 
>>> 1);
>>> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));
>>
>> unsigned long would be nicer than u32.
>
> Indeed. That would make this work on ppc64, too.
> After which ppc64 has an identical copy (u64 = unsigned long on ppc64) below?

That was Matt's homework to notice that ;)

cheers
--
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] powerpc/asm/cacheflush: Cleanup cacheflush function params

2017-07-20 Thread Geert Uytterhoeven
On Thu, Jul 20, 2017 at 1:43 PM, Michael Ellerman  wrote:
> Matt Brown  writes:
>> The cacheflush prototypes currently use start and stop values and each
>> call requires typecasting the address to an unsigned long.
>> This patch changes the cacheflush prototypes to follow the x86 style of
>> using a base and size values, with base being a void pointer.
>>
>> All callers of the cacheflush functions, including drivers, have been
>> modified to conform to the new prototypes.
>>
>> The 64 bit cacheflush functions which were implemented in assembly code
>> (flush_dcache_range, flush_inval_dcache_range) have been translated into
>> C for readability and coherence.

>> --- a/arch/powerpc/include/asm/cacheflush.h
>> +++ b/arch/powerpc/include/asm/cacheflush.h
>> @@ -51,13 +51,13 @@ static inline void __flush_dcache_icache_phys(unsigned 
>> long physaddr)
>>   * Write any modified data cache blocks out to memory and invalidate them.
>>   * Does not invalidate the corresponding instruction cache blocks.
>>   */
>> -static inline void flush_dcache_range(unsigned long start, unsigned long 
>> stop)
>> +static inline void flush_dcache_range(void *start, unsigned long size)
>>  {
>> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
>> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
>> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));
>
> unsigned long would be nicer than u32.

Indeed. That would make this work on ppc64, too.
After which ppc64 has an identical copy (u64 = unsigned long on ppc64) below?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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 6/6] xhci: fix memleak in xhci_run()

2017-07-20 Thread Mathias Nyman
From: Shu Wang 

Found this issue by kmemleak.
xhci_run() did not check return val and free command for
xhci_queue_vendor_command()

unreferenced object 0x88011c0be500 (size 64):
  comm "kworker/0:1", pid 58, jiffies 4294670908 (age 50.420s)
  hex dump (first 32 bytes):
  backtrace:
[] kmemleak_alloc+0x4a/0xa0
[] kmem_cache_alloc_trace+0xca/0x1d0
[] xhci_alloc_command+0x44/0x130
[] xhci_run+0x4cc/0x630
[] usb_add_hcd+0x3bb/0x950
[] usb_hcd_pci_probe+0x188/0x500
[] xhci_pci_probe+0x2c/0x220
[] local_pci_probe+0x45/0xa0
[] work_for_cpu_fn+0x14/0x20
[] process_one_work+0x149/0x360
[] worker_thread+0x1d8/0x3c0
[] kthread+0x109/0x140
[] ret_from_fork+0x25/0x30
[] 0x

Cc: 
Signed-off-by: Shu Wang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5132642..b2ff1ff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -625,8 +625,10 @@ int xhci_run(struct usb_hcd *hcd)
if (!command)
return -ENOMEM;
 
-   xhci_queue_vendor_command(xhci, command, 0, 0, 0,
+   ret = xhci_queue_vendor_command(xhci, command, 0, 0, 0,
TRB_TYPE(TRB_NEC_GET_FW));
+   if (ret)
+   xhci_free_command(xhci, command);
}
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Finished xhci_run for USB2 roothub");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/6] xhci: Bad Ethernet performance plugged in ASM1042A host

2017-07-20 Thread Mathias Nyman
From: Jiahau Chang 

When USB Ethernet is plugged in ASMEDIA ASM1042A xHCI host, bad
performance was manifesting in Web browser use (like download
large file such as ISO image). It is known limitation of
ASM1042A that is not compatible with driver scheduling,
As a workaround we can modify flow control handling of ASM1042A.
The register we modify is changes the behavior

[use quirk bit 28, usleep_range 40-60us, empty non-pci function -Mathias]
Cc: 
Signed-off-by: Jiahau Chang 
Signed-off-by: Ian Pilcher 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/pci-quirks.c | 54 +++
 drivers/usb/host/pci-quirks.h |  2 ++
 drivers/usb/host/xhci-pci.c   |  6 +
 drivers/usb/host/xhci.c   |  6 +
 drivers/usb/host/xhci.h   |  1 +
 5 files changed, 69 insertions(+)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index a9a1e4c..c8989c6 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -77,6 +77,16 @@
 #define USB_INTEL_USB3_PSSEN   0xD8
 #define USB_INTEL_USB3PRM  0xDC
 
+/* ASMEDIA quirk use */
+#define ASMT_DATA_WRITE0_REG   0xF8
+#define ASMT_DATA_WRITE1_REG   0xFC
+#define ASMT_CONTROL_REG   0xE0
+#define ASMT_CONTROL_WRITE_BIT 0x02
+#define ASMT_WRITEREG_CMD  0x10423
+#define ASMT_FLOWCTL_ADDR  0xFA30
+#define ASMT_FLOWCTL_DATA  0xBA
+#define ASMT_PSEUDO_DATA   0
+
 /*
  * amd_chipset_gen values represent AMD different chipset generations
  */
@@ -412,6 +422,50 @@ void usb_amd_quirk_pll_disable(void)
 }
 EXPORT_SYMBOL_GPL(usb_amd_quirk_pll_disable);
 
+static int usb_asmedia_wait_write(struct pci_dev *pdev)
+{
+   unsigned long retry_count;
+   unsigned char value;
+
+   for (retry_count = 1000; retry_count > 0; --retry_count) {
+
+   pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
+
+   if (value == 0xff) {
+   dev_err(&pdev->dev, "%s: check_ready ERROR", __func__);
+   return -EIO;
+   }
+
+   if ((value & ASMT_CONTROL_WRITE_BIT) == 0)
+   return 0;
+
+   usleep_range(40, 60);
+   }
+
+   dev_warn(&pdev->dev, "%s: check_write_ready timeout", __func__);
+   return -ETIMEDOUT;
+}
+
+void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev)
+{
+   if (usb_asmedia_wait_write(pdev) != 0)
+   return;
+
+   /* send command and address to device */
+   pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD);
+   pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR);
+   pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);
+
+   if (usb_asmedia_wait_write(pdev) != 0)
+   return;
+
+   /* send data to device */
+   pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA);
+   pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA);
+   pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);
+}
+EXPORT_SYMBOL_GPL(usb_asmedia_modifyflowcontrol);
+
 void usb_amd_quirk_pll_enable(void)
 {
usb_amd_quirk_pll(0);
diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
index 0222195..6559944 100644
--- a/drivers/usb/host/pci-quirks.h
+++ b/drivers/usb/host/pci-quirks.h
@@ -11,6 +11,7 @@
 void usb_amd_dev_put(void);
 void usb_amd_quirk_pll_disable(void);
 void usb_amd_quirk_pll_enable(void);
+void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev);
 void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev);
 void usb_disable_xhci_ports(struct pci_dev *xhci_pdev);
 void sb800_prefetch(struct device *dev, int on);
@@ -18,6 +19,7 @@
 struct pci_dev;
 static inline void usb_amd_quirk_pll_disable(void) {}
 static inline void usb_amd_quirk_pll_enable(void) {}
+static inline void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) {}
 static inline void usb_amd_dev_put(void) {}
 static inline void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) {}
 static inline void sb800_prefetch(struct device *dev, int on) {}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 53882e2..5b0fa55 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -59,6 +59,8 @@
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_20x43bb
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_10x43bc
 
+#define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI   0x1142
+
 static const char hcd_name[] = "xhci_hcd";
 
 static struct hc_driver __read_mostly xhci_pci_hc_driver;
@@ -217,6 +219,10 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
pdev->device == 0x1142)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 
+   if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
+   pdev->device == PCI_DEVICE_ID_ASMEDIA_1042A_XHCI)
+   xhci->quirks |= XHCI_ASMEDIA_MODIFY_FLOWCONTR

[PATCH 3/6] usb: xhci: Issue stop EP command only when the EP state is running

2017-07-20 Thread Mathias Nyman
From: Shyam Sundar S K 

on AMD platforms with SNPS 3.1 USB controller if stop endpoint command is
issued the controller does not respond, when the EP is not in running
state. HW completes the command execution and reports
"Context State Error" completion code. This is as per the spec. However
HW on receiving the second command additionally marks EP to Flow control
state in HW which is RTL bug. This bug causes the HW not to respond
to any further doorbells that are rung by the driver. This makes the EP
to not functional anymore and causes gross functional failures.

As a workaround, not to hit this problem, it's better to check the EP state
and issue a stop EP command only when the EP is in running state.

As a sidenote, even with this patch there is still a possibility of
triggering the RTL bug if the context state races with the stop endpoint
command as described in xHCI spec 4.6.9

[code simplification and reworded sidenote in commit message -Mathias]
Signed-off-by: Shyam Sundar S K 
Signed-off-by: Nehal Shah 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1adae9e..364f602 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -398,14 +398,21 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
spin_lock_irqsave(&xhci->lock, flags);
for (i = LAST_EP_INDEX; i > 0; i--) {
if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
+   struct xhci_ep_ctx *ep_ctx;
struct xhci_command *command;
+
+   ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, i);
+
+   /* Check ep is running, required by AMD SNPS 3.1 xHC */
+   if (GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)
+   continue;
+
command = xhci_alloc_command(xhci, false, false,
 GFP_NOWAIT);
if (!command) {
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_free_command(xhci, cmd);
return -ENOMEM;
-
}
xhci_queue_stop_endpoint(xhci, command, slot_id, i,
 suspend);
-- 
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 5/6] usb: xhci: fix spinlock recursion for USB2 test mode

2017-07-20 Thread Mathias Nyman
From: Peter Chen 

Both xhci_hub_control and xhci_disable_slot tries to hold spinlock, the
spinlock recursion occurs when enters USB2 test mode. Fix it by unlock
spinlock before calling xhci_disable_slot.

Cc: 
Fixes: 0f1d832ed1fb ("usb: xhci: Add port test modes support for usb2")
Signed-off-by: Peter Chen 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 9ef5f68..00721e8 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -610,12 +610,14 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 
/* Disable all Device Slots */
xhci_dbg(xhci, "Disable all slots\n");
+   spin_unlock_irqrestore(&xhci->lock, *flags);
for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
retval = xhci_disable_slot(xhci, NULL, i);
if (retval)
xhci_err(xhci, "Failed to disable slot %d, %d. Enter 
test mode anyway\n",
 i, retval);
}
+   spin_lock_irqsave(&xhci->lock, *flags);
/* Put all ports to the Disable state by clear PP */
xhci_dbg(xhci, "Disable all port (PP = 0)\n");
/* Power off USB3 ports*/
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/6] xhci: Fix NULL pointer dereference when cleaning up streams for removed host

2017-07-20 Thread Mathias Nyman
This off by one in stream_id indexing caused NULL pointer dereference and
soft lockup on machines with USB attached SCSI devices connected to a
hotpluggable xhci controller.

The code that cleans up pending URBs for dead hosts tried to dereference
a stream ring at the invalid stream_id 0.
ep->stream_info->stream_rings[0] doesn't point to a ring.

Start looping stream_id from 1 like in all the other places in the driver,
and check that the ring exists before trying to kill URBs on it.

Reported-by: rocko r 
Cc: 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c50c902..cc368ad 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -864,13 +864,16 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
(ep->ep_state & EP_GETTING_NO_STREAMS)) {
int stream_id;
 
-   for (stream_id = 0; stream_id < ep->stream_info->num_streams;
+   for (stream_id = 1; stream_id < ep->stream_info->num_streams;
stream_id++) {
+   ring = ep->stream_info->stream_rings[stream_id];
+   if (!ring)
+   continue;
+
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Killing URBs for slot ID %u, ep index 
%u, stream %u",
-   slot_id, ep_index, stream_id + 1);
-   xhci_kill_ring_urbs(xhci,
-   
ep->stream_info->stream_rings[stream_id]);
+   slot_id, ep_index, stream_id);
+   xhci_kill_ring_urbs(xhci, ring);
}
} else {
ring = ep->ring;
-- 
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 4/6] xhci: fix 20000ms port resume timeout

2017-07-20 Thread Mathias Nyman
A uncleared PLC (port link change) bit will prevent furuther port event
interrupts for that port. Leaving it uncleared caused get_port_status()
to timeout after 2ms while waiting to get the final port event
interrupt for resume -> U0 state change.

This is a targeted fix for a specific case where we get a port resume event
racing with xhci resume. The port event interrupt handler notices xHC is
not yet running and bails out early, leaving PLC uncleared.

The whole xhci port resuming needs more attention, but while working on it
it anyways makes sense to always ensure PLC is cleared in get_port_status
before setting a new link state and waiting for its completion.

Cc: 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 364f602..9ef5f68 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -904,6 +904,9 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
clear_bit(wIndex, &bus_state->resuming_ports);
 
set_bit(wIndex, &bus_state->rexit_ports);
+
+   xhci_test_and_clear_bit(xhci, port_array, wIndex,
+   PORT_PLC);
xhci_set_link_state(xhci, port_array, wIndex,
XDEV_U0);
 
-- 
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: [PATCH] xhci: fix memleak in xhci_run()

2017-07-20 Thread Mathias Nyman

On 20.07.2017 08:40, shuw...@redhat.com wrote:

From: Shu Wang 

Found this issue by kmemleak.
xhci_run() did not check return val and free command for
xhci_queue_vendor_command()

unreferenced object 0x88011c0be500 (size 64):
   comm "kworker/0:1", pid 58, jiffies 4294670908 (age 50.420s)
   hex dump (first 32 bytes):
   backtrace:
 [] kmemleak_alloc+0x4a/0xa0
 [] kmem_cache_alloc_trace+0xca/0x1d0
 [] xhci_alloc_command+0x44/0x130
 [] xhci_run+0x4cc/0x630
 [] usb_add_hcd+0x3bb/0x950
 [] usb_hcd_pci_probe+0x188/0x500
 [] xhci_pci_probe+0x2c/0x220
 [] local_pci_probe+0x45/0xa0
 [] work_for_cpu_fn+0x14/0x20
 [] process_one_work+0x149/0x360
 [] worker_thread+0x1d8/0x3c0
 [] kthread+0x109/0x140
 [] ret_from_fork+0x25/0x30
 [] 0x

Signed-off-by: Shu Wang 


Thanks, adding

-Mathias

--
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 0/6] xhci fixes for usb-linus 4.13-rc

2017-07-20 Thread Mathias Nyman
Hi Greg

This series contain ASMedia and AMD host specific fixes, streams
null pointer fix, and rare timeout, memleak and locking issues.

Thanks
Mathias

Jiahau Chang (1):
  xhci: Bad Ethernet performance plugged in ASM1042A host

Mathias Nyman (2):
  xhci: Fix NULL pointer dereference when cleaning up streams for
removed host
  xhci: fix 2ms port resume timeout

Peter Chen (1):
  usb: xhci: fix spinlock recursion for USB2 test mode

Shu Wang (1):
  xhci: fix memleak in xhci_run()

Shyam Sundar S K (1):
  usb: xhci: Issue stop EP command only when the EP state is running

 drivers/usb/host/pci-quirks.c | 54 +++
 drivers/usb/host/pci-quirks.h |  2 ++
 drivers/usb/host/xhci-hub.c   | 14 ++-
 drivers/usb/host/xhci-pci.c   |  6 +
 drivers/usb/host/xhci-ring.c  | 11 +
 drivers/usb/host/xhci.c   | 10 +++-
 drivers/usb/host/xhci.h   |  1 +
 7 files changed, 92 insertions(+), 6 deletions(-)

-- 
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: [PATCH] powerpc/asm/cacheflush: Cleanup cacheflush function params

2017-07-20 Thread Michael Ellerman
Hi Matt,

Thanks for tackling this mess.

Matt Brown  writes:
> The cacheflush prototypes currently use start and stop values and each
> call requires typecasting the address to an unsigned long.
> This patch changes the cacheflush prototypes to follow the x86 style of
> using a base and size values, with base being a void pointer.
>
> All callers of the cacheflush functions, including drivers, have been
> modified to conform to the new prototypes.
>
> The 64 bit cacheflush functions which were implemented in assembly code
> (flush_dcache_range, flush_inval_dcache_range) have been translated into
> C for readability and coherence.
>
> Signed-off-by: Matt Brown 
> ---
>  arch/powerpc/include/asm/cacheflush.h| 47 +
>  arch/powerpc/kernel/misc_64.S| 52 
> 
>  arch/powerpc/mm/dma-noncoherent.c| 15 
>  arch/powerpc/platforms/512x/mpc512x_shared.c | 10 +++---
>  arch/powerpc/platforms/85xx/smp.c|  6 ++--
>  arch/powerpc/sysdev/dart_iommu.c |  5 +--
>  drivers/ata/pata_bf54x.c |  3 +-
>  drivers/char/agp/uninorth-agp.c  |  6 ++--
>  drivers/gpu/drm/drm_cache.c  |  3 +-
>  drivers/macintosh/smu.c  | 15 
>  drivers/mmc/host/bfin_sdh.c  |  3 +-
>  drivers/mtd/nand/bf5xx_nand.c|  6 ++--
>  drivers/soc/fsl/qbman/dpaa_sys.h |  2 +-
>  drivers/soc/fsl/qbman/qman_ccsr.c|  3 +-
>  drivers/spi/spi-bfin5xx.c| 10 +++---
>  drivers/tty/serial/mpsc.c| 46 
>  drivers/usb/musb/blackfin.c  |  6 ++--

I think you want to trim that to powerpc only drivers for now at least.

> diff --git a/arch/powerpc/include/asm/cacheflush.h 
> b/arch/powerpc/include/asm/cacheflush.h
> index 11843e3..b8f04c3 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -51,13 +51,13 @@ static inline void __flush_dcache_icache_phys(unsigned 
> long physaddr)
>   * Write any modified data cache blocks out to memory and invalidate them.
>   * Does not invalidate the corresponding instruction cache blocks.
>   */
> -static inline void flush_dcache_range(unsigned long start, unsigned long 
> stop)
> +static inline void flush_dcache_range(void *start, unsigned long size)
>  {
> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));

unsigned long would be nicer than u32.

And ALIGN_DOWN() should work here I think.

> + unsigned long len = size + (L1_CACHE_BYTES - 1);

And ALIGN?

> @@ -83,22 +83,39 @@ static inline void clean_dcache_range(unsigned long 
> start, unsigned long stop)
>   * to invalidate the cache so the PPC core doesn't get stale data
>   * from the CPM (no cache snooping here :-).
>   */
> -static inline void invalidate_dcache_range(unsigned long start,
> -unsigned long stop)
> +static inline void invalidate_dcache_range(void *start, unsigned long size)
>  {
> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));
> + unsigned long len = size + (L1_CACHE_SHIFT - 1);
>   unsigned long i;
>  
> - for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> + for (i = 0; i < len >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
>   dcbi(addr);
>   mb();   /* sync */
>  }
>  
>  #endif /* CONFIG_PPC32 */
>  #ifdef CONFIG_PPC64
> -extern void flush_dcache_range(unsigned long start, unsigned long stop);
> -extern void flush_inval_dcache_range(unsigned long start, unsigned long 
> stop);
> +static inline void flush_dcache_range(void *start, unsigned long size)
> +{
> + void *addr = (void *)((u64)start & ~(L1_CACHE_BYTES - 1));
> + unsigned long len = size + (L1_CACHE_BYTES - 1);
> + unsigned long i;
> +
> + for (i = 0; i < len >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> + dcbf(addr);
> + mb();   /* sync */
> +}

I'd probably prefer a precursor patch to do the asm -> C conversion, but
I guess that's a pain because then you have to implement both the old
and new logic in C.

Also L1_CACHE_SHIFT is not necessarily == DCACHEL1BLOCKSIZE.

Finally it would be good to see what code the compiler generates out of
this, and see how it compares to the asm version. Not because it's
particularly performance critical (hopefully) but just so we know.

> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c 
> b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index 6b4f4cb..0f3a7d9 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/5

Re: [PATCH 1/2] usb: dwc3: Don't reinitialize core during host bus-suspend/resume

2017-07-20 Thread Manu Gautam
Hi,


On 7/13/2017 1:13 PM, Baolin Wang wrote:
> Hi,
>
> On 13 July 2017 at 15:26, Manu Gautam  wrote:
>>
>> On 7/13/2017 11:33 AM, Baolin Wang wrote:
>>> On 12 July 2017 at 16:58, Manu Gautam  wrote:
 On 7/12/2017 2:06 PM, Baolin Wang wrote:
> Hi,
>
> On 12 July 2017 at 15:25, Manu Gautam  wrote:
>> On 7/12/2017 12:19 PM, Baolin Wang wrote:
>>> Hi,
>>>
>>> On 3 July 2017 at 19:25, Manu Gautam  wrote:
 Driver powers-off PHYs and reinitializes DWC3 core and gadget on
 resume. While this works fine for gadget mode but in host
 mode there is not re-initialization of host stack. Also, resetting
 bus as part of bus_suspend/resume is not correct which could affect
 (or disconnect) connected devices.
 Fix this by not reinitializing core on suspend/resume in host mode
 for HOST only and OTG/drd configurations.
>>> But for some mobile devices, they also need suspend/resume in host
>>> mode which will power off dwc3 controller in glue layer. If we remove
>>> dwc3_core_init() for host mode, I am afraid it can not work well in
>>> host mode after resuming dwc3.
>> Can you point me to a glue driver doing that?
> Yes, now there are no glue driver doing that, but for many vendor
> trees they will do that. (Our platform will power off dwc3 when
> suspending dwc3, but have not upstreamed yet)
 Fine, but glue driver still need to take care of re-initialization on 
 resume
 with current design.
>>> Yes.
>>>
>> If dwc3 is powered off on suspend then Xhci level initialization also
>> needed
>> after
>> performing dwc3_core_init on resume which is currently not present. So
>> this
>> seems
>> to me broken anyway.
> Since we've add runtime PM for xhci-plat driver [1], which means we
> can runtime suspend/resume xhci device from dwc3 [2].
> [1] https://www.spinics.net/lists/linux-usb/msg156077.html
> [2] https://patchwork.kernel.org/patch/9471869/
 Sure. We can discuss this patch once you re-submit.
 One concern I have with the patch is marking ignore_children for dwc3 and 
 as
 part of
 dwc3 suspend/resume invoking
>>> Suppose we will suspend xhci device as dwc3's child, but now dwc3
>>> device is not suspend yet, which will block to suspend xhci device.
>>> Thus we should set ignore children flag to remove the parent
>>> dependency.
>> I may not have understood the issue. In any case children must be suspended 
>> first.
>> Example sequence is:
>> external hub/device (if any) suspend -> root hub  -> xhci -> dwc3 core -> 
>> dwc3 glue.
>> xhci device's runtime/pm suspend should happen before dwc3 gets suspended.
> Since the [2] patch will get/put xhci usage counter to avoid affecting
> other controller's runtime PM, then we need ignore child. But now
> xhci-plat driver has done this (by issuing pm_runtime_forbid()
> default), we can remove the get/put counter in dwc3, then we do not
> need ignore child flag any more.
>
 runtime pm on xhci. Ideally pm core should handle it seamlessly i.e.
 suspending dwc3
 once xhci gets suspended and pm_stay_awake/relax can be used to block
 pm_suspend
 until runtime suspend of glue driver happens.

> But [2] has not applied yet now, I will try to upstream it again. Then
> we can suspend xhci device--> suspend dwc3 host---> glue layer can
> power off, and it is similar when resuming in host mode.
 Do you see any issues with this patch?
 I am able to get runtime suspend/resume happening fine with XHCI on my
 platform.
 It involves runtime suspend/resume of USB HUB and dwc3 glue driver without
 any other
 change.
>>> Great, I will try this patch again.

Baolin, just checking if you were able to test this?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] usb: xhci: fix spinlock recursion for USB2 test mode

2017-07-20 Thread Mathias Nyman

On 19.07.2017 10:28, Peter Chen wrote:

Both xhci_hub_control and xhci_disable_slot tries to hold spinlock, the
spinlock recursion occurs when enters USB2 test mode. Fix it by unlock
spinlock before calling xhci_disable_slot.

Cc: 
Fixes: 0f1d832ed1fb ("usb: xhci: Add port test modes support for usb2")
Signed-off-by: Peter Chen 


Thanks, adding

-Mathias


--
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 v16 2/7] power: add power sequence library

2017-07-20 Thread Peter Chen
On Wed, Jul 19, 2017 at 01:34:11PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 19, 2017 10:56:00 AM Peter Chen wrote:
> > On Tue, Jul 18, 2017 at 07:06:05PM +0200, Rafael J. Wysocki wrote:
> > > On Tue, Jul 18, 2017 at 6:29 AM, Peter Chen  wrote:
> > > > On Mon, Jul 17, 2017 at 03:39:07PM +0200, Rafael J. Wysocki wrote:
> > > >> > Sorry, I should describe more.
> > > >> >
> > > >> > Let's take USB bus as an example, when the new USB device is at the
> > > >> > host port, the device structure at device model is not created until
> > > >> > it is discoverable by the USB bus. If this new USB device needs to be
> > > >> > powered on before can be discoverable by the bus, the device 
> > > >> > structure
> > > >> > will be not created without powering on operation. The code 
> > > >> > usb_alloc_dev
> > > >> > (drivers/usb/core/usb.c) is only called for discoverable device.
> > > >> >
> > > >> > Unlike the other bus, eg, platform bus, it creates device structure
> > > >> > according to DT node. The USB bus was designed for hot plug model, 
> > > >> > the
> > > >> > device structure is for discoverable device. In recent years, we 
> > > >> > begin
> > > >> > to have some hard-wired USB device, Eg, onboard USB-hub, onboard USB 
> > > >> > 4G
> > > >> > Modem, etc at the market. It needs some board level power operation 
> > > >> > before
> > > >> > it can be found by the USB bus. This patch set is designed primarily 
> > > >> > for
> > > >> > fix this kind of problem. You will see at at pwrseq_generic.c, we 
> > > >> > use DT
> > > >> > version clock API of_clk_get and DT version gpio API 
> > > >> > of_get_named_gpio_flags
> > > >> > instead of device structure version, like devm_clk_get and
> > > >> > devm_gpiod_get_optional.
> > > >> >
> > > >> > MMC system has similar use case, it creates power sequence platform
> > > >> > device for this issue, but all those power stuffs (clock, gpio, etc)
> > > >> > may not be suitable as a dedicated virtual device at DT, they are 
> > > >> > belonged
> > > >> > to one physical device, so this patch set is created to see if this 
> > > >> > issue
> > > >> > can be fixed better.
> > > >>
> > > >> OK, thanks for the explanation.
> > > >>
> > > >> The above needs to be part of your problem statement.
> > > >
> > > > Ok, I will add it to cover letter.
> > > >
> > > >> >
> > > >> > The bus will power up all device nodes in this bus according to DT
> > > >> > information, the device structure has not created at this time.
> > > >>
> > > >> OK
> > > >>
> > > >> I still think that the information on power resources depended on by 
> > > >> devices
> > > >> should be used for power management as well as for the initial 
> > > >> power-up.
> > > >>
> > > >> The most straightforward way to arrange for that would be to make it 
> > > >> possible
> > > >> to find the DT node matching the device after the device has been 
> > > >> discovered
> > > >> and struct device created for it, say by USB.  That would require 
> > > >> adding some
> > > >> more information on the device to the DT node, probably.
> > > >
> > > > After the device is created, the device node structure is under struct
> > > > device, say dev->of_node. The most difficulty for this issue is the
> > > > device creation is dynamic and is after the physical device is
> > > > discovered by the bus, the initial power-up is needed before the device
> > > > can be discovered by the bus.
> > > 
> > > So you power up all devices on the bus using the information from
> > > of_nodes upfront.
> > > 
> > 
> > I think your mean call pm_device_power_up(struct device *dev) for bus
> > level device (eg, USB HUB for USB), yeah, I can do that. But we still
> > have below problem:
> > 
> > Where we can put the kinds of power sequence implementation
> > (eg, pwrseq_generic.c) and the match mechanism between device
> > node and kinds of power sequence(eg, of_pwrseq_on at core.c)? These
> > stuffs can be shared among subsystems, and is better to use
> > one framework for it. MMC subsystem adds such things at its core
> > folder (drivers/mmc/core/pwrseq*) currently.
> 
> The power sequence handling code can be generic IMO, because it doesn't
> depend on what bus the devices are on.  It just needs to provide some
> services to its users, but you need a clean interface between it and its
> users, of course.
> 
> There basically are two categories of devices, let's refer to them as USB-type
> and MMC-type.  The difference between them, as far as power sequences go,
> boils down to initialization and discovery.
> 
> For the USB-type ones you need an upfront power-up pass over of_nodes
> under the bus controller.  That needs to be initiated by the controller 
> driver,
> but may be carried out by the common code.

The upfront power-up can't use pm_device_power_up API, since the
power sequence at device node is for the devices under the controller
device, it has to use the power sequence APIs directly, eg,
of_pwrseq_on_list or of_pwrseq_on.

Re: [PATCH] powerpc/asm/cacheflush: Cleanup cacheflush function params

2017-07-20 Thread Geert Uytterhoeven
On Thu, Jul 20, 2017 at 8:28 AM, Matt Brown  wrote:
> The cacheflush prototypes currently use start and stop values and each
> call requires typecasting the address to an unsigned long.
> This patch changes the cacheflush prototypes to follow the x86 style of
> using a base and size values, with base being a void pointer.
>
> All callers of the cacheflush functions, including drivers, have been
> modified to conform to the new prototypes.
>
> The 64 bit cacheflush functions which were implemented in assembly code
> (flush_dcache_range, flush_inval_dcache_range) have been translated into
> C for readability and coherence.
>
> Signed-off-by: Matt Brown 

>  drivers/spi/spi-bfin5xx.c| 10 +++---
>  drivers/usb/musb/blackfin.c  |  6 ++--

These are used on blackfin, so changing them without changing the blackfin
cache ops will break the build.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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