Re: [PATCH] dwc3: Remove wait for wait_end_transfer event.

2018-07-19 Thread Felipe Balbi

Hi,

lemag...@gmail.com writes:
> From: Pierre Le Magourou 
>
> We can't wait for END_OF_TRANSFER event in dwc3_gadget_ep_dequeue()
> because it can be called in an interruption context.
>
> TRBs are now cleared after the END_OF_TRANSFER completion, in the
> interrupt handler.
>
> Signed-off-by: Pierre Le Magourou 
> ---

you forgot to Cc linux-usb

>  drivers/usb/dwc3/core.h   |  14 ++
>  drivers/usb/dwc3/gadget.c | 122 
> +++---
>  2 files changed, 85 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 7a217a36c36b..5e2070183e3a 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -627,6 +627,7 @@ struct dwc3_event_buffer {
>   * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
>   * @lock: spinlock for endpoint request queue traversal
>   * @regs: pointer to first endpoint register
> + * @pending_trb: pointer to pending TRB structure
>   * @trb_pool: array of transaction buffers
>   * @trb_pool_dma: dma address of @trb_pool
>   * @trb_enqueue: enqueue 'pointer' into TRB array
> @@ -654,6 +655,7 @@ struct dwc3_ep {
>   void __iomem*regs;
>  
>   struct dwc3_trb *trb_pool;
> + struct dwc3_pending_trb *pending_trb;
>   dma_addr_t  trb_pool_dma;
>   struct dwc3 *dwc;
>  
> @@ -777,6 +779,18 @@ struct dwc3_trb {
>   u32 ctrl;
>  } __packed;
>  
> +/**
> + * struct dwc3_pending_trb
> + * @dirty_trb: pointer to TRB waiting for END_TRANSFER completion
> + * @extra_trb: true if extra TRB was set to align transfer
> + * @num_pending_sgs: number of pending SGs
> + */
> +struct dwc3_pending_trb {
> + struct dwc3_trb *dirty_trb;
> + bool extra_trb;
> + unsigned int num_pending_sgs;
> +};
> +
>  /**
>   * struct dwc3_hwparams - copy of HWPARAMS registers
>   * @hwparams0: GHWPARAMS0
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 69bf137aab37..dd1f2b74723b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1341,6 +1341,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>  
>   struct dwc3_ep  *dep = to_dwc3_ep(ep);
>   struct dwc3 *dwc = dep->dwc;
> + struct dwc3_pending_trb *p_trb;
>  
>   unsigned long   flags;
>   int ret = 0;
> @@ -1367,63 +1368,29 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>* If request was already started, this means we had to
>* stop the transfer. With that we also need to ignore
>* all TRBs used by the request, however TRBs can only
> -  * be modified after completion of END_TRANSFER
> -  * command. So what we do here is that we wait for
> -  * END_TRANSFER completion and only after that, we jump
> -  * over TRBs by clearing HWO and incrementing dequeue
> -  * pointer.
> +  * be modified after completion of END_TRANSFER command.
>*
> -  * Note that we have 2 possible types of transfers here:
> -  *
> -  * i) Linear buffer request
> -  * ii) SG-list based request
> -  *
> -  * SG-list based requests will have r->num_pending_sgs
> -  * set to a valid number (> 0). Linear requests,
> -  * normally use a single TRB.
> -  *
> -  * For each of these two cases, if r->unaligned flag is
> -  * set, one extra TRB has been used to align transfer
> -  * size to wMaxPacketSize.
> -  *
> -  * All of these cases need to be taken into
> -  * consideration so we don't mess up our TRB ring
> -  * pointers.
> +  * Don't wait for END_TRANSFER completion here because
> +  * dwc3_gadget_ep_dequeue() can be called from within
> +  * the controller's interrupt handler. Save the TRB
> +  * pointer, the number of pending sgs, and the
> +  * extra_trb flag, so that TRBs HWO can be cleared and
> +  * dequeue pointer incremented when the END_TRANSFER
> +  * completion is received.
>*/
> - wait_event_lock_irq(dep->wait_end_transfer,
> - !(dep->flags & 
> DWC3_EP_END_TRANSFER_PENDING),
> - dwc->lock);
>  
>   if (!r->trb)
>   goto out0;
>  
> - if (r->num_pending_sgs) 

[PATCH] usb: core: handle hub C_PORT_OVER_CURRENT condition

2018-07-19 Thread Bin Liu
Based on USB2.0 Spec Section 11.12.5,

  "If a hub has per-port power switching and per-port current limiting,
  an over-current on one port may still cause the power on another port
  to fall below specific minimums. In this case, the affected port is
  placed in the Power-Off state and C_PORT_OVER_CURRENT is set for the
  port, but PORT_OVER_CURRENT is not set."

so let's check C_PORT_OVER_CURRENT too for over current condition.

Fixes: 08d1dec6f405 ("usb:hub set hub->change_bits when over-current happens")
Cc: 
Tested-by: Alessandro Antenucci 
Signed-off-by: Bin Liu 
---
 drivers/usb/core/hub.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index fcae521df29b..1fb266809966 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1142,10 +1142,14 @@ static void hub_activate(struct usb_hub *hub, enum 
hub_activation_type type)
 
if (!udev || udev->state == USB_STATE_NOTATTACHED) {
/* Tell hub_wq to disconnect the device or
-* check for a new connection
+* check for a new connection or over current condition.
+* Based on USB2.0 Spec Section 11.12.5,
+* C_PORT_OVER_CURRENT could be set while
+* PORT_OVER_CURRENT is not. So check for any of them.
 */
if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
-   (portstatus & USB_PORT_STAT_OVERCURRENT))
+   (portstatus & USB_PORT_STAT_OVERCURRENT) ||
+   (portchange & USB_PORT_STAT_C_OVERCURRENT))
set_bit(port1, hub->change_bits);
 
} else if (portstatus & USB_PORT_STAT_ENABLE) {
-- 
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] usb: dwc3: change stream event enable bit back to 13

2018-07-19 Thread Erich E. Hoover
Commit ff3f0789b3dc ("usb: dwc3: use BIT() macro where possible")
changed DWC3_DEPCFG_STREAM_EVENT_EN from bit 13 to bit 12.

Spotted this cleanup typo while looking at diffs between 4.9.35 and
4.14.16 for a separate issue.

Fixes: ff3f0789b3dc ("usb: dwc3: use BIT() macro where possible")

Signed-off-by: Erich E. Hoover 
---
 drivers/usb/dwc3/gadget.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index 578aa856f986..1d6f7969953d 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -25,7 +25,7 @@ struct dwc3;
 #define DWC3_DEPCFG_XFER_IN_PROGRESS_ENBIT(9)
 #define DWC3_DEPCFG_XFER_NOT_READY_EN  BIT(10)
 #define DWC3_DEPCFG_FIFO_ERROR_EN  BIT(11)
-#define DWC3_DEPCFG_STREAM_EVENT_ENBIT(12)
+#define DWC3_DEPCFG_STREAM_EVENT_ENBIT(13)
 #define DWC3_DEPCFG_BINTERVAL_M1(n)(((n) & 0xff) << 16)
 #define DWC3_DEPCFG_STREAM_CAPABLE BIT(24)
 #define DWC3_DEPCFG_EP_NUMBER(n)   (((n) & 0x1f) << 25)
-- 
2.14.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


REGRESSION: usb: dwc2: USB device not seen after boot

2018-07-19 Thread Hal Emmerich
[1.] One line summary of the problem: Regression, USB devices not
recognized if plugged in after system boots
[2.] Full description of the problem/report:

  If a usb device is plugged in before system begins booting into the
kernel, it will be recognized once boot completes.
  However, if a usb device is plugged in once boot completes the
system will not recognized it, no messages are added to dmesg.
  If CONFIG_USB_DWC2_DEBUG is set to yes, two messages are added to
dmesg when a usb device is plugged in:
  dwc2 ff58.usb: gintsts=4001 gintmsk=f000
  dwc2 ff58.usb: Session request interrupt - lx_state=2
  ##Reverting the merge made at commit
6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 (The main 4.17 merge) fixes the
issue##

[3.] Keywords (i.e., modules, networking, kernel): usb, dwc2, kernel, arm
[4.] Kernel information:

[4.1.] Kernel version (from /proc/version):
Linux version 4.17.6 (root@debian-build) (gcc version 5.4.1 20160919
(15:5.4.1+svn241155-1)) #1 SMP PREEMPT Thu Jul 12 21:40:35 CDT 2018

[4.2.] Kernel .config file:

[5.] Most recent kernel version which did not have the bug: 4.16.18


[6.] Output of Oops.. message (if applicable) with symbolic information
   resolved (see Documentation/admin-guide/bug-hunting.rst)
[7.] A small shell script or example program which triggers the
   problem (if possible)
[8.] Environment
[8.1.] Software (add the output of the ver_linux script here)

[8.2.] Processor information (from /proc/cpuinfo):

processor: 0
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1

processor: 1
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1

processor: 2
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1

processor: 3
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1

Hardware: Rockchip (Device Tree)
Revision: 
Serial: 


[8.3.] Module information (from /proc/modules):

vfat 20480 1 - Live 0xbf16f000
fat 65536 1 vfat, Live 0xbf158000
ath9k_htc 65536 0 - Live 0xbf14
ath9k_common 16384 1 ath9k_htc, Live 0xbf137000
ath9k_hw 360448 2 ath9k_htc,ath9k_common, Live 0xbf0d1000
ath 24576 3 ath9k_htc,ath9k_common,ath9k_hw, Live 0xbf0c7000
mac80211 561152 1 ath9k_htc, Live 0xbf00f000
autofs4 36864 2 - Live 0xbf00

[8.5.] PCI information ('lspci -vvv' as root) N/A
[8.6.] SCSI information (from /proc/scsi/scsi) N/A
[8.7.] Other information that might be relevant to the problem
 (please look in /proc and include all information that you
 think to be relevant):
[X.] Other notes, patches, fixes, workarounds:

Reverting the merge made at commit
6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 fixes the issue

Walking through the dmesg logs with and without that commit I found this
difference at the same point in the boot process:

dwc2 ff54.usb: In host mode, hprt0=1501
With the non-functional commit
dwc2 ff54.usb: In host mode, hprt0=1101

This difference corresponds to bit 10 being set in the hprt0 register,
but I can’t tell what this bits function is in hw.h

disabling power_down and lpm in dwc2_set_his_params in params.c
did not help.


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


Re: REGRESSION: usb: dwc2: USB device not seen after boot

2018-07-19 Thread Hal Emmerich
Replying to include I had with someone at synopsys, edited to remove
their info as I don't think their email is public.

> Hi Hal,
>
> Could you please apply this patch ([PATCH] usb: dwc2: Fix host exit from 
> hibernation flow.) from linux-usb mailing list.
>
> Let me know if this patch doesn't fix your issue.
>
> Regards,
And what I sent back:
> Hi,
>
> The patch did not fix my issue.
> I don't think dwc2_host_enter_hibernation() is to blame, as it is never 
> called in any of my tests.
> _dwc2_hcd_suspend() gets called, calling dwc2_enter_partial_power_down() 
> which calls the register backup functions.
> After that, there are no more debug messages that refer to that 
> hsotg->dev until I plug a flash drive into the corresponding port.
> When it is plugged in, I know dwc2_handle_common_inter() is called to 
> handle an interrupt, which calls dwc2_read_common_intr(), which prints 
> the
> first of these two lines.
>
>>> dwc2 ff58.usb: gintsts=4001 gintmsk=f000
>>> dwc2 ff58.usb: Session request interrupt - lx_state=2
> Because gintsts=4001 or gintsts=0101
> dwc2_handle_common_inter() calls dwc2_handle_session_req_intr()
>
>   if (gintsts & GINTSTS_SESSREQINT)
>   dwc2_handle_session_req_intr(hsotg);
>
> which prints the second line, then returns as the controller is not in 
> device mode. dwc2_handle_common_inter() leaving the usb device in a 
> limbo state.
>
>
> without the commit I specified in my original message, this is what 
> plugging in a flash drive to the same port looks like:
>
> [   32.745159] dwc2 ff58.usb: gintsts=0521  gintmsk=f3000806
> [   32.749192] dwc2 ff58.usb:   find
> [   32.856474] dwc2 ff58.usb: SetPortFeature
> [   32.856533] dwc2 ff58.usb: SetPortFeature - USB_PORT_FEAT_RESET
> [   32.856585] dwc2 ff58.usb: In host mode, hprt0=00021501
> [   32.908063] dwc2 ff58.usb: gintsts=0521  gintmsk=f3000806
> [   32.959694] dwc2 ff58.usb: DWC OTG HCD HUB STATUS DATA: Root port 
> status changed
> [   32.959748] dwc2 ff58.usb:   port_connect_status_change: 0
> [   32.959770] dwc2 ff58.usb:   port_reset_change: 1
> [   32.959790] dwc2 ff58.usb:   port_enable_change: 0
> [   32.959808] dwc2 ff58.usb:   port_suspend_change: 0
> [   32.959827] dwc2 ff58.usb:   port_over_current_change: 0
> [   32.970274] dwc2 ff58.usb: ClearPortFeature USB_PORT_FEAT_C_RESET
> [   33.021709] usb 2-1: new high-speed USB device number 2 using dwc2
> [   33.024098] dwc2 ff58.usb: SetPortFeature
> [   33.024109] dwc2 ff58.usb: SetPortFeature - USB_PORT_FEAT_RESET
> [   33.024119] dwc2 ff58.usb: In host mode, hprt0=1101
> [   33.024134] dwc2 ff58.usb: gintsts=0529  gintmsk=f3000806
> [   33.076237] dwc2 ff58.usb: gintsts=0529  gintmsk=f3000806
> [   33.137721] dwc2 ff58.usb: ClearPortFeature USB_PORT_FEAT_C_RESET
> [   33.207735] dwc2 ff58.usb: DWC OTG HCD HUB STATUS DATA: Root port 
> status changed
> [   33.207835] dwc2 ff58.usb:   port_connect_status_change: 0
> [   33.207874] dwc2 ff58.usb:   port_reset_change: 0
> [   33.207910] dwc2 ff58.usb:   port_enable_change: 1
> [   33.207945] dwc2 ff58.usb:   port_suspend_change: 0
> [   33.207979] dwc2 ff58.usb:   port_over_current_change: 0
> [   33.328256] dwc2 ff58.usb: DWC OTG HCD EP DISABLE: 
> bEndpointAddress=0x00, ep->hcpriv=e81e42ce
> [   33.328275] dwc2 ff58.usb: DWC OTG HCD EP DISABLE: 
> bEndpointAddress=0x00, ep->hcpriv=  (null)
> [   33.328287] dwc2 ff58.usb: DWC OTG HCD EP RESET: 
> bEndpointAddress=0x00
> [   33.347535] usb 2-1: New USB device found, idVendor=18a5, 
> idProduct=0302, bcdDevice= 1.00
> [   33.347629] usb 2-1: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=3
> [   33.347720] usb 2-1: Product: STORE N GO
> [   33.347767] usb 2-1: Manufacturer: Verbatim
> [   33.347815] usb 2-1: SerialNumber: 9E457CF1
> [   33.348067] dwc2 ff58.usb: DWC OTG HCD EP RESET: 
> bEndpointAddress=0x01
> [   33.348072] dwc2 ff58.usb: DWC OTG HCD EP RESET: 
> bEndpointAddress=0x82
> [   33.348224] usb-storage 2-1:1.0: USB Mass Storage device detected
> [   33.354164] scsi host1: usb-storage 2-1:1.0
> [   33.354905] dwc2 ff58.usb: ClearPortFeature 
> USB_PORT_FEAT_C_ENABLE
> [   34.403547] scsi 1:0:0:0: Direct-Access Verbatim STORE N GO   
> 8.07 PQ: 0 ANSI: 4
> [   34.414868] sd 1:0:0:0: [sdb] 15564800 512-byte logical blocks: (7.97 
> GB/7.42 GiB)
> [   34.418173] sd 1:0:0:0: [sdb] Write Protect is off
> [   34.418270] sd 1:0:0:0: [sdb] Mode Sense: 23 00 00 00
> [   34.418979] sd 1:0:0:0: [sdb] Write cache: disabled, read cache: 
> enabled, doesn't support DPO or FUA
> [   34.424606]  sdb: sdb1
> [   34.426850] sd 1:0:0:0: [sdb] Attached SCSI removable disk
>
>
> Which the commit, the two lines I've included above are all that get 
> printed to dmesg.
> All I can conclude is that gintsts and gintmsk are not in the correct 
> stat

[PATCH] usb: dwc2: disable power_down on rockchip devices

2018-07-19 Thread Hal Emmerich
>From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001
From: Hal Emmerich 
Date: Thu, 19 Jul 2018 21:48:08 -0500
Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices

 The bug would let the usb controller enter partial power down,
 which was formally known as hibernate, upon boot if nothing was plugged
 in to the port. Partial power down couldn't be exited properly, so any
 usb devices plugged in after boot would not be usable.

 Before the name change, params.hibernation was false by default, so
 _dwc2_hcd_suspend() would skip entering hibernation. With the
 rename, _dwc2_hcd_suspend() was changed to use  params.power_down
 to decide whether or not to enter partial power down.

 Since params.power_down is non-zero by default, it needs to be set
 to 0 for rockchip devices to restore functionality.

 This bug was reported in the linux-usb thread:
 REGRESSION: usb: dwc2: USB device not seen after boot

 The commit that caused this regression is:
 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6

Signed-off-by: Hal Emmerich 
---
 drivers/usb/dwc2/params.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index f03e41879224..492607adc506 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -82,6 +82,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
    p->host_perio_tx_fifo_size = 256;
    p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
    GAHBCFG_HBSTLEN_SHIFT;
+   p->power_down = 0;
 }

 static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg)
--
2.11.0
--
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: REGRESSION: usb: dwc2: USB device not seen after boot

2018-07-19 Thread Hal Emmerich
I believe I have found the root cause of this issue.
Before commit 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6
|_dwc2_hcd_suspend() did not end up calling dwc2_enter_hibernation(),
which was renamed to dwc2_enter_partial_power_down() in the same commit.
_dwc2_hcd_suspend() skipped |
||dwc2_enter_partial_power_down() because of these lines:||

|if (!hsotg->params.hibernation){ goto skip_power_saving; } which now reads: 
||if (!hsotg->params.power_down){ goto skip_power_saving; } The problem is, 
params.power_down does not default to 0 like params.hibernation did so 
|||dwc2_enter_partial_power_down() gets called|, the controller puts the port 
into partial power down, and doesn't correctly leave when a device is plugged 
in causing the port to be unusable. I've submitted a patch to the list to 
address this on rockchip devices: [PATCH] usb: dwc2: disable power_down on 
rockchip devices |||


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