[PATCH] xhci: handle stop ep on invalid CStream
Below Note in xHCI spec 6.4.2.1 describes a Transfer Event is generated for Stop Endpoint Command on invalid CStream: CStream is not valid until a Streams endpoint transitions to the Start Stream state for the first time. A Transfer Event generated by a Stop Endpoint Command shall report '0' in the TRB Pointer and TRB Length fields if the command is executed and CStream is invalid. Refer to section 4.12.1. But, current implementation will output below error messages once driver receives the Transfer Event for invalid CStream: [ 133.184633] xhci_hcd :01:00.2: ERROR Transfer event for unknown stream ring slot 1 ep 2 [ 133.184635] xhci_hcd :01:00.2: @00046044a420 1b00 01038001 This issue can be observed while uas driver is handling SCSI command that connected UAS device doesn't support. UAS device rejects the unsupported command and causes Data-In stream for SCSI command data stopped when CStream is invalid. This patch handles the transfer event to remove false error messages. Signed-off-by: Henry Lin --- drivers/usb/host/xhci-ring.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index a8d92c9..8ae658e 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2267,6 +2267,11 @@ static int handle_tx_event(struct xhci_hcd *xhci, case COMP_RING_UNDERRUN: case COMP_RING_OVERRUN: goto cleanup; + case COMP_STOPPED_LENGTH_INVALID: + if ((ep->ep_state & EP_HAS_STREAMS) && + (event->buffer == 0)) + goto cleanup; + /* else fall through */ default: xhci_err(xhci, "ERROR Transfer event for unknown stream ring slot %u ep %u\n", slot_id, ep_index); -- 2.7.4
Price Inquiry
Hi,friend, This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia. We are glad to know about your company from the web and we are interested in your products. Could you kindly send us your Latest catalog and price list for our trial order. Best Regards, Daniel Murray Purchasing Manager
Re: [PATCH] usb: dwc2: gadget: fix ISOC frame overflow handling
Hi Minas,
On Mon, 12 Nov 2018 08:53:36 +
Minas Harutyunyan wrote:
> On 11/9/2018 10:43 PM, John Keeping wrote:
> > On Fri, 9 Nov 2018 14:36:36 +
> > Minas Harutyunyan wrote:
> >
> >> On 11/9/2018 12:43 PM, Minas Harutyunyan wrote:
> >>> Hi John,
> >>>
> >>> On 11/8/2018 9:37 PM, John Keeping wrote:
> Hi Minas,
>
> On Mon, 5 Nov 2018 08:28:07 +
> Minas Harutyunyan wrote:
>
> > On 10/23/2018 5:43 PM, John Keeping wrote:
> >> By clearing the overrun flag as soon as the target frame is
> >> next incremented, we can end up incrementing the target frame
> >> more than expected in dwc2_gadget_handle_ep_disabled() when the
> >> endpoint's interval is greater than 1. This happens if the
> >> target frame has just wrapped at the point when the endpoint is
> >> disabled and the frame number has not yet done so.
> >>
> >> Instead, wait until the frame number also wraps and then clear
> >> the overrun flag.
> >>
> >> Signed-off-by: John Keeping
> >> ---
> >> drivers/usb/dwc2/gadget.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/dwc2/gadget.c
> >> b/drivers/usb/dwc2/gadget.c index 2d6d2c8244de..8da2c052dfa1
> >> 100644 --- a/drivers/usb/dwc2/gadget.c
> >> +++ b/drivers/usb/dwc2/gadget.c
> >> @@ -117,7 +117,7 @@ static inline void
> >> dwc2_gadget_incr_frame_num(struct dwc2_hsotg_ep *hs_ep) if
> >> (hs_ep->target_frame > DSTS_SOFFN_LIMIT)
> >> { hs_ep->frame_overrun = true; hs_ep->target_frame &=
> >> DSTS_SOFFN_LIMIT;
> >> - } else {
> >> + } else if (hs_ep->parent->frame_number <
> >> hs_ep->target_frame) { hs_ep->frame_overrun = false;
> >>}
> >> }
> >>
> > Did you tested mentioned by you scenario? If you see issue can
> > you provide debug log and point the issue line in the log.
>
> It only reproduces very occasionally so it's difficult to capture
> a full debug log containing the error.
>
> I applied this patch to capture logging specifically around this
> scenario:
>
> -- >8 --
> diff --git a/drivers/usb/dwc2/gadget.c
> b/drivers/usb/dwc2/gadget.c index 220c0f9b89b0..3770b9d3b523
> 100644 --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2722,13 +2722,20 @@ static void
> dwc2_gadget_handle_ep_disabled(struct dwc2_hsotg_ep *hs_ep) }
>
> do {
> + unsigned int target_frame = hs_ep->target_frame;
> + bool frame_overrun = hs_ep->frame_overrun;
> +
> hs_req = get_ep_head(hs_ep);
> if (hs_req)
> dwc2_hsotg_complete_request(hsotg,
> hs_ep, hs_req, -ENODATA);
> +
> dwc2_gadget_incr_frame_num(hs_ep);
> /* Update current frame number value. */
> hsotg->frame_number =
> dwc2_hsotg_read_frameno(hsotg); +
> + dev_warn(hsotg->dev, "%s: expiring request
> frame_number=0x%04x target_frame=0x%04x overrun=%u\n",
> +__func__, hsotg->frame_number,
> target_frame, frame_overrun); } while
> (dwc2_gadget_target_frame_elapsed(hs_ep));
> dwc2_gadget_start_next_request(hs_ep);
> -- 8< --
>
> >>> According above patch in debug log should be printed overrun flag
> >>> also. Could you please resend log with this flag.
> >
> > D'oh! I included a log from an earlier version before I added the
> > overrun flag.
> >
> >> One more request. Please add EP number to debug print.
> >
> > Here's an updated log:
> >
> > -- >8 --
> > [ 264.926385] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled:
> > expiring request ep=2 frame_number=0x2161 target_frame=0x2168
> > overrun=0 [ 265.905413] dwc2 ff58.usb:
> > dwc2_gadget_handle_ep_disabled: expiring request ep=2
> > frame_number=0x3ff9 target_frame=0x0008 overrun=0 [ 265.905421]
> > dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring request
> > ep=2 frame_number=0x3ff9 target_frame=0x0010 overrun=0
> > [ 265.905427] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled:
> > expiring request ep=2 frame_number=0x3ff9 target_frame=0x0018
> > overrun=0 [ 265.905432] dwc2 ff58.usb:
> > dwc2_gadget_handle_ep_disabled: expiring request ep=2
> > frame_number=0x3ff9 target_frame=0x0020 overrun=0 [ 265.905438]
> > dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring request
> > ep=2 frame_number=0x3ff9 target_frame=0x0028 overrun=0
> > [ 265.905443] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled:
> > expiring request ep=2 frame_number=0x3ff9 target_frame=0x0030
> > overrun=0 [ 265.905448] dwc2 ff58.usb:
> > dwc2_gadget_handle_ep_disabled: expiring request ep=2
> > frame_number=0x3
[PATCH] usb: xhci: fix build warning - missing prototype
Fix build warning when building drivers/usb/host/xhci-mem.o due to missing
prototype for xhci_free_virt_devices_depth_first.
This function is only used in xhci-mem.c so just make it static.
Signed-off-by: Jean-Philippe Menil
---
drivers/usb/host/xhci-mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b1f27aa38b10..c5236f5d917a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -933,7 +933,7 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int
slot_id)
* that tt_info, then free the child first. Recursive.
* We can't rely on udev at this point to find child-parent relationships.
*/
-void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
+static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int
slot_id)
{
struct xhci_virt_device *vdev;
struct list_head *tt_list_head;
--
2.17.1
Re: [PATCH v1 1/1] usb: dwc3: Fix NULL pointer exception in dwc3_pci_remove()
Felipe/Greg,
Any comments ?
On 10/17/18 11:40 AM, Kuppuswamy Sathyanarayanan wrote:
In dwc3_pci_quirks() function, gpiod lookup table is only registered for
baytrail SOC. But in dwc3_pci_remove(), we try to unregistered it
without any checks. This leads to NULL pointer de-reference exception in
gpiod_remove_lookup_table() when unloading the module for non baytrail
SOCs. This patch fixes this issue.
Fixes: 5741022cbdf3 ("usb: dwc3: pci: Add GPIO lookup table on platforms
without ACPI GPIO resources")
Cc:
Signed-off-by: Kuppuswamy Sathyanarayanan
Reviewed-by: Heikki Krogerus
---
drivers/usb/dwc3/dwc3-pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 1286076a8890..842795856bf4 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -283,8 +283,10 @@ static int dwc3_pci_probe(struct pci_dev *pci, const
struct pci_device_id *id)
static void dwc3_pci_remove(struct pci_dev *pci)
{
struct dwc3_pci *dwc = pci_get_drvdata(pci);
+ struct pci_dev *pdev = dwc->pci;
- gpiod_remove_lookup_table(&platform_bytcr_gpios);
+ if (pdev->device == PCI_DEVICE_ID_INTEL_BYT)
+ gpiod_remove_lookup_table(&platform_bytcr_gpios);
#ifdef CONFIG_PM
cancel_work_sync(&dwc->wakeup_work);
#endif
--
Sathyanarayanan Kuppuswamy
Linux kernel developer
Re: [PATCH v1 1/5] drivercore: Revert "deferral race condition fix"
Hi Andy,
On 2018-11-10 20:10, Andy Shevchenko wrote:
> Consider the following scenario.
>
> There are two independent devices coupled together by functional dependencies:
> - USB OTG (dwc3-pci)
> - extcon (tested with extcon-intel-mrfld, not yet in upstream)
>
> Each of the driver services a corresponding device is built as a module. In
> the
> Buildroot environment the modules are probed by alphabetical ordering of their
> modaliases. The latter comes to the case when USB OTG driver will be probed
> first followed by extcon one.
>
> So, if the platform anticipates extcon device to be appeared, in the above
> case
> we will get deferred probe of USB OTG, because of ordering.
>
> Now, a cherry on top of the cake, the deferred probing list contains
> the only two modules, i.e. USB OTG and extcon. Due to above circumstances,
> values in the local_trigger_count and deferred_trigger_count are not the same,
> and thus provokes deferred probe triggering again and again.
>
> ...
> [ 20.678332] platform dwc3.0.auto: Retrying from deferred list
> [ 20.694743] platform dwc3.0.auto: Driver dwc3 requests probe deferral
> [ 20.701254] platform dwc3.0.auto: Added to deferred list
> [ 20.706620] platform dwc3.0.auto: driver_deferred_probe_add_trigger 1 2
> [ 20.713732] platform dwc3.0.auto: Retrying from deferred list
> [ 20.730035] platform dwc3.0.auto: Driver dwc3 requests probe deferral
> [ 20.736540] platform dwc3.0.auto: Added to deferred list
> [ 20.741889] platform dwc3.0.auto: driver_deferred_probe_add_trigger 3 4
> [ 20.748991] platform dwc3.0.auto: Retrying from deferred list
> [ 20.765416] platform dwc3.0.auto: Driver dwc3 requests probe deferral
> [ 20.771914] platform dwc3.0.auto: Added to deferred list
> [ 20.777279] platform dwc3.0.auto: driver_deferred_probe_add_trigger 5 6
> ...
>
> Deeper investigation shows the culprit commit 58b116bce136
> ("drivercore: deferral race condition fix") which was dedicated to fix some
> other issue while bringing a regression.
>
> This reverts commit 58b116bce13612e5aa6fcd49ecbd4cf8bb59e835 for good until
> we will have better solution.
if we revert the commit then the original issue will re-surfaces. afaik
it was not only audio which hit the 'last driver to be probed from the
deferred list would never probe, unless we provoke the kernel to load
additional module, or remove/reload the module' issue.
Do I understand correctly that in your case you have two modules
(dwc3-pci and extcon-intel-mrfld) in a deferred probe loop, iow both of
the drivers returns -EPROBE_DEFER and they just spin?
If both is deferring, how this supposed to work?
If we revert 58b116bce13612e5aa6fcd49ecbd4cf8bb59e835, then you might be
hitting the very same issue as described by the commit:
s/davinci_evm sound.3/dwc3-pci
s/davinci-mcasp 4803c000.mcasp/extcon-intel-mrfld
Am I missing something?
>
> Cc: Grant Likely
> Cc: Peter Ujfalusi
> Cc: Greg Kroah-Hartman
> Cc: Mark Brown
> Cc: Felipe Balbi
> Cc: Andrzej Hajda
> Signed-off-by: Andy Shevchenko
> ---
> drivers/base/dd.c | 27 ++-
> 1 file changed, 2 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 169412ee4ae8..9a966e45fda5 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -53,7 +53,6 @@
> static DEFINE_MUTEX(deferred_probe_mutex);
> static LIST_HEAD(deferred_probe_pending_list);
> static LIST_HEAD(deferred_probe_active_list);
> -static atomic_t deferred_trigger_count = ATOMIC_INIT(0);
> static struct dentry *deferred_devices;
> static bool initcalls_done;
>
> @@ -143,17 +142,6 @@ static bool driver_deferred_probe_enable = false;
> * This functions moves all devices from the pending list to the active
> * list and schedules the deferred probe workqueue to process them. It
> * should be called anytime a driver is successfully bound to a device.
> - *
> - * Note, there is a race condition in multi-threaded probe. In the case where
> - * more than one device is probing at the same time, it is possible for one
> - * probe to complete successfully while another is about to defer. If the
> second
> - * depends on the first, then it will get put on the pending list after the
> - * trigger event has already occurred and will be stuck there.
> - *
> - * The atomic 'deferred_trigger_count' is used to determine if a successful
> - * trigger has occurred in the midst of probing a driver. If the trigger
> count
> - * changes in the midst of a probe, then deferred processing should be
> triggered
> - * again.
> */
> static void driver_deferred_probe_trigger(void)
> {
> @@ -166,7 +154,6 @@ static void driver_deferred_probe_trigger(void)
>* into the active list so they can be retried by the workqueue
>*/
> mutex_lock(&deferred_probe_mutex);
> - atomic_inc(&deferred_trigger_count);
> list_splice_tail_init(&deferred_probe_pending_list,
> &deferred_probe_ac
Re: Query on usb/core/devio.c
On Mon, 12 Nov 2018, Mayuresh Kulkarni wrote: > On Fri, 2018-11-09 at 10:33 -0500, Alan Stern wrote: > > On Fri, 9 Nov 2018, Mayuresh Kulkarni wrote: > > > > > > > > > > > > > The driver has no way to tell whether the resume was caused by the > > > > host or by the device. (In fact, it's possible for a resume to be > > > > caused by _both_ the host and the device, if they request it at the > > > > same time.) In the end, it doesn't matter anyway. > > > > > > > Yes agreed to the point that driver as such does not know if its resume is > > > called due to host or remote wake. > > > But based on ioctl call, it should be possible to know, if resume happened > > > due > > > to resume-ioctl or remote-wake (e.g.: using a flag in resume-ioctl), > > > right? > > No, I keep telling you, it is not possible. Furthermore, you should > > never care what the cause was. After all, what difference would it > > make to your program? It shouldn't make any difference -- you should > > do exactly the same thing no matter what the wakeup cause was. > > > > I think I now understand the disconnect between us this point. Below is an > attempt to bridge that, so please bear with me: > 1. In our use-case(s), the end user can "interact" with composite USB device > either by physically interacting with or via an app on Android. > 2. When the interaction is "physical" (e.g.: something that is sensed by a > sensor or a button press), we need "remote-wake" from USB device to host. > 3. When the interaction is via app (e.g.: to toggle some control), we need > "host-wake" from host to USB device. Oliver gave a good explanation of where you are going wrong, but I will fill in some of the missing steps here. > And this is where, the knowledge of wake-up source and difference in handling > comes into picture. > For (2), the host needs to first wake-up and then queue a request to read-out > "what" happened. Wrong. For all you know, the user may have both toggled a control in the app _and_ pressed a button on the device. Therefore the host needs to queue a request to read out what happened _and_ it needs to take whatever actions have been requested by the user. > For (3), the host needs to first wake-up the device and then queue the > command(s) to the take action asked by the end user. Wrong. The user may have triggered a sensor on the device as well as toggled a control in the app, in which case the host does not need to wake up the device (the device will have issued its own remote wakeup) but it does need to queue a request to see if anything has happened. > I agree that in both of above cases, "a USB request" needs to be send from > user- > space to kernel to device. But in my opinion, each of the them has a different > context associated with it. E.g.: for (2), a single read request is sufficient > to "know" what happened while for (3), a write and a read request is needed. In both cases, you always need to query the device's state and you may need to send a command to the device. Hence there's no point in trying to distinguish the two cases. > With that said, due to above reason(s), it is proposed to add 3 "new" ioctls. > Does the calling sequence by user-space I mentioned in previous response, > seems > sensible now? My earlier response still stands. > > > > > > As I understand, the calling sequence by user-space would be like - > > > > > > ioctl(fd, USBDEVFS_SUSPEND); ==> put-down PM ref-count and return > > > > > > ret = ioctl(fd, USBDVEFS_WAIT_FOR_REMOTE_WAKE); ==> will block for resume > > This should be WAIT_FOR_RESUME, not WAIT_FOR_REMOTE_WAKE. > > > > OK. > > > > > > > if (ret == EHOST_WAKE) > > > /* handle host wake case */ > > > else if (ret == EREMOTE_WAKE) > > > /* handle remote wake case */ > > > else > > > /* handle other return values */ > > No way to tell the difference between wakeup causes. The return value > > would be 0 for a resume and -1 if the ioctl was interrupted by a signal > > before the device resumed (or perhaps if the device file was closed > > before the ioctl returned). > > Based on above comment, if we are able to distinguish the "resume" cause, the > user-space will be in better position to take action. But we are not able to distinguish the cause, and even if we were, it's quite possible that the user could interact with the device after the time the host woke it up but before any commands were issued. Therefore the app must always query the device's state. What would you do if the device _wasn't_ suspended when the user interacted with the app? Wouldn't the app still need to find out somehow that the user had also triggered a sensor? The same remains true if the device _was_ suspended. Alan Stern > > I still think this should be implemented in the runtime PM core through > > poll/select in sysfs. But we may as well also offer an interface > > through usbfs. > > > > > > > > When user wants to interact with the device, user space should c
Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel
Sorry, please ignore this. Used incorrect Vinod email address.
On Mon, Nov 12, 2018 at 09:40:49AM -0600, Bin Liu wrote:
> The driver defines three states for a cppi channel.
> - idle: .chan_busy == 0 && not in .pending list
> - pending: .chan_busy == 0 && in .pending list
> - busy: .chan_busy == 1 && not in .pending list
>
> There are cases in which the cppi channel could be in the pending state
> when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
> is called.
>
> cppi41_stop_chan() has a bug for these cases to set channels to idle state.
> It only checks the .chan_busy flag, but not the .pending list, then later
> when cppi41_runtime_resume() is called the channels in .pending list will
> be transitioned to busy state.
>
> Removing channels from the .pending list solves the problem.
>
> Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy")
> Cc: sta...@vger.kernel.org # v3.15+
> Signed-off-by: Bin Liu
> ---
> drivers/dma/ti/cppi41.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index 1497da367710..e507ec36c0d3 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
>
> desc_phys = lower_32_bits(c->desc_phys);
> desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
> - if (!cdd->chan_busy[desc_num])
> + if (!cdd->chan_busy[desc_num]) {
> + struct cppi41_channel *cc, *_ct;
> +
> + /*
> + * channels might still be in the pendling list if
> + * cppi41_dma_issue_pending() is called after
> + * cppi41_runtime_suspend() is called
> + */
> + list_for_each_entry_safe(cc, _ct, &cdd->pending, node) {
> + if (cc != c)
> + continue;
> + list_del(&cc->node);
> + break;
> + }
> return 0;
> + }
>
> ret = cppi41_tear_down_chan(c);
> if (ret)
> --
> 2.17.1
>
[PATCH] dma: cppi41: delete channel from pending list when stop channel
The driver defines three states for a cppi channel.
- idle: .chan_busy == 0 && not in .pending list
- pending: .chan_busy == 0 && in .pending list
- busy: .chan_busy == 1 && not in .pending list
There are cases in which the cppi channel could be in the pending state
when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
is called.
cppi41_stop_chan() has a bug for these cases to set channels to idle state.
It only checks the .chan_busy flag, but not the .pending list, then later
when cppi41_runtime_resume() is called the channels in .pending list will
be transitioned to busy state.
Removing channels from the .pending list solves the problem.
Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy")
Cc: sta...@vger.kernel.org # v3.15+
Signed-off-by: Bin Liu
---
drivers/dma/ti/cppi41.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index 1497da367710..e507ec36c0d3 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
desc_phys = lower_32_bits(c->desc_phys);
desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
- if (!cdd->chan_busy[desc_num])
+ if (!cdd->chan_busy[desc_num]) {
+ struct cppi41_channel *cc, *_ct;
+
+ /*
+* channels might still be in the pendling list if
+* cppi41_dma_issue_pending() is called after
+* cppi41_runtime_suspend() is called
+*/
+ list_for_each_entry_safe(cc, _ct, &cdd->pending, node) {
+ if (cc != c)
+ continue;
+ list_del(&cc->node);
+ break;
+ }
return 0;
+ }
ret = cppi41_tear_down_chan(c);
if (ret)
--
2.17.1
[PATCH] dma: cppi41: delete channel from pending list when stop channel
The driver defines three states for a cppi channel.
- idle: .chan_busy == 0 && not in .pending list
- pending: .chan_busy == 0 && in .pending list
- busy: .chan_busy == 1 && not in .pending list
There are cases in which the cppi channel could be in the pending state
when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
is called.
cppi41_stop_chan() has a bug for these cases to set channels to idle state.
It only checks the .chan_busy flag, but not the .pending list, then later
when cppi41_runtime_resume() is called the channels in .pending list will
be transitioned to busy state.
Removing channels from the .pending list solves the problem.
Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy")
Cc: sta...@vger.kernel.org # v3.15+
Signed-off-by: Bin Liu
---
drivers/dma/ti/cppi41.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index 1497da367710..e507ec36c0d3 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
desc_phys = lower_32_bits(c->desc_phys);
desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
- if (!cdd->chan_busy[desc_num])
+ if (!cdd->chan_busy[desc_num]) {
+ struct cppi41_channel *cc, *_ct;
+
+ /*
+* channels might still be in the pendling list if
+* cppi41_dma_issue_pending() is called after
+* cppi41_runtime_suspend() is called
+*/
+ list_for_each_entry_safe(cc, _ct, &cdd->pending, node) {
+ if (cc != c)
+ continue;
+ list_del(&cc->node);
+ break;
+ }
return 0;
+ }
ret = cppi41_tear_down_chan(c);
if (ret)
--
2.17.1
RE: scsi_set_medium_removal timeout issue
On Mon, 12 Nov 2018, Zengtao (B) wrote: > >> >Something is wrong here. Before sending PREVENT-ALLOW MEDIUM > >> >REMOVAL, the host should issue SYNCHRONIZE CACHE. This will force > >> >fsg_lun_fsync_sub to run, and the host should allow a long timeout > >> >for this command. Then when PREVENT-ALLOW MEDIUM REMOVAL > >is sent, > >> >nothing will need to be flushed. > >> > > >> > >> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it > >> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe > >something > >> wrong with the scsi layer or something wrong with the mass storage > >class driver? > > > >Or it could be something else. Can you please post the dmesg log from > >the host, showing what happens when the device is first plugged in? > > > > I have enabled the SCSI log for the host, please refer to the attachment. The log you attached was incomplete -- it was missing some commands from the beginning. In any case, it wasn't what I wanted. I asked you to post the dmesg log, not the SCSI log. Alan Stern
Re: Query on usb/core/devio.c
On Mo, 2018-11-12 at 12:04 +, Mayuresh Kulkarni wrote: > I think I now understand the disconnect between us this point. Below is an > attempt to bridge that, so please bear with me: > 1. In our use-case(s), the end user can "interact" with composite USB device > either by physically interacting with or via an app on Android. > 2. When the interaction is "physical" (e.g.: something that is sensed by a > sensor or a button press), we need "remote-wake" from USB device to host. > 3. When the interaction is via app (e.g.: to toggle some control), we need > "host-wake" from host to USB device. All of these events can occur at the same time. And you cannot prevent that. > And this is where, the knowledge of wake-up source and difference in handling > comes into picture. > For (2), the host needs to first wake-up and then queue a request to read-out > "what" happened. > For (3), the host needs to first wake-up the device and then queue the > command(s) to the take action asked by the end user. > > I agree that in both of above cases, "a USB request" needs to be send from > user- > space to kernel to device. But in my opinion, each of the them has a different > context associated with it. E.g.: for (2), a single read request is sufficient > to "know" what happened while for (3), a write and a read request is needed. There is a natural race condition between remote and host wake up. You cannot rule out that your app wakes the device up, exactly when user interaction would have woken it anyway. Your app will know what it wants to do with the device. And you can detect that. Hence the correct algorithm is: 1. wake up the device 2. query it for a cause of wake up 3. If the host wants to do IO, do all of it. Regards Oliver
Re: Query on usb/core/devio.c
On Fri, 2018-11-09 at 10:33 -0500, Alan Stern wrote: > On Fri, 9 Nov 2018, Mayuresh Kulkarni wrote: > > > > > > > > > The driver has no way to tell whether the resume was caused by the > > > host or by the device. (In fact, it's possible for a resume to be > > > caused by _both_ the host and the device, if they request it at the > > > same time.) In the end, it doesn't matter anyway. > > > > > Yes agreed to the point that driver as such does not know if its resume is > > called due to host or remote wake. > > But based on ioctl call, it should be possible to know, if resume happened > > due > > to resume-ioctl or remote-wake (e.g.: using a flag in resume-ioctl), right? > No, I keep telling you, it is not possible. Furthermore, you should > never care what the cause was. After all, what difference would it > make to your program? It shouldn't make any difference -- you should > do exactly the same thing no matter what the wakeup cause was. > I think I now understand the disconnect between us this point. Below is an attempt to bridge that, so please bear with me: 1. In our use-case(s), the end user can "interact" with composite USB device either by physically interacting with or via an app on Android. 2. When the interaction is "physical" (e.g.: something that is sensed by a sensor or a button press), we need "remote-wake" from USB device to host. 3. When the interaction is via app (e.g.: to toggle some control), we need "host-wake" from host to USB device. And this is where, the knowledge of wake-up source and difference in handling comes into picture. For (2), the host needs to first wake-up and then queue a request to read-out "what" happened. For (3), the host needs to first wake-up the device and then queue the command(s) to the take action asked by the end user. I agree that in both of above cases, "a USB request" needs to be send from user- space to kernel to device. But in my opinion, each of the them has a different context associated with it. E.g.: for (2), a single read request is sufficient to "know" what happened while for (3), a write and a read request is needed. With that said, due to above reason(s), it is proposed to add 3 "new" ioctls. Does the calling sequence by user-space I mentioned in previous response, seems sensible now? > > > > As I understand, the calling sequence by user-space would be like - > > > > ioctl(fd, USBDEVFS_SUSPEND); ==> put-down PM ref-count and return > > > > ret = ioctl(fd, USBDVEFS_WAIT_FOR_REMOTE_WAKE); ==> will block for resume > This should be WAIT_FOR_RESUME, not WAIT_FOR_REMOTE_WAKE. > OK. > > > > if (ret == EHOST_WAKE) > > /* handle host wake case */ > > else if (ret == EREMOTE_WAKE) > > /* handle remote wake case */ > > else > > /* handle other return values */ > No way to tell the difference between wakeup causes. The return value > would be 0 for a resume and -1 if the ioctl was interrupted by a signal > before the device resumed (or perhaps if the device file was closed > before the ioctl returned). Based on above comment, if we are able to distinguish the "resume" cause, the user-space will be in better position to take action. > > I still think this should be implemented in the runtime PM core through > poll/select in sysfs. But we may as well also offer an interface > through usbfs. > > > > > When user wants to interact with the device, user space should call - > > > > ioctl(fd, USBDEVFS_RESUME); ==> gets PM ref-count, unblock wait-for-remote > > and > > return > Not necessary, although I suppose we should support this. The device > would automatically be resumed anyway, when the user interacts with it. > I think the first comment above applies to this part as well (part related to how end user can interact with the composite USB device). > > > > Here the "get PM ref-count" should cause the resume of root-hub and its port > > followed by resume of device, right? > > As I understand, as a result of this operation, the host controller should > > bring > > back the link to L0. > Right. > > > > > Am I missing some important aspect here? If yes, could you please help > > explain? > Only the things noted above. > > Alan Stern
RE: scsi_set_medium_removal timeout issue
Hi Alan: >-Original Message- >From: Alan Stern [mailto:st...@rowland.harvard.edu] >Sent: Wednesday, October 31, 2018 10:20 PM >To: Zengtao (B) >Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; >gre...@linuxfoundation.org; linux-s...@vger.kernel.org; >linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; >usb-stor...@lists.one-eyed-alien.net >Subject: RE: scsi_set_medium_removal timeout issue > >On Wed, 31 Oct 2018, Zengtao (B) wrote: > >> Hi: >> >> >-Original Message- >> >From: Alan Stern [mailto:st...@rowland.harvard.edu] >> >Sent: Tuesday, October 30, 2018 10:08 PM >> >To: Zengtao (B) >> >Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; >> >gre...@linuxfoundation.org; linux-s...@vger.kernel.org; >> >linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; >> >usb-stor...@lists.one-eyed-alien.net >> >Subject: Re: scsi_set_medium_removal timeout issue >> > >> >On Tue, 30 Oct 2018, Zengtao (B) wrote: >> > >> >> Hi >> >> >> >> I have recently met a scsi_set_medium_removal timeout issue, and >> >> it's related to both SCSI and USB MASS storage. >> >> Since i am not an expert in either scsi or usb mass storage, i am >> >> writing to report the issue and ask for a solution from you guys. >> >> >> >> My test scenario is as follow: >> >> 1.Linux HOST-Linux mass storage gadget(the back store is a >> >> flash >> >partition). >> >> 2.Host mount the device. >> >> 3.Host writes some data to the Mass storage device. >> >> 4.Host Unmount the device. >> >> Both Linux kernels(Host and Device) are Linux 4.9. >> >> Some has reported the same issue a long time ago, but it remains >there. >> >> https://www.spinics.net/lists/linux-usb/msg53739.html >> >> >> >> For the issue itself, there is my observation: >> >> In the step 4, the Host will issue an >> >PREVENT_ALLOW_MEDIUM_REMOVAL scsi command. >> >> and and timeout happens due to the device 's very slow >> >fsg_lun_fsync_sub. >> > >> >Something is wrong here. Before sending PREVENT-ALLOW MEDIUM >> >REMOVAL, the host should issue SYNCHRONIZE CACHE. This will force >> >fsg_lun_fsync_sub to run, and the host should allow a long timeout >> >for this command. Then when PREVENT-ALLOW MEDIUM REMOVAL >is sent, >> >nothing will need to be flushed. >> > >> >> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it >> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe >something >> wrong with the scsi layer or something wrong with the mass storage >class driver? > >Or it could be something else. Can you please post the dmesg log from >the host, showing what happens when the device is first plugged in? > I have enabled the SCSI log for the host, please refer to the attachment. Thanks. Zengtao ~ # ~ # ~ # ~ # ~ # mkfs.vfat /dev/sda1 moun~ # ~ # mount /dev/sda1 /mnt ~ # ~ # time dd if=/dev/zero of=/mnt/test0 bs=16k count=5120 5120+0 records in 5120+0 records out 83886080 bytes (80.0MB) copied, 2.970369 seconds, 26.9MB/s real0m 2.97s user0m 0.01s sys 0m 0.76s ~ # ~ # umount /mnt sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 12 f6 00 00 80 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b700 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b700 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 16 46 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 16 46 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0 sd 0:0:0:0: Notifying upper driver of completion (result 0) sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800 sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 17 36 00 00 f0 00 sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK dr
Re: [PATCH v1 2/5] extcon: Return -EPROBE_DEFER when extcon device is not found
On Sat, Nov 10, 2018 at 08:10:58PM +0200, Andy Shevchenko wrote:
> All current users of extcon_get_extcon_dev() API considers
> an extcon device a mandatory to appear. Thus, they all convert
> NULL pointer to -EPROBE_DEFER error code.
>
> There is one more caller anticipated with the same requirements.
>
> To decrease a code duplication and a burden to the callers,
> return -EPROBE_DEFER directly from extcon_get_extcon_dev().
>
> Signed-off-by: Andy Shevchenko
Reviewed-by: Heikki Krogerus
> ---
> drivers/extcon/extcon-axp288.c| 4 ++--
> drivers/extcon/extcon.c | 2 +-
> drivers/power/supply/axp288_charger.c | 8
> drivers/usb/phy/phy-omap-otg.c| 6 +++---
> drivers/usb/typec/tcpm/fusb302.c | 4 ++--
> 5 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index a983708b77a6..3472d3b756ed 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -360,8 +360,8 @@ static int axp288_extcon_probe(struct platform_device
> *pdev)
> name = acpi_dev_get_first_match_name("INT3496", NULL, -1);
> if (name) {
> info->id_extcon = extcon_get_extcon_dev(name);
> - if (!info->id_extcon)
> - return -EPROBE_DEFER;
> + if (IS_ERR(info->id_extcon))
> + return PTR_ERR(info->id_extcon);
>
> dev_info(dev, "controlling USB role\n");
> } else {
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 5ab0498be652..2bd0f2f33f05 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -884,7 +884,7 @@ struct extcon_dev *extcon_get_extcon_dev(const char
> *extcon_name)
> if (!strcmp(sd->name, extcon_name))
> goto out;
> }
> - sd = NULL;
> + sd = ERR_PTR(-EPROBE_DEFER);
> out:
> mutex_unlock(&extcon_dev_list_lock);
> return sd;
> diff --git a/drivers/power/supply/axp288_charger.c
> b/drivers/power/supply/axp288_charger.c
> index 735658ee1c60..8558577fccf5 100644
> --- a/drivers/power/supply/axp288_charger.c
> +++ b/drivers/power/supply/axp288_charger.c
> @@ -768,17 +768,17 @@ static int axp288_charger_probe(struct platform_device
> *pdev)
> info->regmap_irqc = axp20x->regmap_irqc;
>
> info->cable.edev = extcon_get_extcon_dev(AXP288_EXTCON_DEV_NAME);
> - if (info->cable.edev == NULL) {
> + if (IS_ERR(info->cable.edev)) {
> dev_dbg(&pdev->dev, "%s is not ready, probe deferred\n",
> AXP288_EXTCON_DEV_NAME);
> - return -EPROBE_DEFER;
> + return PTR_ERR(info->cable.edev);
> }
>
> if (acpi_dev_present(USB_HOST_EXTCON_HID, NULL, -1)) {
> info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_NAME);
> - if (info->otg.cable == NULL) {
> + if (IS_ERR(info->otg.cable)) {
> dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe
> deferred\n");
> - return -EPROBE_DEFER;
> + return PTR_ERR(info->otg.cable);
> }
> dev_info(&pdev->dev,
>"Using " USB_HOST_EXTCON_HID " extcon for usb-id\n");
> diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
> index ee0863c6553e..605314ddcd3d 100644
> --- a/drivers/usb/phy/phy-omap-otg.c
> +++ b/drivers/usb/phy/phy-omap-otg.c
> @@ -91,12 +91,12 @@ static int omap_otg_probe(struct platform_device *pdev)
> int ret;
> u32 rev;
>
> - if (!config || !config->extcon)
> + if (!config)
> return -ENODEV;
>
> extcon = extcon_get_extcon_dev(config->extcon);
> - if (!extcon)
> - return -EPROBE_DEFER;
> + if (IS_ERR(extcon))
> + return PTR_ERR(extcon);
>
> otg_dev = devm_kzalloc(&pdev->dev, sizeof(*otg_dev), GFP_KERNEL);
> if (!otg_dev)
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 43b64d9309d0..6d332066202b 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -1767,8 +1767,8 @@ static int fusb302_probe(struct i2c_client *client,
>*/
> if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
> chip->extcon = extcon_get_extcon_dev(name);
> - if (!chip->extcon)
> - return -EPROBE_DEFER;
> + if (IS_ERR(chip->extcon))
> + return PTR_ERR(chip->extcon);
> }
>
> chip->vbus = devm_regulator_get(chip->dev, "vbus");
> --
> 2.19.1
--
heikki
Re: [PATCH v1 4/5] usb: dwc3: drd: Switch to device property for 'extcon' handling
Hi, Andy Shevchenko writes: > Switch to device property for 'extcon' handling. > No functional change intended. > > Signed-off-by: Andy Shevchenko > --- > drivers/usb/dwc3/drd.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c > index e7e421521a34..5dc4cddd5b68 100644 > --- a/drivers/usb/dwc3/drd.c > +++ b/drivers/usb/dwc3/drd.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > > #include "debug.h" > #include "core.h" > @@ -485,8 +486,8 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 > *dwc) > struct device_node *np_phy, *np_conn; > struct extcon_dev *edev; > > - if (of_property_read_bool(dev->of_node, "extcon")) > - return extcon_get_edev_by_phandle(dwc->dev, 0); > + if (device_property_read_bool(dev, "extcon")) > + return extcon_get_edev_by_phandle(dev, 0); No reservations against any of the patches. What is the idea for getting these merged upstream? I don't mind taking 4-5 through my tree, but what are the other folks considering? -- balbi signature.asc Description: PGP signature
Re: [PATCH] HID: add driver for U2F Zero built-in LED and RNG
On Thu, 1 Nov 2018, Andrej Shadura wrote:
> Hi everyone,
>
> I’ve got a comment from Nick Kossifidis that I probably shouldn’t set
> RNG’s quality to 1024. Adding linux-crypto@ to the loop.
So, what was this about? Is there any resolution to it? :)
Thanks.
>
> On 23/10/2018 16:46, Andrej Shadura wrote:
> > U2F Zero supports custom commands for blinking the LED and getting data
> > from the internal hardware RNG. Expose the blinking function as a LED
> > device, and the internal hardware RNG as an HWRNG so that it can be used
> > to feed the enthropy pool.
> >
> > Signed-off-by: Andrej Shadura
> > ---
> > drivers/hid/Kconfig | 15 ++
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-u2fzero.c | 371 ++
> > 4 files changed, 388 insertions(+)
> > create mode 100644 drivers/hid/hid-u2fzero.c
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 61e1953ff921..3de6bf202f20 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -975,6 +975,21 @@ config HID_UDRAW_PS3
> > Say Y here if you want to use the THQ uDraw gaming tablet for
> > the PS3.
> >
> > +config HID_U2FZERO
> > + tristate "U2F Zero LED and RNG support"
> > + depends on HID
> > + depends on LEDS_CLASS
> > + help
> > + Support for the LED of the U2F Zero device.
> > +
> > + U2F Zero supports custom commands for blinking the LED
> > + and getting data from the internal hardware RNG.
> > + The internal hardware can be used to feed the enthropy pool.
> > +
> > + U2F Zero only supports blinking its LED, so this driver doesn't
> > + allow setting the brightness to anything but 1, which will
> > + trigger a single blink and immediately reset to back 0.
> > +
> > config HID_WACOM
> > tristate "Wacom Intuos/Graphire tablet support (USB)"
> > depends on USB_HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index bd7ac53b75c5..617b1a928b6a 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -107,6 +107,7 @@ obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
> > obj-$(CONFIG_HID_TIVO) += hid-tivo.o
> > obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
> > obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
> > +obj-$(CONFIG_HID_U2FZERO) += hid-u2fzero.o
> > obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
> > obj-$(CONFIG_HID_UDRAW_PS3)+= hid-udraw-ps3.o
> > obj-$(CONFIG_HID_LED) += hid-led.o
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index bc49909aba8e..070a2fcd8a18 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -309,6 +309,7 @@
> > #define USB_DEVICE_ID_CYGNAL_RADIO_SI470X 0x818a
> > #define USB_DEVICE_ID_FOCALTECH_FT_MULTITOUCH 0x81b9
> > #define USB_DEVICE_ID_CYGNAL_CP21120xea90
> > +#define USB_DEVICE_ID_U2F_ZERO 0x8acf
> >
> > #define USB_DEVICE_ID_CYGNAL_RADIO_SI4713 0x8244
> >
> > diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
> > new file mode 100644
> > index ..bd4077c93c97
> > --- /dev/null
> > +++ b/drivers/hid/hid-u2fzero.c
> > @@ -0,0 +1,371 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * U2F Zero LED and RNG driver
> > + *
> > + * Copyright 2018 Andrej Shadura
> > + * Loosely based on drivers/hid/hid-led.c
> > + * and drivers/usb/misc/chaoskey.c
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation, version 2.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "usbhid/usbhid.h"
> > +#include "hid-ids.h"
> > +
> > +#define DRIVER_SHORT "u2fzero"
> > +
> > +#define HID_REPORT_SIZE64
> > +
> > +/* We only use broadcast (CID-less) messages */
> > +#define CID_BROADCAST 0x
> > +
> > +struct u2f_hid_msg {
> > + u32 cid;
> > + union {
> > + struct {
> > + u8 cmd;
> > + u8 bcnth;
> > + u8 bcntl;
> > + u8 data[HID_REPORT_SIZE - 7];
> > + } init;
> > + struct {
> > + u8 seq;
> > + u8 data[HID_REPORT_SIZE - 5];
> > + } cont;
> > + };
> > +} __packed;
> > +
> > +struct u2f_hid_report {
> > + u8 report_type;
> > + struct u2f_hid_msg msg;
> > +} __packed;
> > +
> > +#define U2F_HID_MSG_LEN(f) (size_t)(((f).init.bcnth << 8) + (f).init.bcntl)
> > +
> > +/* Custom extensions to the U2FHID protocol */
> > +#define U2F_CUSTOM_GET_RNG 0x21
> > +#define U2F_CUSTOM_WINK0x24
> > +
> > +struct u2fzero_device {
> > + struct hid_device *hdev;
> > + struct urb *urb; /* URB for the RNG data */
> > +
Re: [PATCH 3/3] usb: dwc3: Support option to disable USB2 LPM
On 12/11/18 13:03, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: Also can we have some consistency in usage of '-' vs '_'? >>> Right.. I agree. I've been using '-' as it is the preferred syntax as >>> most of the properties, but some old properties use '_'. Do you have any >>> suggestion? >> >> I'd keep it consistent to "snps,usb3_lpm_capable" so we avoid mistakes >> when writing the DT. >> Felipe? > > _ are not really accepted in DT, except for legacy properties. What we > can do is rename usb3_lpm_capable to usb3-lpm-capable and change code to > try with _ if - fails. Then we can introduce the new property using - > I agree. This is a better approach. cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH 3/3] usb: dwc3: Support option to disable USB2 LPM
Hi, Roger Quadros writes: >>> Also can we have some consistency in usage of '-' vs '_'? >> Right.. I agree. I've been using '-' as it is the preferred syntax as >> most of the properties, but some old properties use '_'. Do you have any >> suggestion? > > I'd keep it consistent to "snps,usb3_lpm_capable" so we avoid mistakes > when writing the DT. > Felipe? _ are not really accepted in DT, except for legacy properties. What we can do is rename usb3_lpm_capable to usb3-lpm-capable and change code to try with _ if - fails. Then we can introduce the new property using - -- balbi signature.asc Description: PGP signature
Re: Adding NovAtel USB vendor & device ID to Kernel
On Thu, Nov 08, 2018 at 01:07:47AM +, SNELL James wrote: > Hello, > We produce extremely high-end GNSS (GPS, etc) receivers that are often > used for a very wide range of applications. Our receivers can be > connected to via USB, which will provide 3 USB-to-serial ports that > can be used to issue commands and get receiver data. Do your products support other serial interfaces as well (e.g. UART or i2c)? > We typically get Linux users to create a udev file so their systems > attach the USB serial ports to /dev. > > I just noticed that when my receiver enumerates, dmesg outputs: > [ 414.374523] usb 1-1.1.3: new full-speed USB device number 8 using dwc_otg > [ 414.508473] usb 1-1.1.3: New USB device found, idVendor=09d7, > idProduct=0100 > [ 414.508488] usb 1-1.1.3: New USB device strings: Mfr=1, Product=2, > SerialNumber=3 > [ 414.508497] usb 1-1.1.3: Product: NovAtel GPS Receiver > [ 414.508505] usb 1-1.1.3: Manufacturer: NovAtel Inc. > [ 414.508514] usb 1-1.1.3: SerialNumber: DMGW18050122R > [ 414.511608] usbserial_generic 1-1.1.3:1.0: The "generic" usb-serial driver > is only for testing and one-off prototypes. > [ 414.511624] usbserial_generic 1-1.1.3:1.0: Tell > mailto:linux-usb@vger.kernel.org to add your device to a proper driver. > [ 414.511636] usbserial_generic 1-1.1.3:1.0: generic converter detected > [ 414.512004] usb 1-1.1.3: generic converter now attached to ttyUSB0 > [ 414.512352] usb 1-1.1.3: generic converter now attached to ttyUSB1 > [ 414.512805] usb 1-1.1.3: generic converter now attached to ttyUSB2 > # lsusb -s 001:008 -v > I think it would be "nice" if our receiver's USB-delivered serial > ports attached to /dev as /dev/novatel0, .. /dev/novatelN or > (/dev/gnss0 .. /dev/gnssN). Though if they continued to appear as > /dev/ttyUSB0 .. /dev/ttyUSBN, that'd also be great. As Oliver mentioned it would be useful to know what USB-serial converter chip you use in the device, if any, or of this is a custom implementation. Since 4.19 we actually have GNSS subsystem in the kernel, which if we implement a driver for your devices would show up as /dev/gnssN with an associated attribute describing the GNSS type (reflecting the supported protocol, e.g. NMEA, UBX or SiRF). What protocols do your devices use (besides NMEA)? Do use any common GNSS receivers chips, or do you have your own? What are the three ports used for, or is that configurable? > I'm not entirely sure if the dmesg output that's directed me here is > really intended for this sort of request. If not, I'm willing to make > my own git merge request, though I've not toyed much with the Linux > Kernel, so tips would be extremely appreciated. > > Can we please get our Vendor ID (0x09d7) and Product ID (0x0100) added > to the Kernel in a sensical manner? Depending a bit on your answers to the above questions, we could also add your VID/PID to a USB-serial driver, which would give you ttyUSBN devices without any need to mess with modprobe. > The information contained in this e-mail may contain confidential or > privileged material and is intended only for the stated addressee(s). > If you are not a valid addressee, the use, disclosure, copying or > distribution of this information is prohibited and may be unlawful. > If you have received this message in error, please notify the sender > immediately and delete all copies of the message from your computer. > Notwithstanding any applicable legislation which may provide for > contracts to be formed from electronic communication, this email does > not create, form part of, or vary any contract, nor is it otherwise > intended to bind any Hexagon group company. You need to remove this footer when dealing with the mailing lists, though. Thanks, Johan
Re: [PATCH 3/3] usb: dwc3: Support option to disable USB2 LPM
Thinh, On 12/11/18 07:29, Thinh Nguyen wrote: > Hi Roger, > > On 11/9/2018 3:58 AM, Roger Quadros wrote: >> Hi, >> >> On 08/11/18 04:10, Thinh Nguyen wrote: >>> Support the option to disable USB2 LPM. Set xhci "usb2-lpm-disable" >>> property via "snps,usb2-lpm-disable" property. >>> >>> Signed-off-by: Thinh Nguyen >>> --- >>> drivers/usb/dwc3/core.c | 2 ++ >>> drivers/usb/dwc3/core.h | 2 ++ >>> drivers/usb/dwc3/host.c | 5 - >>> 3 files changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index a4068a7b95dd..f6b80a545a78 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -1248,6 +1248,8 @@ static void dwc3_get_properties(struct dwc3 *dwc) >>> &hird_threshold); >>> dwc->usb3_lpm_capable = device_property_read_bool(dev, >>> "snps,usb3_lpm_capable"); >>> + dwc->usb2_lpm_disable = device_property_read_bool(dev, >>> + "snps,usb2-lpm-disable"); >> Can we use the same logic as usb3_lpm instead? >> i.e. enable USB2 LPM only if "snps,usb2_lpm_capable" is present in DT. >> This is because older platforms that are not tested for usb2 lpm >> might break if you enable it by default. > > I follow the same logic as usb-xhci property. The usb2-lpm-disable > property from xHCI has been around for awhile. Do you suggest to change > the property for xHCI then? No. I see it now that you are just setting the XHCI property and not doing any change in the dwc3 functionality itself. I think your patch is correct. >> Also can we have some consistency in usage of '-' vs '_'? > Right.. I agree. I've been using '-' as it is the preferred syntax as > most of the properties, but some old properties use '_'. Do you have any > suggestion? I'd keep it consistent to "snps,usb3_lpm_capable" so we avoid mistakes when writing the DT. Felipe? cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH] USB: serial: mos7840: Add a product ID for the new product
On Mon, Nov 12, 2018 at 03:16:05PM +0800, Jackychou wrote:
> From: JackyChou
>
> Add a new PID 0x7843 to the driver.
> Let the new products be able to set up 3 serial ports with the driver.
>
> Signed-off-by: JackyChou
> ---
> drivers/usb/serial/mos7840.c | 26 ++
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index b42bad85097a..7667b22fa2f7 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -94,6 +94,7 @@
> /* The native mos7840/7820 component */
> #define USB_VENDOR_ID_MOSCHIP 0x9710
> #define MOSCHIP_DEVICE_ID_7840 0x7840
> +#define MOSCHIP_DEVICE_ID_7843 0x7843
> #define MOSCHIP_DEVICE_ID_7820 0x7820
> #define MOSCHIP_DEVICE_ID_7810 0x7810
> /* The native component can have its vendor/device id's overridden
> @@ -176,6 +177,7 @@ enum mos7840_flag {
>
> static const struct usb_device_id id_table[] = {
> {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)},
> + {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7843)},
> {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)},
> {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)},
> {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)},
> @@ -298,7 +300,7 @@ static int mos7840_set_uart_reg(struct usb_serial_port
> *port, __u16 reg,
> val = val & 0x00ff;
> /* For the UART control registers, the application number need
> to be Or'ed */
> - if (port->serial->num_ports == 4) {
> + if (port->serial->num_ports == 4 || port->serial->num_ports == 3) {
Please use num_ports > 2 here.
> val |= ((__u16)port->port_number + 1) << 8;
> } else {
> if (port->port_number == 0) {
> @@ -332,7 +334,7 @@ static int mos7840_get_uart_reg(struct usb_serial_port
> *port, __u16 reg,
> return -ENOMEM;
>
> /* Wval is same as application number */
> - if (port->serial->num_ports == 4) {
> + if (port->serial->num_ports == 4 || port->serial->num_ports == 3) {
Same here.
> Wval = ((__u16)port->port_number + 1) << 8;
> } else {
> if (port->port_number == 0) {
> @@ -2071,9 +2073,12 @@ static int mos7840_probe(struct usb_serial *serial,
> VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
>
> /* For a MCS7840 device GPIO0 must be set to 1 */
> - if (buf[0] & 0x01)
> - device_type = MOSCHIP_DEVICE_ID_7840;
> - else if (mos7810_check(serial))
> + if (buf[0] & 0x01) {
> + if (product == MOSCHIP_DEVICE_ID_7843)
> + device_type = MOSCHIP_DEVICE_ID_7843;
What about 7843 devices that use a different PID? Is there a reliable
way to detect the type without relying on PID?
Otherwise, there's no point in verifying GPIO0 for these ones.
> + else
> + device_type = MOSCHIP_DEVICE_ID_7840;
> + } else if (mos7810_check(serial))
Nit: if you add braces to one of the branches you need to add it to all
of them.
> device_type = MOSCHIP_DEVICE_ID_7810;
> else
> device_type = MOSCHIP_DEVICE_ID_7820;
> @@ -2091,7 +2096,10 @@ static int mos7840_calc_num_ports(struct usb_serial
> *serial,
> int device_type = (unsigned long)usb_get_serial_data(serial);
> int num_ports;
>
> - num_ports = (device_type >> 4) & 0x000F;
> + if (device_type == MOSCHIP_DEVICE_ID_7843)
> + num_ports = 3;
> + else
> + num_ports = (device_type >> 4) & 0x000F;
>
> /*
>* num_ports is currently never zero as device_type is one of
> @@ -2146,7 +2154,8 @@ static int mos7840_port_probe(struct usb_serial_port
> *port)
> mos7840_port->SpRegOffset = 0x0;
> mos7840_port->ControlRegOffset = 0x1;
> mos7840_port->DcrRegOffset = 0x4;
> - } else if ((mos7840_port->port_num == 2) && (serial->num_ports == 4)) {
> + } else if ((mos7840_port->port_num == 2) &&
> +((serial->num_ports == 4) || (serial->num_ports == 3))) {
> mos7840_port->SpRegOffset = 0x8;
> mos7840_port->ControlRegOffset = 0x9;
> mos7840_port->DcrRegOffset = 0x16;
> @@ -2154,7 +2163,8 @@ static int mos7840_port_probe(struct usb_serial_port
> *port)
> mos7840_port->SpRegOffset = 0xa;
> mos7840_port->ControlRegOffset = 0xb;
> mos7840_port->DcrRegOffset = 0x19;
> - } else if ((mos7840_port->port_num == 3) && (serial->num_ports == 4)) {
> + } else if ((mos7840_port->port_num == 3) &&
> +((serial->num_ports == 4) || (serial->num_ports == 3))) {
> mos7840_port->SpRegOffset = 0xa;
> mos7840_port->ControlRegOffset = 0xb;
> mos7840_port->DcrRegOffset = 0x19;
This is getting rather ugly. Can't
Inquiry 12/11/2018
Hi,friend, This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia. We are glad to know about your company from the web and we are interested in your products. Could you kindly send us your Latest catalog and price list for our trial order. Best Regards, Daniel Murray Purchasing Manager
Re: WARNING in usb_submit_urb (4)
syzbot has found a reproducer for the following crash on: HEAD commit:e12e00e388de Merge tag 'kbuild-fixes-v4.20' of git://git.k.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=100e4ef540 kernel config: https://syzkaller.appspot.com/x/.config?x=8f215f21f041a0d7 dashboard link: https://syzkaller.appspot.com/bug?extid=7634edaea4d0b341c625 compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11ce6fbd40 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+7634edaea4d0b341c...@syzkaller.appspotmail.com hub 3-0:1.0: 8 ports detected hub 3-0:1.0: USB hub found hub 3-0:1.0: 8 ports detected [ cut here ] URB 5e43faa5 submitted while active WARNING: CPU: 0 PID: 6240 at drivers/usb/core/urb.c:363 usb_submit_urb+0x11cf/0x14e0 drivers/usb/core/urb.c:363 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 6240 Comm: kworker/0:1 Not tainted 4.20.0-rc1+ #332 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: events_power_efficient hub_init_func2 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x244/0x39d lib/dump_stack.c:113 panic+0x2ad/0x55c kernel/panic.c:188 __warn.cold.8+0x20/0x45 kernel/panic.c:540 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969 RIP: 0010:usb_submit_urb+0x11cf/0x14e0 drivers/usb/core/urb.c:363 Code: ee e8 95 62 7b fc 45 84 ed 0f 85 e2 f6 ff ff e8 b7 61 7b fc 48 89 de 48 c7 c7 a0 32 92 88 c6 05 d4 45 0d 05 01 e8 11 c5 44 fc <0f> 0b e9 c0 f6 ff ff c7 45 a0 01 00 00 00 e9 65 f7 ff ff 41 bc ed RSP: 0018:8881c1f974f0 EFLAGS: 00010282 RAX: RBX: 8881c83bb800 RCX: RDX: RSI: 8165e7e5 RDI: 0005 RBP: 8881c1f97560 R08: 8881bc062040 R09: ed103b5c3ef8 R10: ed103b5c3ef8 R11: 8881dae1f7c7 R12: fff0 R13: R14: 0009 R15: 8881c1f976f8 hub_activate+0xcab/0x1940 drivers/usb/core/hub.c:1215 hub_init_func2+0x1e/0x30 drivers/usb/core/hub.c:1240 process_one_work+0xc90/0x1c40 kernel/workqueue.c:2153 worker_thread+0x17f/0x1390 kernel/workqueue.c:2296 kthread+0x35a/0x440 kernel/kthread.c:246 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352 Kernel Offset: disabled Rebooting in 86400 seconds..
Re: [PATCH] Improve the accuracy of the baud rate generator by using round-to-closest instead of truncating when calculating baud rate divisors.
On Wed, Oct 31, 2018 at 09:16:48PM +0100, Nikolaj Fogh wrote: > I have experienced that the ftdi_sio driver gives less-than-optimal baud > rates as the driver truncates instead of rounds to nearest during baud rate > divisor calculation. Please break your lines at 72 cols or so, and use the common subject prefix (e.g. "USB: serial: ftdi_sio: improve...") with a concise summary. > This patch improves on the baud rate generation. The generated baud rate > corresponds to the optimal baud rate achievable with the chip. This is what > the windows driver gives as well. How did you verify this? Did you trace and compare the divisors actually requested by the Windows driver, or did you measure the resulting rates using a scope? Thanks, Johan
Re: [PATCH -next] USB: serial: mos7840: remove set but not used variables 'st, data1, iflag'
On Fri, Oct 26, 2018 at 08:25:49PM +0800, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/usb/serial/mos7840.c: In function 'mos7840_interrupt_callback':
> drivers/usb/serial/mos7840.c:604:14: warning:
> variable 'st' set but not used [-Wunused-but-set-variable]
>
> drivers/usb/serial/mos7840.c: In function 'mos7840_write':
> drivers/usb/serial/mos7840.c:1303:17: warning:
> variable 'data1' set but not used [-Wunused-but-set-variable]
>
> drivers/usb/serial/mos7840.c:1700:11: warning:
> variable 'iflag' set but not used [-Wunused-but-set-variable]
>
> They are never used since introduction in
> commit 3f5429746d91 ("USB: Moschip 7840 USB-Serial Driver")
>
> Signed-off-by: YueHaibing
Applied, thanks.
Johan
Re: [PATCH] Add support of TI ICDI to USB simple serial device
On Fri, Oct 26, 2018 at 07:38:07PM +0800, Dashi Cao wrote:
> TI In-Circuit Debug Interface (ICDI) is a debugging interface for TI ARM
> microcontrollers. It has four USB interfaces and the first two of them are
> presented as standard ACM serial device. The 3rd interface is the debugging
> interface and it can be driven as a Linux USB simple serial device. With it,
> debugging sessions and firmware up/down loading are supported on Linux.
Please break your lines at 72 column or so.
And use the common subject prefix (e.g. "USB: serial: add support
of TI ICD...").
> Signed-off-by: Dashi Cao
You never replied to Felipe's question whether you had verified that this
doesn't break OpenOCD?
> ---
> drivers/usb/serial/usb-serial-simple.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/usb/serial/usb-serial-simple.c
> b/drivers/usb/serial/usb-serial-simple.c
> index 4d0273508043..ae43088b659e 100644
> --- a/drivers/usb/serial/usb-serial-simple.c
> +++ b/drivers/usb/serial/usb-serial-simple.c
> @@ -109,6 +109,11 @@ DEVICE(suunto, SUUNTO_IDS);
> { USB_DEVICE(0x908, 0x0004) }
> DEVICE(siemens_mpi, SIEMENS_IDS);
>
> +/* TI In-Circuit Debug Interface */
> +#define ICDI_IDS() \
> + { USB_DEVICE_INTERFACE_CLASS(0x1cbe, 0x00fd, USB_CLASS_VENDOR_SPEC) }
> +DEVICE(ti_icdi, ICDI_IDS);
Please use a TI_ prefix for ICDI_IDS as well.
Can you post the lsusb -v output (or usb-devices) for the device for
reference?
> +
> /* All of the above structures mushed into two lists */
> static struct usb_serial_driver * const serial_drivers[] = {
> &carelink_device,
> @@ -124,6 +129,7 @@ static struct usb_serial_driver * const serial_drivers[]
> = {
> &hp4x_device,
> &suunto_device,
> &siemens_mpi_device,
> + &ti_icdi_device,
> NULL
> };
>
> @@ -141,6 +147,7 @@ static const struct usb_device_id id_table[] = {
> HP4X_IDS(),
> SUUNTO_IDS(),
> SIEMENS_IDS(),
> + ICDI_IDS(),
> { },
> };
> MODULE_DEVICE_TABLE(usb, id_table);
Thanks,
Johan
Re: [PATCH 06/10] phy: Add usb phy support for hi3660 Soc of Hisilicon
Hi, On 27/10/18 3:28 PM, Yu Chen wrote: > This driver handles usb phy power on and shutdown for hi3660 Soc of > Hisilicon. > > Cc: Kishon Vijay Abraham I > Cc: "David S. Miller" > Cc: Greg Kroah-Hartman > Cc: Mauro Carvalho Chehab > Cc: Andrew Morton > Cc: Arnd Bergmann > Cc: Shawn Guo > Cc: Pengcheng Li > Cc: Jianguo Sun > Cc: Masahiro Yamada > Cc: Jiancheng Xue > Cc: John Stultz > Cc: Binghui Wang > Signed-off-by: Yu Chen > --- > MAINTAINERS | 2 + > drivers/phy/hisilicon/Kconfig | 10 ++ > drivers/phy/hisilicon/Makefile | 1 + > drivers/phy/hisilicon/phy-hi3660-usb3.c | 254 > > 4 files changed, 267 insertions(+) > create mode 100644 drivers/phy/hisilicon/phy-hi3660-usb3.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index e485449f7811..7adf167588ee 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15294,7 +15294,9 @@ M:Binghui Wang > L: linux-usb@vger.kernel.org > S: Maintained > F: Documentation/devicetree/bindings/usb/dwc3-hisi.txt > +F: Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt > F: drivers/usb/dwc3/dwc3-hisi.c > +F: drivers/phy/hisilicon/phy-hi3660-usb3.c > > USB ISP116X DRIVER > M: Olav Kongas > diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig > index b40ee54a1a50..3c142f08987c 100644 > --- a/drivers/phy/hisilicon/Kconfig > +++ b/drivers/phy/hisilicon/Kconfig > @@ -12,6 +12,16 @@ config PHY_HI6220_USB > > To compile this driver as a module, choose M here. > > +config PHY_HI3660_USB > + tristate "hi3660 USB PHY support" > + depends on (ARCH_HISI && ARM64) || COMPILE_TEST > + select GENERIC_PHY > + select MFD_SYSCON > + help > + Enable this to support the HISILICON HI3660 USB PHY. > + > + To compile this driver as a module, choose M here. > + > config PHY_HISTB_COMBPHY > tristate "HiSilicon STB SoCs COMBPHY support" > depends on (ARCH_HISI && ARM64) || COMPILE_TEST > diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile > index f662a4fe18d8..75ba64e2faf8 100644 > --- a/drivers/phy/hisilicon/Makefile > +++ b/drivers/phy/hisilicon/Makefile > @@ -1,4 +1,5 @@ > obj-$(CONFIG_PHY_HI6220_USB) += phy-hi6220-usb.o > +obj-$(CONFIG_PHY_HI3660_USB) += phy-hi3660-usb3.o > obj-$(CONFIG_PHY_HISTB_COMBPHY) += phy-histb-combphy.o > obj-$(CONFIG_PHY_HISI_INNO_USB2) += phy-hisi-inno-usb2.o > obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o > diff --git a/drivers/phy/hisilicon/phy-hi3660-usb3.c > b/drivers/phy/hisilicon/phy-hi3660-usb3.c > new file mode 100644 > index ..041c542750e2 > --- /dev/null > +++ b/drivers/phy/hisilicon/phy-hi3660-usb3.c > @@ -0,0 +1,254 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Phy provider for USB 3.0 controller on HiSilicon 3660 platform > + * > + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd. > + * http://www.huawei.com > + * > + * Authors: Yu Chen > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ This license text doesn't have to added explicitly. It should be already governed by SPDX line. > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PERI_CRG_CLK_EN4 (0x40) > +#define PERI_CRG_CLK_DIS4(0x44) > +#define GT_CLK_USB3OTG_REF BIT(0) > +#define GT_ACLK_USB3OTG BIT(1) > + > +#define PERI_CRG_RSTEN4 (0x90) > +#define PERI_CRG_RSTDIS4 (0x94) > +#define IP_RST_USB3OTGPHY_PORBIT(3) > +#define IP_RST_USB3OTG BIT(5) > + > +#define PERI_CRG_ISODIS (0x148) > +#define USB_REFCLK_ISO_ENBIT(25) > + > +#define PCTRL_PERI_CTRL3 (0x10) > +#define PCTRL_PERI_CTRL3_MSK_START (16) > +#define USB_TCXO_EN BIT(1) > + > +#define PCTRL_PERI_CTRL24(0x64) > +#define SC_CLK_USB3PHY_3MUX1_SEL BIT(25) > + > +#define USBOTG3_CTRL0(0x00) > +#define SC_USB3PHY_ABB_GT_EN BIT(15) > + > +#define USBOTG3_CTRL2(0x08) > +#define USBOTG3CTRL2_POWERDOWN_HSP BIT(0) > +#define USBOTG3CTRL2_POWERDOWN_SSP BIT(1) > + > +#define USBOTG3_CTRL3(0x0C) > +#
Re: [PATCH -next] USB: serial: quatech2: remove set but not used variable 'port_priv'
On Thu, Oct 11, 2018 at 07:44:33AM +, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/usb/serial/quatech2.c: In function 'qt2_process_read_urb':
> drivers/usb/serial/quatech2.c:503:27: warning:
> variable 'port_priv' set but not used [-Wunused-but-set-variable]
>
> It not used any more after
> commit 2be818a116b2 ("Revert "USB: quatech2: only write to the tty if the
> port is open."")
>
> Signed-off-by: YueHaibing
Now applied. Thanks for including the not-used-since information.
Johan
Re: [PATCH] usb: dwc2: gadget: fix ISOC frame overflow handling
Hi John,
On 11/9/2018 10:43 PM, John Keeping wrote:
> Hi Minas,
>
> On Fri, 9 Nov 2018 14:36:36 +
> Minas Harutyunyan wrote:
>
>> On 11/9/2018 12:43 PM, Minas Harutyunyan wrote:
>>> Hi John,
>>>
>>> On 11/8/2018 9:37 PM, John Keeping wrote:
Hi Minas,
On Mon, 5 Nov 2018 08:28:07 +
Minas Harutyunyan wrote:
> On 10/23/2018 5:43 PM, John Keeping wrote:
>> By clearing the overrun flag as soon as the target frame is next
>> incremented, we can end up incrementing the target frame more
>> than expected in dwc2_gadget_handle_ep_disabled() when the
>> endpoint's interval is greater than 1. This happens if the
>> target frame has just wrapped at the point when the endpoint is
>> disabled and the frame number has not yet done so.
>>
>> Instead, wait until the frame number also wraps and then clear
>> the overrun flag.
>>
>> Signed-off-by: John Keeping
>> ---
>> drivers/usb/dwc2/gadget.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c
>> b/drivers/usb/dwc2/gadget.c index 2d6d2c8244de..8da2c052dfa1
>> 100644 --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -117,7 +117,7 @@ static inline void
>> dwc2_gadget_incr_frame_num(struct dwc2_hsotg_ep *hs_ep) if
>> (hs_ep->target_frame > DSTS_SOFFN_LIMIT) { hs_ep->frame_overrun =
>> true; hs_ep->target_frame &= DSTS_SOFFN_LIMIT;
>> -} else {
>> +} else if (hs_ep->parent->frame_number <
>> hs_ep->target_frame) { hs_ep->frame_overrun = false;
>> }
>> }
>>
> Did you tested mentioned by you scenario? If you see issue can you
> provide debug log and point the issue line in the log.
It only reproduces very occasionally so it's difficult to capture
a full debug log containing the error.
I applied this patch to capture logging specifically around this
scenario:
-- >8 --
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 220c0f9b89b0..3770b9d3b523 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2722,13 +2722,20 @@ static void
dwc2_gadget_handle_ep_disabled(struct dwc2_hsotg_ep *hs_ep) }
do {
+ unsigned int target_frame = hs_ep->target_frame;
+ bool frame_overrun = hs_ep->frame_overrun;
+
hs_req = get_ep_head(hs_ep);
if (hs_req)
dwc2_hsotg_complete_request(hsotg,
hs_ep, hs_req, -ENODATA);
+
dwc2_gadget_incr_frame_num(hs_ep);
/* Update current frame number value. */
hsotg->frame_number =
dwc2_hsotg_read_frameno(hsotg); +
+ dev_warn(hsotg->dev, "%s: expiring request
frame_number=0x%04x target_frame=0x%04x overrun=%u\n",
+__func__, hsotg->frame_number,
target_frame, frame_overrun); } while
(dwc2_gadget_target_frame_elapsed(hs_ep));
dwc2_gadget_start_next_request(hs_ep);
-- 8< --
>>> According above patch in debug log should be printed overrun flag
>>> also. Could you please resend log with this flag.
>
> D'oh! I included a log from an earlier version before I added the
> overrun flag.
>
>> One more request. Please add EP number to debug print.
>
> Here's an updated log:
>
> -- >8 --
> [ 264.926385] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x2161 target_frame=0x2168 overrun=0
> [ 265.905413] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0008 overrun=0
> [ 265.905421] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0010 overrun=0
> [ 265.905427] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0018 overrun=0
> [ 265.905432] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0020 overrun=0
> [ 265.905438] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0028 overrun=0
> [ 265.905443] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0030 overrun=0
> [ 265.905448] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0038 overrun=0
> [ 265.905454] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_frame=0x0040 overrun=0
> [ 265.905459] dwc2 ff58.usb: dwc2_gadget_handle_ep_disabled: expiring
> request ep=2 frame_number=0x3ff9 target_fra
[PATCH 2/6] xhci: handle port status events for removed USB3 hcd
At xhci removal the USB3 hcd (shared_hcd) is removed before the primary
USB2 hcd. Interrupts for port status changes may still occur for USB3
ports after the shared_hcd is freed, causing NULL pointer dereference.
Check if xhci->shared_hcd is still valid before handing USB3 port events
Cc:
Reported-by: Peter Chen
Tested-by: Jack Pham
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ring.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a8d92c9..80d464e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1556,6 +1556,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
goto cleanup;
}
+ /* We might get interrupts after shared_hcd is removed */
+ if (port->rhub == &xhci->usb3_rhub && xhci->shared_hcd == NULL) {
+ xhci_dbg(xhci, "ignore port event for removed USB3 hcd\n");
+ bogus_port_status = true;
+ goto cleanup;
+ }
+
hcd = port->rhub->hcd;
bus_state = &xhci->bus_state[hcd_index(hcd)];
hcd_portnum = port->hcd_portnum;
--
2.7.4
[PATCH 1/6] xhci: Fix leaking USB3 shared_hcd at xhci removal
Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()
The shared_hcd is removed and freed in xhci by first calling
usb_remove_hcd(xhci->shared_hcd), and later
usb_put_hcd(xhci->shared_hcd)
Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have
disconnected their devices.") the shared_hcd was never properly put as
xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd) was
called.
shared_hcd (USB3) is removed before primary hcd (USB2).
While removing the primary hcd we might need to handle xhci interrupts
to cleanly remove last USB2 devices, therefore we need to set
xhci->shared_hcd to NULL before removing the primary hcd to let xhci
interrupt handler know shared_hcd is no longer available.
xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before
adding them. so to keep the correct reverse removal order use a temporary
shared_hcd variable for them.
For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both
HCDs before adding them")
Fixes: fe190ed0d602 ("xhci: Do not halt the host until both HCD have
disconnected their devices.")
Cc: Joel Stanley
Cc: Chunfeng Yun
Cc: Thierry Reding
Cc: Jianguo Sun
Cc:
Reported-by: Jack Pham
Tested-by: Jack Pham
Tested-by: Peter Chen
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-histb.c | 6 --
drivers/usb/host/xhci-mtk.c | 6 --
drivers/usb/host/xhci-pci.c | 1 +
drivers/usb/host/xhci-plat.c | 6 --
drivers/usb/host/xhci-tegra.c | 1 +
drivers/usb/host/xhci.c | 2 --
6 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
index 27f0016..3c4abb5 100644
--- a/drivers/usb/host/xhci-histb.c
+++ b/drivers/usb/host/xhci-histb.c
@@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device *dev)
struct xhci_hcd_histb *histb = platform_get_drvdata(dev);
struct usb_hcd *hcd = histb->hcd;
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct usb_hcd *shared_hcd = xhci->shared_hcd;
xhci->xhc_state |= XHCI_STATE_REMOVING;
- usb_remove_hcd(xhci->shared_hcd);
+ usb_remove_hcd(shared_hcd);
+ xhci->shared_hcd = NULL;
device_wakeup_disable(&dev->dev);
usb_remove_hcd(hcd);
- usb_put_hcd(xhci->shared_hcd);
+ usb_put_hcd(shared_hcd);
xhci_histb_host_disable(histb);
usb_put_hcd(hcd);
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 71d0d33..60987c7 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)
struct xhci_hcd_mtk *mtk = platform_get_drvdata(dev);
struct usb_hcd *hcd = mtk->hcd;
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct usb_hcd *shared_hcd = xhci->shared_hcd;
- usb_remove_hcd(xhci->shared_hcd);
+ usb_remove_hcd(shared_hcd);
+ xhci->shared_hcd = NULL;
device_init_wakeup(&dev->dev, false);
usb_remove_hcd(hcd);
- usb_put_hcd(xhci->shared_hcd);
+ usb_put_hcd(shared_hcd);
usb_put_hcd(hcd);
xhci_mtk_sch_exit(mtk);
xhci_mtk_clks_disable(mtk);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 01c5705..1fb448c 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -380,6 +380,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
if (xhci->shared_hcd) {
usb_remove_hcd(xhci->shared_hcd);
usb_put_hcd(xhci->shared_hcd);
+ xhci->shared_hcd = NULL;
}
/* Workaround for spurious wakeups at shutdown with HSW */
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 32b5574..ef09cb0 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -362,14 +362,16 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
struct clk *reg_clk = xhci->reg_clk;
+ struct usb_hcd *shared_hcd = xhci->shared_hcd;
xhci->xhc_state |= XHCI_STATE_REMOVING;
- usb_remove_hcd(xhci->shared_hcd);
+ usb_remove_hcd(shared_hcd);
+ xhci->shared_hcd = NULL;
usb_phy_shutdown(hcd->usb_phy);
usb_remove_hcd(hcd);
- usb_put_hcd(xhci->shared_hcd);
+ usb_put_hcd(shared_hcd);
clk_disable_unprepare(clk);
clk_disable_unprepare(reg_clk);
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 6b5db34..938ff06 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1303,6 +1303,7 @@ static int tegra_xusb_remove(struct platform_device *pdev)
usb_remove_hcd(xhci->shared_hcd);
usb_put_hcd(xhci->shared_hcd);
+ xhci->shared_hcd = NULL;
usb_remove_hcd(tegra->hcd);
usb_put_hc
[PATCH 0/6] xhci fixes for usb-linus
A few xhci fixes for usb-linus, among othres fixing a hcd leak at removal, setting timout values correctly, and making sure USB3 and USB2 ports don't get mixed up in resume when port speed field may be unreliable. This series was sent to Greg and other patch stakeholders earlier,but due to a typo never made it to linux-usb mailing list. Resending it. -Mathias Aaron Ma (2): usb: xhci: fix uninitialized completion when USB3 port got wrong status usb: xhci: fix timeout for transition from RExit to U0 Cherian, George (1): xhci: Add quirk to workaround the errata seen on Cavium Thunder-X2 Soc Mathias Nyman (2): xhci: Fix leaking USB3 shared_hcd at xhci removal xhci: handle port status events for removed USB3 hcd Sandeep Singh (1): xhci: Add check for invalid byte size error when UAS devices are connected. drivers/usb/host/xhci-histb.c | 6 -- drivers/usb/host/xhci-hub.c | 6 +++--- drivers/usb/host/xhci-mtk.c | 6 -- drivers/usb/host/xhci-pci.c | 6 ++ drivers/usb/host/xhci-plat.c | 6 -- drivers/usb/host/xhci-ring.c | 45 +-- drivers/usb/host/xhci-tegra.c | 1 + drivers/usb/host/xhci.c | 2 -- drivers/usb/host/xhci.h | 3 ++- 9 files changed, 67 insertions(+), 14 deletions(-) -- 2.7.4
[PATCH 4/6] usb: xhci: fix uninitialized completion when USB3 port got wrong status
From: Aaron Ma
Realtek USB3.0 Card Reader [0bda:0328] reports wrong port status on
Cannon lake PCH USB3.1 xHCI [8086:a36d] after resume from S3,
after clear port reset it works fine.
Since this device is registered on USB3 roothub at boot,
when port status reports not superspeed, xhci_get_port_status will call
an uninitialized completion in bus_state[0].
Kernel will hang because of NULL pointer.
Restrict the USB2 resume status check in USB2 roothub to fix hang issue.
Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-hub.c | 2 +-
drivers/usb/host/xhci-ring.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 12eea73..60e4ac7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -876,7 +876,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
status |= USB_PORT_STAT_SUSPEND;
}
if ((raw_port_status & PORT_PLS_MASK) == XDEV_RESUME &&
- !DEV_SUPERSPEED_ANY(raw_port_status)) {
+ !DEV_SUPERSPEED_ANY(raw_port_status) && hcd->speed < HCD_USB3) {
if ((raw_port_status & PORT_RESET) ||
!(raw_port_status & PORT_PE))
return 0x;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 730a6ec..250b758 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1646,7 +1646,7 @@ static void handle_port_status(struct xhci_hcd *xhci,
* RExit to a disconnect state). If so, let the the driver know it's
* out of the RExit state.
*/
- if (!DEV_SUPERSPEED_ANY(portsc) &&
+ if (!DEV_SUPERSPEED_ANY(portsc) && hcd->speed < HCD_USB3 &&
test_and_clear_bit(hcd_portnum,
&bus_state->rexit_ports)) {
complete(&bus_state->rexit_done[hcd_portnum]);
--
2.7.4
[PATCH 3/6] xhci: Add check for invalid byte size error when UAS devices are connected.
From: Sandeep Singh Observed "TRB completion code (27)" error which corresponds to Stopped - Length Invalid error(xhci spec section 4.17.4) while connecting USB to SATA bridge. Looks like this case was not considered when the following patch[1] was committed. Hence adding this new check which can prevent the invalid byte size error. [1] ade2e3a xhci: handle transfer events without TRB pointer Cc: Signed-off-by: Sandeep Singh cc: Nehal Shah cc: Shyam Sundar S K Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 80d464e..730a6ec 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2273,6 +2273,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, goto cleanup; case COMP_RING_UNDERRUN: case COMP_RING_OVERRUN: + case COMP_STOPPED_LENGTH_INVALID: goto cleanup; default: xhci_err(xhci, "ERROR Transfer event for unknown stream ring slot %u ep %u\n", -- 2.7.4
[PATCH 5/6] usb: xhci: fix timeout for transition from RExit to U0
From: Aaron Ma
This definition is used by msecs_to_jiffies in milliseconds.
According to the comments, max rexit timeout should be 20ms.
Align with the comments to properly calculate the delay.
Verified on Sunrise Point-LP and Cannon Lake.
Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-hub.c | 4 ++--
drivers/usb/host/xhci.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 60e4ac7..da98a11 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -921,7 +921,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
time_left = wait_for_completion_timeout(
&bus_state->rexit_done[wIndex],
msecs_to_jiffies(
- XHCI_MAX_REXIT_TIMEOUT));
+ XHCI_MAX_REXIT_TIMEOUT_MS));
spin_lock_irqsave(&xhci->lock, flags);
if (time_left) {
@@ -935,7 +935,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
} else {
int port_status = readl(port->addr);
xhci_warn(xhci, "Port resume took longer than
%i msec, port status = 0x%x\n",
- XHCI_MAX_REXIT_TIMEOUT,
+ XHCI_MAX_REXIT_TIMEOUT_MS,
port_status);
status |= USB_PORT_STAT_SUSPEND;
clear_bit(wIndex, &bus_state->rexit_ports);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index bf0b369..5f0c4f1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1680,7 +1680,7 @@ struct xhci_bus_state {
* It can take up to 20 ms to transition from RExit to U0 on the
* Intel Lynx Point LP xHCI host.
*/
-#defineXHCI_MAX_REXIT_TIMEOUT (20 * 1000)
+#defineXHCI_MAX_REXIT_TIMEOUT_MS 20
static inline unsigned int hcd_index(struct usb_hcd *hcd)
{
--
2.7.4
[PATCH 6/6] xhci: Add quirk to workaround the errata seen on Cavium Thunder-X2 Soc
From: "Cherian, George"
Implement workaround for ThunderX2 Errata-129 (documented in
CN99XX Known Issues" available at Cavium support site).
As per ThunderX2errata-129, USB 2 device may come up as USB 1
if a connection to a USB 1 device is followed by another connection to
a USB 2 device, the link will come up as USB 1 for the USB 2 device.
Resolution: Reset the PHY after the USB 1 device is disconnected.
The PHY reset sequence is done using private registers in XHCI register
space. After the PHY is reset we check for the PLL lock status and retry
the operation if it fails. From our tests, retrying 4 times is sufficient.
Add a new quirk flag XHCI_RESET_PLL_ON_DISCONNECT to invoke the workaround
in handle_xhci_port_status().
Cc: sta...@vger.kernel.org
Signed-off-by: George Cherian
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-pci.c | 5 +
drivers/usb/host/xhci-ring.c | 35 ++-
drivers/usb/host/xhci.h | 1 +
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 1fb448c..a951526 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -248,6 +248,11 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241)
xhci->quirks |= XHCI_LIMIT_ENDPOINT_INTERVAL_7;
+ if ((pdev->vendor == PCI_VENDOR_ID_BROADCOM ||
+pdev->vendor == PCI_VENDOR_ID_CAVIUM) &&
+pdev->device == 0x9026)
+ xhci->quirks |= XHCI_RESET_PLL_ON_DISCONNECT;
+
if (xhci->quirks & XHCI_RESET_ON_RESUME)
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
"QUIRK: Resetting on resume");
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 250b758..6575058 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1521,6 +1521,35 @@ static void handle_device_notification(struct xhci_hcd
*xhci,
usb_wakeup_notification(udev->parent, udev->portnum);
}
+/*
+ * Quirk hanlder for errata seen on Cavium ThunderX2 processor XHCI
+ * Controller.
+ * As per ThunderX2errata-129 USB 2 device may come up as USB 1
+ * If a connection to a USB 1 device is followed by another connection
+ * to a USB 2 device.
+ *
+ * Reset the PHY after the USB device is disconnected if device speed
+ * is less than HCD_USB3.
+ * Retry the reset sequence max of 4 times checking the PLL lock status.
+ *
+ */
+static void xhci_cavium_reset_phy_quirk(struct xhci_hcd *xhci)
+{
+ struct usb_hcd *hcd = xhci_to_hcd(xhci);
+ u32 pll_lock_check;
+ u32 retry_count = 4;
+
+ do {
+ /* Assert PHY reset */
+ writel(0x6F, hcd->regs + 0x1048);
+ udelay(10);
+ /* De-assert the PHY reset */
+ writel(0x7F, hcd->regs + 0x1048);
+ udelay(200);
+ pll_lock_check = readl(hcd->regs + 0x1070);
+ } while (!(pll_lock_check & 0x1) && --retry_count);
+}
+
static void handle_port_status(struct xhci_hcd *xhci,
union xhci_trb *event)
{
@@ -1654,8 +1683,12 @@ static void handle_port_status(struct xhci_hcd *xhci,
goto cleanup;
}
- if (hcd->speed < HCD_USB3)
+ if (hcd->speed < HCD_USB3) {
xhci_test_and_clear_bit(xhci, port, PORT_PLC);
+ if ((xhci->quirks & XHCI_RESET_PLL_ON_DISCONNECT) &&
+ (portsc & PORT_CSC) && !(portsc & PORT_CONNECT))
+ xhci_cavium_reset_phy_quirk(xhci);
+ }
cleanup:
/* Update event ring dequeue pointer before dropping the lock */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 5f0c4f1..260b259 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1849,6 +1849,7 @@ struct xhci_hcd {
#define XHCI_INTEL_USB_ROLE_SW BIT_ULL(31)
#define XHCI_ZERO_64B_REGS BIT_ULL(32)
#define XHCI_DEFAULT_PM_RUNTIME_ALLOW BIT_ULL(33)
+#define XHCI_RESET_PLL_ON_DISCONNECT BIT_ULL(34)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
--
2.7.4
