RE: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support

2019-05-16 Thread Biju Das 
+ Rob

Hi Chunfeng Yun,

Thanks for the feedback.

> Subject: Re: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
> switch support
> 
> On Wed, 2019-05-15 at 13:09 +0100, Biju Das wrote:
> > The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
> > usb type-c drp port controller. This patch adds dual role switch
> > support for the type-c connector using the usb role switch class framework.
> >
> > Signed-off-by: Biju Das 
> > ---
> >  V5-->V6
> >* Added graph api's to find the role supported by the connector.
> >  V4-->V5
> >* Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10902537/)
> >  V3-->V4
> >* No Change
> >  V2-->V3
> >* Incorporated Shimoda-san's review comment
> >  (https://patchwork.kernel.org/patch/10852507/)
> >* Used renesas,usb-role-switch property for differentiating USB
> >  role switch associated with Type-C port controller driver.
> >  V1-->V2
> >* Driver uses usb role clas for handling dual role switch and handling
> >  connect/disconnect events instead of extcon.
> > ---
> >  drivers/usb/gadget/udc/renesas_usb3.c | 121
> > --
> >  1 file changed, 114 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..1d41998 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -24,6 +24,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  /* register definitions */
> > @@ -351,6 +352,8 @@ struct renesas_usb3 {
> > int disabled_count;
> >
> > struct usb_request *ep0_req;
> > +
> > +   enum usb_role connection_state;
> > u16 test_mode;
> > u8 ep0_buf[USB3_EP0_BUF_SIZE];
> > bool softconnect;
> > @@ -359,6 +362,7 @@ struct renesas_usb3 {
> > bool extcon_usb;/* check vbus and set EXTCON_USB
> */
> > bool forced_b_device;
> > bool start_to_connect;
> > +   bool dual_role_sw;
> >  };
> >
> >  #define gadget_to_renesas_usb3(_gadget)\
> > @@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&usb3->lock, flags);
> > -   usb3_set_mode_by_role_sw(usb3, host);
> > -   usb3_vbus_out(usb3, a_dev);
> > +   if (!usb3->dual_role_sw || usb3->connection_state !=
> USB_ROLE_NONE) {
> > +   usb3_set_mode_by_role_sw(usb3, host);
> > +   usb3_vbus_out(usb3, a_dev);
> > +   }
> > /* for A-Peripheral or forced B-device mode */
> > if ((!host && a_dev) || usb3->start_to_connect)
> > usb3_connect(usb3);
> > @@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3
> > *usb3)  {
> > usb3->extcon_host = usb3_is_a_device(usb3);
> >
> > -   if (usb3->extcon_host && !usb3->forced_b_device)
> > +   if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3-
> >forced_b_device)
> > +   || usb3->connection_state == USB_ROLE_HOST)
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false); @@ -2343,14 +2350,65
> @@
> > static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> > return cur_role;
> >  }
> >
> > -static int renesas_usb3_role_switch_set(struct device *dev,
> > -   enum usb_role role)
> > +static void handle_ext_role_switch_states(struct device *dev,
> > +   enum usb_role role)
> > +{
> > +   struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > +   struct device *host = usb3->host_dev;
> > +   enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> > +
> > +   switch (role) {
> > +   case USB_ROLE_NONE:
> > +   usb3->connection_state = USB_ROLE_NONE;
> > +   if (usb3->driver)
> > +   usb3_disconnect(usb3);
> > +   usb3_vbus_out(usb3, false);
> > +   break;
> > +   case USB_ROLE_DEVICE:
> > +   if (usb3->connection_state == USB_ROLE_NONE) {
> > +   usb3->connection_state = USB_ROLE_DEVICE;
> > +   usb3_set_mode(usb3, false);
> > +   if (usb3->driver)
> > +   usb3_connect(usb3);
> > +   } else if (cur_role == USB_ROLE_HOST)  {
> > +   device_release_driver(host);
> > +   usb3_set_mode(usb3, false);
> > +   if (usb3->driver)
> > +   usb3_connect(usb3);
> > +   }
> > +   usb3_vbus_out(usb3, false);
> > +   break;
> > +   case USB_ROLE_HOST:
> > +   if (usb3->connection_state == USB_ROLE_NONE) {
> > +   if (usb3->driver)
> > +   usb3_disconnect(usb3);
> > +
> > +   usb3->connection_state = USB_ROLE_HOST;
> > +   usb3_set_mode(usb3, true);
> > +

Re: Linux crash when using FTDI FT232R USB to serial converter on AMD boards.

2019-05-16 Thread Oliver Neukum 
On Mi, 2019-05-15 at 16:15 +0200,  starost...@gmail.com  wrote:
> Dne 15.5.2019 v 15:54 Oliver Neukum napsal(a):
> > 1. Determine whether the bug depends on the use of an IOMMU
> 
> No, bug not depends on the use of an IOMMU. System crash on both cases.

Interesting.

> > 2.Send a new report to the corresponding mailing list
> 
> Which mailing list is correct?

In that case linux-usb@vger.kernel.org

HTH
Oliver

Re: Linux crash when using FTDI FT232R USB to serial converter on AMD boards.

2019-05-16 Thread starost...@gmail.com 

Dne 16.5.2019 v 9:58 Oliver Neukum napsal(a):

2.Send a new report to the corresponding mailing list

Which mailing list is correct?

In that case linux-usb@vger.kernel.org

HTH
Oliver

Is there some rules how to send bug report? Or I can send report with 
"my amateur description"?


starosta

Re: Linux crash when using FTDI FT232R USB to serial converter on AMD boards.

2019-05-16 Thread Oliver Neukum 
On Do, 2019-05-16 at 10:20 +0200,  starost...@gmail.com  wrote:
> Dne 16.5.2019 v 9:58 Oliver Neukum napsal(a):
> > > > 2.Send a new report to the corresponding mailing list
> > > 
> > > Which mailing list is correct?
> > 
> > In that case linux-usb@vger.kernel.org
> > 
> > HTH
> > Oliver
> > 
> 
> Is there some rules how to send bug report? Or I can send report with 
> "my amateur description"?

Mention

1. kernel version
2. whether this is known to be a regression
3. include the log with iommu disabled and mention that you disabled
the IOMMU
4. Include the output of lsusb

HTH
Oliver

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

2019-05-16 Thread Chunfeng Yun 
On Thu, 2019-05-16 at 12:09 +0530, Nagarjuna Kristam wrote:
> This patch adds UDC driver for tegra XUSB 3.0 device mode controller.
> XUSB device mode controller supports 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 | 3807 
> +++
>  3 files changed, 3818 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 ef0259a..b35856c 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -440,6 +440,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 Tegra Superspeed USB 3.0 Device Controller"
> + depends on ARCH_TEGRA
> + help
> +  Enables NVIDIA Tegra USB 3.0 device mode controller driver.
> +
> +  Say "y" to link the driver statically, or "m" to build a
> +  dynamically linked module called "tegra_xudc" and force all
> +  gadget drivers to also be dynamically linked.
> +
>  source "drivers/usb/gadget/udc/aspeed-vhub/Kconfig"
>  
>  #
> diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
> index 897f648..1c55c96 100644
> --- a/drivers/usb/gadget/udc/Makefile
> +++ b/drivers/usb/gadget/udc/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_BCM63XX_UDC)   += bcm63xx_udc.o
>  obj-$(CONFIG_USB_FSL_USB2)   += fsl_usb2_udc.o
>  fsl_usb2_udc-y   := fsl_udc_core.o
>  fsl_usb2_udc-$(CONFIG_ARCH_MXC)  += fsl_mxc_udc.o
> +obj-$(CONFIG_USB_TEGRA_XUDC) += tegra_xudc.o
>  obj-$(CONFIG_USB_M66592) += m66592-udc.o
>  obj-$(CONFIG_USB_R8A66597)   += r8a66597-udc.o
>  obj-$(CONFIG_USB_RENESAS_USB3)   += renesas_usb3.o
> diff --git a/drivers/usb/gadget/udc/tegra_xudc.c 
> b/drivers/usb/gadget/udc/tegra_xudc.c
> new file mode 100644
> index 000..6cd4675
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/tegra_xudc.c
> @@ -0,0 +1,3807 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NVIDIA Tegra XUSB device mode controller
> + *
> + * Copyright (c) 2013-2019, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2015, Google Inc.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* XUSB_DEV registers */
> +#define SPARAM 0x000
> +#define  SPARAM_ERSTMAX_SHIFT 16
> +#define  SPARAM_ERSTMAX_MASK 0x1f
use GENMASK() to generate mask?

> +#define DB 0x004
> +#define  DB_TARGET_SHIFT 8
> +#define  DB_TARGET_MASK 0xff
> +#define  DB_STREAMID_SHIFT 16
> +#define  DB_STREAMID_MASK 0x
> +#define ERSTSZ 0x008
> +#define  ERSTSZ_ERSTXSZ_SHIFT(x) ((x) * 16)
> +#define  ERSTSZ_ERSTXSZ_MASK 0x
> +#define ERSTXBALO(x) (0x010 + 8 * (x))
> +#define ERSTXBAHI(x) (0x014 + 8 * (x))
> +#define ERDPLO 0x020
> +#define  ERDPLO_EHB BIT(3)
> +#define ERDPHI 0x024
> +#define EREPLO 0x028
> +#define  EREPLO_ECS BIT(0)
> +#define  EREPLO_SEGI BIT(1)
> +#define EREPHI 0x02c
> +#define CTRL 0x030
> +#define  CTRL_RUN BIT(0)
> +#define  CTRL_LSE BIT(1)
> +#define  CTRL_IE BIT(4)
> +#define  CTRL_SMI_EVT BIT(5)
> +#define  CTRL_SMI_DSE BIT(6)
> +#define  CTRL_EWE BIT(7)
> +#define  CTRL_DEVADDR_SHIFT 24
> +#define  CTRL_DEVADDR_MASK 0x7f
> +#define  CTRL_ENABLE BIT(31)
> +#define ST 0x034
> +#define  ST_RC BIT(0)
> +#define  ST_IP BIT(4)
> +#define RT_IMOD  0x038
> +#define  RT_IMOD_IMODI_SHIFT 0
> +#define  RT_IMOD_IMODI_MASK 0x
> +#define  RT_IMOD_IMODC_SHIFT 16
> +#define  RT_IMOD_IMODC_MASK 0x
> +#define PORTSC 0x03c
> +#define  PORTSC_CCS BIT(0)
> +#define  PORTSC_PED BIT(1)
> +#define  PORTSC_PR BIT(4)
> +#define  PORTSC_PLS_SHIFT 5
> +#define  PORTSC_PLS_MASK 0xf
> +#define  PORTSC_PLS_U0 0x0
> +#define  PORTSC_PLS_U2 0x2
> +#define  PORTSC_PLS_U3 0x3
> +#define  PORTSC_PLS_DISABLED 0x4
> +#define  PORTSC_PLS_RXDETECT 0x5
> +#define  PORTSC_PLS_INACTIVE 0x6
> +#define  PORTSC_PLS_RESUME 0xf
> +#define  PORTSC_PS_SHIFT 10
> +#define  PORTSC_PS_MASK 0xf
> +#define  PORTSC_PS_UNDEFINED 0x0
> +#define  PORTSC_PS_FS 0x1
> +#define  PORTSC_PS_LS 0x2
> +#define  PORTSC_PS_HS 0x3
> +#define  PORTSC_PS_SS 0x4
> +#define  PORTSC_LWS BIT(16)
> +#define  PORTSC_CSC BIT(17)
> +#define  PORTSC_WRC BIT(19)
> +#define  PORTSC_PRC BIT(21)
> +#define  PORTSC_PLC BIT(22)
> +#define  PORTSC_CEC BIT(23)
> +#define  PORTSC_WPR BIT(30)
> +#define  PORTSC_CHANGE_MASK (PORTSC_CSC | PORTSC_WRC | PORTSC_PRC | \
> +  PORTSC_PLC | PORTSC_CEC)
> +#defi

Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread Laurentiu Tudor 
Hi Fredrik,

Thanks very much for taking the time to look into this. Please see 
comments inline.

On 15.05.2019 19:28, Fredrik Noring wrote:
> Hi Lauretniu,
> 
>> I think that any recent kernel will do, so I'd say your current branch
>> should be fine.
> 
> The kernel oopses with "unable to handle kernel paging request at virtual
> address 000aba0b" in hcd_alloc_coherent via usb_hcd_map_urb_for_dma. 

By any chance, does this address looks like the dma_addr that the device 
should target?

> This relates to patch 2/3 that I didn't quite understand, where
> 
> - retval = dma_declare_coherent_memory(dev, mem->start,
> -  mem->start - mem->parent->start,
> -  resource_size(mem));
> 
> becomes
> 
> + retval = gen_pool_add_virt(hcd->localmem_pool,
> +(unsigned long)mem->start -
> + mem->parent->start,
> +mem->start, resource_size(mem),
> 
> so that arguments two and three switch places. Given
> 
> int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>   dma_addr_t device_addr, size_t size);
> 
> and
> 
> int gen_pool_add_virt(struct gen_pool *pool, unsigned long virt, phys_addr_t 
> phys
>size_t size, int nid)
> 
> it seems that the device address (dma_addr_t device_addr) now becomes a
> virtual address (unsigned long virt). Is that intended?

Actually, I think I'm misusing genalloc and also it appears that i need 
to add a mapping on the phys address. So my plan is to change the 
"unsigned long virt" to be the void * returned by the mapping operation 
and the phys_addr_t be the dma_addr_t. I'll return with a patch.

Regarding the usage of unsigned long in genalloc, yeah seems a bit 
strange but by looking at other users in the kernel it appears that 
that's the design.

---
Best Regards, Laurentiu

> My corresponding patch is below for reference. I applied your 1/3 patch
> to test it.
> 
> Fredrik
> 
> diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
> --- a/drivers/usb/host/ohci-ps2.c
> +++ b/drivers/usb/host/ohci-ps2.c
> @@ -7,6 +7,7 @@
>*/
>   
>   #include 
> +#include 
>   #include 
>   #include 
>   #include 
> @@ -92,12 +93,12 @@ static irqreturn_t ohci_ps2_irq(struct usb_hcd *hcd)
>   return ohci_irq(hcd); /* Call normal IRQ handler. */
>   }
>   
> -static int iopheap_alloc_coherent(struct platform_device *pdev,
> - size_t size, int flags)
> +static int iopheap_alloc_coherent(struct platform_device *pdev, size_t size)
>   {
>   struct device *dev = &pdev->dev;
>   struct usb_hcd *hcd = platform_get_drvdata(pdev);
>   struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
> + int err;
>   
>   if (ps2priv->iop_dma_addr != 0)
>   return 0;
> @@ -112,28 +113,37 @@ static int iopheap_alloc_coherent(struct 
> platform_device *pdev,
>   return -ENOMEM;
>   }
>   
> - if (dma_declare_coherent_memory(dev,
> - iop_bus_to_phys(ps2priv->iop_dma_addr),
> - ps2priv->iop_dma_addr, size, flags)) {
> - dev_err(dev, "dma_declare_coherent_memory failed\n");
> - iop_free(ps2priv->iop_dma_addr);
> - ps2priv->iop_dma_addr = 0;
> - return -ENOMEM;
> + hcd->localmem_pool = devm_gen_pool_create(dev,
> + PAGE_SHIFT, dev_to_node(dev), DRV_NAME);
> + if (IS_ERR(hcd->localmem_pool)) {
> + err = PTR_ERR(hcd->localmem_pool);
> + goto out_err;
> + }
> +
> + err = gen_pool_add_virt(hcd->localmem_pool, ps2priv->iop_dma_addr,
> + iop_bus_to_phys(ps2priv->iop_dma_addr), size, dev_to_node(dev));
> + if (err < 0) {
> + dev_err(dev, "gen_pool_add_virt failed with %d\n", err);
> + goto out_err;
>   }
>   
>   return 0;
> +
> +out_err:
> + iop_free(ps2priv->iop_dma_addr);
> + ps2priv->iop_dma_addr = 0;
> +
> + return err;
>   }
>   
>   static void iopheap_free_coherent(struct platform_device *pdev)
>   {
> - struct device *dev = &pdev->dev;
>   struct usb_hcd *hcd = platform_get_drvdata(pdev);
>   struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
>   
>   if (ps2priv->iop_dma_addr == 0)
>   return;
>   
> - dma_release_declared_memory(dev);
>   iop_free(ps2priv->iop_dma_addr);
>   ps2priv->iop_dma_addr = 0;
>   }
> @@ -177,8 +187,7 @@ static int ohci_hcd_ps2_probe(struct platform_device 
> *pdev)
>   goto err;
>   }
>   
> - ret = iopheap_alloc_coherent(pdev,
> - DMA_BUFFER_SIZE, DMA_MEMORY_EXCLUSIVE);
> + ret = iopheap_alloc_coherent(pdev, DMA_BUFFER_SIZE);
>   if (ret != 0)
>   goto err_alloc_coherent;
>   
>

[PATCH v4 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread laurentiu . tudor 
From: Laurentiu Tudor 

For HCs that have local memory, replace the current DMA API usage
with a genalloc generic allocator to manage the mappings for these
devices.
This is in preparation for dropping the existing "coherent" dma
mem declaration APIs. Current implementation was relying on a short
circuit in the DMA API that in the end, was acting as an allocator
for these type of devices.

Only compiled tested, so any volunteers willing to test are most welcome.

Thank you!

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Changes in v4:
 - created mapping for local mem
 - fix genalloc misuse

Changes in v3:
 - rearranged calls into genalloc simplifying the code a bit (Christoph)
 - dropped RFC prefix

Changes in v2:
 - use genalloc also in core usb (based on comment from Robin)
 - moved genpool decl to usb_hcd to be accesible from core usb

Laurentiu Tudor (3):
  USB: use genalloc for USB HCs with local memory
  usb: host: ohci-sm501: init genalloc for local memory
  usb: host: ohci-tmio: init genalloc for local memory

 drivers/usb/core/buffer.c | 15 +---
 drivers/usb/host/ohci-hcd.c   | 23 +---
 drivers/usb/host/ohci-sm501.c | 66 +--
 drivers/usb/host/ohci-tmio.c  | 32 -
 include/linux/usb/hcd.h   |  3 ++
 5 files changed, 95 insertions(+), 44 deletions(-)

-- 
2.17.1

Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread Laurentiu Tudor 
Hi Fredrick,

On 15.05.2019 19:28, Fredrik Noring wrote:
> Hi Lauretniu,
> 
>> I think that any recent kernel will do, so I'd say your current branch
>> should be fine.
> 
> The kernel oopses with "unable to handle kernel paging request at virtual
> address 000aba0b" in hcd_alloc_coherent via usb_hcd_map_urb_for_dma. This
> relates to patch 2/3 that I didn't quite understand, where
> 
> - retval = dma_declare_coherent_memory(dev, mem->start,
> -  mem->start - mem->parent->start,
> -  resource_size(mem));
> 
> becomes
> 
> + retval = gen_pool_add_virt(hcd->localmem_pool,
> +(unsigned long)mem->start -
> + mem->parent->start,
> +mem->start, resource_size(mem),
> 
> so that arguments two and three switch places. Given
> 
> int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>   dma_addr_t device_addr, size_t size);
> 
> and
> 
> int gen_pool_add_virt(struct gen_pool *pool, unsigned long virt, phys_addr_t 
> phys
>size_t size, int nid)
> 
> it seems that the device address (dma_addr_t device_addr) now becomes a
> virtual address (unsigned long virt). Is that intended?
> 
> My corresponding patch is below for reference. I applied your 1/3 patch
> to test it.

I took your code, added the missing mapping and placed it in a patch. 
Please see attached (compile tested only).

---
Thanks & Best Regards, Laurentiu
From 510c34990bbe899a2e4f43f3b904183f126f81de Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor 
Date: Thu, 16 May 2019 14:35:06 +0300
Subject: [PATCH] usb: host: ohci-ps2: init genalloc for local memory

In preparation for dropping the existing "coherent" dma mem declaration
APIs, replace the current dma_declare_coherent_memory() based mechanism
with the creation of a genalloc pool that will be used in the OHCI
subsystem as replacement for the DMA APIs.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor 
---
 drivers/usb/host/ohci-ps2.c | 44 ++---
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
index 7669b7358bc3..454e5e3ac96e 100755
--- a/drivers/usb/host/ohci-ps2.c
+++ b/drivers/usb/host/ohci-ps2.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -92,12 +93,13 @@ static irqreturn_t ohci_ps2_irq(struct usb_hcd *hcd)
 	return ohci_irq(hcd); /* Call normal IRQ handler. */
 }
 
-static int iopheap_alloc_coherent(struct platform_device *pdev,
-	size_t size, int flags)
+static int iopheap_alloc_coherent(struct platform_device *pdev, size_t size)
 {
 	struct device *dev = &pdev->dev;
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
+	void __iomem *local_mem;
+	int err;
 
 	if (ps2priv->iop_dma_addr != 0)
 		return 0;
@@ -112,28 +114,45 @@ static int iopheap_alloc_coherent(struct platform_device *pdev,
 		return -ENOMEM;
 	}
 
-	if (dma_declare_coherent_memory(dev,
-			iop_bus_to_phys(ps2priv->iop_dma_addr),
-			ps2priv->iop_dma_addr, size, flags)) {
-		dev_err(dev, "dma_declare_coherent_memory failed\n");
-		iop_free(ps2priv->iop_dma_addr);
-		ps2priv->iop_dma_addr = 0;
-		return -ENOMEM;
+	hcd->localmem_pool = devm_gen_pool_create(dev, PAGE_SHIFT,
+		  dev_to_node(dev), DRV_NAME);
+	if (IS_ERR(hcd->localmem_pool)) {
+		err = PTR_ERR(hcd->localmem_pool);
+		goto out_err;
+	}
+
+	local_mem = devm_ioremap(dev,
+ iop_bus_to_phys(ps2priv->iop_dma_addr),
+ size);
+	if (!local_mem) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	err = gen_pool_add_virt(hcd->localmem_pool, (unsigned long)local_mem,
+ps2priv->iop_dma_addr, size, dev_to_node(dev));
+	if (err < 0) {
+		dev_err(dev, "gen_pool_add_virt failed with %d\n", err);
+		goto out_err;
 	}
 
 	return 0;
+
+out_err:
+	iop_free(ps2priv->iop_dma_addr);
+	ps2priv->iop_dma_addr = 0;
+
+	return err;
 }
 
 static void iopheap_free_coherent(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
 
 	if (ps2priv->iop_dma_addr == 0)
 		return;
 
-	dma_release_declared_memory(dev);
 	iop_free(ps2priv->iop_dma_addr);
 	ps2priv->iop_dma_addr = 0;
 }
@@ -177,8 +196,7 @@ static int ohci_hcd_ps2_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	ret = iopheap_alloc_coherent(pdev,
-		DMA_BUFFER_SIZE, DMA_MEMORY_EXCLUSIVE);
+	ret = iopheap_alloc_coherent(pdev, DMA_BUFFER_SIZE);
 	if (ret != 0)
 		goto err_alloc_coherent;
 
-- 
2.17.1

[PATCH v4 2/3] usb: host: ohci-sm501: init genalloc for local memory

2019-05-16 Thread laurentiu . tudor 
From: Laurentiu Tudor 

In preparation for dropping the existing "coherent" dma mem declaration
APIs, replace the current dma_declare_coherent_memory() based mechanism
with the creation of a genalloc pool that will be used in the OHCI
subsystem as replacement for the DMA APIs.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor 
---
 drivers/usb/host/ohci-sm501.c | 68 +--
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c
index c26228c25f99..1a25f5396e3e 100644
--- a/drivers/usb/host/ohci-sm501.c
+++ b/drivers/usb/host/ohci-sm501.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -92,6 +93,7 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device 
*pdev)
struct resource *res, *mem;
int retval, irq;
struct usb_hcd *hcd = NULL;
+   void __iomem *local_mem;
 
irq = retval = platform_get_irq(pdev, 0);
if (retval < 0)
@@ -110,40 +112,18 @@ static int ohci_hcd_sm501_drv_probe(struct 
platform_device *pdev)
goto err0;
}
 
-   /* The sm501 chip is equipped with local memory that may be used
-* by on-chip devices such as the video controller and the usb host.
-* This driver uses dma_declare_coherent_memory() to make sure
-* usb allocations with dma_alloc_coherent() allocate from
-* this local memory. The dma_handle returned by dma_alloc_coherent()
-* will be an offset starting from 0 for the first local memory byte.
-*
-* So as long as data is allocated using dma_alloc_coherent() all is
-* fine. This is however not always the case - buffers may be allocated
-* using kmalloc() - so the usb core needs to be told that it must copy
-* data into our local memory if the buffers happen to be placed in
-* regular memory. The HCD_LOCAL_MEM flag does just that.
-*/
-
-   retval = dma_declare_coherent_memory(dev, mem->start,
-mem->start - mem->parent->start,
-resource_size(mem));
-   if (retval) {
-   dev_err(dev, "cannot declare coherent memory\n");
-   goto err1;
-   }
-
/* allocate, reserve and remap resources for registers */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res == NULL) {
dev_err(dev, "no resource definition for registers\n");
retval = -ENOENT;
-   goto err2;
+   goto err1;
}
 
hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd) {
retval = -ENOMEM;
-   goto err2;
+   goto err1;
}
 
hcd->rsrc_start = res->start;
@@ -164,6 +144,43 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device 
*pdev)
 
ohci_hcd_init(hcd_to_ohci(hcd));
 
+   /* The sm501 chip is equipped with local memory that may be used
+* by on-chip devices such as the video controller and the usb host.
+* This driver uses genalloc so that usb allocations with
+* gen_pool_dma_alloc() allocate from this local memory. The dma_handle
+* returned by gen_pool_dma_alloc() will be an offset starting from 0
+* for the first local memory byte.
+*
+* So as long as data is allocated using gen_pool_dma_alloc() all is
+* fine. This is however not always the case - buffers may be allocated
+* using kmalloc() - so the usb core needs to be told that it must copy
+* data into our local memory if the buffers happen to be placed in
+* regular memory. The HCD_LOCAL_MEM flag does just that.
+*/
+
+   hcd->localmem_pool = devm_gen_pool_create(dev, PAGE_SHIFT,
+ dev_to_node(dev),
+ "ohci-sm501");
+   if (IS_ERR(hcd->localmem_pool)) {
+   retval = PTR_ERR(hcd->localmem_pool);
+   goto err5;
+   }
+
+   local_mem = devm_ioremap(dev, mem->start, resource_size(mem));
+   if (!local_mem) {
+   retval = -ENOMEM;
+   goto err5;
+   }
+
+   retval = gen_pool_add_virt(hcd->localmem_pool,
+  (unsigned long)local_mem,
+  mem->start - mem->parent->start,
+  resource_size(mem),
+  dev_to_node(dev));
+   if (retval < 0) {
+   dev_err(dev, "failed to add to pool: %d\n", retval);
+   goto err5;
+   }
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (retval)
goto err5;
@@ -181,8 +198,6 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device 
*pdev)

[PATCH v4 3/3] usb: host: ohci-tmio: init genalloc for local memory

2019-05-16 Thread laurentiu . tudor 
From: Laurentiu Tudor 

In preparation for dropping the existing "coherent" dma mem declaration
APIs, replace the current dma_declare_coherent_memory() based mechanism
with the creation of a genalloc pool that will be used in the OHCI
subsystem as replacement for the DMA APIs.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor 
---
 drivers/usb/host/ohci-tmio.c | 32 
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ohci-tmio.c b/drivers/usb/host/ohci-tmio.c
index f88a0370659f..1c39099b9870 100644
--- a/drivers/usb/host/ohci-tmio.c
+++ b/drivers/usb/host/ohci-tmio.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*-*/
 
@@ -188,6 +189,7 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device 
*dev)
struct resource *config = platform_get_resource(dev, IORESOURCE_MEM, 1);
struct resource *sram = platform_get_resource(dev, IORESOURCE_MEM, 2);
int irq = platform_get_irq(dev, 0);
+   void __iomem *local_mem;
struct tmio_hcd *tmio;
struct ohci_hcd *ohci;
struct usb_hcd *hcd;
@@ -224,11 +226,6 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device 
*dev)
goto err_ioremap_regs;
}
 
-   ret = dma_declare_coherent_memory(&dev->dev, sram->start, sram->start,
-   resource_size(sram));
-   if (ret)
-   goto err_dma_declare;
-
if (cell->enable) {
ret = cell->enable(dev);
if (ret)
@@ -239,6 +236,28 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device 
*dev)
ohci = hcd_to_ohci(hcd);
ohci_hcd_init(ohci);
 
+   hcd->localmem_pool = devm_gen_pool_create(&dev->dev, PAGE_SHIFT,
+ dev_to_node(&dev->dev),
+ "ohci-sm501");
+   if (IS_ERR(hcd->localmem_pool)) {
+   ret = PTR_ERR(hcd->localmem_pool);
+   goto err_enable;
+   }
+
+   local_mem = devm_ioremap(&dev->dev, sram->start, resource_size(sram));
+   if (!local_mem) {
+   ret = -ENOMEM;
+   goto err_enable;
+   }
+
+   ret = gen_pool_add_virt(hcd->localmem_pool, (unsigned long)local_mem,
+   sram->start, resource_size(sram),
+   dev_to_node(&dev->dev));
+   if (ret < 0) {
+   dev_err(&dev->dev, "failed to add to pool: %d\n", ret);
+   goto err_enable;
+   }
+
ret = usb_add_hcd(hcd, irq, 0);
if (ret)
goto err_add_hcd;
@@ -254,8 +273,6 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device 
*dev)
if (cell->disable)
cell->disable(dev);
 err_enable:
-   dma_release_declared_memory(&dev->dev);
-err_dma_declare:
iounmap(hcd->regs);
 err_ioremap_regs:
iounmap(tmio->ccr);
@@ -276,7 +293,6 @@ static int ohci_hcd_tmio_drv_remove(struct platform_device 
*dev)
tmio_stop_hc(dev);
if (cell->disable)
cell->disable(dev);
-   dma_release_declared_memory(&dev->dev);
iounmap(hcd->regs);
iounmap(tmio->ccr);
usb_put_hcd(hcd);
-- 
2.17.1

[PATCH v4 1/3] USB: use genalloc for USB HCs with local memory

2019-05-16 Thread laurentiu . tudor 
From: Laurentiu Tudor 

For HCs that have local memory, replace the current DMA API usage
with a genalloc generic allocator to manage the mappings for these
devices.
This is in preparation for dropping the existing "coherent" dma
mem declaration APIs. Current implementation was relying on a short
circuit in the DMA API that in the end, was acting as an allocator
for these type of devices.

For context, see thread here: https://lkml.org/lkml/2019/4/22/357

Signed-off-by: Laurentiu Tudor 
---
 drivers/usb/core/buffer.c   | 15 +++
 drivers/usb/host/ohci-hcd.c | 23 ++-
 include/linux/usb/hcd.h |  3 +++
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index f641342cdec0..22a8f3f5679b 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -124,10 +125,12 @@ void *hcd_buffer_alloc(
if (size == 0)
return NULL;
 
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);
+
/* some USB hosts just use PIO */
if (!IS_ENABLED(CONFIG_HAS_DMA) ||
-   (!is_device_dma_capable(bus->sysdev) &&
-!(hcd->driver->flags & HCD_LOCAL_MEM))) {
+   !is_device_dma_capable(bus->sysdev)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
@@ -152,9 +155,13 @@ void hcd_buffer_free(
if (!addr)
return;
 
+   if (hcd->driver->flags & HCD_LOCAL_MEM) {
+   gen_pool_free(hcd->localmem_pool, (unsigned long)addr, size);
+   return;
+   }
+
if (!IS_ENABLED(CONFIG_HAS_DMA) ||
-   (!is_device_dma_capable(bus->sysdev) &&
-!(hcd->driver->flags & HCD_LOCAL_MEM))) {
+   !is_device_dma_capable(bus->sysdev)) {
kfree(addr);
return;
}
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 210181fd98d2..f9462c372943 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -505,8 +506,15 @@ static int ohci_init (struct ohci_hcd *ohci)
timer_setup(&ohci->io_watchdog, io_watchdog_func, 0);
ohci->prev_frame_no = IO_WATCHDOG_OFF;
 
-   ohci->hcca = dma_alloc_coherent (hcd->self.controller,
-   sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL);
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   ohci->hcca = gen_pool_dma_alloc(hcd->localmem_pool,
+   sizeof(*ohci->hcca),
+   &ohci->hcca_dma);
+   else
+   ohci->hcca = dma_alloc_coherent(hcd->self.controller,
+   sizeof(*ohci->hcca),
+   &ohci->hcca_dma,
+   GFP_KERNEL);
if (!ohci->hcca)
return -ENOMEM;
 
@@ -990,9 +998,14 @@ static void ohci_stop (struct usb_hcd *hcd)
remove_debug_files (ohci);
ohci_mem_cleanup (ohci);
if (ohci->hcca) {
-   dma_free_coherent (hcd->self.controller,
-   sizeof *ohci->hcca,
-   ohci->hcca, ohci->hcca_dma);
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   gen_pool_free(hcd->localmem_pool,
+ (unsigned long)ohci->hcca,
+ sizeof(*ohci->hcca));
+   else
+   dma_free_coherent(hcd->self.controller,
+ sizeof(*ohci->hcca),
+ ohci->hcca, ohci->hcca_dma);
ohci->hcca = NULL;
ohci->hcca_dma = 0;
}
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index bb57b5af4700..1b0fc3cebc08 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -216,6 +216,9 @@ struct usb_hcd {
 #defineHC_IS_RUNNING(state) ((state) & __ACTIVE)
 #defineHC_IS_SUSPENDED(state) ((state) & __SUSPEND)
 
+   /* allocator for HCs having local memory */
+   struct gen_pool *localmem_pool;
+
/* more shared queuing code would be good; it should support
 * smarter scheduling, handle transaction translators, etc;
 * input size of periodic table to an interrupt scheduler.
-- 
2.17.1

Re: Linux crash when using FTDI FT232R USB to serial converter on AMD boards.

2019-05-16 Thread starost...@gmail.com 

Dne 16.5.2019 v 10:34 Oliver Neukum napsal(a):

Mention

1. kernel version
2. whether this is known to be a regression

Sorry for question, can you more specify what you mean?


3. include the log with iommu disabled and mention that you disabled
the IOMMU
4. Include the output of lsusb

HTH
Oliver

Thank you
starosta

[PATCH] USB: serial: mos7840: Prefer 'unsigned int' to bare use of 'unsigned'

2019-05-16 Thread parna . naveenkumar 
From: Naveen Kumar Parna 

This fixes checkpatch.pl warning "WARNING: Prefer 'unsigned int' to
bare use of 'unsigned'".

Signed-off-by: Naveen Kumar Parna 
---
 drivers/usb/serial/mos7840.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index a698d46ba773..a610af4dea3f 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -1682,7 +1682,7 @@ static void mos7840_change_port_settings(struct 
tty_struct *tty,
struct moschip_port *mos7840_port, struct ktermios *old_termios)
 {
int baud;
-   unsigned cflag;
+   unsigned int cflag;
__u8 lData;
__u8 lParity;
__u8 lStop;
-- 
2.17.1

Re: Linux crash when using FTDI FT232R USB to serial converter on AMD boards.

2019-05-16 Thread Oliver Neukum 
On Do, 2019-05-16 at 14:29 +0200,  starost...@gmail.com  wrote:
> Dne 16.5.2019 v 10:34 Oliver Neukum napsal(a):
> > Mention
> > 
> > 1. kernel version
> > 2. whether this is known to be a regression
> 
> Sorry for question, can you more specify what you mean?

You will be asked whether this worked in earlier version of the
kernel. The answer would be: yes, no, unknown (why)

HTH
Oliver

Kernel crash with FTDI FT232R on AMD boards.

2019-05-16 Thread starost...@gmail.com 

Hello,
when I try to read EEPROM memory from FT232R chip (USB to serial 
converter), system crash after a seconds.


1) Configuration
ASUS PRIME A320M-K, latest bios version 4801, default settings.
Ubuntu server 19.04 with kernel 5.1.1-050101-generic: 
https://kernel.ubuntu.com/~kernel-ppa/mainline/v5.1.1/


2) lsusboutput
Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 002: ID 04f3:0103 Elan Microelectronics Corp. ActiveJet 
K-2024 Multimedia Keyboard

Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 002: ID 0403:6001 Future Technology Devices 
International, Ltd FT232 Serial (UART) IC

Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

3) kern.log with IOMMU disabled
https://paste.ee/p/nuCPp

4) Notes
- problem is, when FT232R is connected to USB2.0 port, when is connected 
to USB3 port, this works fine
- another test - same hardware, Debian 9.8 with kernel 4.19 - system 
crash too


-- starosta
May 16 15:06:45 test-ubnt kernel: [0.00] Linux version 
5.1.1-050101-generic (kernel@kathleen) (gcc version 8.3.0 (Ubuntu 
8.3.0-12ubuntu1)) #201905110631 SMP Sat May 11 06:33:50 UTC 2019
May 16 15:06:45 test-ubnt kernel: [0.00] Command line: 
BOOT_IMAGE=/boot/vmlinuz-5.1.1-050101-generic 
root=UUID=91232716-4b81-4e1d-9d58-c2ad4d090e93 ro
May 16 15:06:45 test-ubnt kernel: [0.00] KERNEL supported cpus:
May 16 15:06:45 test-ubnt kernel: [0.00]   Intel GenuineIntel
May 16 15:06:45 test-ubnt kernel: [0.00]   AMD AuthenticAMD
May 16 15:06:45 test-ubnt kernel: [0.00]   Hygon HygonGenuine
May 16 15:06:45 test-ubnt kernel: [0.00]   Centaur CentaurHauls
May 16 15:06:45 test-ubnt kernel: [0.00] x86/fpu: Supporting XSAVE 
feature 0x001: 'x87 floating point registers'
May 16 15:06:45 test-ubnt kernel: [0.00] x86/fpu: Supporting XSAVE 
feature 0x002: 'SSE registers'
May 16 15:06:45 test-ubnt kernel: [0.00] x86/fpu: Supporting XSAVE 
feature 0x004: 'AVX registers'
May 16 15:06:45 test-ubnt kernel: [0.00] x86/fpu: xstate_offset[2]:  
576, xstate_sizes[2]:  256
May 16 15:06:45 test-ubnt kernel: [0.00] x86/fpu: Enabled xstate 
features 0x7, context size is 832 bytes, using 'compacted' format.
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-provided physical RAM map:
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x-0x0009] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x000a-0x000f] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x0010-0x09e0] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x09e1-0x09ff] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x0a00-0x0a1f] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x0a20-0x0a20afff] ACPI NVS
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x0a20b000-0x0aff] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x0b00-0x0b01] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x0b02-0xb198bfff] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xb198c000-0xb19a5fff] ACPI data
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xb19a6000-0xba7f3fff] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xba7f4000-0xba8d] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xba8e-0xba8ecfff] ACPI data
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xba8ed000-0xba9eefff] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xba9ef000-0xbada6fff] ACPI NVS
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xbada7000-0xbbaadfff] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xbbaae000-0xbbb4efff] type 20
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xbbb4f000-0xbdff] usable
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xbe00-0xdfff] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xf800-0xfbff] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0xfd10-0xfd1f] reserved
May 16 15:06:45 test-ubnt kernel: [0.00] BIOS-e820: [mem 
0x000

Re: Linux crash when using FTDI FT232R USB to serial converter on AMD boards.

2019-05-16 Thread starost...@gmail.com 

Dne 16.5.2019 v 15:11 Oliver Neukum napsal(a):


You will be asked whether this worked in earlier version of the
kernel. The answer would be: yes, no, unknown (why)

HTH
Oliver


Thank you, I sent new report.
starosta

Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread Fredrik Noring 
Hi Laurentiu,

> > The kernel oopses with "unable to handle kernel paging request at virtual
> > address 000aba0b" in hcd_alloc_coherent via usb_hcd_map_urb_for_dma. 
> 
> By any chance, does this address looks like the dma_addr that the device 
> should target?

Yes, that looks like a typical device address suitable for its DMA.

> Actually, I think I'm misusing genalloc and also it appears that i need 
> to add a mapping on the phys address. So my plan is to change the 
> "unsigned long virt" to be the void * returned by the mapping operation 
> and the phys_addr_t be the dma_addr_t. I'll return with a patch.

Great, I'm happy to test your next patch revision.

Fredrik

Re: Kernel crash with FTDI FT232R on AMD boards.

2019-05-16 Thread Johan Hovold 
On Thu, May 16, 2019 at 03:35:42PM +0200, starost...@gmail.com wrote:
> Hello,
> when I try to read EEPROM memory from FT232R chip (USB to serial 
> converter), system crash after a seconds.

You should mention that you're using libusb and the vendor's ftdi
library. Specifically, the kernels ftdi_sio driver is not involved.

> 1) Configuration
> ASUS PRIME A320M-K, latest bios version 4801, default settings.
> Ubuntu server 19.04 with kernel 5.1.1-050101-generic: 
> https://kernel.ubuntu.com/~kernel-ppa/mainline/v5.1.1/
> 
> 2) lsusboutput
> Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 003 Device 002: ID 04f3:0103 Elan Microelectronics Corp. ActiveJet 
> K-2024 Multimedia Keyboard
> Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 001 Device 002: ID 0403:6001 Future Technology Devices 
> International, Ltd FT232 Serial (UART) IC
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> 
> 3) kern.log with IOMMU disabled
> https://paste.ee/p/nuCPp

Good that you were able to rule out the iommu, but you forgot to CC the
xhci maintainer (added Mathias again).

It could be good idea to include a link to thread with your initial
report as well:

https://lkml.kernel.org/r/04503197-a0a9-8b35-6c65-c10f296aa...@gmail.com

> 4) Notes
> - problem is, when FT232R is connected to USB2.0 port, when is connected 
> to USB3 port, this works fine
> - another test - same hardware, Debian 9.8 with kernel 4.19 - system 
> crash too
> 
> -- starosta

Including only the obviously relevant part of your log below.

> May 16 15:06:45 test-ubnt kernel: [0.00] Linux version 
> 5.1.1-050101-generic (kernel@kathleen) (gcc version 8.3.0 (Ubuntu 
> 8.3.0-12ubuntu1)) #201905110631 SMP Sat May 11 06:33:50 UTC 2019

...

> May 16 15:07:03 test-ubnt kernel: [   30.042564] usbserial: USB Serial 
> deregistering driver FTDI USB Serial Device
> May 16 15:07:03 test-ubnt kernel: [   30.042759] ftdi_sio ttyUSB0: FTDI USB 
> Serial Device converter now disconnected from ttyUSB0
> May 16 15:07:03 test-ubnt kernel: [   30.042792] usbcore: deregistering 
> interface driver ftdi_sio
> May 16 15:07:03 test-ubnt kernel: [   30.042842] ftdi_sio 1-9:1.0: device 
> disconnected
> May 16 15:07:54 test-ubnt kernel: [   81.751630] xhci_hcd :01:00.0: WARN 
> Set TR Deq Ptr cmd failed due to incorrect slot or ep state.
> May 16 15:07:54 test-ubnt kernel: [   81.990340] general protection fault: 
>  [#1] SMP NOPTI
> May 16 15:07:54 test-ubnt kernel: [   81.993345] CPU: 3 PID: 1058 Comm: 
> readua Not tainted 5.1.1-050101-generic #201905110631
> May 16 15:07:54 test-ubnt kernel: [   81.996509] Hardware name: System 
> manufacturer System Product Name/PRIME A320M-K, BIOS 4801 04/25/2019
> May 16 15:07:54 test-ubnt kernel: [   81.998598] RIP: 
> 0010:__kmalloc+0xa5/0x220
> May 16 15:07:54 test-ubnt kernel: [   82.000379] Code: 65 49 8b 50 08 65 4c 
> 03 05 90 c5 d8 54 4d 8b 38 4d 85 ff 0f 84 2e 01 00 00 41 8b 59 20 49 8b 39 48 
> 8d 4a 01 4c 89 f8 4c 01 fb <48> 33 1b 49 33 99 38 01 00 00 65 48 0f c7 0f 0f 
> 94 c0 84 c0 74 bd
> May 16 15:07:54 test-ubnt kernel: [   82.004031] RSP: 0018:b415c0f1fcd0 
> EFLAGS: 00010202
> May 16 15:07:54 test-ubnt kernel: [   82.005847] RAX: 74536432697a6001 RBX: 
> 74536432697a6001 RCX: 4a47
> May 16 15:07:54 test-ubnt kernel: [   82.007654] RDX: 4a46 RSI: 
> 0cc0 RDI: 000281a0
> May 16 15:07:54 test-ubnt kernel: [   82.009463] RBP: b415c0f1fd00 R08: 
> 9e259aee81a0 R09: 9e259a806b80
> May 16 15:07:54 test-ubnt kernel: [   82.011277] R10: 9e258ac616f0 R11: 
> 9e2591f3a400 R12: 0cc0
> May 16 15:07:54 test-ubnt kernel: [   82.013085] R13: 1000 R14: 
> 9e259a806b80 R15: 74536432697a6001
> May 16 15:07:54 test-ubnt kernel: [   82.014878] FS:  () 
> GS:9e259aec(0063) knlGS:f6bbeb40
> May 16 15:07:54 test-ubnt kernel: [   82.016670] CS:  0010 DS: 002b ES: 002b 
> CR0: 80050033
> May 16 15:07:54 test-ubnt kernel: [   82.018450] CR2: f7d23eac CR3: 
> 000115b2c000 CR4: 003406e0
> May 16 15:07:54 test-ubnt kernel: [   82.020233] Call Trace:
> May 16 15:07:54 test-ubnt kernel: [   82.021995]  ? 
> proc_do_submiturb+0xaf1/0xc70
> May 16 15:07:54 test-ubnt kernel: [   82.023748]  
> proc_do_submiturb+0xaf1/0xc70
> May 16 15:07:54 test-ubnt kernel: [   82.025486]  
> proc_submiturb_compat+0x81/0xb0
> May 16 15:07:54 test-ubnt kernel: [   82.027226]  usbdev_do_ioctl+0x930/0xd70
> May 16 15:07:54 test-ubnt kernel: [   82.028964]  ? call_rwsem_wake+0x1b/0x30
> May 16 15:07:54 test-ubnt kernel: [   82.030681]  ? _copy_from_user+0x3e/0x60
> May 16 15:07:54 test-ubnt kernel: [   82.032359]  
> usbdev_compat_ioctl+0x10/0x20
> Ma

Re: Kernel crash with FTDI FT232R on AMD boards.

2019-05-16 Thread Mathias Nyman 

On 16.5.2019 16.56, Johan Hovold wrote:

On Thu, May 16, 2019 at 03:35:42PM +0200, starost...@gmail.com wrote:

Hello,
when I try to read EEPROM memory from FT232R chip (USB to serial
converter), system crash after a seconds.


You should mention that you're using libusb and the vendor's ftdi
library. Specifically, the kernels ftdi_sio driver is not involved.


1) Configuration
ASUS PRIME A320M-K, latest bios version 4801, default settings.
Ubuntu server 19.04 with kernel 5.1.1-050101-generic:
https://kernel.ubuntu.com/~kernel-ppa/mainline/v5.1.1/

2) lsusboutput
Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 002: ID 04f3:0103 Elan Microelectronics Corp. ActiveJet
K-2024 Multimedia Keyboard
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 002: ID 0403:6001 Future Technology Devices
International, Ltd FT232 Serial (UART) IC
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

3) kern.log with IOMMU disabled
https://paste.ee/p/nuCPp


Good that you were able to rule out the iommu, but you forgot to CC the
xhci maintainer (added Mathias again).

It could be good idea to include a link to thread with your initial
report as well:

https://lkml.kernel.org/r/04503197-a0a9-8b35-6c65-c10f296aa...@gmail.com


4) Notes
- problem is, when FT232R is connected to USB2.0 port, when is connected
to USB3 port, this works fine
- another test - same hardware, Debian 9.8 with kernel 4.19 - system
crash too

-- starosta


Including only the obviously relevant part of your log below.


May 16 15:06:45 test-ubnt kernel: [0.00] Linux version 
5.1.1-050101-generic (kernel@kathleen) (gcc version 8.3.0 (Ubuntu 
8.3.0-12ubuntu1)) #201905110631 SMP Sat May 11 06:33:50 UTC 2019


...


May 16 15:07:03 test-ubnt kernel: [   30.042564] usbserial: USB Serial 
deregistering driver FTDI USB Serial Device
May 16 15:07:03 test-ubnt kernel: [   30.042759] ftdi_sio ttyUSB0: FTDI USB 
Serial Device converter now disconnected from ttyUSB0
May 16 15:07:03 test-ubnt kernel: [   30.042792] usbcore: deregistering 
interface driver ftdi_sio
May 16 15:07:03 test-ubnt kernel: [   30.042842] ftdi_sio 1-9:1.0: device 
disconnected
May 16 15:07:54 test-ubnt kernel: [   81.751630] xhci_hcd :01:00.0: WARN 
Set TR Deq Ptr cmd failed due to incorrect slot or ep state.


logs and traces from xhci could tell more details about what is going on,
This line indicates a URB cancellation issues.

To get xhci traces and logs please do:

mount -t debugfs none /sys/kernel/debug
echo 'module xhci_hcd =p' >/sys/kernel/debug/dynamic_debug/control
echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control
echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable
< read EEPROM using the usb serial converter >
Send output of dmesg
Send content of /sys/kernel/debug/tracing/trace

-Mathias

[PATCH v10 2/2] usb: xhci: Add Clear_TT_Buffer

2019-05-16 Thread Jim Lin 
USB 2.0 specification chapter 11.17.5 says "as part of endpoint halt
processing for full-/low-speed endpoints connected via a TT, the host
software must use the Clear_TT_Buffer request to the TT to ensure
that the buffer is not in the busy state".

In our case, a full-speed speaker (ConferenceCam) is behind a high-
speed hub (ConferenceCam Connect), sometimes once we get STALL on a
request we may continue to get STALL with the folllowing requests,
like Set_Interface.

Here we invoke usb_hub_clear_tt_buffer() to send Clear_TT_Buffer
request to the hub of the device for the following Set_Interface
requests to the device to get ACK successfully.

Signed-off-by: Jim Lin 
---
v2: xhci_clear_tt_buffer_complete: add static, shorter indentation
, remove its claiming in xhci.h
v3: Add description for clearing_tt (xhci.h)
v4: Remove clearing_tt flag because hub_tt_work has hub->tt.lock
to protect for Clear_TT_Buffer to be run serially.
Remove xhci_clear_tt_buffer_complete as it's not necessary.
Same reason as the above.
Extend usb_hub_clear_tt_buffer parameter
v5: Not extending usb_hub_clear_tt_buffer parameter
Add description.
v6: Remove unused parameter slot_id from xhci_clear_hub_tt_buffer
v7: Add devaddr field in "struct usb_device"
v8: split as two patches
v9: no change flag
v10: Add EP_CLEARING_TT flag

 drivers/usb/host/xhci-ring.c | 27 ++-
 drivers/usb/host/xhci.c  | 17 +
 drivers/usb/host/xhci.h  |  5 +
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9215a28dad40..11d498b6c09b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -399,7 +399,7 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci,
 * stream once the endpoint is on the HW schedule.
 */
if ((ep_state & EP_STOP_CMD_PENDING) || (ep_state & SET_DEQ_PENDING) ||
-   (ep_state & EP_HALTED))
+   (ep_state & EP_HALTED) || (ep_state & EP_CLEARING_TT))
return;
writel(DB_VALUE(ep_index, stream_id), db_addr);
/* The CPU has better things to do at this point than wait for a
@@ -433,6 +433,13 @@ static void ring_doorbell_for_active_rings(struct xhci_hcd 
*xhci,
}
 }
 
+void xhci_ring_doorbell_for_active_rings(struct xhci_hcd *xhci,
+   unsigned int slot_id,
+   unsigned int ep_index)
+{
+   ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
+}
+
 /* Get the right ring for the given slot_id, ep_index and stream_id.
  * If the endpoint supports streams, boundary check the URB's stream ID.
  * If the endpoint doesn't support streams, return the singular endpoint ring.
@@ -1786,6 +1793,23 @@ struct xhci_segment *trb_in_td(struct xhci_hcd *xhci,
return NULL;
 }
 
+static void xhci_clear_hub_tt_buffer(struct xhci_hcd *xhci, struct xhci_td *td,
+   struct xhci_virt_ep *ep)
+{
+   /*
+* As part of low/full-speed endpoint-halt processing
+* we must clear the TT buffer (USB 2.0 specification 11.17.5).
+*/
+   if (td->urb->dev->tt && !usb_pipeint(td->urb->pipe) &&
+   (td->urb->dev->tt->hub != xhci_to_hcd(xhci)->self.root_hub) &&
+   !(ep->ep_state & EP_CLEARING_TT)) {
+   ep->ep_state |= EP_CLEARING_TT;
+   td->urb->ep->hcpriv = td->urb->dev;
+   if (usb_hub_clear_tt_buffer(td->urb))
+   ep->ep_state &= ~EP_CLEARING_TT;
+   }
+}
+
 static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci,
unsigned int slot_id, unsigned int ep_index,
unsigned int stream_id, struct xhci_td *td,
@@ -1804,6 +1828,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd 
*xhci,
if (reset_type == EP_HARD_RESET) {
ep->ep_state |= EP_HARD_CLEAR_TOGGLE;
xhci_cleanup_stalled_ring(xhci, ep_index, stream_id, td);
+   xhci_clear_hub_tt_buffer(xhci, td, ep);
}
xhci_ring_cmd_db(xhci);
 }
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 68b393e5a453..0dc108e61a89 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5133,6 +5133,22 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
 }
 EXPORT_SYMBOL_GPL(xhci_gen_setup);
 
+static void xhci_clear_tt_buffer_complete(struct usb_hcd *hcd,
+   struct usb_host_endpoint *ep)
+{
+   struct xhci_hcd *xhci;
+   struct usb_device *udev;
+   unsigned int slot_id;
+   unsigned int ep_index;
+
+   xhci = hcd_to_xhci(hcd);
+   udev = (struct usb_device *)ep->hcpriv;
+   slot_id = udev->slot_id;
+   ep_index = xhci_get_endpoint_index(&ep->desc);
+   xhci->devs[slot_id]->eps[ep_index].ep_state &= ~EP_CLEARING_TT;
+   xhci_ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
+}
+
 static const struct hc_driver xhci_hc_driver =

[PATCH v10 1/2] usb: xhci : Add devaddr in struct usb_device

2019-05-16 Thread Jim Lin 
The Clear_TT_Buffer request sent to the hub includes the address of
the LS/FS child device in wValue field. usb_hub_clear_tt_buffer()
uses udev->devnum to set the address wValue. This won't work for
devices connected to xHC.

For other host controllers udev->devnum is the same as the address of
the usb device, chosen and set by usb core. With xHC the controller
hardware assigns the address, and won't be the same as devnum.

Here we add devaddr in "struct usb_device" for
usb_hub_clear_tt_buffer() to use.

Signed-off-by: Jim Lin 
---
v2: xhci_clear_tt_buffer_complete: add static, shorter indentation
, remove its claiming in xhci.h
v3: Add description for clearing_tt (xhci.h)
v4: Remove clearing_tt flag because hub_tt_work has hub->tt.lock
to protect for Clear_TT_Buffer to be run serially.
Remove xhci_clear_tt_buffer_complete as it's not necessary.
Same reason as the above.
Extend usb_hub_clear_tt_buffer parameter
v5: Not extending usb_hub_clear_tt_buffer parameter
Add description.
v6: Remove unused parameter slot_id from xhci_clear_hub_tt_buffer
v7: Add devaddr field in "struct usb_device"
v8: split as two patches, change type from int to u8 for devaddr.
v9: Use pahole to find place to put devaddr in struct usb_device.
Remove space between type cast and variable.
hub.c changed from v8
clear->devinfo |= (u16) (udev->devaddr << 4);
to 
clear->devinfo |= ((u16)udev->devaddr) << 4;
to solve a problem if devaddr is larger than 16.
v10 Initialize devaddr in xhci_setup_device()
Move devaddr to be below "u8 level"

 drivers/usb/core/hub.c  | 4 +++-
 drivers/usb/host/xhci.c | 1 +
 include/linux/usb.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 15a2934dc29d..0d4b289be103 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -873,7 +873,7 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
/* info that CLEAR_TT_BUFFER needs */
clear->tt = tt->multi ? udev->ttport : 1;
clear->devinfo = usb_pipeendpoint (pipe);
-   clear->devinfo |= udev->devnum << 4;
+   clear->devinfo |= ((u16)udev->devaddr) << 4;
clear->devinfo |= usb_pipecontrol(pipe)
? (USB_ENDPOINT_XFER_CONTROL << 11)
: (USB_ENDPOINT_XFER_BULK << 11);
@@ -2125,6 +2125,8 @@ static void update_devnum(struct usb_device *udev, int 
devnum)
/* The address for a WUSB device is managed by wusbcore. */
if (!udev->wusb)
udev->devnum = devnum;
+   if (!udev->devaddr)
+   udev->devaddr = (u8)devnum;
 }
 
 static void hub_free_dev(struct usb_device *udev)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 7fa58c99f126..68b393e5a453 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4096,6 +4096,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct 
usb_device *udev,
/* Zero the input context control for later use */
ctrl_ctx->add_flags = 0;
ctrl_ctx->drop_flags = 0;
+   udev->devaddr = (u8)(le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK);
 
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
   "Internal device address = %d",
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 4229eb74bd2c..af68e31118f8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -580,6 +580,7 @@ struct usb3_lpm_parameters {
  * @bus_mA: Current available from the bus
  * @portnum: parent port number (origin 1)
  * @level: number of USB hub ancestors
+ * @devaddr: device address, XHCI: assigned by HW, others: same as devnum
  * @can_submit: URBs may be submitted
  * @persist_enabled:  USB_PERSIST enabled for this device
  * @have_langid: whether string_langid is valid
@@ -663,6 +664,7 @@ struct usb_device {
unsigned short bus_mA;
u8 portnum;
u8 level;
+   u8 devaddr;
 
unsigned can_submit:1;
unsigned persist_enabled:1;
-- 
2.1.4

[PATCH v10 0/2] usb: xhci: Add Clear_TT_Buffer

2019-05-16 Thread Jim Lin 
USB 2.0 specification chapter 11.17.5 says "as part of endpoint halt
processing for full-/low-speed endpoints connected via a TT, the host
software must use the Clear_TT_Buffer request to the TT to ensure
that the buffer is not in the busy state".

In our case, a full-speed speaker (ConferenceCam) is behind a high-
speed hub (ConferenceCam Connect), sometimes once we get STALL on a
request we may continue to get STALL with the folllowing requests,
like Set_Interface.

Solution is to invoke usb_hub_clear_tt_buffer() to send
Clear_TT_Buffer request to the hub of the device for the following
Set_Interface requests to the device to get ACK successfully.

The Clear_TT_Buffer request sent to the hub includes the address of
the LS/FS child device in wValue field. usb_hub_clear_tt_buffer()
uses udev->devnum to set the address wValue. This won't work for
devices connected to xHC.

For other host controllers udev->devnum is the same as the address of
the usb device, chosen and set by usb core. With xHC the controller
hardware assigns the address, and won't be the same as devnum.

Here we have two patches.
One is to add devaddr in struct usb_device for
usb_hub_clear_tt_buffer() to use.
Another is to invoke usb_hub_clear_tt_buffer() for halt processing.
 
Signed-off-by: Jim Lin 

Jim Lin (2):
  usb: xhci : Add devaddr in struct usb_device
  usb: xhci: Add Clear_TT_Buffer

 drivers/usb/core/hub.c   |  4 +++-
 drivers/usb/host/xhci-ring.c | 27 ++-
 drivers/usb/host/xhci.c  | 18 ++
 drivers/usb/host/xhci.h  |  5 +
 include/linux/usb.h  |  2 ++
 5 files changed, 54 insertions(+), 2 deletions(-)

-- 
2.1.4

Re: [PATCH v10 1/2] usb: xhci : Add devaddr in struct usb_device

2019-05-16 Thread Alan Stern 
On Thu, 16 May 2019, Jim Lin wrote:

> The Clear_TT_Buffer request sent to the hub includes the address of
> the LS/FS child device in wValue field. usb_hub_clear_tt_buffer()
> uses udev->devnum to set the address wValue. This won't work for
> devices connected to xHC.
> 
> For other host controllers udev->devnum is the same as the address of
> the usb device, chosen and set by usb core. With xHC the controller
> hardware assigns the address, and won't be the same as devnum.
> 
> Here we add devaddr in "struct usb_device" for
> usb_hub_clear_tt_buffer() to use.
> 
> Signed-off-by: Jim Lin 
> ---

Aside from the "xhci:" part of the Subject line (it's not really 
appropriate because this is a modification of the USB core more than of 
the xhci-hcd driver),

Acked-by: Alan Stern 


> v2: xhci_clear_tt_buffer_complete: add static, shorter indentation
> , remove its claiming in xhci.h
> v3: Add description for clearing_tt (xhci.h)
> v4: Remove clearing_tt flag because hub_tt_work has hub->tt.lock
> to protect for Clear_TT_Buffer to be run serially.
> Remove xhci_clear_tt_buffer_complete as it's not necessary.
> Same reason as the above.
> Extend usb_hub_clear_tt_buffer parameter
> v5: Not extending usb_hub_clear_tt_buffer parameter
> Add description.
> v6: Remove unused parameter slot_id from xhci_clear_hub_tt_buffer
> v7: Add devaddr field in "struct usb_device"
> v8: split as two patches, change type from int to u8 for devaddr.
> v9: Use pahole to find place to put devaddr in struct usb_device.
> Remove space between type cast and variable.
> hub.c changed from v8
> clear->devinfo |= (u16) (udev->devaddr << 4);
> to 
> clear->devinfo |= ((u16)udev->devaddr) << 4;
> to solve a problem if devaddr is larger than 16.
> v10 Initialize devaddr in xhci_setup_device()
> Move devaddr to be below "u8 level"
> 
>  drivers/usb/core/hub.c  | 4 +++-
>  drivers/usb/host/xhci.c | 1 +
>  include/linux/usb.h | 2 ++
>  3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 15a2934dc29d..0d4b289be103 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -873,7 +873,7 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
>   /* info that CLEAR_TT_BUFFER needs */
>   clear->tt = tt->multi ? udev->ttport : 1;
>   clear->devinfo = usb_pipeendpoint (pipe);
> - clear->devinfo |= udev->devnum << 4;
> + clear->devinfo |= ((u16)udev->devaddr) << 4;
>   clear->devinfo |= usb_pipecontrol(pipe)
>   ? (USB_ENDPOINT_XFER_CONTROL << 11)
>   : (USB_ENDPOINT_XFER_BULK << 11);
> @@ -2125,6 +2125,8 @@ static void update_devnum(struct usb_device *udev, int 
> devnum)
>   /* The address for a WUSB device is managed by wusbcore. */
>   if (!udev->wusb)
>   udev->devnum = devnum;
> + if (!udev->devaddr)
> + udev->devaddr = (u8)devnum;
>  }
>  
>  static void hub_free_dev(struct usb_device *udev)
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 7fa58c99f126..68b393e5a453 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4096,6 +4096,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, 
> struct usb_device *udev,
>   /* Zero the input context control for later use */
>   ctrl_ctx->add_flags = 0;
>   ctrl_ctx->drop_flags = 0;
> + udev->devaddr = (u8)(le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK);
>  
>   xhci_dbg_trace(xhci, trace_xhci_dbg_address,
>  "Internal device address = %d",
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 4229eb74bd2c..af68e31118f8 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -580,6 +580,7 @@ struct usb3_lpm_parameters {
>   * @bus_mA: Current available from the bus
>   * @portnum: parent port number (origin 1)
>   * @level: number of USB hub ancestors
> + * @devaddr: device address, XHCI: assigned by HW, others: same as devnum
>   * @can_submit: URBs may be submitted
>   * @persist_enabled:  USB_PERSIST enabled for this device
>   * @have_langid: whether string_langid is valid
> @@ -663,6 +664,7 @@ struct usb_device {
>   unsigned short bus_mA;
>   u8 portnum;
>   u8 level;
> + u8 devaddr;
>  
>   unsigned can_submit:1;
>   unsigned persist_enabled:1;
>

[PATCH] USB: Add LPM quirk for Surface Dock GigE adapter

2019-05-16 Thread Maximilian Luz 
Without USB_QUIRK_NO_LPM ethernet will not work and rtl8152 will
complain with

r8152 : Stop submitting intr, status -71

Adding the quirk resolves this. As the dock is externally powered, this
should not have any drawbacks.

Signed-off-by: Maximilian Luz 
---
 drivers/usb/core/quirks.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 8bc35d53408b..6082b008969b 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -209,6 +209,9 @@ static const struct usb_device_id usb_quirk_list[] = {
/* Microsoft LifeCam-VX700 v2.0 */
{ USB_DEVICE(0x045e, 0x0770), .driver_info = USB_QUIRK_RESET_RESUME },
 
+   /* Microsoft Surface Dock Ethernet (RTL8153 GigE) */
+   { USB_DEVICE(0x045e, 0x07c6), .driver_info = USB_QUIRK_NO_LPM },
+
/* Cherry Stream G230 2.0 (G85-231) and 3.0 (G85-232) */
{ USB_DEVICE(0x046a, 0x0023), .driver_info = USB_QUIRK_RESET_RESUME },
 
-- 
2.21.0

Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread Fredrik Noring 
Hi Laurentiu,

> I took your code, added the missing mapping and placed it in a patch. 
> Please see attached (compile tested only).

Thanks! Unfortunately, the OHCI fails with errors such as

usb 1-1: device descriptor read/64, error -12

that I tracked down to the calls

   hub_port_init
-> usb_control_msg
-> usb_internal_control_msg
-> usb_start_wait_urb
-> usb_submit_urb
-> usb_hcd_submit_urb
-> hcd->driver->urb_enqueue
-> ohci_urb_enqueue
-> ed_get
-> ed_alloc
-> dma_pool_zalloc
-> dma_pool_alloc
-> pool_alloc_page,

which returns NULL. Then I noticed

/* pool_alloc_page() might sleep, so temporarily drop &pool->lock */

that might be a problem considering that the HCD handles pool memory in
IRQ handlers, for instance:

   do_IRQ
-> generic_handle_irq
-> handle_level_irq
-> handle_irq_event
-> handle_irq_event_percpu
-> __handle_irq_event_percpu
-> usb_hcd_irq
-> ohci_irq
-> ohci_work
-> finish_urb
-> __usb_hcd_giveback_urb
-> usb_hcd_unmap_urb_for_dma
-> hcd_buffer_free

Also, DMA_BUFFER_SIZE in ohci-ps2.c is only 256 KiB in total. Is the new
pool implementation at least as efficient as the previous one?

Fredrik

[PATCH] USB: OHCI: remove space before open square bracket '['

2019-05-16 Thread parna . naveenkumar 
From: Naveen Kumar Parna 

This patch removes following checkpatch.pl error in usb/host/ohci-pci.c file.
ERROR: space prohibited before open square bracket '['

Signed-off-by: Naveen Kumar Parna 
---
 drivers/usb/host/ohci-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index fbcd34911025..a033f7d855e0 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -274,7 +274,7 @@ static const struct ohci_driver_overrides pci_overrides 
__initconst = {
.reset =ohci_pci_reset,
 };
 
-static const struct pci_device_id pci_ids [] = { {
+static const struct pci_device_id pci_ids[] = { {
/* handle any USB OHCI controller */
PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_OHCI, ~0),
.driver_data =  (unsigned long) &ohci_pci_hc_driver,
-- 
2.17.1

[REPOST PATCH v2 3/3] ARM: dts: rockchip: Allow wakeup from rk3288-veyron's dwc2 USB ports

2019-05-16 Thread Douglas Anderson 
We want to be able to wake from USB if a device is plugged in that
wants remote wakeup.  Enable it on both dwc2 controllers.

NOTE: this is added specifically to veyron and not to rk3288 in
general since it's not known whether all rk3288 boards are designed to
support USB wakeup.  It is plausible that some boards could shut down
important rails in S3.

Also note that currently wakeup doesn't seem to happen unless you use
the "deep" suspend mode (where SDRAM is turned off).  Presumably the
shallow suspend mode is gating some sort of clock that's important but
I couldn't easily figure out how to get it working.

Signed-off-by: Douglas Anderson 
---

Changes in v2:
- rk3288-veyron dts patch new for v2.

 arch/arm/boot/dts/rk3288-veyron.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi 
b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 1252522392c7..1d8bfed7830c 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -424,6 +424,7 @@
 
 &usb_host1 {
status = "okay";
+   snps,need-phy-for-wake;
 };
 
 &usb_otg {
@@ -432,6 +433,7 @@
assigned-clocks = <&cru SCLK_USBPHY480M_SRC>;
assigned-clock-parents = <&usbphy0>;
dr_mode = "host";
+   snps,need-phy-for-wake;
 };
 
 &vopb {
-- 
2.21.0.1020.gf2820cf01a-goog

[REPOST PATCH v2 1/3] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB

2019-05-16 Thread Douglas Anderson 
Some SoCs with a dwc2 USB controller may need to keep the PHY on to
support remote wakeup.  Allow specifying this as a device tree
property.

Signed-off-by: Douglas Anderson 
---
For relevant prior discussion on this patch, see:

https://lkml.kernel.org/r/1435017144-2971-3-git-send-email-diand...@chromium.org

I didn't make any changes from the prior version since I never found
out what Rob thought of my previous arguments.  If folks want a
change, perhaps they could choose from these options:

1. Assume that all dwc2 hosts would like to keep their PHY on for
   suspend if there's a USB wakeup enabled, thus we totally drop this
   binding.  This doesn't seem super great to me since I'd bet that
   many devices that use dwc2 weren't designed for USB wakeup (they
   may not keep enough clocks or rails on) so we might be wasting
   power for nothing.
2. Rename this property to "snps,wakeup-from-suspend-with-phy" to make
   it more obvious that this property is intended both to document
   that wakeup from suspend is possible and that we need the PHY for
   said wakeup.
3. Rename this property to "snps,can-wakeup-from-suspend" and assume
   it's implicit that if we can wakeup from suspend that we need to
   keep the PHY on.  If/when someone shows that a device exists using
   dwc2 where we can wakeup from suspend without the PHY they can add
   a new property.

NOTE FOR REPOST:
- In v2 Rob said [1] he'd prefer something based on the SoC
  compatibility string, but that doesn't work because not all boards
  will have the regulator setup / board design / suspend logic
  necessary to make this work.

[1] https://lkml.kernel.org/r/20190430012328.GA25660@bogus


Changes in v2: None

 Documentation/devicetree/bindings/usb/dwc2.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 49eac0dc86b0..aafff3a6904d 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -42,6 +42,8 @@ Refer to phy/phy-bindings.txt for generic phy consumer 
properties
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget 
mode.
+- snps,need-phy-for-wake: If present indicates that the phy needs to be left
+  on for remote wakeup during suspend.
 - snps,reset-phy-on-wake: If present indicates that we need to reset the PHY 
when
   we detect a wakeup.  This is due to a hardware 
errata.
 
@@ -58,4 +60,5 @@ Example:
clock-names = "otg";
phys = <&usbphy>;
phy-names = "usb2-phy";
+   snps,need-phy-for-wake;
 };
-- 
2.21.0.1020.gf2820cf01a-goog

[REPOST PATCH v2 2/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

2019-05-16 Thread Douglas Anderson 
If the 'snps,need-phy-for-wake' is set in the device tree then:

- We know that we can wakeup, so call device_set_wakeup_capable().
  The USB core will use this knowledge to enable wakeup by default.
- We know that we should keep the PHY on during suspend if something
  on our root hub needs remote wakeup.  This requires the patch (USB:
  Export usb_wakeup_enabled_descendants()).  Note that we don't keep
  the PHY on at suspend time if it's not needed because it would be a
  power draw.

If we later find some users of dwc2 that can support wakeup without
keeping the PHY on we may want to add a way to call
device_set_wakeup_capable() without keeping the PHY on at suspend
time.

Signed-off-by: Douglas Anderson 
Signed-off-by: Chris Zhong 
---
For relevant prior discussion of this idea, see:

https://lkml.kernel.org/r/1436207224-21849-4-git-send-email-diand...@chromium.org

If I'm reading all the responses correctly folks were of the opinion
that this patch is still the right way to go.

Changes in v2:
- Rebased to mainline atop rk3288 remote wake quirk series.

 drivers/usb/dwc2/core.h |  5 +
 drivers/usb/dwc2/platform.c | 43 +++--
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 152ac41dfb2d..73c1e998f27a 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -861,6 +861,9 @@ struct dwc2_hregs_backup {
  * @hibernated:True if core is hibernated
  * @reset_phy_on_wake: Quirk saying that we should assert PHY reset on a
  * remote wakeup.
+ * @phy_off_for_suspend: Status of whether we turned the PHY off at suspend.
+ * @need_phy_for_wake: Quirk saying that we should keep the PHY on at
+ * suspend if we need USB to wake us up.
  * @frame_number:   Frame number read from the core. For both device
  * and host modes. The value ranges are from 0
  * to HFNUM_MAX_FRNUM.
@@ -1049,6 +1052,8 @@ struct dwc2_hsotg {
unsigned int ll_hw_enabled:1;
unsigned int hibernated:1;
unsigned int reset_phy_on_wake:1;
+   unsigned int need_phy_for_wake:1;
+   unsigned int phy_off_for_suspend:1;
u16 frame_number;
 
struct phy *phy;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index d10a7f8daec3..31be644d1273 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -47,7 +47,9 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 
 #include "core.h"
@@ -447,6 +449,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
if (retval)
goto error;
 
+   hsotg->need_phy_for_wake =
+   of_property_read_bool(dev->dev.of_node,
+ "snps,need-phy-for-wake");
+
/*
 * Reset before dwc2_get_hwparams() then it could get power-on real
 * reset value form registers.
@@ -478,6 +484,14 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg->gadget_enabled = 1;
}
 
+   /*
+* If we need PHY for wakeup we must be wakeup capable.
+* When we have a device that can wake without the PHY we
+* can adjust this condition.
+*/
+   if (hsotg->need_phy_for_wake)
+   device_set_wakeup_capable(&dev->dev, true);
+
hsotg->reset_phy_on_wake =
of_property_read_bool(dev->dev.of_node,
  "snps,reset-phy-on-wake");
@@ -513,6 +527,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
return retval;
 }
 
+static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
+{
+   struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
+
+   if (!dwc2->ll_hw_enabled)
+   return false;
+
+   /* If the controller isn't allowed to wakeup then we can power off. */
+   if (!device_may_wakeup(dwc2->dev))
+   return true;
+
+   /*
+* We don't want to power off the PHY if something under the
+* root hub has wakeup enabled.
+*/
+   if (usb_wakeup_enabled_descendants(root_hub))
+   return false;
+
+   /* No reason to keep the PHY powered, so allow poweroff */
+   return true;
+}
+
 static int __maybe_unused dwc2_suspend(struct device *dev)
 {
struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
@@ -521,8 +557,10 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
if (dwc2_is_device_mode(dwc2))
dwc2_hsotg_suspend(dwc2);
 
-   if (dwc2->ll_hw_enabled)
+   if (dwc2_can_poweroff_phy(dwc2)) {
ret = __dwc2_lowlevel_hw_disable(dwc2);
+   dwc2->phy_off_for_suspend = true;
+   }
 
return ret;
 }
@@ -532,11 +570,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
struct dwc2_hsotg *dwc

[REPOST PATCH v2 0/3] USB: dwc2: Allow wakeup from suspend; enable for rk3288-veyron

2019-05-16 Thread Douglas Anderson 
This is a re-post of the last 3 patches of a series I posted earlier
at:
  https://lkml.kernel.org/r/20190418001356.124334-1-diand...@chromium.org

The first two patches were applied but the last three weren't because
they didn't apply at the time.  They apply fine now so are ready to
land.

Changes in v2:
- Rebased to mainline atop rk3288 remote wake quirk series.
- rk3288-veyron dts patch new for v2.

Douglas Anderson (3):
  Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
  USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
  ARM: dts: rockchip: Allow wakeup from rk3288-veyron's dwc2 USB ports

 .../devicetree/bindings/usb/dwc2.txt  |  3 ++
 arch/arm/boot/dts/rk3288-veyron.dtsi  |  2 +
 drivers/usb/dwc2/core.h   |  5 +++
 drivers/usb/dwc2/platform.c   | 43 ++-
 4 files changed, 51 insertions(+), 2 deletions(-)

-- 
2.21.0.1020.gf2820cf01a-goog