UAS - General Protection Fault when plugging in USB 3.0 Mass Storage

2014-09-27 Thread Alan Barker
Hi, I have a 2TB Seagate Backup drive that works fine when used in my
laptop's USB 2.0 port yet when plugged into the USB 3.0 port my system
hangs with a Kernel Panic/GPF and needs a hard reset.

I'm running Fedora 20 on an ASUS UX31E laptop. I've just upgraded from
the base release kernel 3.11.10-301.fc20 straight to 3.16.2-201.fc20 and
the problem appeared. (new laptop install hence the large jump). Rolling
back to the 3.11.x kernel lets the drive work again. I tried a few newer
3.16.x kernels (and some slightly older ones 3.15.x) but the problem
still exists. 

I've since been able to work around the problem by blacklisting the UAS
driver, which lets me plug in the device without a fault, but doesn't
recognise the drive for use. I've now settled on using an Option rule in
modprobe.d directory to force the device down to the usb_storage driver
instead of UAS. This now works fine with no problems on 3.16.2-201
kernel however, it is not the newer driver. 

I'm fairly sure this is coming from the UAS driver, although we can
never be certain...

I also noticed that there have been a few of these issues before
(http://www.spinics.net/lists/linux-usb/msg109480.html) so whilst i've
been able to work around it, i'm still keen to stick around and help you
guys out and fix the problem - where i can.

I appreciate your work and please let me know if there is anything I can
do to help.

(I've pasted relevant logs below)

Alan Barker.



[Kernel messages]

Sep 25 22:10:51 holon2.local kernel: usb 3-1: new SuperSpeed USB device number 
6 using xhci_hcd
Sep 25 22:10:51 holon2.local kernel: usb 3-1: New USB device found, 
idVendor=0bc2, idProduct=ab20
Sep 25 22:10:51 holon2.local kernel: usb 3-1: New USB device strings: Mfr=2, 
Product=3, SerialNumber=1
Sep 25 22:10:51 holon2.local kernel: usb 3-1: Product: Backup+  SL
Sep 25 22:10:51 holon2.local kernel: usb 3-1: Manufacturer: Seagate
Sep 25 22:10:51 holon2.local kernel: usb 3-1: SerialNumber: NA75VFTG
Sep 25 22:10:51 holon2.local kernel: scsi10 : uas
Sep 25 22:10:51 holon2.local kernel: scsi 10:0:0:0: Direct-Access Seagate  
Backup+  SL  A905 PQ: 0 ANSI: 6
Sep 25 22:10:51 holon2.local kernel: sd 10:0:0:0: Attached scsi generic sg1 
type 0
Sep 25 22:10:51 holon2.local kernel: sd 10:0:0:0: [sdb] Spinning up disk...
Sep 25 22:10:51 holon2.local mtp-probe[29531]: checking bus 3, device 6: 
"/sys/devices/pci:00/:00:1c.3/:03:00.0/usb3/3-1"
Sep 25 22:10:51 holon2.local mtp-probe[29531]: bus: 3, device: 6 was not an MTP 
device
Sep 25 22:10:55 holon2.local kernel: ready
Sep 25 22:10:55 holon2.local kernel: sd 10:0:0:0: [sdb] 3907029167 512-byte 
logical blocks: (2.00 TB/1.81 TiB)
Sep 25 22:10:56 holon2.local kernel: sd 10:0:0:0: [sdb] Write Protect is off
Sep 25 22:10:56 holon2.local kernel: sd 10:0:0:0: [sdb] Mode Sense: 4f 00 00 00
Sep 25 22:10:56 holon2.local kernel: sd 10:0:0:0: [sdb] Write cache: enabled, 
read cache: enabled, doesn't support DPO or FUA
Sep 25 22:10:56 holon2.local kernel: general protection fault:  [#1] SMP 
Sep 25 22:10:56 holon2.local kernel: Modules linked in: cdc_acm xt_conntrack 
iptable_raw iptable_security xt_CHECKSUM iptable_mangle ipt_MASQUERADE 
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack 
vhost_net vhost macvtap macvlan tun lockd sunrpc ppp_mppe ppp_deflate bsd_comp 
ppp_async crc_ccitt ppp_generic slhc ses enclosure uas usb_storage ccm tcp_lp 
asix usbnet mii binfmt_misc rfcomm fuse bridge stp llc bnep vfat fat 
asus_nb_wmi asus_wmi sparse_keymap iTCO_wdt iTCO_vendor_support 
x86_pkg_temp_thermal coretemp kvm_intel snd_hda_codec_hdmi kvm arc4 
snd_hda_codec_realtek snd_hda_codec_generic ath9k crct10dif_pclmul ath9k_common 
crc32_pclmul snd_hda_intel ath9k_hw crc32c_intel snd_hda_controller ath 
snd_hda_codec ghash_clmulni_intel uvcvideo ath3k snd_hwdep mac80211 btusb 
snd_seq bluetooth microcode
Sep 25 22:10:56 holon2.local kernel:  videobuf2_vmalloc videobuf2_memops 
videobuf2_core cfg80211 snd_seq_device v4l2_common videodev joydev snd_pcm 
media serio_raw rfkill wmi snd_timer tpm_tis mei_me snd mei i2c_i801 lpc_ich 
tpm soundcore mfd_core shpchp i915 i2c_algo_bit drm_kms_helper drm i2c_core 
video [last unloaded: iptable_raw]
Sep 25 22:10:56 holon2.local kernel: CPU: 0 PID: 25783 Comm: Chrome_ChildIOT 
Not tainted 3.16.2-201.fc20.x86_64 #1
Sep 25 22:10:56 holon2.local kernel: Hardware name: ASUSTeK Computer Inc. 
UX31E/UX31E, BIOS UX31E.211 01/20/2012
Sep 25 22:10:56 holon2.local kernel: task: 8800064c ti: 
8800114b4000 task.ti: 8800114b4000
Sep 25 22:10:56 holon2.local kernel: RIP: 0010:[]  
[] __kmalloc_node_track_caller+0x1cb/0x290
Sep 25 22:10:56 holon2.local kernel: RSP: 0018:8800114b7b90  EFLAGS: 
00010246
Sep 25 22:10:56 holon2.local kernel: RAX:  RBX: 
8801389b9000 RCX: 815e751d
Sep 25 22:10:56 holon2.local kernel: RDX: 00f7449e RSI: 
 RDI: 017f
Sep 25 22:10:56 holon2.local ke

problems with usb stick after suspend and wake up

2014-09-27 Thread Norbert Preining
Dear all,

(please Cc)

I am running latest kernel (3.17-rc6) and I see corruption of an usb3.0
device usb stick. Strange errors, impossibility to mount.

Repeated unplugging and replugging helps sometimes, in all cases
recovery is necessary.

Is this a know problem, a problem of my system
Debian/sid, uptodate, self-compiled kernel
of the suspend program (that initiates the suspend), 
the usb stick and file system, or the USB subsystem?

I happily provide loads of further details about hardware, logs, whatever,
but first I ask before sending tons of useless stuff!?

Any idea?

Thanks a lot

Norbert


PREINING, Norbert   http://www.preining.info
JAIST, Japan TeX Live & Debian Developer
GPG: 0x860CDC13   fp: F7D8 A928 26E3 16A1 9FA0  ACF0 6CAC A448 860C DC13

--
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 fix for 3.17 1/2] xhci: Check for eps[ep_index].ring being NULL after an usb_device_reset

2014-09-27 Thread Hans de Goede
This commit fixes the following oops:

[10238.622067] scsi host3: uas_eh_bus_reset_handler start
[10240.766164] usb 3-4: reset SuperSpeed USB device number 3 using xhci_hcd
[10245.779365] usb 3-4: device descriptor read/8, error -110
[10245.883331] usb 3-4: reset SuperSpeed USB device number 3 using xhci_hcd
[10250.897603] usb 3-4: device descriptor read/8, error -110
[10251.058200] BUG: unable to handle kernel NULL pointer dereference at 
0040
[10251.058244] IP: [] xhci_check_streams_endpoint+0x91/0x140

[10251.059473] Call Trace:
[10251.059487]  [] 
xhci_calculate_streams_and_bitmask+0xbc/0x130
[10251.059520]  [] xhci_alloc_streams+0x10f/0x5a0
[10251.059548]  [] ? check_preempt_curr+0x75/0xa0
[10251.059575]  [] ? ttwu_do_wakeup+0x2c/0x100
[10251.059601]  [] ? ttwu_do_activate.constprop.111+0x66/0x70
[10251.059635]  [] usb_alloc_streams+0xab/0xf0
[10251.059662]  [] uas_configure_endpoints+0x128/0x150 [uas]
[10251.059694]  [] uas_post_reset+0x3c/0xb0 [uas]
[10251.059722]  [] usb_reset_device+0x1b9/0x2a0
[10251.059749]  [] uas_eh_bus_reset_handler+0xb2/0x190 [uas]
[10251.059781]  [] scsi_try_bus_reset+0x53/0x110
[10251.059808]  [] scsi_eh_bus_reset+0xf7/0x270


The problem is the following call sequence (simplified):

1) usb_reset_device
2)  usb_reset_and_verify_device
2)   hub_port_init
3)hub_port_finish_reset
3) xhci_discover_or_reset_device
This frees xhci->devs[slot_id]->eps[ep_index].ring for all eps but 0
4)usb_get_device_descriptor
   This fails
5)   hub_port_init fails
6)  usb_reset_and_verify_device fails, does not restore device config
7)  uas_post_reset
8)   xhci_alloc_streams
  NULL deref on the free-ed ring

The real problem here is that xhci_discover_or_reset_device frees the rings
during a reset, and if usb_reset_and_verify_device then fails to restore the
device configuration (and thus re-alloc the rings), that we're then left with
a device which from the usb_device_driver's pov is still functional (its
disconnect method will get called when khub gets around to it), but in
reality is not.

This commit adds a check for this condition to xhci_check_args(), which should
cover all public entry points into the xhci driver.

Cc: sta...@vger.kernel.org
Reported-by: Claudio Bizzarri 
Signed-off-by: Hans de Goede 
---
 drivers/usb/host/xhci.c | 37 ++---
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 642791c..64160ff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1146,10 +1146,12 @@ unsigned int xhci_last_valid_endpoint(u32 added_ctxs)
  * returns 0 this is a root hub; returns -EINVAL for NULL pointers.
  */
 static int xhci_check_args(struct usb_hcd *hcd, struct usb_device *udev,
-   struct usb_host_endpoint *ep, int check_ep, bool check_virt_dev,
-   const char *func) {
+   struct usb_host_endpoint *ep, bool check_ep,
+   bool check_ep_ring, bool check_virt_dev, const char *func)
+{
struct xhci_hcd *xhci;
-   struct xhci_virt_device *virt_dev;
+   struct xhci_virt_device *virt_dev = NULL;
+   unsigned int ep_index;
 
if (!hcd || (check_ep && !ep) || !udev) {
pr_debug("xHCI %s called with invalid args\n", func);
@@ -1176,6 +1178,18 @@ static int xhci_check_args(struct usb_hcd *hcd, struct 
usb_device *udev,
}
}
 
+   /*
+* If we get called after a reset, then usb_reset_device ->
+* xhci_discover_or_reset_device may have free-ed eps[ep_index].ring,
+* without it being setup again because of the usb_reset_device
+* failing to re-configure the device.
+*/
+   if (check_ep_ring) {
+   ep_index = xhci_get_endpoint_index(&ep->desc);
+   if (virt_dev->eps[ep_index].ring == NULL)
+   return -ENODEV;
+   }
+
if (xhci->xhc_state & XHCI_STATE_HALTED)
return -ENODEV;
 
@@ -1281,7 +1295,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
int size, i;
 
if (!urb || xhci_check_args(hcd, urb->dev, urb->ep,
-   true, true, __func__) <= 0)
+   true, true, true, __func__) <= 0)
return -EINVAL;
 
slot_id = urb->dev->slot_id;
@@ -1596,7 +1610,7 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
u32 new_add_flags, new_drop_flags;
int ret;
 
-   ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
+   ret = xhci_check_args(hcd, udev, ep, true, false, true, __func__);
if (ret <= 0)
return ret;
xhci = hcd_to_xhci(hcd);
@@ -1675,7 +1689,7 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_virt_device *virt_dev;
int ret = 0;
 
-   ret = xhci_check_args(hcd,

[PATCH fix for 3.17 2/2] xhci: Silence "xHCI xhci_drop_endpoint called with disabled ep ..." messages

2014-09-27 Thread Hans de Goede
Silence the "xHCI xhci_drop_endpoint called with disabled ep ..." messages
appearing on a successful usb device reset.

Signed-off-by: Hans de Goede 
---
 drivers/usb/host/xhci.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 64160ff..5d985e4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1636,6 +1636,12 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
 
ep_index = xhci_get_endpoint_index(&ep->desc);
ep_ctx = xhci_get_ep_ctx(xhci, out_ctx, ep_index);
+   /*
+* If drop_endpoint gets called after a usb_device_reset, then
+* xhci_discover_or_reset_device has already dropped the eps
+*/
+   if (xhci->devs[udev->slot_id]->eps[ep_index].ring == NULL)
+   return 0;
/* If the HC already knows the endpoint is disabled,
 * or the HCD has noted it is disabled, ignore this request
 */
-- 
2.1.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: [PATCH fix for 3.17 2/2] xhci: Silence "xHCI xhci_drop_endpoint called with disabled ep ..." messages

2014-09-27 Thread Hans de Goede
Hi,

On 09/27/2014 03:01 PM, Hans de Goede wrote:
> Silence the "xHCI xhci_drop_endpoint called with disabled ep ..." messages
> appearing on a successful usb device reset.
> 
> Signed-off-by: Hans de Goede 

Note no Cc stable for this one, even though it did get send to stable
accidentally, sorry.

Regards,

Hans

> ---
>  drivers/usb/host/xhci.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 64160ff..5d985e4 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1636,6 +1636,12 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
> usb_device *udev,
>  
>   ep_index = xhci_get_endpoint_index(&ep->desc);
>   ep_ctx = xhci_get_ep_ctx(xhci, out_ctx, ep_index);
> + /*
> +  * If drop_endpoint gets called after a usb_device_reset, then
> +  * xhci_discover_or_reset_device has already dropped the eps
> +  */
> + if (xhci->devs[udev->slot_id]->eps[ep_index].ring == NULL)
> + return 0;
>   /* If the HC already knows the endpoint is disabled,
>* or the HCD has noted it is disabled, ignore this request
>*/
> 
--
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: problems with usb stick after suspend and wake up

2014-09-27 Thread Alan Stern
On Sat, 27 Sep 2014, Norbert Preining wrote:

> Dear all,
> 
> (please Cc)
> 
> I am running latest kernel (3.17-rc6) and I see corruption of an usb3.0
> device usb stick. Strange errors, impossibility to mount.
> 
> Repeated unplugging and replugging helps sometimes, in all cases
> recovery is necessary.
> 
> Is this a know problem, a problem of my system
> Debian/sid, uptodate, self-compiled kernel
> of the suspend program (that initiates the suspend), 
> the usb stick and file system, or the USB subsystem?
> 
> I happily provide loads of further details about hardware, logs, whatever,
> but first I ask before sending tons of useless stuff!?
> 
> Any idea?

I do not recognize this problem based on your description.  Please post 
the dmesg log.

Alan Stern

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


Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-09-27 Thread Peter Chen
On Thu, Sep 25, 2014 at 07:37:50PM -0500, Felipe Balbi wrote:
> HI,
> 
> On Fri, Sep 26, 2014 at 07:39:34AM +0800, Peter Chen wrote:
> > On Thu, Sep 25, 2014 at 09:15:53AM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> > > > > 
> > > > > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > > > > (dwc3, musb, chipidea) you are talking about, right? Except for
> > > > > creating another platform driver as well as related DT node 
> > > > > (optional),
> > > > > are there any advantages compared to current IP core driver structure?
> > > > 
> > > > Having a library module usually requires less code, and is more
> > > > consistent with other drivers, which helps new developers understand
> > > > what the driver is doing.
> > > 
> > > I beg to differ. You end-up having to pass function pointers through
> > > platform_data to handle differences which could be handled by the core
> > > IP.
> > > 
> > > > The most important aspect though is the DT binding: once the structure
> > > > with separate kernel drivers leaks out into the DT format, you can't
> > > > easily change the driver any more, e.g. to make a property that was
> > > > introduced for one glue driver more general so it can get handled by
> > > > the common part. Having a single node lets us convert to the library
> > > > model later, so that would be a strong reason to keep the DT binding
> > > > simple, without child nodes.
> > > > 
> > > > Following that logic, it turns into another reason for using the library
> > > > model to start with, because otherwise the child device does not have
> > > > any DT properties itself and has to rely on the parent device 
> > > > properties,
> > > > which somewhat complicates the driver structure.
> > > 
> > > this is bullcrap. Take a look at
> > > Documentation/devicetree/bindings/usb/dwc3.txt:
> > > 
> > > synopsys DWC3 CORE
> > > 
> > > DWC3- USB3 CONTROLLER
> > > 
> > > Required properties:
> > >  - compatible: must be "snps,dwc3"
> > >  - reg : Address and length of the register set for the device
> > >  - interrupts: Interrupts used by the dwc3 controller.
> > > 
> > > Optional properties:
> > >  - usb-phy : array of phandle for the PHY device.  The first element
> > >in the array is expected to be a handle to the USB2/HS PHY and
> > >the second element is expected to be a handle to the USB3/SS PHY
> > >  - phys: from the *Generic PHY* bindings
> > >  - phy-names: from the *Generic PHY* bindings
> > >  - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
> > > 
> > > This is usually a subnode to DWC3 glue to which it is connected.
> > > 
> > > dwc3@4a03 {
> > >   compatible = "snps,dwc3";
> > >   reg = <0x4a03 0xcfff>;
> > >   interrupts = <0 92 4>
> > >   usb-phy = <&usb2_phy>, <&usb3,phy>;
> > >   tx-fifo-resize;
> > > };
> > > 
> > > This contains all the attributes it needs to work. In fact, this could
> > > be used without any glue.
> > > 
> > 
> > If you want to add "vbus-supply" core node to support ID switch, what's
> > the plan for that?
> 
> send a patch ? Just make sure it's optional because not everybody needs
> a vbus-supply. Also, DRD will still take a few merge windows to become a
> reality. I don't want to merge something before considering it carefuly,
> otherwise we will having which is broken and doesn't work for everybody
> ;-)
> 
> > Is it possible that the glue layer needs to access core register
> > (eg, portsc.phcd during suspend)? The wrapper IP may wait some signals
> > at core IP to change.
> 
> why would a glue layer need to access registers from the core ? That
> sounds very odd. I haven't seen that and will, definitely, NACK such a
> patch :-)
> 
> can you further describe why you think a glue layer might need to access
> core IP's registers ?
> 

My mistake, we can just wait core layer for finishing before doing PHY
and wrapper operation.

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


Re: [PATCH 1/2] USB: EHCI: add portpower callback and override

2014-09-27 Thread Peter Chen
On Fri, Sep 26, 2014 at 10:42:35AM -0400, Alan Stern wrote:
> On Fri, 26 Sep 2014, Michael Grzeschik wrote:
> 
> > The current EHCI implementation is prepared to toggle the
> > PORT_POWER bit to enable or disable a USB-Port. In some
> > cases this port power can not be toggled by the PORT_POWER
> > bit but instead i.e. by an external GPIO.
> > This patch adds an callback to be triggered as well.
> > The host needs to assign a capable portpower callback.
> 
> The idea is reasonable enough, I suppose (but where does this stop?  
> Before you know it, every function normally carried out by the host 
> controller will actually be handled by a platform-specific piece of 
> hardware!)
> 
> The implementation leaves something to be desired.  Instead of keeping
> the current port-power code in ehci-hcd inline, as it is, you should
> define a default callback routine to do the work.  Then the override
> can replace the default callback.
> 

Agree, but one more thing that I find the clear_port_feature(PORT_POWER)
for roothub doesn't be called when I remove the hcd, we hope
the portsc.pp and external gpio can be in single .portpower API.

-- 
Best Regards,
Peter Chen
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-27 Thread Huang Rui
On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
> On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index b0f4d52..6138c5d 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > >  }
> > > >  
> > > >  /**
> > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > + * @dwc: Pointer to our controller context structure
> > > > + */
> > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > +{
> > > > +   u32 reg = 0;
> > > > +
> > > > +   reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | 
> > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > +   | DWC3_GUSB3PIPECTL_UX_EXITINPX | 
> > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > +   | DWC3_GUSB3PIPECTL_DEPOCHANGE | 
> > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > 
> > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > > erratum before I can accept it. You have also duplicated the bit in this
> > > initialization.
> > > 
> > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > > 
> > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > > only be proven to work with this bit after going through full USB
> > > certification.
> > > 
> > 
> > What do you mean of the faulty PHY?
> > We would use AMD PHY to talk with DWC3 controller.
> 
> Look at the description of each of those bits and you'll see it mentions
> they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> example:
> 
>   "
>   This bit is added for SS PHY workaround where SS PHY ...
>   "
> 

Got it, so faulty PHY you meant that some special PHYs didn't not meet
the standards someplace.

For simulation board, we used vendor provided PHY. But on SOC, we
will use AMD PHY. I will re-check again to confirm which workarounds
need on simulation board and which workarounds need on SOC.

> It's alright that AMD PHY needs this bit, but then, let's get
> confirmation from IP/SoC/SilVal team and add a proper comment stating
> why we need them. This is so we don't forget that $version of AMD's PHY
> needs workarounds for A, B, and C silicon errata.
> 

Yes, but currently, I needn't write AMD own phy driver. There isn't
any requirement from HW side to program the phy register. So I used
NOP USB transceiver driver till now. 

> Also, I'd have to ask you to provide full boot logs of your platform
> booting with my testing/next (where all the latest patches are) plus
> your patches. 

I will provide the boot logs to you. Actually, I already did the
gadget mass storage and gadget zero testing with my patches under 3.14
before.

> Then, load g_mass_storage with a backing storage of your
> choice and run my msc.c/msc.sh tools which you can get from [1] and [2];
> post the logs for that too. Last, but not least, please USB30CV (chapter
> 9 and Link Layer test, at least) just so we know there isn't anything
> new with your version of the core, which I suppose is 2.80a, based on
> the LPM Errata bits.
> 

OK, will post the logs to you.

> This is just because I don't have access to the HW myself, so I can't
> verify your patches. One thing I can tell you, with my testing/next,
> dwc3 is really stable. I have every test passing except for Halt
> Endpoint which I'm debugging right now.
> 

OK, I got it. Will rebase my patches to testing/next.

> > > > +   reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | 
> > > > DWC3_GUSB3PIPECTL_TX_DEEPH(1);
> > > 
> > > DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
> > > need to see an erratum.
> > > 
> > > > +   dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > > > +
> > > > +   mdelay(100);
> > > > +}
> > > > +
> > > > +/**
> > > >   * dwc3_free_one_event_buffer - Frees one event buffer
> > > >   * @dwc: Pointer to our controller context structure
> > > >   * @evt: Pointer to event buffer to be freed
> > > > @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > > > if (ret)
> > > > goto err0;
> > > >  
> > > > +   if (dwc->quirks & DWC3_AMD_NL_PLAT)
> > > > +   dwc3_core_nl_set_pipe3(dwc);
> > > > +
> > > > reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > > > +
> > > > reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> > > > -   reg &= ~DWC3_GCTL_DISSCRAMBLE;
> > > > +
> > > > +   if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > +   reg |= DWC3_GCTL_DISSCRAMBLE;
> > > 
> > > you're disabling scrambling ? What about EMI ? Why doesn't your device
> > > work with scrambling ? I need to see an erratum before I can accept
> > > this.
> > > 
> > 
> > Sorry, disabling scrambling is workaround for debugg

Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-09-27 Thread Huang Rui
On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
> On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index b0f4d52..6138c5d 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > >  }
> > > >  



> > > > +   reg |= DWC3_GCTL_GBLHIBERNATIONEN;
> > > 
> > > looks like this should always be set for all versions of the core
> > > configured with hibernation.
> > > 
> > 
> > yes, amd nl chip will support hibernation.
> 
> in that case, we do this:
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4d4e854..584dcde 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>   case DWC3_GHWPARAMS1_EN_PWROPT_HIB:
>   /* enable hibernation here */
>   dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4);
> + reg |= DWC3_GCTL_GBLHIBERNATIONEN;
>   break;
>   default:
>   dev_dbg(dwc->dev, "No power optimization available\n");
> 
> that'll work for everybody with hibernation enabled in coreConsultant.
> 

Right, I will do it like this on V2.

Thanks,
Rui
--
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