[no subject]

[林敬樺]

投資できる1億5000万ドル相当の取引があります
あなたの国のどんなビジネスエリアでも、より多くの情報のために私に答えてください。
電子メール：normansbs1...@126.com
ここにのみあなたの応答を転送する：normansbs1...@126.com

I have a business deal worth $150 Million USD which can be invested
in any business area in your country, reply to me for more info via
Email: normansbs1...@126.com
Forward your response here only: 
normansbs1...@126.com

Re: [PATCH v4 0/7] usb: typec: ucsi: Remaining changes for v5.2

[Greg Kroah-Hartman]

On Tue, Apr 23, 2019 at 05:21:44PM +0300, Heikki Krogerus wrote:
> Hi Greg,
> 
> Ajay noticed that I was not considering earlier releases of the UCSI
> specification with a specific UCSI command, GET_CURRENT_CAM, in v3.
> The definition for the command was changed in UCSI specification v1.1.
> The issue is now fixed.

Looks good, thanks for sticking with these, now all queued up.

greg k-h

[PATCH] xhci: update bounce buffer with correct sg num

[Henry Lin]

This change fixes a data corruption issue occurred on USB hard disk for
the case that bounce buffer is used during transferring data.

While updating data between sg list and bounce buffer, current
implementation passes mapped sg number (urb->num_mapped_sgs) to
sg_pcopy_from_buffer() and sg_pcopy_to_buffer(). This causes data
not get copied if target buffer is located in the elements after
mapped sg elements. This change passes sg number for full list to
fix issue.

Besides, for copying data from bounce buffer, calling dma_unmap_single()
on the bounce buffer before copying data to sg list can avoid cache issue.

Signed-off-by: Henry Lin 
---
 drivers/usb/host/xhci-ring.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9215a28..f5d0908 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -656,6 +656,7 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd 
*xhci,
struct device *dev = xhci_to_hcd(xhci)->self.controller;
struct xhci_segment *seg = td->bounce_seg;
struct urb *urb = td->urb;
+   size_t len;
 
if (!ring || !seg || !urb)
return;
@@ -666,11 +667,14 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd 
*xhci,
return;
}
 
-   /* for in tranfers we need to copy the data from bounce to sg */
-   sg_pcopy_from_buffer(urb->sg, urb->num_mapped_sgs, seg->bounce_buf,
-seg->bounce_len, seg->bounce_offs);
dma_unmap_single(dev, seg->bounce_dma, ring->bounce_buf_len,
 DMA_FROM_DEVICE);
+   /* for in tranfers we need to copy the data from bounce to sg */
+   len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs, seg->bounce_buf,
+seg->bounce_len, seg->bounce_offs);
+   if (len != seg->bounce_len)
+   xhci_warn(xhci, "WARN Wrong bounce buffer read length: %ld != 
%d\n",
+   len, seg->bounce_len);
seg->bounce_len = 0;
seg->bounce_offs = 0;
 }
@@ -3123,6 +3127,7 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct 
urb *urb, u32 enqd_len,
unsigned int unalign;
unsigned int max_pkt;
u32 new_buff_len;
+   size_t len;
 
max_pkt = usb_endpoint_maxp(&urb->ep->desc);
unalign = (enqd_len + *trb_buff_len) % max_pkt;
@@ -3153,8 +3158,12 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct 
urb *urb, u32 enqd_len,
 
/* create a max max_pkt sized bounce buffer pointed to by last trb */
if (usb_urb_dir_out(urb)) {
-   sg_pcopy_to_buffer(urb->sg, urb->num_mapped_sgs,
+   len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
   seg->bounce_buf, new_buff_len, enqd_len);
+   if (len != seg->bounce_len)
+   xhci_warn(xhci,
+   "WARN Wrong bounce buffer write length: %ld != 
%d\n",
+   len, seg->bounce_len);
seg->bounce_dma = dma_map_single(dev, seg->bounce_buf,
 max_pkt, DMA_TO_DEVICE);
} else {
-- 
2.7.4

Re: [PATCH v1 00/14] usb: dwc2: Fix and improve power saving modes.

[Felipe Balbi]

Artur Petrosyan  writes:
> This patch set, fixes and improves partial power down and hibernation power
> saving modes. Also, adds support for entering/exiting hibernation by
> system issued suspend/resume.
>
> This series contains patches which were submitted to LKML. However, a part
> of those patches didn't reach to LKML because of local issue related to
> smtp server.
>
> The patches which reached to LKML are:
>
> - usb: dwc2: Add part. power down exit from dwc2_conn_id_status_change().
> - usb: dwc2: Add port conn. sts. checking in _dwc2_hcd_resume() function.
> - usb: dwc2: Fix suspend state in host mode for partial power down.
> - usb: dwc2: Fix wakeup detected and session request interrupt handlers.
> - usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
> - usb: dwc2: Fix dwc2_restore_device_registers() function.
>
> The patches which didn't reach to LKML are:
>
> - usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
> - usb: dwc2: Clear GINTSTS_RESTOREDONE bit after restore is generated.
> - usb: dwc2: Clear fifo_map when resetting core.
> - usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
> - usb: dwc2: Fix hibernation between host and device modes.
> - usb: dwc2: Update dwc2_handle_usb_suspend_intr function.
> - usb: dwc2: Add default param to control power optimization.
> - usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
>
> Submitting all of the patches together in this version.
>
> Changes from V0:
>  - Replaced 1 with DWC2_POWER_DOWN_PARAM_PARTIAL in commit
>"9eed02b9fe96 usb: dwc2: Fix wakeup detected and session request
>interrupt handlers.
>
>
> Artur Petrosyan (14):
>   usb: dwc2: Fix dwc2_restore_device_registers() function.
>   usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
>   usb: dwc2: Fix wakeup detected and session request interrupt handlers.
>   usb: dwc2: Fix suspend state in host mode for partial power down.
>   usb: dwc2: Add port conn. sts. checking in _dwc2_hcd_resume()
> function.
>   usb: dwc2: Add part. power down exit from
> dwc2_conn_id_status_change().
>   usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
>   usb: dwc2: Add default param to control power optimization.
>   usb: dwc2: Update dwc2_handle_usb_suspend_intr function.
>   usb: dwc2: Fix hibernation between host and device modes.
>   usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
>   usb: dwc2: Clear fifo_map when resetting core.
>   usb: dwc2: Clear GINTSTS_RESTOREDONE bit after restore is generated.
>   usb: dwc2: Add enter/exit hibernation from system issued
> suspend/resume

patches don't apply.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 01/14] usb: dwc2: Fix dwc2_restore_device_registers() function.

[Felipe Balbi]

Artur Petrosyan  writes:

> - Added backup of DCFG register.
> - Added Set the Power-On Programming done bit.
>
> Signed-off-by: Artur Petrosyan 

won't apply.


-- 
balbi


signature.asc
Description: PGP signature

Re: USB fuzzing with syzbot

[Andrey Konovalov]

On Wed, Apr 24, 2019 at 6:05 PM Andrey Konovalov  wrote:
>
> On Fri, Apr 19, 2019 at 10:35 AM Greg Kroah-Hartman
>  wrote:
> >
> > > 2. Is there an easy way to figure out which config options enable
> > > drivers reachable over USB?
> >
> > Looking for all options that depend on USB is a good start.
> >
> > > Right now our kernel config is based on one of the Debian kernel
> > > configs, that supposedly enables enough relevant USB drivers. At the
> > > same time it enables a lot of other unnecessary stuff, which makes the
> > > kernel big and long to compile. Ideally, we would to have a way to
> > > auto-generate a kernel config that enables all the relevant (enabled
> > > by at least one of the distros) USB drivers. I've looked at whether
> > > it's possible to figure out which particular options in some kernel
> > > config are related to USB, but it seems that neither the option names,
> > > nor the way they are grouped in the config file, are representative
> > > enough.
> >
> > Yeah, it's hard to just carve out this type of configuration, but here's
> > what I have done in the past to try to be sure I enabled all USB drivers
> > in my kernel configuration.
> >
> > First, start with a "minimally working configuration" by running:
> > make localmodconfig
> > on a working system, with the needed modules for booting and operating
> > properly already loaded.
> >
> > That gives you a .config file that should take only minutes to build,
> > compared to much longer for the normal distro configuration (also be
> > sure to disable some debugging options so you don't spend extra time
> > building and stripping symbols).
> >
> > Boot and make sure that configuration works.
> >
> > Then, take that .config and do:
> > - disable USB from the configuration by deleting the:
> > CONFIG_USB_SUPPORT=y
> >   option from your .config
> > - run 'make oldconfig' to disable all USB drivers
> > - turn USB back on by setting CONFIG_USB_SUPPORT=y back on in
> >   your .config
> > - run 'make oldconfig' and answer 'y' or 'm' to all of the
> >   driver options you are presented with.
> >
> > That usually catches almost all of them.  Sometimes you need to make
> > sure you have some other subsystem enabled (like SCSI), but odds are, if
> > you start with a "normally stripped down" configuration that works, you
> > should be fine.
>
> I suspect that make localmodconfig (+ switching CONFIG_USB_SUPPORT off
> and on) would likely include a lot of stuff that we don't need (there
> are many options that are =y, but not related to USB at all), but it
> definitely sounds better than what I have right now (converting almost
> all =m into =y). I'll give it a shot, thanks!

I've tried this and unfortunately it doesn't work as desired. The
reason is that localmodconfig will only enable options for the modules
that are currently loaded, and if a module that some USB driver
depends on is not loaded, then this driver won't be enabled after yes
| make oldconfig. For example my machine didn't have the cfg80211
module loaded, and thus e.g. CONFIG_AT76C50X_USB didn't get enabled
after oldconfig. However when I plug in a wireless USB adapter,
cfg80211 gets loaded together with the USB driver for that adapter. I
guess the same applies to other kinds of dependency modules (e.g.
bluetooth). So this would only work if all the dependency modules are
already loaded.

Re: [PATCH 1/5] usb: dwc2: Move UTMI_PHY_DATA defines closer

[Felipe Balbi]

Jules Maselbas  writes:

> Makes GHWCFG4_UTMI_PHY_DATA* defines closer to their relative shift and
> mask defines to improve readability.
>
> Signed-off-by: Jules Maselbas 

Doesn't apply. Please make sure to rebase on testing/next

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH V2 7/8] usb: gadget: Add UDC driver for tegra XUSB device mode controller

[Felipe Balbi]

Nagarjuna Kristam  writes:

> This patch adds UDC driver for tegra XUSB 3.0 device mode controller.
> XUSB device mode controller support SS, HS and FS modes
>
> Based on work by:
>   Mark Kuo 
>   Andrew Bresticker 
>
> Signed-off-by: Nagarjuna Kristam 
> ---
>  drivers/usb/gadget/udc/Kconfig  |   10 +
>  drivers/usb/gadget/udc/Makefile |1 +
>  drivers/usb/gadget/udc/tegra_xudc.c | 3702 
> +++
>  3 files changed, 3713 insertions(+)
>  create mode 100644 drivers/usb/gadget/udc/tegra_xudc.c
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 0a16cbd..f6f469c 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -439,6 +439,16 @@ config USB_GADGET_XILINX
> dynamically linked module called "udc-xilinx" and force all
> gadget drivers to also be dynamically linked.
>  
> +config USB_TEGRA_XUDC
> + tristate "NVIDIA Superspeed USB 3.0 Device Controller"
> + depends on ARCH_TEGRA

no compile_test?

-- 
balbi


signature.asc
Description: PGP signature

[balbi-usb:testing/next 19/42] drivers/usb/dwc2/core_intr.c:448:25: error: 'struct dwc2_hsotg' has no member named 'phy_reset_work'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
testing/next
head:   2e3cfcbbb1403feeac5949f32dcd2724de3ff40c
commit: a0a493835f9a2e4c690c41145def88223997db2b [19/42] usb: dwc2: optionally 
assert phy reset when waking up
config: x86_64-randconfig-x000-201916 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout a0a493835f9a2e4c690c41145def88223997db2b
# save the attached .config to linux build tree
make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>):

   drivers/usb/dwc2/core_intr.c: In function 'dwc2_handle_wakeup_detected_intr':
>> drivers/usb/dwc2/core_intr.c:448:25: error: 'struct dwc2_hsotg' has no 
>> member named 'phy_reset_work'
schedule_work(&hsotg->phy_reset_work);
^~

vim +448 drivers/usb/dwc2/core_intr.c

   388  
   389  /*
   390   * This interrupt indicates that the DWC_otg controller has detected a
   391   * resume or remote wakeup sequence. If the DWC_otg controller is in
   392   * low power mode, the handler must brings the controller out of low
   393   * power mode. The controller automatically begins resume signaling.
   394   * The handler schedules a time to stop resume signaling.
   395   */
   396  static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
   397  {
   398  int ret;
   399  
   400  /* Clear interrupt */
   401  dwc2_writel(hsotg, GINTSTS_WKUPINT, GINTSTS);
   402  
   403  dev_dbg(hsotg->dev, "++Resume or Remote Wakeup Detected 
Interrupt++\n");
   404  dev_dbg(hsotg->dev, "%s lxstate = %d\n", __func__, 
hsotg->lx_state);
   405  
   406  if (hsotg->lx_state == DWC2_L1) {
   407  dwc2_wakeup_from_lpm_l1(hsotg);
   408  return;
   409  }
   410  
   411  if (dwc2_is_device_mode(hsotg)) {
   412  dev_dbg(hsotg->dev, "DSTS=0x%0x\n",
   413  dwc2_readl(hsotg, DSTS));
   414  if (hsotg->lx_state == DWC2_L2) {
   415  u32 dctl = dwc2_readl(hsotg, DCTL);
   416  
   417  /* Clear Remote Wakeup Signaling */
   418  dctl &= ~DCTL_RMTWKUPSIG;
   419  dwc2_writel(hsotg, dctl, DCTL);
   420  ret = dwc2_exit_partial_power_down(hsotg, true);
   421  if (ret && (ret != -ENOTSUPP))
   422  dev_err(hsotg->dev, "exit power_down 
failed\n");
   423  
   424  call_gadget(hsotg, resume);
   425  }
   426  /* Change to L0 state */
   427  hsotg->lx_state = DWC2_L0;
   428  } else {
   429  if (hsotg->params.power_down)
   430  return;
   431  
   432  if (hsotg->lx_state != DWC2_L1) {
   433  u32 pcgcctl = dwc2_readl(hsotg, PCGCTL);
   434  
   435  /* Restart the Phy Clock */
   436  pcgcctl &= ~PCGCTL_STOPPCLK;
   437  dwc2_writel(hsotg, pcgcctl, PCGCTL);
   438  
   439  /*
   440   * If we've got this quirk then the PHY is 
stuck upon
   441   * wakeup.  Assert reset.  This will propagate 
out and
   442   * eventually we'll re-enumerate the device.  
Not great
   443   * but the best we can do.  We can't call 
phy_reset()
   444   * at interrupt time but there's no hurry, so 
we'll
   445   * schedule it for later.
   446   */
   447  if (hsotg->reset_phy_on_wake)
 > 448  schedule_work(&hsotg->phy_reset_work);
   449  
   450  mod_timer(&hsotg->wkp_timer,
   451jiffies + msecs_to_jiffies(71));
   452  } else {
   453  /* Change to L0 state */
   454  hsotg->lx_state = DWC2_L0;
   455  }
   456  }
   457  }
   458  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Command ring abort xhci_handshake question

[Bollinger, Seth]

Hello All,

Recently I’ve been troubleshooting a problem with the host controller dying and 
noticed that when the controller dies, the system was unresponsive for a _long_ 
period of time.  When I explored the code a bit, I saw that xhci_handshake will 
spin with interrupts disabled for up to 5 seconds.  I also saw places where it 
would spin for up to 10 seconds (I have not experienced this though…).  5 
seconds with interrupts disabled seems like an eternity.

Is there a reason that it’s done this way?  It looks like the code was 
inherited from ehci, so the code has been used for a long period of time.

Would it be possible to wait for the command ring stopped event for 5 seconds, 
then check the CRR bit and halt and die if it’s not cleared?

Thanks!

Seth

Re: [PATCH V2 7/8] usb: gadget: Add UDC driver for tegra XUSB device mode controller

[Thierry Reding]

On Thu, Apr 25, 2019 at 04:00:05PM +0300, Felipe Balbi wrote:
> Nagarjuna Kristam  writes:
> 
> > This patch adds UDC driver for tegra XUSB 3.0 device mode controller.
> > XUSB device mode controller support SS, HS and FS modes
> >
> > Based on work by:
> >   Mark Kuo 
> >   Andrew Bresticker 
> >
> > Signed-off-by: Nagarjuna Kristam 
> > ---
> >  drivers/usb/gadget/udc/Kconfig  |   10 +
> >  drivers/usb/gadget/udc/Makefile |1 +
> >  drivers/usb/gadget/udc/tegra_xudc.c | 3702 
> > +++
> >  3 files changed, 3713 insertions(+)
> >  create mode 100644 drivers/usb/gadget/udc/tegra_xudc.c
> >
> > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > index 0a16cbd..f6f469c 100644
> > --- a/drivers/usb/gadget/udc/Kconfig
> > +++ b/drivers/usb/gadget/udc/Kconfig
> > @@ -439,6 +439,16 @@ config USB_GADGET_XILINX
> >   dynamically linked module called "udc-xilinx" and force all
> >   gadget drivers to also be dynamically linked.
> >  
> > +config USB_TEGRA_XUDC
> > +   tristate "NVIDIA Superspeed USB 3.0 Device Controller"
> > +   depends on ARCH_TEGRA
> 
> no compile_test?

That's not possible right now. The driver depends on functions that
don't have dummy implementations to support COMPILE_TEST. I suppose
that's something that we could change, but does it need to be part
of this initial submission?

On that note:

Nagarjuna, I think we have PHY_TEGRA_XUSB as at least one other
dependency. Without that the driver could be enabled but fail to link
because of the missing implementations that that driver would've
provided.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH V2 4/8] dt-bindings: usb: Add NVIDIA Tegra XUSB device mode controller binding

[Thierry Reding]

On Mon, Mar 11, 2019 at 04:41:52PM +0530, Nagarjuna Kristam wrote:
> Add device-tree binding documentation for the XUSB device mode controller
> present on tegra210 SoC. This controller supports USB 3.0 specification
> 
> Based on work by Andrew Bresticker .
> 
> Signed-off-by: Nagarjuna Kristam 
> ---
>  .../devicetree/bindings/usb/nvidia,tegra-xudc.txt  | 105 
> +
>  1 file changed, 105 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt

Hi Nagarjuna,

when you resend this, make sure to Cc devicet...@vger.kernel.org on this
patch. We need review from one of the device tree bindings maintainers
before this can be applied, and they won't review if they don't receive
the patch. =)

Thierry

> 
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt 
> b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> new file mode 100644
> index 000..990655d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> @@ -0,0 +1,105 @@
> +Device tree binding for NVIDIA Tegra XUSB device mode controller (XUDC)
> +===
> +
> +The Tegra XUDC controller supports both USB 2.0 HighSpeed/FullSpeed and
> +USB 3.0 SuperSpeed protocols.
> +
> +Required properties:
> +
> +- compatible: For Tegra210, must contain "nvidia,tegra210-xudc".
> +- reg: Must contain the base and length of the XUSB device registers, XUSB 
> device
> +  PCI Config registers and XUSB device controller registers.
> +- interrupts: Must contain the XUSB device interrupt
> +- clocks: Must contain an entry for ell clocks used.
> +  See ../clock/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +   - xusb_device
> +   - xusb_ss
> +   - xusb_ss_src
> +   - xusb_hs_src
> +   - xusb_fs_src
> +- nvidia,xusb-padctl: phandle to the XUSB pad controller that is used to
> +  configure the USB pads used by the XUDC controller
> +- power-domains: A list of PM domain specifiers that reference each 
> power-domain
> +  used by the XUSB device mode controller. This list must comprise of a 
> specifier
> +  for the XUSBA and XUSBB power-domains. See ../power/power_domain.txt and
> +  ../arm/tegra/nvidia,tegra20-pmc.txt for details.
> +- power-domain-names: A list of names that represent each of the specifiers 
> in
> +  the 'power-domains' property. Must include 'xusb_ss' and 'xusb_device'
> +
> +For Tegra210:
> +- avddio-usb-supply: PCIe/USB3 analog logic power supply. Must supply 1.05 V.
> +- hvdd-usb-supply: USB controller power supply. Must supply 3.3 V.
> +- avdd-pll-utmip-supply: UTMI PLL power supply. Must supply 1.8 V.
> +
> +- phys: Must contain an entry for each entry in phy-names.
> +  See ../phy/phy-bindings.txt for details.
> +- extcon-usb: Must contains an extcon-usb entry which detects
> +  USB VBUS pin. See ../extcon/extcon-usb-gpio.txt for details.
> +
> +Optional properties:
> +
> +- phy-names: Should include an entry for each PHY used by the controller.
> +  Names must be "usb2", and "usb3" if support SuperSpeed device mode.
> +  - "usb3" phy, SuperSpeed (SSTX+/SSTX-/SSRX+/SSRX-) data lines
> +  - "usb2" phy, USB 2.0 (D+/D-) data lines
> +
> +Example:
> +
> + pmc: pmc@7000e400 {
> + compatible = "nvidia,tegra210-pmc";
> + reg = <0x0 0x7000e400 0x0 0x400>;
> + clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
> + clock-names = "pclk", "clk32k_in";
> +
> + powergates {
> + pd_xusbss: xusba {
> + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>;
> + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>;
> + #power-domain-cells = <0>;
> + };
> +
> + pd_xusbdev: xusbb {
> + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>;
> + resets = <&tegra_car 95>;
> + #power-domain-cells = <0>;
> + };
> + };
> + };
> +
> + xudc@700d {
> + compatible = "nvidia,tegra210-xudc";
> + reg = <0x0 0x700d 0x0 0x8000>,
> + <0x0 0x700d8000 0x0 0x1000>,
> + <0x0 0x700d9000 0x0 0x1000>;
> +
> + interrupts = <0 44 0x4>;
> +
> + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>,
> + <&tegra_car TEGRA210_CLK_XUSB_SS>,
> + <&tegra_car TEGRA210_CLK_XUSB_SSP_SRC>,
> + <&tegra_car TEGRA210_CLK_XUSB_HS_SRC>,
> + <&tegra_car TEGRA210_CLK_XUSB_FS_SRC>;
> + clock-names = "xusb_device", "xusb_ss", "xusb_ss_src",
> +   "xusb_hs_src", "xusb_fs_src";
> +
> + power-domains = <&pd_xusbdev>, <&pd_xusbss>;
> +  

Re: [PATCH v1 00/14] usb: dwc2: Fix and improve power saving modes.

[Artur Petrosyan]

Hi,

On 4/25/2019 16:43, Felipe Balbi wrote:
> Artur Petrosyan  writes:
>> This patch set, fixes and improves partial power down and hibernation power
>> saving modes. Also, adds support for entering/exiting hibernation by
>> system issued suspend/resume.
>>
>> This series contains patches which were submitted to LKML. However, a part
>> of those patches didn't reach to LKML because of local issue related to
>> smtp server.
>>
>> The patches which reached to LKML are:
>>
>> - usb: dwc2: Add part. power down exit from dwc2_conn_id_status_change().
>> - usb: dwc2: Add port conn. sts. checking in _dwc2_hcd_resume() function.
>> - usb: dwc2: Fix suspend state in host mode for partial power down.
>> - usb: dwc2: Fix wakeup detected and session request interrupt handlers.
>> - usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
>> - usb: dwc2: Fix dwc2_restore_device_registers() function.
>>
>> The patches which didn't reach to LKML are:
>>
>> - usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
>> - usb: dwc2: Clear GINTSTS_RESTOREDONE bit after restore is generated.
>> - usb: dwc2: Clear fifo_map when resetting core.
>> - usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
>> - usb: dwc2: Fix hibernation between host and device modes.
>> - usb: dwc2: Update dwc2_handle_usb_suspend_intr function.
>> - usb: dwc2: Add default param to control power optimization.
>> - usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
>>
>> Submitting all of the patches together in this version.
>>
>> Changes from V0:
>>   - Replaced 1 with DWC2_POWER_DOWN_PARAM_PARTIAL in commit
>> "9eed02b9fe96 usb: dwc2: Fix wakeup detected and session request
>> interrupt handlers.
>>
>>
>> Artur Petrosyan (14):
>>usb: dwc2: Fix dwc2_restore_device_registers() function.
>>usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
>>usb: dwc2: Fix wakeup detected and session request interrupt handlers.
>>usb: dwc2: Fix suspend state in host mode for partial power down.
>>usb: dwc2: Add port conn. sts. checking in _dwc2_hcd_resume()
>>  function.
>>usb: dwc2: Add part. power down exit from
>>  dwc2_conn_id_status_change().
>>usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
>>usb: dwc2: Add default param to control power optimization.
>>usb: dwc2: Update dwc2_handle_usb_suspend_intr function.
>>usb: dwc2: Fix hibernation between host and device modes.
>>usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
>>usb: dwc2: Clear fifo_map when resetting core.
>>usb: dwc2: Clear GINTSTS_RESTOREDONE bit after restore is generated.
>>usb: dwc2: Add enter/exit hibernation from system issued
>>  suspend/resume
> 
> patches don't apply.
> 

Do we need to wait for Minas's acknowledge or there is problem related 
to the patches?

-- 
Regards,
Artur

Re: [PATCH V2 1/8] phy: tegra: xusb: t210: add XUSB dual mode support

[Thierry Reding]

On Mon, Mar 11, 2019 at 04:41:49PM +0530, Nagarjuna Kristam wrote:
> The device tree bindings document the "mode" property of "ports"
> subnodes, but the driver was not parsing the property. In preparation
> for adding role switching, parse the property at probe time and
> confgiure the port capabilities accordingly
> 
> Based on work by JC Kuo .
> 
> Signed-off-by: Nagarjuna Kristam 
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 22 +++---
>  drivers/phy/tegra/xusb.c  | 24 +++-
>  drivers/phy/tegra/xusb.h  |  4 +++-
>  3 files changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c 
> b/drivers/phy/tegra/xusb-tegra210.c
> index 05bee32..4beebcc 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2019, NVIDIA CORPORATION.  All rights reserved.
>   * Copyright (C) 2015 Google, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -47,7 +47,10 @@
>  #define XUSB_PADCTL_USB2_PAD_MUX_USB2_BIAS_PAD_XUSB 0x1
>  
>  #define XUSB_PADCTL_USB2_PORT_CAP 0x008
> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(x) (0x0 << ((x) * 4))
>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(x) (0x1 << ((x) * 4))
> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(x) (0x2 << ((x) * 4))
> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(x) (0x3 << ((x) * 4))
>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(x) (0x3 << ((x) * 4))
>  
>  #define XUSB_PADCTL_SS_PORT_MAP 0x014
> @@ -72,6 +75,7 @@
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(x) (0x084 + (x) * 0x40)
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT 7
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK 0x3
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL 0x1
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18 (1 << 6)
>  
>  #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x) (0x088 + (x) * 0x40)
> @@ -965,7 +969,14 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>  
>   value = padctl_readl(padctl, XUSB_PADCTL_USB2_PORT_CAP);
>   value &= ~XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(index);
> - value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
> + if (port->mode == USB_DR_MODE_UNKNOWN)
> + value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(index);
> + else if (port->mode == USB_DR_MODE_PERIPHERAL)
> + value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(index);
> + else if (port->mode == USB_DR_MODE_HOST)
> + value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
> + else if (port->mode == USB_DR_MODE_OTG)
> + value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(index);
>   padctl_writel(padctl, value, XUSB_PADCTL_USB2_PORT_CAP);
>  
>   value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> @@ -997,7 +1008,12 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>   value &= ~(XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK <<
>  XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT);
> - value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
> + if (port->mode == USB_DR_MODE_HOST)
> + value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
> + else
> + value |=
> +   XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL <<
> +   XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT;
>   padctl_writel(padctl, value,
> XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>  
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 5b3b886..c6178a0 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2019, NVIDIA CORPORATION.  All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -542,13 +542,35 @@ static void tegra_xusb_port_unregister(struct 
> tegra_xusb_port *port)
>   device_unregister(&port->dev);
>  }
>  
> +static const char *const modes[] = {
> + [USB_DR_MODE_UNKNOWN] = "",
> + [USB_DR_MODE_HOST] = "host",
> + [USB_DR_MODE_PERIPHERAL] = "peripheral",
> + [USB_DR_MODE_OTG] = "otg",
> +};
> +
>  static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
>  {
>   struct tegra_xusb_port *port = &usb2->base;
>   struct device_node *np = port->dev.of_node;
> + const char *mode;
>  
>   usb2->internal = of_property_read_bool(np, "nvidia,internal");

Re: Command ring abort xhci_handshake question

[Alan Stern]

On Thu, 25 Apr 2019, Bollinger, Seth wrote:

> Hello All,
> 
> Recently I’ve been troubleshooting a problem with the host
> controller dying and noticed that when the controller dies, the
> system was unresponsive for a _long_ period of time.  When I explored
> the code a bit, I saw that xhci_handshake will spin with interrupts
> disabled for up to 5 seconds.  I also saw places where it would spin
> for up to 10 seconds (I have not experienced this though…).  5
> seconds with interrupts disabled seems like an eternity.

Are you certain that those places with the very long timeouts are 
called with interrupts disabled?

> Is there a reason that it’s done this way?  It looks like the code
> was inherited from ehci, so the code has been used for a long period
> of time.

In ehci-hcd, the long timeouts occur only with interrupts enabled.

Alan Stern

> Would it be possible to wait for the command ring stopped event for 5
> seconds, then check the CRR bit and halt and die if it’s not
> cleared?
> 
> Thanks!
> 
> Seth

Re: USB fuzzing with syzbot

[Greg Kroah-Hartman]

On Thu, Apr 25, 2019 at 02:44:11PM +0200, Andrey Konovalov wrote:
> On Wed, Apr 24, 2019 at 6:05 PM Andrey Konovalov  
> wrote:
> >
> > On Fri, Apr 19, 2019 at 10:35 AM Greg Kroah-Hartman
> >  wrote:
> > >
> > > > 2. Is there an easy way to figure out which config options enable
> > > > drivers reachable over USB?
> > >
> > > Looking for all options that depend on USB is a good start.
> > >
> > > > Right now our kernel config is based on one of the Debian kernel
> > > > configs, that supposedly enables enough relevant USB drivers. At the
> > > > same time it enables a lot of other unnecessary stuff, which makes the
> > > > kernel big and long to compile. Ideally, we would to have a way to
> > > > auto-generate a kernel config that enables all the relevant (enabled
> > > > by at least one of the distros) USB drivers. I've looked at whether
> > > > it's possible to figure out which particular options in some kernel
> > > > config are related to USB, but it seems that neither the option names,
> > > > nor the way they are grouped in the config file, are representative
> > > > enough.
> > >
> > > Yeah, it's hard to just carve out this type of configuration, but here's
> > > what I have done in the past to try to be sure I enabled all USB drivers
> > > in my kernel configuration.
> > >
> > > First, start with a "minimally working configuration" by running:
> > > make localmodconfig
> > > on a working system, with the needed modules for booting and operating
> > > properly already loaded.
> > >
> > > That gives you a .config file that should take only minutes to build,
> > > compared to much longer for the normal distro configuration (also be
> > > sure to disable some debugging options so you don't spend extra time
> > > building and stripping symbols).
> > >
> > > Boot and make sure that configuration works.
> > >
> > > Then, take that .config and do:
> > > - disable USB from the configuration by deleting the:
> > > CONFIG_USB_SUPPORT=y
> > >   option from your .config
> > > - run 'make oldconfig' to disable all USB drivers
> > > - turn USB back on by setting CONFIG_USB_SUPPORT=y back on in
> > >   your .config
> > > - run 'make oldconfig' and answer 'y' or 'm' to all of the
> > >   driver options you are presented with.
> > >
> > > That usually catches almost all of them.  Sometimes you need to make
> > > sure you have some other subsystem enabled (like SCSI), but odds are, if
> > > you start with a "normally stripped down" configuration that works, you
> > > should be fine.
> >
> > I suspect that make localmodconfig (+ switching CONFIG_USB_SUPPORT off
> > and on) would likely include a lot of stuff that we don't need (there
> > are many options that are =y, but not related to USB at all), but it
> > definitely sounds better than what I have right now (converting almost
> > all =m into =y). I'll give it a shot, thanks!
> 
> I've tried this and unfortunately it doesn't work as desired. The
> reason is that localmodconfig will only enable options for the modules
> that are currently loaded, and if a module that some USB driver
> depends on is not loaded, then this driver won't be enabled after yes
> | make oldconfig. For example my machine didn't have the cfg80211
> module loaded, and thus e.g. CONFIG_AT76C50X_USB didn't get enabled
> after oldconfig. However when I plug in a wireless USB adapter,
> cfg80211 gets loaded together with the USB driver for that adapter. I
> guess the same applies to other kinds of dependency modules (e.g.
> bluetooth). So this would only work if all the dependency modules are
> already loaded.

Yes, sorry, I thought I said that with:

> > > First, start with a "minimally working configuration" by running:
> > > make localmodconfig
> > > on a working system, with the needed modules for booting and operating
> > > properly already loaded.

I guess "working system" implied everything that you _knew_ you wanted
to have loaded :)

Sorry about the dependancy mess, hopefully you have sorted this out
better now.

thanks,

greg k-h

Re: Command ring abort xhci_handshake question

[Bollinger, Seth]

> On Apr 25, 2019, at 9:22 AM, Alan Stern  wrote:
>
> Are you certain that those places with the very long timeouts are 
> called with interrupts disabled?

I must admit, I have not reviewed all uses of the long xhci_handshakes.  :)  I 
have reviewed the following instance.

In usb/host/xhci-ring.c:xhci_abort_cmd_ring():

ret = xhci_handshake(&xhci->op_regs->cmd_ring,
CMD_RING_RUNNING, 0, 5 * 1000 * 1000);
if (ret < 0) {
xhci_err(xhci, "Abort failed to stop command ring: %d\n", ret);
xhci_halt(xhci);
xhci_hc_died(xhci);
return ret;
}
/*
 * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
 * however on some host hw the CMD_RING_RUNNING bit is correctly cleared
 * but the completion event in never sent. Wait 2 secs (arbitrary
 * number) to handle those cases after negation of CMD_RING_RUNNING.
 */
spin_unlock_irqrestore(&xhci->lock, flags);

The spin_unlock occurs before the completion wait, so I assume that interrupts 
are disabled in the spin above.  Also, the system does nothing during this 
period.  No SSH, no serial, no log messages, etc.

Seth

Re: [PATCH V2 2/8] phy: tegra: xusb: t210: add usb3 port fake support

[Thierry Reding]

On Mon, Mar 11, 2019 at 04:41:50PM +0530, Nagarjuna Kristam wrote:
> On Tegra210, usb2 only otg/peripheral ports dont work in device mode.
> They need an assosciated usb3 port to work in device mode. Identify
> an unused usb3 port and assign it as a fake USB3 port to USB2 only
> port whose mode is otg/peripheral
> 
> Based on work by BH Hsieh .
> 
> Signed-off-by: Nagarjuna Kristam 
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 40 
> +++
>  drivers/phy/tegra/xusb.c  | 29 
>  drivers/phy/tegra/xusb.h  |  6 ++
>  3 files changed, 75 insertions(+)

I think this looks a whole lot better than the initial version. One or
two minor comments below.

> diff --git a/drivers/phy/tegra/xusb-tegra210.c 
> b/drivers/phy/tegra/xusb-tegra210.c
> index 4beebcc..48478bc4 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -58,6 +58,7 @@
>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_SHIFT(x) ((x) * 5)
>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(x) (0x7 << ((x) * 5))
>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(x, v) (((v) & 0x7) << ((x) * 5))
> +#define XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED 0x7
>  
>  #define XUSB_PADCTL_ELPG_PROGRAM1 0x024
>  #define XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN (1 << 31)
> @@ -952,6 +953,15 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>  
>   priv = to_tegra210_xusb_padctl(padctl);
>  
> + if (port->usb3_port_fake != -1) {
> + value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> + value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> + port->usb3_port_fake);
> + value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(
> + port->usb3_port_fake, index);
> + padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> + }
> +
>   value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>   value &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
>   XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
> @@ -1086,6 +1096,15 @@ static int tegra210_usb2_phy_power_off(struct phy *phy)
>  
>   mutex_lock(&padctl->lock);
>  
> + if (port->usb3_port_fake != -1) {
> + value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> + value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> + port->usb3_port_fake);
> + value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(port->usb3_port_fake,
> + XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED);
> + padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> + }
> +
>   if (WARN_ON(pad->enable == 0))
>   goto out;
>  
> @@ -1784,6 +1803,27 @@ static const struct tegra_xusb_pad_soc * const 
> tegra210_pads[] = {
>  
>  static int tegra210_usb2_port_enable(struct tegra_xusb_port *port)
>  {
> + struct tegra_xusb_usb2_port *usb2 = to_usb2_port(port);
> + struct tegra_xusb_padctl *padctl = port->padctl;
> +
> + /* Disable usb3_port_fake usage by default and assign if needed */
> + usb2->usb3_port_fake = -1;
> + if (usb2 && (usb2->mode == USB_DR_MODE_OTG ||
> + usb2->mode == USB_DR_MODE_PERIPHERAL)) {
> + if (!tegra_xusb_usb3_port_has_companion(padctl, port->index)) {
> + int fake = tegra_xusb_find_unused_usb3_port(padctl);
> +
> + if (fake >= 0) {
> + dev_dbg(&port->dev, "Found unused usb3 port: 
> %d\n",
> +  fake);
> + usb2->usb3_port_fake = fake;
> + } else {
> + dev_err(&port->dev, "usb2-%u has neither a 
> companion nor an unused usb3 port to fake it\n",
> + port->index);
> + return -ENODEV;
> + }
> + }
> + }

Generally it's preferable to check for errors and deal with success in
the regular code path. So the above would be:

if (fake < 0) {
dev_err(...);
return -ENODEV;
}

dev_dbg(...);
...

Also, the error message is slightly redundant because &port->dev will
already output the usb2-%u part. Also, perhaps try to make that message
a little shorter. Something like:

dev_err(&port->dev, "no unused USB3 ports available\n");

Also, I don't think this is the right place to do this. There's really
no way for the ->enable() callbacks to fail. Or at least failures will
be ignored, so there's no chance for the driver to do anything about the
failure. It also looks somewhat tacked on.

But how about if we do this as part of tegra_xusb_usb3_port_parse_dt()
or t

Re: [PATCH] xhci: update bounce buffer with correct sg num

[Mathias Nyman]

On 25.4.2019 13.43, Henry Lin wrote:

This change fixes a data corruption issue occurred on USB hard disk for
the case that bounce buffer is used during transferring data.

While updating data between sg list and bounce buffer, current
implementation passes mapped sg number (urb->num_mapped_sgs) to
sg_pcopy_from_buffer() and sg_pcopy_to_buffer(). This causes data
not get copied if target buffer is located in the elements after
mapped sg elements. This change passes sg number for full list to
fix issue.


Nice catch, thanks.



Besides, for copying data from bounce buffer, calling dma_unmap_single()
on the bounce buffer before copying data to sg list can avoid cache issue.


Sure, no need to keep it mapped anymore at this stage.

I'll add this to the queue

-Mathias
 

Re: [balbi-usb:testing/next 19/42] drivers/usb/dwc2/core_intr.c:448:25: error: 'struct dwc2_hsotg' has no member named 'phy_reset_work'

[Doug Anderson]

Hi,

On Thu, Apr 25, 2019 at 6:14 AM kbuild test robot  wrote:
>
>439  /*
>440   * If we've got this quirk then the PHY is 
> stuck upon
>441   * wakeup.  Assert reset.  This will 
> propagate out and
>442   * eventually we'll re-enumerate the device.  
> Not great
>443   * but the best we can do.  We can't call 
> phy_reset()
>444   * at interrupt time but there's no hurry, so 
> we'll
>445   * schedule it for later.
>446   */
>447  if (hsotg->reset_phy_on_wake)
>  > 448  schedule_work(&hsotg->phy_reset_work);

Doh!  I didn't notice that the part of the structure I put this in has:

#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)

I'll whip something up real quick that should fix it.  Sorry about that!

-Doug

Re: [PATCH V2 3/8] phy: tegra: xusb: t210: add vbus override support

[Thierry Reding]

On Mon, Mar 11, 2019 at 04:41:51PM +0530, Nagarjuna Kristam wrote:
> Tegra XUSB device control driver needs to control vbus override
> during its operations, add API for the support
> 
> Signed-off-by: Nagarjuna Kristam 
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 61 
> +++
>  drivers/phy/tegra/xusb.c  | 28 --
>  drivers/phy/tegra/xusb.h  |  2 ++
>  include/linux/phy/tegra/xusb.h|  6 ++--
>  4 files changed, 92 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c 
> b/drivers/phy/tegra/xusb-tegra210.c
> index 48478bc4..be1a870 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -73,6 +73,10 @@
>  #define XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(x) (1 << (1 + (x)))
>  #define XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(x) (1 << (8 + (x)))
>  
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(x) (0x080 + (x) * 0x40)
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP (1 << 18)
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN (1 << 22)
> +
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(x) (0x084 + (x) * 0x40)
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT 7
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK 0x3
> @@ -235,6 +239,12 @@
>  #define XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(x) (0xa74 + (x) * 0x40)
>  #define XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL 0xfcf01368
>  
> +#define XUSB_PADCTL_USB2_VBUS_ID 0xc60
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON (1 << 14)
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT 18
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK 0xf
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_FLOATING 8
> +
>  struct tegra210_xusb_fuse_calibration {
>   u32 hs_curr_level[4];
>   u32 hs_term_range_adj;
> @@ -2009,6 +2019,55 @@ static const struct tegra_xusb_port_ops 
> tegra210_usb3_port_ops = {
>   .map = tegra210_usb3_port_map,
>  };
>  
> +static int tegra210_xusb_padctl_vbus_override(struct tegra_xusb_padctl 
> *padctl,
> +   bool set)

I think "status" would perhaps be somewhat more meaningful.

> +{
> + u32 reg;

The rest of the driver uses "u32 value". It'd be good to be consistent.

> +
> + dev_dbg(padctl->dev, "%s vbus override\n", set ? "set" : "clear");
> +
> + reg = padctl_readl(padctl, XUSB_PADCTL_USB2_VBUS_ID);
> + if (set) {
> + reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
> + reg &= ~(XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK <<
> +XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT);
> + reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_FLOATING <<
> +  XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT;
> + } else
> + reg &= ~XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;

This could use some blank lines to separate blocks and make it more
readable.

> + padctl_writel(padctl, reg, XUSB_PADCTL_USB2_VBUS_ID);
> +
> + return 0;
> +}
> +
> +static int tegra210_utmi_port_reset(struct phy *phy)
> +{
> + struct tegra_xusb_padctl *padctl;
> + struct tegra_xusb_lane *lane;
> + struct device *dev;
> + u32 reg;

u32 value

> +
> + if (!phy)
> + return -ENODEV;

When would this happen?

> +
> + lane = phy_get_drvdata(phy);
> + padctl = lane->pad->padctl;
> + dev = padctl->dev;
> +
> + reg = padctl_readl(padctl,
> + XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(0));

Usually subsequent lines are indented so that they align with the first
argument of the first line.

> + dev_dbg(dev, "BATTERY_CHRG_OTGPADX_CTL0(0): 0x%x\n", reg);

You can use %#x to avoid having to explicitly provide the 0x prefix.
Also, is this really useful for debugging? We could add trace support to
this driver (to padctl_readl() and padctl_writel() for example) to allow
for more flexible tracing of register programming sequences.

> +
> + if ((reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP) ||
> + (reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN)) {
> + dev_dbg(dev, "Toggle vbus\n");

This one is pretty redundant because the function calls below each
already output something to that effect.

> + tegra210_xusb_padctl_vbus_override(padctl, false);
> + tegra210_xusb_padctl_vbus_override(padctl, true);
> + return 1;
> + }
> + return 0;
> +}
> +
>  static int
>  tegra210_xusb_read_fuse_calibration(struct tegra210_xusb_fuse_calibration 
> *fuse)
>  {
> @@ -2071,6 +2130,8 @@ static const struct tegra_xusb_padctl_ops 
> tegra210_xusb_padctl_ops = {
>   .remove = tegra210_xusb_padctl_remove,
>   .usb3_set_lfps_detect = tegra210_usb3_set_lfps_detect,
>   .hsic_set_idle = tegra210_hsic_set_idle,
> + .vbus_override = tegra210_xusb_padctl_vbus_override,
> + .utmi_port_reset = tegra210_utmi_port_reset,
> 

Re: [PATCH V2 4/8] dt-bindings: usb: Add NVIDIA Tegra XUSB device mode controller binding

[Thierry Reding]

On Mon, Mar 11, 2019 at 04:41:52PM +0530, Nagarjuna Kristam wrote:
> Add device-tree binding documentation for the XUSB device mode controller
> present on tegra210 SoC. This controller supports USB 3.0 specification

Tegra210, please. "... supports the USB 3.0 ...". Also end sentences
with a fullstop.

> 
> Based on work by Andrew Bresticker .
> 
> Signed-off-by: Nagarjuna Kristam 
> ---
>  .../devicetree/bindings/usb/nvidia,tegra-xudc.txt  | 105 
> +
>  1 file changed, 105 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt 
> b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> new file mode 100644
> index 000..990655d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> @@ -0,0 +1,105 @@
> +Device tree binding for NVIDIA Tegra XUSB device mode controller (XUDC)
> +===
> +
> +The Tegra XUDC controller supports both USB 2.0 HighSpeed/FullSpeed and
> +USB 3.0 SuperSpeed protocols.
> +
> +Required properties:
> +
> +- compatible: For Tegra210, must contain "nvidia,tegra210-xudc".
> +- reg: Must contain the base and length of the XUSB device registers, XUSB 
> device
> +  PCI Config registers and XUSB device controller registers.
> +- interrupts: Must contain the XUSB device interrupt
> +- clocks: Must contain an entry for ell clocks used.

s/ell/all/

> +  See ../clock/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +   - xusb_device
> +   - xusb_ss
> +   - xusb_ss_src
> +   - xusb_hs_src
> +   - xusb_fs_src

It'd be good to explain what each of these are.

> +- nvidia,xusb-padctl: phandle to the XUSB pad controller that is used to
> +  configure the USB pads used by the XUDC controller
> +- power-domains: A list of PM domain specifiers that reference each 
> power-domain
> +  used by the XUSB device mode controller. This list must comprise of a 
> specifier
> +  for the XUSBA and XUSBB power-domains. See ../power/power_domain.txt and
> +  ../arm/tegra/nvidia,tegra20-pmc.txt for details.
> +- power-domain-names: A list of names that represent each of the specifiers 
> in
> +  the 'power-domains' property. Must include 'xusb_ss' and 'xusb_device'
> +
> +For Tegra210:
> +- avddio-usb-supply: PCIe/USB3 analog logic power supply. Must supply 1.05 V.
> +- hvdd-usb-supply: USB controller power supply. Must supply 3.3 V.
> +- avdd-pll-utmip-supply: UTMI PLL power supply. Must supply 1.8 V.

My understanding is that this last supply is really needed for the XUSB
pad controller to bring up the PLL. In fact, I've just moved the same
supply to the XUSB pad controller from the XUSB controller for all of
the supported boards because having this in the XUSB controller would
fail under some circumstances.

> +
> +- phys: Must contain an entry for each entry in phy-names.
> +  See ../phy/phy-bindings.txt for details.
> +- extcon-usb: Must contains an extcon-usb entry which detects

In the example below, this is simply "extcon".

> +  USB VBUS pin. See ../extcon/extcon-usb-gpio.txt for details.
> +
> +Optional properties:
> +
> +- phy-names: Should include an entry for each PHY used by the controller.
> +  Names must be "usb2", and "usb3" if support SuperSpeed device mode.
> +  - "usb3" phy, SuperSpeed (SSTX+/SSTX-/SSRX+/SSRX-) data lines
> +  - "usb2" phy, USB 2.0 (D+/D-) data lines

Why are these optional? phys is required and references phy-names
explicitly, so I think that effectively makes these phy-names required
as well.

> +
> +Example:
> +
> + pmc: pmc@7000e400 {
> + compatible = "nvidia,tegra210-pmc";
> + reg = <0x0 0x7000e400 0x0 0x400>;
> + clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
> + clock-names = "pclk", "clk32k_in";
> +
> + powergates {
> + pd_xusbss: xusba {
> + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>;
> + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>;
> + #power-domain-cells = <0>;
> + };
> +
> + pd_xusbdev: xusbb {
> + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>;
> + resets = <&tegra_car 95>;
> + #power-domain-cells = <0>;
> + };
> + };
> + };
> +
> + xudc@700d {
> + compatible = "nvidia,tegra210-xudc";
> + reg = <0x0 0x700d 0x0 0x8000>,
> + <0x0 0x700d8000 0x0 0x1000>,
> + <0x0 0x700d9000 0x0 0x1000>;
> +
> + interrupts = <0 44 0x4>;

This should use symbolic names defined in the following includes:

dt-bindings/interrupt-controller

[PATCH] usb: xhci: avoid null pointer deref when bos field is NULL

[Schmid, Carsten]

Hi,

some weeks ago, when stabilizing a production kernel,
i have fixed a bug in the xhci driver.
Mathias (Nyman) was involved checking the patch, and he wrote:
..
Subject: Re: help needed - kernel crash in USB driver

Hi Zainie

I can look at a upstream fix for usb core,
but we will have to wait for the 5.1 kernel two week merge window
to finish before we can send anything upstream.

While waiting for that the customer proposal will work,
it will avoid this one particular null pointer dereference case.

Their patch also otherwise makes sense.
Even if it doesn't fix the root cause there is no need for the
xhci driver to touch udev->bos when disabling usb2 lpm.
We should encourage them to send it upstream to get it as a
generic driver improvement as well.

Thanks
Mathias


I now had the time to port the patch onto 5.1 kernel tree, and can provide the 
patch.
It's the first time i give a patch for upstream approval.
Tried to follow the rules, and used checkpatch.pl as written.
Please find the result below.

Best regards
Carsten


>From b497444e3632ff4a0aa88d7cb84ce2a75f55295a Mon Sep 17 00:00:00 2001
From: Carsten Schmid 
Date: Fri, 8 Mar 2019 15:15:52 +0100
Subject: [PATCH] usb: xhci: avoid null pointer deref when bos field is NULL

With defective USB sticks we see the following error happen:
usb 1-3: new high-speed USB device number 6 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: device descriptor read/64, error -71
usb 1-3: new high-speed USB device number 7 using xhci_hcd
usb 1-3: device descriptor read/64, error -71
usb 1-3: unable to get BOS descriptor set
usb 1-3: New USB device found, idVendor=0781, idProduct=5581
usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
...
BUG: unable to handle kernel NULL pointer dereference at 0008

This comes from the following place:
[ 1660.215380] IP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.222092] PGD 0 P4D 0
[ 1660.224918] Oops:  [#1] PREEMPT SMP NOPTI
[ 1660.425520] CPU: 1 PID: 38 Comm: kworker/1:1 Tainted: P U  W  O
4.14.67-apl #1
[ 1660.434277] Workqueue: usb_hub_wq hub_event [usbcore]
[ 1660.439918] task: a295b6ae4c80 task.stack: ad458015
[ 1660.446532] RIP: 0010:xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd]
[ 1660.453821] RSP: 0018:ad4580153c70 EFLAGS: 00010046
[ 1660.459655] RAX:  RBX: a295b4d7c000 RCX: 0002
[ 1660.467625] RDX: 0002 RSI: 984a55b2 RDI: 984a55b2
[ 1660.475586] RBP: ad4580153cc8 R08: 00d6520a R09: 0001
[ 1660.483556] R10: ad4580a004a0 R11: 0286 R12: a295b4d7c000
[ 1660.491525] R13: 00010648 R14: a295a84e1800 R15: 
[ 1660.499494] FS:  () GS:a295bfc8() 
knlGS:
[ 1660.508530] CS:  0010 DS:  ES:  CR0: 80050033
[ 1660.514947] CR2: 0008 CR3: 00025a114000 CR4: 003406a0
[ 1660.522917] Call Trace:
[ 1660.525657]  usb_set_usb2_hardware_lpm+0x3d/0x70 [usbcore]
[ 1660.531792]  usb_disable_device+0x242/0x260 [usbcore]
[ 1660.537439]  usb_disconnect+0xc1/0x2b0 [usbcore]
[ 1660.542600]  hub_event+0x596/0x18f0 [usbcore]
[ 1660.547467]  ? trace_preempt_on+0xdf/0x100
[ 1660.552040]  ? process_one_work+0x1c1/0x410
[ 1660.556708]  process_one_work+0x1d2/0x410
[ 1660.561184]  ? preempt_count_add.part.3+0x21/0x60
[ 1660.566436]  worker_thread+0x2d/0x3f0
[ 1660.570522]  kthread+0x122/0x140
[ 1660.574123]  ? process_one_work+0x410/0x410
[ 1660.578792]  ? kthread_create_on_node+0x60/0x60
[ 1660.583849]  ret_from_fork+0x3a/0x50
[ 1660.587839] Code: 00 49 89 c3 49 8b 84 24 50 16 00 00 8d 4a ff 48 8d 04 c8 
48 89 ca 4c 8b 10 45 8b 6a 04 48 8b 00 48 89 45 c0 49 8b 86 80 03 00 00 <48> 8b 
40 08 8b 40 03 0f 1f 44 00 00 45 85 ff 0f 84 81 01 00 00
[ 1660.608980] RIP: xhci_set_usb2_hardware_lpm+0xdf/0x3d0 [xhci_hcd] RSP: 
ad4580153c70
[ 1660.617921] CR2: 0008

Tracking this down shows that udev->bos is NULL in the following code:
(xhci.c, in xhci_set_usb2_hardware_lpm)
field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);  <<< here

xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
enable ? "enable" : "disable", port_num + 1);

if (enable) {
/* Host supports BESL timeout instead of HIRD */
if (udev->usb2_hw_lpm_besl_capable) {
/* if device doesn't have a preferred BESL value use a
 * default one which works with mixed HIRD and BESL
 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
 */
if ((field & USB_BESL_SUPPORT) &&
(field & USB_BESL_BASELINE_VALID))
hird = USB_GET_BESL_BASELINE(field);
else
hir

Re: [PATCH 1/5] usb: dwc2: Move UTMI_PHY_DATA defines closer

[Jules Maselbas]

Hi Felipe,

On Thu, Apr 25, 2019 at 03:52:16PM +0300, Felipe Balbi wrote:
> Jules Maselbas  writes:
> 
> > Makes GHWCFG4_UTMI_PHY_DATA* defines closer to their relative shift and
> > mask defines to improve readability.
> >
> > Signed-off-by: Jules Maselbas 
> 
> Doesn't apply. Please make sure to rebase on testing/next

I've applied this patch serie on v5.1-rc6 and I didn't saw any problems.
I also found that theses patchs are already included in testing/next.
Did I miss something? 


Regards,
--
Jules

Re: [balbi-usb:testing/next 19/42] drivers/usb/dwc2/core_intr.c:448:25: error: 'struct dwc2_hsotg' has no member named 'phy_reset_work'

[Doug Anderson]

Hi,

On Thu, Apr 25, 2019 at 8:02 AM Doug Anderson  wrote:
>
> Hi,
>
> On Thu, Apr 25, 2019 at 6:14 AM kbuild test robot  wrote:
> >
> >439  /*
> >440   * If we've got this quirk then the PHY is 
> > stuck upon
> >441   * wakeup.  Assert reset.  This will 
> > propagate out and
> >442   * eventually we'll re-enumerate the 
> > device.  Not great
> >443   * but the best we can do.  We can't call 
> > phy_reset()
> >444   * at interrupt time but there's no hurry, 
> > so we'll
> >445   * schedule it for later.
> >446   */
> >447  if (hsotg->reset_phy_on_wake)
> >  > 448  
> > schedule_work(&hsotg->phy_reset_work);
>
> Doh!  I didn't notice that the part of the structure I put this in has:
>
> #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
>
> I'll whip something up real quick that should fix it.  Sorry about that!

https://lkml.kernel.org/r/20190425154021.4465-1-diand...@chromium.org

-Doug

[PATCH 5/5] USB: cdc-acm: clean up throttle handling

[Johan Hovold]

Clean up the throttle implementation by dropping the redundant
throttle_req flag which was a remnant from back when USB serial had only
a single read URB, something which was later carried over to cdc-acm.

Also convert the throttled flag to an atomic bit flag.

Signed-off-by: Johan Hovold 
---
 drivers/usb/class/cdc-acm.c | 33 -
 drivers/usb/class/cdc-acm.h |  3 +--
 2 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index c03aa8550980..183b41753c98 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -468,7 +468,6 @@ static void acm_read_bulk_callback(struct urb *urb)
 {
struct acm_rb *rb = urb->context;
struct acm *acm = rb->instance;
-   unsigned long flags;
int status = urb->status;
bool stopped = false;
bool stalled = false;
@@ -525,15 +524,10 @@ static void acm_read_bulk_callback(struct urb *urb)
return;
}
 
-   /* throttle device if requested by tty */
-   spin_lock_irqsave(&acm->read_lock, flags);
-   acm->throttled = acm->throttle_req;
-   if (!acm->throttled) {
-   spin_unlock_irqrestore(&acm->read_lock, flags);
-   acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
-   } else {
-   spin_unlock_irqrestore(&acm->read_lock, flags);
-   }
+   if (test_bit(ACM_THROTTLED, &acm->flags))
+   return;
+
+   acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
 }
 
 /* data interface wrote those outgoing bytes */
@@ -670,10 +664,7 @@ static int acm_port_activate(struct tty_port *port, struct 
tty_struct *tty)
/*
 * Unthrottle device in case the TTY was closed while throttled.
 */
-   spin_lock_irq(&acm->read_lock);
-   acm->throttled = 0;
-   acm->throttle_req = 0;
-   spin_unlock_irq(&acm->read_lock);
+   clear_bit(ACM_THROTTLED, &acm->flags);
 
retval = acm_submit_read_urbs(acm, GFP_KERNEL);
if (retval)
@@ -841,27 +832,19 @@ static void acm_tty_throttle(struct tty_struct *tty)
 {
struct acm *acm = tty->driver_data;
 
-   spin_lock_irq(&acm->read_lock);
-   acm->throttle_req = 1;
-   spin_unlock_irq(&acm->read_lock);
+   set_bit(ACM_THROTTLED, &acm->flags);
 }
 
 static void acm_tty_unthrottle(struct tty_struct *tty)
 {
struct acm *acm = tty->driver_data;
-   unsigned int was_throttled;
 
-   spin_lock_irq(&acm->read_lock);
-   was_throttled = acm->throttled;
-   acm->throttled = 0;
-   acm->throttle_req = 0;
-   spin_unlock_irq(&acm->read_lock);
+   clear_bit(ACM_THROTTLED, &acm->flags);
 
/* Matches the smp_mb__after_atomic() in acm_read_bulk_callback(). */
smp_mb();
 
-   if (was_throttled)
-   acm_submit_read_urbs(acm, GFP_KERNEL);
+   acm_submit_read_urbs(acm, GFP_KERNEL);
 }
 
 static int acm_tty_break_ctl(struct tty_struct *tty, int state)
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 515aad0847ee..ca1c026382c2 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -108,6 +108,7 @@ struct acm {
unsigned long flags;
 #  define EVENT_TTY_WAKEUP 0
 #  define EVENT_RX_STALL   1
+#  define ACM_THROTTLED2
struct usb_cdc_line_coding line;/* bits, stop, parity */
struct work_struct work;/* work queue entry for 
line discipline waking up */
unsigned int ctrlin;/* input control lines 
(DCD, DSR, RI, break, overruns) */
@@ -122,8 +123,6 @@ struct acm {
unsigned int ctrl_caps; /* control capabilities 
from the class specific header */
unsigned int susp_count;/* number of suspended 
interfaces */
unsigned int combined_interfaces:1; /* control and data 
collapsed */
-   unsigned int throttled:1;   /* actually throttled */
-   unsigned int throttle_req:1;/* throttle requested */
u8 bInterval;
struct usb_anchor delayed;  /* writes queued for a 
device about to be woken */
unsigned long quirks;
-- 
2.21.0

[PATCH 3/5] USB: serial: generic: drop unnecessary goto

[Johan Hovold]

Drop an unnecessary goto from a write-urb completion error path.

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/generic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 67cef3ef1e5f..1be8bea372a2 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -463,10 +463,9 @@ void usb_serial_generic_write_bulk_callback(struct urb 
*urb)
default:
dev_err_console(port, "%s - nonzero urb status: %d\n",
__func__, status);
-   goto resubmit;
+   break;
}
 
-resubmit:
usb_serial_generic_write_start(port, GFP_ATOMIC);
usb_serial_port_softint(port);
 }
-- 
2.21.0

[PATCH 0/5] USB: fix tty unthrottle races

[Johan Hovold]

This series fixes a couple of long-standing issues in USB serial and
cdc-acm which essentially share the same implementation.

As noted by Oliver a few years back, read-urb completion can race with
unthrottle() running on another CPU and this can potentially lead to
memory corruption. This particular bug in cdc-acm was unfortunately
reintroduced a year later.

There's also a second race due to missing memory barriers which could
theoretically lead to the port staying throttled until reopened on
weakly ordered systems. A second set of memory barriers should address
that.

I would appreciate your keen eyes on this one to make sure I got the
barriers right.

I noticed there's some on-going discussion about the atomic memory
barriers that Alan's involved in, and I'll try to catch up on his
data-race work as well. I'm still a little concerned about whether the
smp_mb__before_atomic() is sufficient to prevent the compiler from
messing things up without adding READ_ONCE().

Note that none of these have stable tags as the issues have been there
for eight years or so without anyone noticing (besides Oliver).

Still feels good to clean up your own mess.

Note that the cdc-acm patches have so far only been compile tested.

Johan


Johan Hovold (5):
  USB: serial: fix unthrottle races
  USB: serial: clean up throttle handling
  USB: serial: generic: drop unnecessary goto
  USB: cdc-acm: fix unthrottle races
  USB: cdc-acm: clean up throttle handling

 drivers/usb/class/cdc-acm.c  | 63 +++---
 drivers/usb/class/cdc-acm.h  |  3 +-
 drivers/usb/serial/generic.c | 76 +++-
 include/linux/usb/serial.h   |  5 +--
 4 files changed, 75 insertions(+), 72 deletions(-)

-- 
2.21.0

[PATCH 2/5] USB: serial: clean up throttle handling

[Johan Hovold]

Clean up the throttle implementation by dropping the redundant
throttle_req flag which was a remnant from back when there was only a
single read URB.

Also convert the throttled flag to an atomic bit flag.

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/generic.c | 34 --
 include/linux/usb/serial.h   |  5 +
 2 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 0fff4968ea1b..67cef3ef1e5f 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -106,12 +106,8 @@ void usb_serial_generic_deregister(void)
 int usb_serial_generic_open(struct tty_struct *tty, struct usb_serial_port 
*port)
 {
int result = 0;
-   unsigned long flags;
 
-   spin_lock_irqsave(&port->lock, flags);
-   port->throttled = 0;
-   port->throttle_req = 0;
-   spin_unlock_irqrestore(&port->lock, flags);
+   clear_bit(USB_SERIAL_THROTTLED, &port->flags);
 
if (port->bulk_in_size)
result = usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
@@ -375,7 +371,6 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 {
struct usb_serial_port *port = urb->context;
unsigned char *data = urb->transfer_buffer;
-   unsigned long flags;
bool stopped = false;
int status = urb->status;
int i;
@@ -429,15 +424,10 @@ void usb_serial_generic_read_bulk_callback(struct urb 
*urb)
if (stopped)
return;
 
-   /* Throttle the device if requested by tty */
-   spin_lock_irqsave(&port->lock, flags);
-   port->throttled = port->throttle_req;
-   if (!port->throttled) {
-   spin_unlock_irqrestore(&port->lock, flags);
-   usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC);
-   } else {
-   spin_unlock_irqrestore(&port->lock, flags);
-   }
+   if (test_bit(USB_SERIAL_THROTTLED, &port->flags))
+   return;
+
+   usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_read_bulk_callback);
 
@@ -485,23 +475,16 @@ EXPORT_SYMBOL_GPL(usb_serial_generic_write_bulk_callback);
 void usb_serial_generic_throttle(struct tty_struct *tty)
 {
struct usb_serial_port *port = tty->driver_data;
-   unsigned long flags;
 
-   spin_lock_irqsave(&port->lock, flags);
-   port->throttle_req = 1;
-   spin_unlock_irqrestore(&port->lock, flags);
+   set_bit(USB_SERIAL_THROTTLED, &port->flags);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_throttle);
 
 void usb_serial_generic_unthrottle(struct tty_struct *tty)
 {
struct usb_serial_port *port = tty->driver_data;
-   int was_throttled;
 
-   spin_lock_irq(&port->lock);
-   was_throttled = port->throttled;
-   port->throttled = port->throttle_req = 0;
-   spin_unlock_irq(&port->lock);
+   clear_bit(USB_SERIAL_THROTTLED, &port->flags);
 
/*
 * Matches the smp_mb__after_atomic() in
@@ -509,8 +492,7 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
 */
smp_mb();
 
-   if (was_throttled)
-   usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+   usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_unthrottle);
 
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 1c19f77ed541..d8bdab8f3c26 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -28,6 +28,7 @@
 
 /* USB serial flags */
 #define USB_SERIAL_WRITE_BUSY  0
+#define USB_SERIAL_THROTTLED   1
 
 /**
  * usb_serial_port: structure for the specific ports of a device.
@@ -67,8 +68,6 @@
  * @flags: usb serial port flags
  * @write_wait: a wait_queue_head_t used by the port.
  * @work: work queue entry for the line discipline waking up.
- * @throttled: nonzero if the read urb is inactive to throttle the device
- * @throttle_req: nonzero if the tty wants to throttle us
  * @dev: pointer to the serial device
  *
  * This structure is used by the usb-serial core and drivers for the specific
@@ -115,8 +114,6 @@ struct usb_serial_port {
unsigned long   flags;
wait_queue_head_t   write_wait;
struct work_struct  work;
-   charthrottled;
-   charthrottle_req;
unsigned long   sysrq; /* sysrq timeout */
struct device   dev;
 };
-- 
2.21.0

[PATCH 4/5] USB: cdc-acm: fix unthrottle races

[Johan Hovold]

Fix two long-standing bugs which could potentially lead to memory
corruption or leave the port throttled until it is reopened (on weakly
ordered systems), respectively, when read-URB completion races with
unthrottle().

First, the URB must not be marked as free before processing is complete
to prevent it from being submitted by unthrottle() on another CPU.

CPU 1   CPU 2

complete()  unthrottle()
  process_urb();
  smp_mb__before_atomic();
  set_bit(i, free);   if (test_and_clear_bit(i, free))
  submit_urb();

Second, the URB must be marked as free before checking the throttled
flag to prevent unthrottle() on another CPU from failing to observe that
the URB needs to be submitted if complete() sees that the throttled flag
is set.

CPU 1   CPU 2

complete()  unthrottle()
  set_bit(i, free);   throttled = 0;
  smp_mb__after_atomic(); smp_mb();
  if (throttled)  if (test_and_clear_bit(i, free))
  return; submit_urb();

Note that test_and_clear_bit() only implies barriers when the test is
successful. To handle the case where the URB is still in use an explicit
barrier needs to be added to unthrottle() for the second race condition.

Also note that the first race was fixed by 36e59e0d70d6 ("cdc-acm: fix
race between callback and unthrottle") back in 2015, but the bug was
reintroduced a year later.

Fixes: 1aba579f3cf5 ("cdc-acm: handle read pipe errors")
Fixes: 088c64f81284 ("USB: cdc-acm: re-write read processing")
Signed-off-by: Johan Hovold 
---
 drivers/usb/class/cdc-acm.c | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index ec666eb4b7b4..c03aa8550980 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -470,12 +470,12 @@ static void acm_read_bulk_callback(struct urb *urb)
struct acm *acm = rb->instance;
unsigned long flags;
int status = urb->status;
+   bool stopped = false;
+   bool stalled = false;
 
dev_vdbg(&acm->data->dev, "got urb %d, len %d, status %d\n",
rb->index, urb->actual_length, status);
 
-   set_bit(rb->index, &acm->read_urbs_free);
-
if (!acm->dev) {
dev_dbg(&acm->data->dev, "%s - disconnected\n", __func__);
return;
@@ -488,15 +488,16 @@ static void acm_read_bulk_callback(struct urb *urb)
break;
case -EPIPE:
set_bit(EVENT_RX_STALL, &acm->flags);
-   schedule_work(&acm->work);
-   return;
+   stalled = true;
+   break;
case -ENOENT:
case -ECONNRESET:
case -ESHUTDOWN:
dev_dbg(&acm->data->dev,
"%s - urb shutting down with status: %d\n",
__func__, status);
-   return;
+   stopped = true;
+   break;
default:
dev_dbg(&acm->data->dev,
"%s - nonzero urb status received: %d\n",
@@ -505,10 +506,24 @@ static void acm_read_bulk_callback(struct urb *urb)
}
 
/*
-* Unthrottle may run on another CPU which needs to see events
-* in the same order. Submission has an implict barrier
+* Make sure URB processing is done before marking as free to avoid
+* racing with unthrottle() on another CPU. Matches the barriers
+* implied by the test_and_clear_bit() in acm_submit_read_urb().
 */
smp_mb__before_atomic();
+   set_bit(rb->index, &acm->read_urbs_free);
+   /*
+* Make sure URB is marked as free before checking the throttled flag
+* to avoid racing with unthrottle() on another CPU. Matches the
+* smp_mb() in unthrottle().
+*/
+   smp_mb__after_atomic();
+
+   if (stopped || stalled) {
+   if (stalled)
+   schedule_work(&acm->work);
+   return;
+   }
 
/* throttle device if requested by tty */
spin_lock_irqsave(&acm->read_lock, flags);
@@ -842,6 +857,9 @@ static void acm_tty_unthrottle(struct tty_struct *tty)
acm->throttle_req = 0;
spin_unlock_irq(&acm->read_lock);
 
+   /* Matches the smp_mb__after_atomic() in acm_read_bulk_callback(). */
+   smp_mb();
+
if (was_throttled)
acm_submit_read_urbs(acm, GFP_KERNEL);
 }
-- 
2.21.0

[PATCH 1/5] USB: serial: fix unthrottle races

[Johan Hovold]

Fix two long-standing bugs which could potentially lead to memory
corruption or leave the port throttled until it is reopened (on weakly
ordered systems), respectively, when read-URB completion races with
unthrottle().

First, the URB must not be marked as free before processing is complete
to prevent it from being submitted by unthrottle() on another CPU.

CPU 1   CPU 2

complete()  unthrottle()
  process_urb();
  smp_mb__before_atomic();
  set_bit(i, free);   if (test_and_clear_bit(i, free))
  submit_urb();

Second, the URB must be marked as free before checking the throttled
flag to prevent unthrottle() on another CPU from failing to observe that
the URB needs to be submitted if complete() sees that the throttled flag
is set.

CPU 1   CPU 2

complete()  unthrottle()
  set_bit(i, free);   throttled = 0;
  smp_mb__after_atomic(); smp_mb();
  if (throttled)  if (test_and_clear_bit(i, free))
  return; submit_urb();

Note that test_and_clear_bit() only implies barriers when the test is
successful. To handle the case where the URB is still in use an explicit
barrier needs to be added to unthrottle() for the second race condition.

Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs")
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/generic.c | 39 +---
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 2274d9625f63..0fff4968ea1b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -376,6 +376,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
struct usb_serial_port *port = urb->context;
unsigned char *data = urb->transfer_buffer;
unsigned long flags;
+   bool stopped = false;
int status = urb->status;
int i;
 
@@ -383,33 +384,51 @@ void usb_serial_generic_read_bulk_callback(struct urb 
*urb)
if (urb == port->read_urbs[i])
break;
}
-   set_bit(i, &port->read_urbs_free);
 
dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i,
urb->actual_length);
switch (status) {
case 0:
+   usb_serial_debug_data(&port->dev, __func__, urb->actual_length,
+   data);
+   port->serial->type->process_read_urb(urb);
break;
case -ENOENT:
case -ECONNRESET:
case -ESHUTDOWN:
dev_dbg(&port->dev, "%s - urb stopped: %d\n",
__func__, status);
-   return;
+   stopped = true;
+   break;
case -EPIPE:
dev_err(&port->dev, "%s - urb stopped: %d\n",
__func__, status);
-   return;
+   stopped = true;
+   break;
default:
dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
__func__, status);
-   goto resubmit;
+   break;
}
 
-   usb_serial_debug_data(&port->dev, __func__, urb->actual_length, data);
-   port->serial->type->process_read_urb(urb);
+   /*
+* Make sure URB processing is done before marking as free to avoid
+* racing with unthrottle() on another CPU. Matches the barriers
+* implied by the test_and_clear_bit() in
+* usb_serial_generic_submit_read_urb().
+*/
+   smp_mb__before_atomic();
+   set_bit(i, &port->read_urbs_free);
+   /*
+* Make sure URB is marked as free before checking the throttled flag
+* to avoid racing with unthrottle() on another CPU. Matches the
+* smp_mb() in unthrottle().
+*/
+   smp_mb__after_atomic();
+
+   if (stopped)
+   return;
 
-resubmit:
/* Throttle the device if requested by tty */
spin_lock_irqsave(&port->lock, flags);
port->throttled = port->throttle_req;
@@ -484,6 +503,12 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
port->throttled = port->throttle_req = 0;
spin_unlock_irq(&port->lock);
 
+   /*
+* Matches the smp_mb__after_atomic() in
+* usb_serial_generic_read_bulk_callback().
+*/
+   smp_mb();
+
if (was_throttled)
usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
 }
-- 
2.21.0

Re: Use device tree to disable U1/U2 in gadget devices based on DWC3

[Rob Weber]

Hi Everyone,

On Wed, Apr 24, 2019 at 02:33:48PM -0400, Alan Stern wrote:
> On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:
> 
> > Hi Balbi and other USB developers.
> > 
> > I am developing camera devices based on Xilinx ZynqMP, using the
> > gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> > controller and the build-in Cirrus SERDES as phy.
> > Testing with a number of hosts and Windows 7, has shown sporadic
> > reconnects when leaving U2/U1, caused by failing link training, where
> > the host resets the bus. Sometime it also means it reconnect via
> > USB2.
> > 
> > So to overcome this, I will like to have the option for disabling
> > U1/U2 on the core when working with those hosts.
> > 
> > Currently I have made a hack in ep0.c where I return EINVAL in
> > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting
> > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> > dwc3_ep0_set_config
> > Will though prefer a solution possible to upstream, so was thinking
> > about adding two devicetree bindings.
> > 
> > * snps,u1_disable_as_gadget: When set the core will not enable U1 if
> > requested from host, nor initiate U1.
> > * snps,u2_disable_as_gadget: When set the core will not enable U2 if
> > requested from host, nor initiate U2.
> > 
> > If you think this might be something which can be upstreamed I will
> > prepare the code and send a patch for discussion.
> > On the other hand, if you think that disabling U1/U2 via device tree
> > as suggested should not be a feature no need for me to try making it
> > a feature.
> 
> Speaking as an interested bystander, I would first wonder why your
> hardware fails during link training.  If it is properly designed, that
> should not happen.  The fact that it does happen suggests your devices
> might also experience problems during normal data transport.

I was recently debugging a similar problem where my device would
improperly transition from U2 to U0. This caused the connection to reset
and the device to be re-enumerated as a USB2 device. Our design uses an
Intel CherryTrail z8550 SoC that has an internal dwc3 USB device
controller. I worked with Felipe a couple of weeks ago [1] to come up with
the same workaround you mentioned above. I disable device-initiated U1/U2
in ep0_set_config and also return -EINVAL when handling SetFeature
requests from the USB host. I've attached my patch for kernel 4.9.115
below for reference.

Although we have applied this workaround, we still have not identified a
root cause of the issue. Intel has no known errata related to the symptoms
we are experiencing. We've done quite a bit of analysis on our design and
are pretty confident we've followed all design guidelines for USB.

Cheers,
Rob Weber

[1] https://marc.info/?t=15534993561&r=1&w=2

---
drivers/usb/dwc3/ep0.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 2331469f943d..78b75d65b435 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -433,10 +433,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
return -EINVAL;

reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-if (set)
-reg |= DWC3_DCTL_INITU1ENA;
-else
+if (set) {
+/* reg |= DWC3_DCTL_INITU1ENA; */
+dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U1 Enable");
+return -EINVAL;
+} else {
reg &= ~DWC3_DCTL_INITU1ENA;
+}
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
break;

@@ -448,10 +451,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
return -EINVAL;

reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-if (set)
-reg |= DWC3_DCTL_INITU2ENA;
-else
+if (set) {
+/* reg |= DWC3_DCTL_INITU2ENA; */
+dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U2 Enable");
+return -EINVAL;
+} else {
reg &= ~DWC3_DCTL_INITU2ENA;
+}
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
break;

@@ -567,7 +573,7 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct 
usb_ctrlrequest *ctrl)
enum usb_device_state state = dwc->gadget.state;
u32 cfg;
int ret;
-u32 reg;
+/* u32 reg; */

cfg = le16_to_cpu(ctrl->wValue);

@@ -594,9 +600,9 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct 
usb_ctrlrequest *ctrl)
 * Enable transition to U1/U2 state when
 * nothing is pending from application.
 */
-reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA);
-dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+/* reg = dwc3_readl(dwc->regs, DWC3_DCTL); */
+/* reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPT

[usb:usb-next 54/62] drivers/usb/misc/usb251xb.c:248:50: error: dereferencing pointer to incomplete type 'struct gpio_chip'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-next
head:   a4d6a2989dc3f2f2bcd25ca53dd187a1de68ffac
commit: 6e3c8beb4f92a18a65e521cc5fe75874b6e2c860 [54/62] usb: usb251xb: Lock 
i2c-bus segment the hub resides
config: x86_64-randconfig-s5-04260238 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 6e3c8beb4f92a18a65e521cc5fe75874b6e2c860
# save the attached .config to linux build tree
make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>):

   drivers/usb/misc/usb251xb.c: In function 'usb251x_check_gpio_chip':
>> drivers/usb/misc/usb251xb.c:248:50: error: dereferencing pointer to 
>> incomplete type 'struct gpio_chip'
 ret = usb251xb_check_dev_children(&adap->dev, gc->parent);
 ^~

vim +248 drivers/usb/misc/usb251xb.c

   235  
   236  static int usb251x_check_gpio_chip(struct usb251xb *hub)
   237  {
   238  struct gpio_chip *gc = gpiod_to_chip(hub->gpio_reset);
   239  struct i2c_adapter *adap = hub->i2c->adapter;
   240  int ret;
   241  
   242  if (!hub->gpio_reset)
   243  return 0;
   244  
   245  if (!gc)
   246  return -EINVAL;
   247  
 > 248  ret = usb251xb_check_dev_children(&adap->dev, gc->parent);
   249  if (ret) {
   250  dev_err(hub->dev, "Reset GPIO chip is at the same 
i2c-bus\n");
   251  return -EINVAL;
   252  }
   253  
   254  return 0;
   255  }
   256  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH 1/2] usb: dwc3: Fix default lpm_nyet_threshold value

[Thinh Nguyen]

The max possible value for DCTL.LPM_NYET_THRES is 15 and not 255. Change
the default value to 15.

Cc: sta...@vger.kernel.org
Fixes: 80caf7d21adc ("usb: dwc3: add lpm erratum support")
Signed-off-by: Thinh Nguyen 
---
 drivers/usb/dwc3/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2c9110ef9865..c6735f0c1424 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1221,7 +1221,7 @@ static void dwc3_get_properties(struct dwc3 *dwc)
u8  tx_max_burst_prd;
 
/* default to highest possible threshold */
-   lpm_nyet_threshold = 0xff;
+   lpm_nyet_threshold = 0xf;
 
/* default to -3.5dB de-emphasis */
tx_de_emphasis = 1;
-- 
2.11.0

Re: [PATCH 0/5] USB: fix tty unthrottle races

[Alan Stern]

On Thu, 25 Apr 2019, Johan Hovold wrote:

> This series fixes a couple of long-standing issues in USB serial and
> cdc-acm which essentially share the same implementation.
> 
> As noted by Oliver a few years back, read-urb completion can race with
> unthrottle() running on another CPU and this can potentially lead to
> memory corruption. This particular bug in cdc-acm was unfortunately
> reintroduced a year later.
> 
> There's also a second race due to missing memory barriers which could
> theoretically lead to the port staying throttled until reopened on
> weakly ordered systems. A second set of memory barriers should address
> that.
> 
> I would appreciate your keen eyes on this one to make sure I got the
> barriers right.
> 
> I noticed there's some on-going discussion about the atomic memory
> barriers that Alan's involved in, and I'll try to catch up on his
> data-race work as well. I'm still a little concerned about whether the
> smp_mb__before_atomic() is sufficient to prevent the compiler from
> messing things up without adding READ_ONCE().

I think your changes in patches 1 and 4 are correct.  Regardless of the
issues still undergoing discussion elsewhere, smp_mb__before_atomic()
should cause both the compiler and the CPU to order every preceding
operation (READ_ONCE or not) before the atomic op.

Alan Stern

> Note that none of these have stable tags as the issues have been there
> for eight years or so without anyone noticing (besides Oliver).
> 
> Still feels good to clean up your own mess.
> 
> Note that the cdc-acm patches have so far only been compile tested.
> 
> Johan
> 
> 
> Johan Hovold (5):
>   USB: serial: fix unthrottle races
>   USB: serial: clean up throttle handling
>   USB: serial: generic: drop unnecessary goto
>   USB: cdc-acm: fix unthrottle races
>   USB: cdc-acm: clean up throttle handling
> 
>  drivers/usb/class/cdc-acm.c  | 63 +++---
>  drivers/usb/class/cdc-acm.h  |  3 +-
>  drivers/usb/serial/generic.c | 76 +++-
>  include/linux/usb/serial.h   |  5 +--
>  4 files changed, 75 insertions(+), 72 deletions(-)
> 
> 

[PATCH 2/2] usb: dwc3: Rename DWC3_DCTL_LPM_ERRATA

[Thinh Nguyen]

The macro name DWC3_DCTL_LPM_ERRATA is uninformative and does not do
masking. Remove DWC3_DCTL_LPM_ERRATA_MASK and rename
DWC3_DCTL_LPM_ERRATA to DWC3_DCTL_NYET_THRES with proper masking.

Signed-off-by: Thinh Nguyen 
---
 drivers/usb/dwc3/core.h   | 3 +--
 drivers/usb/dwc3/gadget.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1528d395b156..f19cbeb01087 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -406,8 +406,7 @@
 #define DWC3_DCTL_TRGTULST_SS_INACT(DWC3_DCTL_TRGTULST(6))
 
 /* These apply for core versions 1.94a and later */
-#define DWC3_DCTL_LPM_ERRATA_MASK  DWC3_DCTL_LPM_ERRATA(0xf)
-#define DWC3_DCTL_LPM_ERRATA(n)((n) << 20)
+#define DWC3_DCTL_NYET_THRES(n)(((n) & 0xf) << 20)
 
 #define DWC3_DCTL_KEEP_CONNECT BIT(19)
 #define DWC3_DCTL_L1_HIBER_EN  BIT(18)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 2bb0ff9608d3..1d808d15e886 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2863,7 +2863,7 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 
*dwc)
"LPM Erratum not available on dwc3 revisions < 
2.40a\n");
 
if (dwc->has_lpm_erratum && dwc->revision >= DWC3_REVISION_240A)
-   reg |= DWC3_DCTL_LPM_ERRATA(dwc->lpm_nyet_threshold);
+   reg |= DWC3_DCTL_NYET_THRES(dwc->lpm_nyet_threshold);
 
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
} else {
-- 
2.11.0

[PATCH] usb: dwc3: debug: Print GET_STATUS(device) tracepoint

[Thinh Nguyen]

DWC3 is missing the printing of control request GET_STATUS(device)
tracepoint. This patch prints that.

Signed-off-by: Thinh Nguyen 
---
 drivers/usb/dwc3/debug.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index 6759a7efd8d5..068259fdfb0c 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -250,6 +250,9 @@ static inline void dwc3_decode_get_status(__u8 t, __u16 i, 
__u16 l, char *str,
size_t size)
 {
switch (t & USB_RECIP_MASK) {
+   case USB_RECIP_DEVICE:
+   snprintf(str, size, "Get Device Status(Length = %d)", l);
+   break;
case USB_RECIP_INTERFACE:
snprintf(str, size, "Get Interface Status(Intf = %d, Length = 
%d)",
i, l);
-- 
2.11.0

[PATCH] usb: dwc3: Do core validation early on probe

[Thinh Nguyen]

The setting of the dr_mode may need to check the controller's revision.
The revision is set in the dwc3_core_is_valid(), which comes after
dr_mode setting. Let's move it closer to the start of the dwc3_probe()
function and before calling dwc3_get_dr_mode().

Signed-off-by: Thinh Nguyen 
---
 drivers/usb/dwc3/core.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c6735f0c1424..4aff1d8dbc4f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -896,12 +896,6 @@ static int dwc3_core_init(struct dwc3 *dwc)
u32 reg;
int ret;
 
-   if (!dwc3_core_is_valid(dwc)) {
-   dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n");
-   ret = -ENODEV;
-   goto err0;
-   }
-
/*
 * Write Linux Version Code to our GUID register so it's easy to figure
 * out which kernel version a bug was found.
@@ -1429,6 +1423,11 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->regs   = regs;
dwc->regs_size  = resource_size(&dwc_res);
 
+   if (!dwc3_core_is_valid(dwc)) {
+   dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n");
+   return -ENODEV;
+   }
+
dwc3_get_properties(dwc);
 
dwc->reset = devm_reset_control_get_optional_shared(dev, NULL);
-- 
2.11.0

[PATCH] usb: dwc3: gadget: Set lpm_capable

[Thinh Nguyen]

All DWC3 controllers are LPM capable. Report that in the
usb_gadget.lpm_capable for the gadget driver to properly output the
bcdUSB value in the descriptor.

Signed-off-by: Thinh Nguyen 
---
 drivers/usb/dwc3/gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 1d808d15e886..d67655384eb2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3301,6 +3301,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
dwc->gadget.sg_supported= true;
dwc->gadget.name= "dwc3-gadget";
dwc->gadget.is_otg  = dwc->dr_mode == USB_DR_MODE_OTG;
+   dwc->gadget.lpm_capable = true;
 
/*
 * FIXME We might be setting max_speed to 

Re: configfs on dwc3: msc enum failed if three functions defined

[Jack Pham]

Hi Bin,

On Mon, Apr 22, 2019 at 08:43:57AM -0500, Bin Liu wrote:
> Hi Felipe,
> 
> I am having an issue with dwc3 on TI AM57x device, and would like to ask
> for your comments.
> 
> I use configfs to create a multi-function gadget on dwc3, mass_storage
> is the last function, it seems if I create 3 functions, the mass_storage
> enumeration will fail on the host. It works fine if only create 2
> functions.
> 
> The dwc3 tracepoints log shows after all the ep0 transfers for
> mass_storage, the very first epXin transfer is not complete - dwc3
> programmed the urb, but never generates RX completion event. This also
> matches the bus analyzer trace - dwc3 NAKs the very first IN token for
> ever.
> 
> I use the attached script to create the gadget, The macro FUNCS in the
> beginning of the script defines the functions to be created.
> 
> Any comments are appreciated.

A stab in the dark here but what is the value of GTXFIFOSIZ(X)[15:0]
for epXin on your device? Is it at least wMaxPacketSize? Depending on
the default hardware values it might be deficient as compared to the
working endpoint that gets assigned in your 2-function config.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usbip: stub_rx: tidy the indenting in is_clear_halt_cmd()

[shuah]

On 4/24/19 3:54 AM, Dan Carpenter wrote:

There is an extra space character before the return statement.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index 97b09a42a10c..f3230bed18af 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -17,9 +17,9 @@ static int is_clear_halt_cmd(struct urb *urb)
  
  	req = (struct usb_ctrlrequest *) urb->setup_packet;
  
-	 return (req->bRequest == USB_REQ_CLEAR_FEATURE) &&

-(req->bRequestType == USB_RECIP_ENDPOINT) &&
-(req->wValue == USB_ENDPOINT_HALT);
+   return (req->bRequest == USB_REQ_CLEAR_FEATURE) &&
+  (req->bRequestType == USB_RECIP_ENDPOINT) &&
+  (req->wValue == USB_ENDPOINT_HALT);
  }
  
  static int is_set_interface_cmd(struct urb *urb)




Thanks for the patch.

Acked-by: Shuah Khan 

thanks,
-- Shuah

Re: [PATCH 0/5] USB: fix tty unthrottle races

[Johan Hovold]

On Thu, Apr 25, 2019 at 04:58:20PM -0400, Alan Stern wrote:
> On Thu, 25 Apr 2019, Johan Hovold wrote:
> 
> > This series fixes a couple of long-standing issues in USB serial and
> > cdc-acm which essentially share the same implementation.
> > 
> > As noted by Oliver a few years back, read-urb completion can race with
> > unthrottle() running on another CPU and this can potentially lead to
> > memory corruption. This particular bug in cdc-acm was unfortunately
> > reintroduced a year later.
> > 
> > There's also a second race due to missing memory barriers which could
> > theoretically lead to the port staying throttled until reopened on
> > weakly ordered systems. A second set of memory barriers should address
> > that.
> > 
> > I would appreciate your keen eyes on this one to make sure I got the
> > barriers right.
> > 
> > I noticed there's some on-going discussion about the atomic memory
> > barriers that Alan's involved in, and I'll try to catch up on his
> > data-race work as well. I'm still a little concerned about whether the
> > smp_mb__before_atomic() is sufficient to prevent the compiler from
> > messing things up without adding READ_ONCE().
> 
> I think your changes in patches 1 and 4 are correct.  Regardless of the
> issues still undergoing discussion elsewhere, smp_mb__before_atomic()
> should cause both the compiler and the CPU to order every preceding
> operation (READ_ONCE or not) before the atomic op.

Ok, thanks for taking a look!

Johan

[PATCH] USB: serial: drop unused iflag macro

[Johan Hovold]

Drop the RELEVANT_IFLAG() macro which essentially hasn't been used for
over a decade except in some remnant debug printks that were recently
removed.

Signed-off-by: Johan Hovold 
---
 include/linux/usb/serial.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 9d378826248b..1b631ea476ee 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -23,9 +23,6 @@
 /* The maximum number of ports one device can grab at once */
 #define MAX_NUM_PORTS  16
 
-/* parity check flag */
-#define RELEVANT_IFLAG(iflag)  (iflag & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
-
 /* USB serial flags */
 #define USB_SERIAL_WRITE_BUSY  0
 #define USB_SERIAL_THROTTLED   1
-- 
2.21.0

Re: [PATCH 0/9] USB: serial: fix initial-termios handling

[Johan Hovold]

On Sun, Apr 21, 2019 at 02:21:45PM +0200, Johan Hovold wrote:
> A few drivers override the default initial termios settings provided by
> USB serial core, but due to a long-standing bug, the terminal settings
> were being reset on every (first) open rather than on first use after
> probe as intended.
> 
> This series fixes the bug and cleans up the various init_termios
> implementations somewhat.

> Johan Hovold (9):
>   USB: serial: fix initial-termios handling
>   USB: serial: ark3116: drop redundant init_termios
>   USB: serial: cypress_m8: drop unused driver data flag
>   USB: serial: cypress_m8: drop unused termios
>   USB: serial: cypress_m8: clean up initial-termios handling
>   USB: serial: iuu_phoenix: drop bogus initial cflag
>   USB: serial: iuu_phoenix: simplify init_termios
>   USB: serial: oti6858: simplify init_termios
>   USB: serial: spcp8x5: simplify init_termios

I have applied these now.

Johan