Re: [PATCH v2 4/6] usb: dwc3: gadget: add remaining sg entries to ring

[John Youn]

Hi Felipe,

On 8/15/2016 4:02 AM, Felipe Balbi wrote:
> Upon transfer completion after a full ring, let's
> add more TRBs to our ring in order to complete our
> request successfully.
> 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/dwc3/gadget.c | 36 
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 90b3d7965136..6483991c8013 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -888,14 +888,13 @@ static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
>  static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
>   struct dwc3_request *req, unsigned int trbs_left)
>  {
> - struct usb_request *request = >request;
> - struct scatterlist *sg = request->sg;
> + struct scatterlist *sg = req->sg;
>   struct scatterlist *s;
>   unsigned intlength;
>   dma_addr_t  dma;
>   int i;
>  
> - for_each_sg(sg, s, request->num_mapped_sgs, i) {
> + for_each_sg(sg, s, req->num_pending_sgs, i) {
>   unsigned chain = true;
>  
>   length = sg_dma_len(s);
> @@ -945,7 +944,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
>   return;
>  
>   list_for_each_entry_safe(req, n, >pending_list, list) {
> - if (req->request.num_mapped_sgs > 0)
> + if (req->num_pending_sgs > 0)
>   dwc3_prepare_one_trb_sg(dep, req, trbs_left--);
>   else
>   dwc3_prepare_one_trb_linear(dep, req, trbs_left--);
> @@ -1071,6 +1070,9 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
> struct dwc3_request *req)
>   if (ret)
>   return ret;
>  
> + req->sg = req->request.sg;
> + req->num_pending_sgs= req->request.num_mapped_sgs;
> +
>   list_add_tail(>list, >pending_list);
>  
>   if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
> @@ -1935,22 +1937,30 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, 
> struct dwc3_ep *dep,
>   int ret;
>  
>   list_for_each_entry_safe(req, n, >started_list, list) {
> -
> + unsigned length;
> + unsigned actual;
>   int chain;
>  
> - chain = req->request.num_mapped_sgs > 0;
> + length = req->request.length;
> + chain = req->num_pending_sgs > 0;
>   if (chain) {
> - struct scatterlist *sg = req->request.sg;
> + struct scatterlist *sg = req->sg;
>   struct scatterlist *s;
> + unsigned int pending = req->num_pending_sgs;
>   unsigned int i;
>  
> - for_each_sg(sg, s, req->request.num_mapped_sgs, i) {
> + for_each_sg(sg, s, pending, i) {
>   trb = >trb_pool[dep->trb_dequeue];
>   count += trb->size & DWC3_TRB_SIZE_MASK;
>   dwc3_ep_inc_deq(dep);
>  
> + req->sg = sg_next(s);
> + req->num_pending_sgs--;
> +
>   ret = __dwc3_cleanup_done_trbs(dwc, dep, req, 
> trb,
>   event, status, chain);
> + if (ret)
> + break;
>   }
>   } else {
>   trb = >trb_pool[dep->trb_dequeue];
> @@ -1968,11 +1978,13 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, 
> struct dwc3_ep *dep,
>* should receive and we simply bounce the request back to the
>* gadget driver for further processing.
>*/
> - req->request.actual += req->request.length - count;
> - dwc3_gadget_giveback(dep, req, status);
> + actual = length - req->request.actual;
> + req->request.actual = actual;
>  
> - if (ret)
> - break;
> + if (ret && chain && (actual < length) && req->num_pending_sgs)
> + return __dwc3_gadget_kick_transfer(dep, 0);
> +
> + dwc3_gadget_giveback(dep, req, status);
>   }
>  
>   /*
> 

On your testing/next, I see considerable slow down and file
corruption in mass storage.

After bisecting, this patch seems to be the first one that shows
problems. The device fails even enumeration here.

I haven't looked in detail at the changes but, do you have any ideas?

I have attached driver logs.

Regards,
John


log.tar.gz
Description: application/gzip

Re: usb: chipidea: hdc: kernel panic during shutdown

[Stefan Wahren]

Hi Alan,

> Alan Stern  hat am 24. August 2016 um 20:55
> geschrieben:
> 
> 
> On Wed, 24 Aug 2016, Stefan Wahren wrote:
> 
> > Hi,
> > 
> > [add Li Jun to CC]
> > 
> > > Alan Stern  hat am 24. August 2016 um 15:45
> > > geschrieben:
> > > 
> > > 
> > > On Wed, 24 Aug 2016, Peter Chen wrote:
> > > 
> > > > On Tue, Aug 23, 2016 at 09:17:02PM +0200, Stefan Wahren wrote:
> > > > > Hi,
> > > > > 
> > > > > i'm using a iMX233-OLinuXino board and the kernel panics during
> > > > > shutdown
> > > > > with
> > > > > 4.8.0-rc2-next-20160819:
> > > > > 
> > > > > [  420.04] ci_hdrc ci_hdrc.0: remove, state 1
> > > > > [  420.05] usb usb1: USB disconnect, device number 1
> > > > > [  420.06] usb 1-1: USB disconnect, device number 2
> > > > > [  420.06] usb 1-1.1: USB disconnect, device number 3
> > > > > [  420.09] smsc95xx 1-1.1:1.0 eth0: unregister 'smsc95xx'
> > > > > usb-ci_hdrc.0-1.1,
> > > > > smsc95xx USB 2.0 Ethernet
> > > > > [  420.29] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> > > > > [  420.30] Unable to handle kernel NULL pointer dereference at
> > > > > virtual
> > > > > address 0118
> > > > > [  420.30] pgd = c2ea4000
> > > > > [  420.30] [0118] *pgd=
> > > > > [  420.30] Internal error: Oops: 5 [#1] ARM
> > > > > [  420.30] Modules linked in:
> > > > > [  420.30] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted
> > > > > 4.8.0-rc2-next-20160819 #1
> > > > > [  420.30] Hardware name: Freescale MXS (Device Tree)
> > > > > [  420.30] task: c349 task.stack: c348e000
> > > > > [  420.30] PC is at usb_hcd_irq+0x0/0x34
> > > > > [  420.30] LR is at ci_irq+0x58/0x12c
> > > 
> > > > I am afraid the hcd is freed before the interrupt triggered. Would you
> > > > please try below changes:
> > > > 
> > > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > > > index 96ae695..61237a9 100644
> > > > --- a/drivers/usb/chipidea/host.c
> > > > +++ b/drivers/usb/chipidea/host.c
> > > > @@ -103,7 +103,7 @@ static const struct ehci_driver_overrides
> > > > ehci_ci_overrides = {
> > > > static irqreturn_t host_irq(struct ci_hdrc *ci)
> > > > {
> > > > -   return usb_hcd_irq(ci->irq, ci->hcd);
> > > > +   return ci->hcd ? usb_hcd_irq(ci->irq, ci->hcd) : IRQ_NONE;
> > > > }
> > > 
> > > This should not be needed.  Instead, the driver should make sure that 
> > > the interrupt handler has been fully unregistered before the hcd is 
> > > deallocated.
> > 
> > according to ci_hdrc_probe() from the chipidea core the IRQ seems to be
> > requested via devm_request_irq() with the flag IRQF_SHARED. 
> > 
> > I have the suspicion the following commit triggers the kernel panic:
> > 
> > 43a404577a93 ("usb: chipidea: host: set host to be null after hcd is freed")
> 
> No, that's not the cause.  Without that commit, you would try to access 
> deallocated memory instead of trying to dereference a NULL pointer, but 
> the kernel would still oops.
> 
> Instead, how about setting ci->role to CI_ROLE_END and then calling
> synchronize_irq(ci->irq) in host_stop(), before the usb_put_hcd()?

i tried the following patch:

--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -185,6 +185,8 @@ static void host_stop(struct ci_hdrc *ci)
 
if (hcd) {
usb_remove_hcd(hcd);
+   ci_role_stop(ci);
+   synchronize_irq(ci->irq);
usb_put_hcd(hcd);
if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci) &&
(ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON))

the i get the following during shutdown:

[  102.17] ci_hdrc ci_hdrc.0: remove, state 1
[  102.18] usb usb1: USB disconnect, device number 1
[  102.18] usb 1-1: USB disconnect, device number 2
[  102.19] usb 1-1.1: USB disconnect, device number 3
[  102.22] smsc95xx 1-1.1:1.0 eth0: unregister 'smsc95xx' usb-ci_hdrc.0-1.1,
smsc95xx USB 2.0 Ethernet
[  102.41] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
[  102.42] ci_hdrc ci_hdrc.0: remove, state 0
[  102.42] Unable to handle kernel NULL pointer dereference at virtual
address 0090
[  102.43] pgd = c2e74000
[  102.43] [0090] *pgd=
[  102.44] Internal error: Oops: 5 [#1] ARM
[  102.44] Modules linked in:
[  102.44] CPU: 0 PID: 1 Comm: systemd-shutdow Tainted: GW
  4.8.0-rc3-00026-gcad9d20-dirty #4
[  102.44] Hardware name: Freescale MXS (Device Tree)
[  102.44] task: c349 task.stack: c348e000
[  102.44] PC is at sysfs_remove_group+0x18/0x9c
[  102.44] LR is at usb_remove_hcd+0x3c/0x1ac
[  102.44] pc : []lr : []psr: 6013
[  102.44] sp : c348fd48  ip : c349  fp : be87dc58
[  102.44] r10: c08cd48c  r9 :   r8 : c348fe5c
[  102.44] r7 : c354bc44  r6 : c08a5160  r5 : 0078  r4 : c08afd2c
[  102.44] r3 : c349  r2 :   r1 : 

[RESEND PATCH, v5 3/5] usb: xhci-mtk: make IPPC register optional

[Chunfeng Yun]

Make IPPC register optional to support host side of dual-role mode,
due to it is moved into common glue layer for simplification.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/host/xhci-mtk.c |   36 +---
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 79959f1..4bf99b9 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -94,6 +94,9 @@ static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
int ret;
int i;
 
+   if (ippc == NULL)
+   return 0;
+
/* power on host ip */
value = readl(>ip_pw_ctr1);
value &= ~CTRL1_IP_HOST_PDN;
@@ -139,6 +142,9 @@ static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk)
int ret;
int i;
 
+   if (ippc == NULL)
+   return 0;
+
/* power down all u3 ports */
for (i = 0; i < mtk->num_u3_ports; i++) {
value = readl(>u3_ctrl_p[i]);
@@ -173,6 +179,9 @@ static int xhci_mtk_ssusb_config(struct xhci_hcd_mtk *mtk)
struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
u32 value;
 
+   if (ippc == NULL)
+   return 0;
+
/* reset whole ip */
value = readl(>ip_pw_ctr0);
value |= CTRL0_IP_SW_RST;
@@ -475,6 +484,7 @@ static void xhci_mtk_quirks(struct device *dev, struct 
xhci_hcd *xhci)
 /* called during probe() after chip reset completes */
 static int xhci_mtk_setup(struct usb_hcd *hcd)
 {
+   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
int ret;
 
@@ -482,12 +492,21 @@ static int xhci_mtk_setup(struct usb_hcd *hcd)
ret = xhci_mtk_ssusb_config(mtk);
if (ret)
return ret;
+   }
+
+   ret = xhci_gen_setup(hcd, xhci_mtk_quirks);
+   if (ret)
+   return ret;
+
+   if (usb_hcd_is_primary_hcd(hcd)) {
+   mtk->num_u3_ports = xhci->num_usb3_ports;
+   mtk->num_u2_ports = xhci->num_usb2_ports;
ret = xhci_mtk_sch_init(mtk);
if (ret)
return ret;
}
 
-   return xhci_gen_setup(hcd, xhci_mtk_quirks);
+   return ret;
 }
 
 static int xhci_mtk_probe(struct platform_device *pdev)
@@ -586,7 +605,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
mtk->hcd = platform_get_drvdata(pdev);
platform_set_drvdata(pdev, mtk);
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mac");
hcd->regs = devm_ioremap_resource(dev, res);
if (IS_ERR(hcd->regs)) {
ret = PTR_ERR(hcd->regs);
@@ -595,11 +614,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-   mtk->ippc_regs = devm_ioremap_resource(dev, res);
-   if (IS_ERR(mtk->ippc_regs)) {
-   ret = PTR_ERR(mtk->ippc_regs);
-   goto put_usb2_hcd;
+   mtk->ippc_regs = NULL;
+   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ippc");
+   if (res) {  /* ippc register is optional */
+   mtk->ippc_regs = devm_ioremap_resource(dev, res);
+   if (IS_ERR(mtk->ippc_regs)) {
+   ret = PTR_ERR(mtk->ippc_regs);
+   goto put_usb2_hcd;
+   }
}
 
for (phy_num = 0; phy_num < mtk->num_phys; phy_num++) {
-- 
1.7.9.5

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

[RESEND PATCH V5, 0/5] Add MediaTek USB3 DRD Driver

[Chunfeng Yun]

>From e60d29d748a4e9f412c9bb08458083e97d3f523d Mon Sep 17 00:00:00 2001
From: Chunfeng Yun 
Date: Tue, 9 Aug 2016 16:12:31 +0800
Subject: [PATCH V5, 0/5] Add MediaTek USB3 DRD Driver

These patches introduce the MediaTek USB3 dual-role controller
driver.

The driver can be configured as Dual-Role Device (DRD),
Peripheral Only and Host Only (xHCI) modes. It works well
with Mass Storage, RNDIS and g_zero on FS/HS and SS. And it is
tested on MT8173 platform which only contains USB2.0 device IP,
and on MT6290 platform which contains USB3.0 device IP.

Change in v5:
1. modify some comments
2. rename some unsuitable variables
3. add reg-names property for host node
4. add USB_MTU3_DEBUG to control debug messages

Change in v4:
1. fix build errors on non-mediatek platforms
2. provide manual dual-role switch via debugfs instead of sysfs

Change in v3:
1. fix some typo error
2. rename mtu3.txt to mt8173-mtu3.txt

Change in v2:
1. modify binding docs according to suggestions
2. modify some comments and remove some dummy blank lines
3. fix memory leakage

Change in v2:
1. modify binding docs according to suggestions
2. modify some comments and remove some dummy blank lines
3. fix memory leakage


Chunfeng Yun (5):
  dt-bindings: mt8173-xhci: support host side of dual-role mode
  dt-bindings: mt8173-mtu3: add devicetree bindings
  usb: xhci-mtk: make IPPC register optional
  usb: Add MediaTek USB3 DRD Driver
  arm64: dts: mediatek: add USB3 DRD driver

 .../devicetree/bindings/usb/mt8173-mtu3.txt|   87 ++
 .../devicetree/bindings/usb/mt8173-xhci.txt|   54 +-
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts|   46 +-
 arch/arm64/boot/dts/mediatek/mt8173.dtsi   |   29 +-
 drivers/usb/Kconfig|2 +
 drivers/usb/Makefile   |1 +
 drivers/usb/host/xhci-mtk.c|   36 +-
 drivers/usb/mtu3/Kconfig   |   54 ++
 drivers/usb/mtu3/Makefile  |   19 +
 drivers/usb/mtu3/mtu3.h|  422 ++
 drivers/usb/mtu3/mtu3_core.c   |  874 +++
 drivers/usb/mtu3/mtu3_dr.c |  375 +
 drivers/usb/mtu3/mtu3_dr.h |  108 +++
 drivers/usb/mtu3/mtu3_gadget.c |  731 
 drivers/usb/mtu3/mtu3_gadget_ep0.c |  879 
 drivers/usb/mtu3/mtu3_host.c   |  294 +++
 drivers/usb/mtu3/mtu3_hw_regs.h|  473 +++
 drivers/usb/mtu3/mtu3_plat.c   |  490 +++
 drivers/usb/mtu3/mtu3_qmu.c|  599 +
 drivers/usb/mtu3/mtu3_qmu.h|   43 +
 20 files changed, 5598 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
 create mode 100644 drivers/usb/mtu3/Kconfig
 create mode 100644 drivers/usb/mtu3/Makefile
 create mode 100644 drivers/usb/mtu3/mtu3.h
 create mode 100644 drivers/usb/mtu3/mtu3_core.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.h
 create mode 100644 drivers/usb/mtu3/mtu3_gadget.c
 create mode 100644 drivers/usb/mtu3/mtu3_gadget_ep0.c
 create mode 100644 drivers/usb/mtu3/mtu3_host.c
 create mode 100644 drivers/usb/mtu3/mtu3_hw_regs.h
 create mode 100644 drivers/usb/mtu3/mtu3_plat.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.h

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

[RESEND PATCH, v5 4/5] usb: Add MediaTek USB3 DRD Driver

[Chunfeng Yun]

This patch adds support for the MediaTek USB3 controller
integrated into MT8173. It can be configured as Dual-Role
Device (DRD), Peripheral Only and Host Only (xHCI) modes.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/Kconfig|2 +
 drivers/usb/Makefile   |1 +
 drivers/usb/mtu3/Kconfig   |   54 +++
 drivers/usb/mtu3/Makefile  |   19 +
 drivers/usb/mtu3/mtu3.h|  422 +
 drivers/usb/mtu3/mtu3_core.c   |  874 +++
 drivers/usb/mtu3/mtu3_dr.c |  375 +++
 drivers/usb/mtu3/mtu3_dr.h |  108 +
 drivers/usb/mtu3/mtu3_gadget.c |  731 ++
 drivers/usb/mtu3/mtu3_gadget_ep0.c |  879 
 drivers/usb/mtu3/mtu3_host.c   |  294 
 drivers/usb/mtu3/mtu3_hw_regs.h|  473 +++
 drivers/usb/mtu3/mtu3_plat.c   |  490 
 drivers/usb/mtu3/mtu3_qmu.c|  599 
 drivers/usb/mtu3/mtu3_qmu.h|   43 ++
 15 files changed, 5364 insertions(+)
 create mode 100644 drivers/usb/mtu3/Kconfig
 create mode 100644 drivers/usb/mtu3/Makefile
 create mode 100644 drivers/usb/mtu3/mtu3.h
 create mode 100644 drivers/usb/mtu3/mtu3_core.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.h
 create mode 100644 drivers/usb/mtu3/mtu3_gadget.c
 create mode 100644 drivers/usb/mtu3/mtu3_gadget_ep0.c
 create mode 100644 drivers/usb/mtu3/mtu3_host.c
 create mode 100644 drivers/usb/mtu3/mtu3_hw_regs.h
 create mode 100644 drivers/usb/mtu3/mtu3_plat.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.h

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8689dcb..9ca0bf0 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -95,6 +95,8 @@ source "drivers/usb/usbip/Kconfig"
 
 endif
 
+source "drivers/usb/mtu3/Kconfig"
+
 source "drivers/usb/musb/Kconfig"
 
 source "drivers/usb/dwc3/Kconfig"
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..7791af6 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_USB_DWC2)+= dwc2/
 obj-$(CONFIG_USB_ISP1760)  += isp1760/
 
 obj-$(CONFIG_USB_MON)  += mon/
+obj-$(CONFIG_USB_MTU3) += mtu3/
 
 obj-$(CONFIG_PCI)  += host/
 obj-$(CONFIG_USB_EHCI_HCD) += host/
diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
new file mode 100644
index 000..25cd619
--- /dev/null
+++ b/drivers/usb/mtu3/Kconfig
@@ -0,0 +1,54 @@
+# For MTK USB3.0 IP
+
+config USB_MTU3
+   tristate "MediaTek USB3 Dual Role controller"
+   depends on EXTCON && (USB || USB_GADGET) && HAS_DMA
+   depends on ARCH_MEDIATEK || COMPILE_TEST
+   select USB_XHCI_MTK if USB_SUPPORT && USB_XHCI_HCD
+   help
+ Say Y or M here if your system runs on MediaTek SoCs with
+ Dual Role SuperSpeed USB controller. You can select usb
+ mode as peripheral role or host role, or both.
+
+ If you don't know what this is, please say N.
+
+ Choose M here to compile this driver as a module, and it
+ will be called mtu3.ko.
+
+
+if USB_MTU3
+choice
+   bool "MTU3 Mode Selection"
+   default USB_MTU3_DUAL_ROLE if (USB && USB_GADGET)
+   default USB_MTU3_HOST if (USB && !USB_GADGET)
+   default USB_MTU3_GADGET if (!USB && USB_GADGET)
+
+config USB_MTU3_HOST
+   bool "Host only mode"
+   depends on USB=y || USB=USB_MTU3
+   help
+ Select this when you want to use MTU3 in host mode only,
+ thereby the gadget feature will be regressed.
+
+config USB_MTU3_GADGET
+   bool "Gadget only mode"
+   depends on USB_GADGET=y || USB_GADGET=USB_MTU3
+   help
+ Select this when you want to use MTU3 in gadget mode only,
+ thereby the host feature will be regressed.
+
+config USB_MTU3_DUAL_ROLE
+   bool "Dual Role mode"
+   depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || 
USB_GADGET=USB_MTU3))
+   help
+ This is the default mode of working of MTU3 controller where
+ both host and gadget features are enabled.
+
+endchoice
+
+config USB_MTU3_DEBUG
+   bool "Enable Debugging Messages"
+   help
+ Say Y here to enable debugging messages in the MTU3 Driver.
+
+endif
diff --git a/drivers/usb/mtu3/Makefile b/drivers/usb/mtu3/Makefile
new file mode 100644
index 000..3e17ff7
--- /dev/null
+++ b/drivers/usb/mtu3/Makefile
@@ -0,0 +1,19 @@
+
+ccflags-$(CONFIG_USB_MTU3_DEBUG)   += -DDEBUG
+
+obj-$(CONFIG_USB_MTU3) += mtu3.o
+
+mtu3-y := mtu3_plat.o
+
+ifneq ($(filter y,$(CONFIG_USB_MTU3_HOST) $(CONFIG_USB_MTU3_DUAL_ROLE)),)
+   mtu3-y  += mtu3_host.o
+endif
+
+ifneq ($(filter y,$(CONFIG_USB_MTU3_GADGET) $(CONFIG_USB_MTU3_DUAL_ROLE)),)
+   mtu3-y  += mtu3_core.o 

[RESEND PATCH, v5 1/5] dt-bindings: mt8173-xhci: support host side of dual-role mode

[Chunfeng Yun]

Some resources, such as IPPC register etc, shared with device
driver are moved into common glue layer when xHCI driver is the
host side of dual-role mode and they should be changed as optional
properties if they are required ones before. For clarity, add
a new part of binding to support host side of dual-role mode.

Additionally add optional properties of pinctrl for host only mode

Signed-off-by: Chunfeng Yun 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/usb/mt8173-xhci.txt|   54 +++-
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt 
b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
index b3a7ffa..2a930bd 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
@@ -2,10 +2,18 @@ MT8173 xHCI
 
 The device node for Mediatek SOC USB3.0 host controller
 
+There are two scenarios: the first one only supports xHCI driver;
+the second one supports dual-role mode, and the host is based on xHCI
+driver. Take account of backward compatibility, we divide bindings
+into two parts.
+
+1st: only supports xHCI driver
+
+
 Required properties:
  - compatible : should contain "mediatek,mt8173-xhci"
- - reg : specifies physical base address and size of the registers,
-   the first one for MAC, the second for IPPC
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be "mac" for xHCI MAC and "ippc" for IP port control
  - interrupts : interrupt used by the controller
  - power-domains : a phandle to USB power domain node to control USB's
mtcmos
@@ -27,12 +35,16 @@ Optional properties:
control register, it depends on "mediatek,wakeup-src".
  - vbus-supply : reference to the VBUS regulator;
  - usb3-lpm-capable : supports USB3.0 LPM
+ - pinctrl-names : a pinctrl state named "default" must be defined
+ - pinctrl-0 : pin control group
+   See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
 
 Example:
 usb30: usb@1127 {
compatible = "mediatek,mt8173-xhci";
reg = <0 0x1127 0 0x1000>,
  <0 0x11280700 0 0x0100>;
+   reg-names = "mac", "ippc";
interrupts = ;
power-domains = < MT8173_POWER_DOMAIN_USB>;
clocks = < CLK_TOP_USB30_SEL>,
@@ -49,3 +61,41 @@ usb30: usb@1127 {
mediatek,syscon-wakeup = <>;
mediatek,wakeup-src = <1>;
 };
+
+2nd: dual-role mode with xHCI driver
+
+
+In the case, xhci is added as subnode to mtu3. An example and the DT binding
+details of mtu3 can be found in:
+Documentation/devicetree/bindings/usb/mtu3.txt
+
+Required properties:
+ - compatible : should contain "mediatek,mt8173-xhci"
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be "mac" for xHCI MAC
+ - interrupts : interrupt used by the host controller
+ - power-domains : a phandle to USB power domain node to control USB's
+   mtcmos
+ - vusb33-supply : regulator of USB avdd3.3v
+
+ - clocks : a list of phandle + clock-specifier pairs, one for each
+   entry in clock-names
+ - clock-names : must be
+   "sys_ck": for clock of xHCI MAC
+
+Optional properties:
+ - vbus-supply : reference to the VBUS regulator;
+ - usb3-lpm-capable : supports USB3.0 LPM
+
+Example:
+usb30: usb@1127 {
+   compatible = "mediatek,mt8173-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>;
+   clock-names = "sys_ck";
+   vusb33-supply = <_vusb_reg>;
+   usb3-lpm-capable;
+};
-- 
1.7.9.5

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

[RESEND PATCH, v5 2/5] dt-bindings: mt8173-mtu3: add devicetree bindings

[Chunfeng Yun]

add a DT binding doc for MediaTek USB3 DRD driver

Signed-off-by: Chunfeng Yun 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/usb/mt8173-mtu3.txt|   87 
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/mt8173-mtu3.txt

diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt 
b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
new file mode 100644
index 000..e049d19
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
@@ -0,0 +1,87 @@
+The device node for Mediatek USB3.0 DRD controller
+
+Required properties:
+ - compatible : should be "mediatek,mt8173-mtu3"
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be "mac" for device IP and "ippc" for IP port control
+ - interrupts : interrupt used by the device IP
+ - power-domains : a phandle to USB power domain node to control USB's
+   mtcmos
+ - vusb33-supply : regulator of USB avdd3.3v
+ - clocks : a list of phandle + clock-specifier pairs, one for each
+   entry in clock-names
+ - clock-names : must contain "sys_ck" for clock of controller;
+   "wakeup_deb_p0" and "wakeup_deb_p1" are optional, they are
+   depends on "mediatek,enable-wakeup"
+ - phys : a list of phandle + phy specifier pairs
+ - dr_mode : should be one of "host", "peripheral" or "otg",
+   refer to usb/generic.txt
+
+Optional properties:
+ - #address-cells, #size-cells : should be '2' if the device has sub-nodes
+   with 'reg' property
+ - ranges : allows valid 1:1 translation between child's address space and
+   parent's address space
+ - extcon : external connector for vbus and idpin changes detection, needed
+   when supports dual-role mode.
+ - vbus-supply : reference to the VBUS regulator, needed when supports
+   dual-role mode.
+ - pinctl-names : a pinctrl state named "default" must be defined,
+   "id_float" and "id_ground" are optinal which depends on
+   "mediatek,enable-manual-drd"
+ - pinctrl-0 : pin control group
+   See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - maximum-speed : valid arguments are "super-speed", "high-speed" and
+   "full-speed"; refer to usb/generic.txt
+ - enable-manual-drd : supports manual dual-role switch via debugfs; usually
+   used when receptacle is TYPE-A and also wants to support dual-role
+   mode.
+ - mediatek,enable-wakeup : supports ip sleep wakeup used by host mode
+ - mediatek,syscon-wakeup : phandle to syscon used to access USB wakeup
+   control register, it depends on "mediatek,enable-wakeup".
+
+Sub-nodes:
+The xhci should be added as subnode to mtu3 as shown in the following example
+if host mode is enabled. The DT binding details of xhci can be found in:
+Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+
+Example:
+ssusb: usb@11271000 {
+   compatible = "mediatek,mt8173-mtu3";
+   reg = <0 0x11271000 0 0x3000>,
+ <0 0x11280700 0 0x0100>;
+   reg-names = "mac", "ippc";
+   interrupts = ;
+   phys = <_port0 PHY_TYPE_USB3>,
+  <_port1 PHY_TYPE_USB2>;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>,
+< CLK_PERI_USB0>,
+< CLK_PERI_USB1>;
+   clock-names = "sys_ck",
+ "wakeup_deb_p0",
+ "wakeup_deb_p1";
+   vusb33-supply = <_vusb_reg>;
+   vbus-supply = <_p0_vbus>;
+   extcon = <_usb>;
+   dr_mode = "otg";
+   mediatek,enable-wakeup;
+   mediatek,syscon-wakeup = <>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+
+   usb_host: xhci@1127 {
+   compatible = "mediatek,mt8173-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>;
+   clock-names = "sys_ck";
+   vusb33-supply = <_vusb_reg>;
+   status = "disabled";
+   };
+};
-- 
1.7.9.5

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

[RESEND PATCH, v5 5/5] arm64: dts: mediatek: add USB3 DRD driver

[Chunfeng Yun]

USB3 DRD driver is added for MT8173-EVB, and xHCI driver
becomes its subnode

Signed-off-by: Chunfeng Yun 
---
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts |   46 +--
 arch/arm64/boot/dts/mediatek/mt8173.dtsi|   29 +
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts 
b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
index 7453a47..682dfd7 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
@@ -34,6 +34,11 @@
 
chosen { };
 
+   extcon_usb: extcon_iddig {
+   compatible = "linux,extcon-usb-gpio";
+   id-gpio = < 16 GPIO_ACTIVE_HIGH>;
+   };
+
usb_p1_vbus: regulator@0 {
compatible = "regulator-fixed";
regulator-name = "usb_vbus";
@@ -42,6 +47,16 @@
gpio = < 130 GPIO_ACTIVE_HIGH>;
enable-active-high;
};
+
+   usb_p0_vbus: regulator@1 {
+   compatible = "regulator-fixed";
+   regulator-name = "vbus";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   gpio = < 9 GPIO_ACTIVE_HIGH>;
+   enable-active-high;
+   };
+
 };
 
  {
@@ -205,6 +220,20 @@
bias-pull-down = ;
};
};
+
+   usb_id_pins_float: usb_iddig_pull_up {
+   pins_iddig {
+   pinmux = ;
+   bias-pull-up;
+   };
+   };
+
+   usb_id_pins_ground: usb_iddig_pull_down {
+   pins_iddig {
+   pinmux = ;
+   bias-pull-down;
+   };
+   };
 };
 
  {
@@ -431,12 +460,25 @@
status = "okay";
 };
 
+ {
+   vusb33-supply = <_vusb_reg>;
+   vbus-supply = <_p0_vbus>;
+   extcon = <_usb>;
+   dr_mode = "otg";
+   mediatek,enable-wakeup;
+   pinctrl-names = "default", "id_float", "id_ground";
+   pinctrl-0 = <_id_pins_float>;
+   pinctrl-1 = <_id_pins_float>;
+   pinctrl-2 = <_id_pins_ground>;
+   status = "okay";
+};
+
  {
status = "okay";
 };
 
- {
+_host {
vusb33-supply = <_vusb_reg>;
vbus-supply = <_p1_vbus>;
-   mediatek,wakeup-src = <1>;
+   status = "okay";
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 10f638f..925948a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -668,11 +668,14 @@
status = "disabled";
};
 
-   usb30: usb@1127 {
-   compatible = "mediatek,mt8173-xhci";
-   reg = <0 0x1127 0 0x1000>,
+   ssusb: usb@11271000 {
+   compatible = "mediatek,mt8173-mtu3";
+   reg = <0 0x11271000 0 0x3000>,
  <0 0x11280700 0 0x0100>;
-   interrupts = ;
+   reg-names = "mac", "ippc";
+   interrupts = ;
+   phys = <_port0 PHY_TYPE_USB3>,
+  <_port1 PHY_TYPE_USB2>;
power-domains = < MT8173_POWER_DOMAIN_USB>;
clocks = < CLK_TOP_USB30_SEL>,
 < CLK_PERI_USB0>,
@@ -680,10 +683,22 @@
clock-names = "sys_ck",
  "wakeup_deb_p0",
  "wakeup_deb_p1";
-   phys = <_port0 PHY_TYPE_USB3>,
-  <_port1 PHY_TYPE_USB2>;
mediatek,syscon-wakeup = <>;
-   status = "okay";
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+
+   usb_host: xhci@1127 {
+   compatible = "mediatek,mt8173-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < 
MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>;
+   clock-names = "sys_ck";
+   status = "disabled";
+   };
};
 
u3phy: usb-phy@1129 {
-- 
1.7.9.5

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

Re: [PATCH V5, 0/5] Add MediaTek USB3 DRD Driver

[chunfeng yun]

Hi,

On Wed, 2016-08-24 at 13:29 +0200, Oliver Neukum wrote:
> On Wed, 2016-08-24 at 14:42 +0800, chunfeng yun wrote:
> > Dear all,
> > 
> > Could you please help me to review the code? 
> 
> Is the structure
> 
> struct qmu_gpd
> 
> shared with the hardware? Do I read this correctly that
> you do PIO to endpoint 0 but DMA to the others?
> 
Yes, you are right.

> Could you resend the series?
> 
I will do it soon

Thank you.

>   Regards
>   Oliver
> 
> 


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

[RFT PATCH v2 0/4] usb: dwc2: Fix core reset and force mode delays

[John Youn]

The following patch series addresses the core reset and force mode
delay problems seen on some platforms.

It is basically the same as this series of patches sent earlier this
year:

http://marc.info/?l=linux-usb=145921907525708=2

Except the first patch is omitted (already upstream) and the last
patch is broken up into two patches.

This series is trying to account for a variable delay from the IDDIG
debounce filter when switching modes. This delay is a function of the
PHY clock speed and can range from 5-50 ms. This delay must be taken
into account on core reset and force modes. A full explanation is
provided in the patch commit log and code comments.

Patch 1 is a prerequisite to this fix.

Patch 2 implements the delay for core reset.

Patch 3 implements the delay for set/clear force modes.

Patch 4 makes further optimizations to remove unnecessary calls to
reset and force mode and avoid extra delays in the driver.

RK3188 platforms:

Michael Niewoehner reported problems with the previous set of patches
that I suspect have to do with the last patch of this series.

On Raspberry PI platforms:

Stefan Wahren reported problems that should be solved by patches 1-3
of this series.

Appreciate testing on these and any other platforms.

Patch 1-2 can probably be merged right now as they shouldn't break
anything.

Patch 3 should solve the RPI problems and shouldn't have any issues on
rk3188 either.

Patch 4 likely will have the same issue as with rk3188 and will need
to be investigated at a later time.

Regards,
John

John Youn (4):
  usb: dwc2: gadget: Only initialize device if in device mode
  usb: dwc2: Add delay to core soft reset
  usb: dwc2: Properly account for the force mode delays
  usb: dwc2: Force mode optimizations

 drivers/usb/dwc2/core.c | 195 
 drivers/usb/dwc2/core.h |   2 +-
 drivers/usb/dwc2/gadget.c   |   7 +-
 drivers/usb/dwc2/hcd.c  |   6 +-
 drivers/usb/dwc2/hw.h   |   1 +
 drivers/usb/dwc2/platform.c |   9 +-
 6 files changed, 143 insertions(+), 77 deletions(-)

-- 
2.9.0

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

[RFT PATCH v2 2/4] usb: dwc2: Add delay to core soft reset

[John Youn]

Add a delay to the core soft reset function to account for the IDDIG
debounce filter.

If the current mode is host, either due to the force mode bit being
set (which persists after core reset) or the connector id pin, a core
soft reset will temporarily reset the mode to device and a delay from
the IDDIG debounce filter will occur before going back to host mode.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.c | 97 +
 drivers/usb/dwc2/core.h |  1 +
 drivers/usb/dwc2/hw.h   |  1 +
 3 files changed, 99 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 4135a5f..bb903e2 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -238,6 +238,78 @@ int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg)
return ret;
 }
 
+/**
+ * dwc2_wait_for_mode() - Waits for the controller mode.
+ * @hsotg: Programming view of the DWC_otg controller.
+ * @host_mode: If true, waits for host mode, otherwise device mode.
+ * @timeout:   Time to wait in ms.
+ */
+static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,
+  bool host_mode,
+  unsigned int timeout)
+{
+   ktime_t start;
+   ktime_t end;
+
+   dev_vdbg(hsotg->dev, "Waiting for %s mode\n",
+host_mode ? "host" : "device");
+
+   start = ktime_get();
+
+   while (1) {
+   s64 ms;
+
+   if (dwc2_is_host_mode(hsotg) == host_mode) {
+   dev_vdbg(hsotg->dev, "%s mode set\n",
+host_mode ? "Host" : "Device");
+   break;
+   }
+
+   end = ktime_get();
+   ms = ktime_to_ms(ktime_sub(end, start));
+
+   if (ms >= (s64)timeout) {
+   dev_warn(hsotg->dev, "%s: Couldn't set %s mode\n",
+__func__, host_mode ? "host" : "device");
+   break;
+   }
+
+   usleep_range(1000, 2000);
+   }
+}
+
+/**
+ * dwc2_iddig_filter_enabled() - Returns true if the IDDIG debounce
+ * filter is enabled.
+ */
+static bool dwc2_iddig_filter_enabled(struct dwc2_hsotg *hsotg)
+{
+   u32 gsnpsid;
+   u32 ghwcfg4;
+
+   if (!dwc2_hw_is_otg(hsotg))
+   return false;
+
+   /* Check if core configuration includes the IDDIG filter. */
+   ghwcfg4 = dwc2_readl(hsotg->regs + GHWCFG4);
+   if (!(ghwcfg4 & GHWCFG4_IDDIG_FILT_EN))
+   return false;
+
+   /*
+* Check if the IDDIG debounce filter is bypassed. Available
+* in core version >= 3.10a.
+*/
+   gsnpsid = dwc2_readl(hsotg->regs + GSNPSID);
+   if (gsnpsid >= DWC2_CORE_REV_3_10a) {
+   u32 gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+
+   if (gotgctl & GOTGCTL_DBNCE_FLTR_BYPASS)
+   return false;
+   }
+
+   return true;
+}
+
 /*
  * Do core a soft reset of the core.  Be careful with this because it
  * resets all the internal state machines of the core.
@@ -246,9 +318,30 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 {
u32 greset;
int count = 0;
+   bool wait_for_host_mode = false;
 
dev_vdbg(hsotg->dev, "%s()\n", __func__);
 
+   /*
+* If the current mode is host, either due to the force mode
+* bit being set (which persists after core reset) or the
+* connector id pin, a core soft reset will temporarily reset
+* the mode to device. A delay from the IDDIG debounce filter
+* will occur before going back to host mode.
+*
+* Determine whether we will go back into host mode after a
+* reset and account for this delay after the reset.
+*/
+   if (dwc2_iddig_filter_enabled(hsotg)) {
+   u32 gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+   u32 gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+
+   if (!(gotgctl & GOTGCTL_CONID_B) ||
+   (gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+   wait_for_host_mode = true;
+   }
+   }
+
/* Core Soft Reset */
greset = dwc2_readl(hsotg->regs + GRSTCTL);
greset |= GRSTCTL_CSFTRST;
@@ -277,6 +370,10 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
}
} while (!(greset & GRSTCTL_AHBIDLE));
 
+   /* Wait up to 50 ms for the IDDIG debounce filter. */
+   if (wait_for_host_mode)
+   dwc2_wait_for_mode(hsotg, true, 50);
+
return 0;
 }
 
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index d645512..2a21a04 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -890,6 +890,7 @@ struct dwc2_hsotg {
 #define DWC2_CORE_REV_2_92a0x4f54292a
 #define DWC2_CORE_REV_2_94a0x4f54294a
 #define DWC2_CORE_REV_3_00a0x4f54300a
+#define DWC2_CORE_REV_3_10a

[RFT PATCH v2 4/4] usb: dwc2: Force mode optimizations

[John Youn]

If the dr_mode is USB_DR_MODE_OTG, forcing the mode is needed during
driver probe to get the host and device specific HW parameters. Then we
clear the force mode bits so that the core operates in OTG mode.

The force mode bits should not be touched at any other time during the
driver lifetime and they should be preserved whenever the GUSBCFG
register is written to. The force mode bit values will presist across
soft resets of the core.

If the dr_mode is either USB_DR_MODE_HOST or USB_DR_MODE_PERIPHERAL, the
force mode is set just once at probe to configure the core as either a
host or peripheral.

Given the above, we no longer need any other reset delays, force delays,
or any forced modes anywhere else in the driver. So replace all calls to
dwc2_core_reset_and_force_dr_mode() with dwc2_core_reset() and remove
all other unnecessary delays.

Also remove the dwc2_force_mode_if_needed() function since the "if
needed" part is already taken care of by the polling in
dwc2_force_mode().

Finally, remove all other calls to dwc2_clear_force_mode().

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.c | 66 ++---
 drivers/usb/dwc2/core.h |  1 -
 drivers/usb/dwc2/hcd.c  |  6 ++---
 drivers/usb/dwc2/platform.c |  9 ++-
 4 files changed, 25 insertions(+), 57 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index caad6121..9b83b20 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -377,14 +377,14 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
return 0;
 }
 
-/*
- * Force the mode of the controller.
+/**
+ * dwc2_force_mode() - Force the mode of the controller.
  *
  * Forcing the mode is needed for two cases:
  *
  * 1) If the dr_mode is set to either HOST or PERIPHERAL we force the
  * controller to stay in a particular mode regardless of ID pin
- * changes. We do this usually after a core reset.
+ * changes. We do this once during probe.
  *
  * 2) During probe we want to read reset values of the hw
  * configuration registers that are only available in either host or
@@ -401,7 +401,7 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
  * the filter is configured and enabled. We poll the current mode of
  * the controller to account for this delay.
  */
-static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
+static void dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
 {
u32 gusbcfg;
u32 set;
@@ -413,17 +413,17 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, 
bool host)
 * Force mode has no effect if the hardware is not OTG.
 */
if (!dwc2_hw_is_otg(hsotg))
-   return false;
+   return;
 
/*
 * If dr_mode is either peripheral or host only, there is no
 * need to ever force the mode to the opposite mode.
 */
if (WARN_ON(host && hsotg->dr_mode == USB_DR_MODE_PERIPHERAL))
-   return false;
+   return;
 
if (WARN_ON(!host && hsotg->dr_mode == USB_DR_MODE_HOST))
-   return false;
+   return;
 
gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
 
@@ -450,6 +450,11 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
 {
u32 gusbcfg;
 
+   if (!dwc2_hw_is_otg(hsotg))
+   return;
+
+   dev_dbg(hsotg->dev, "Clearing force mode bits\n");
+
gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
@@ -481,25 +486,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg)
}
 }
 
-/*
- * Do core a soft reset of the core.  Be careful with this because it
- * resets all the internal state machines of the core.
- *
- * Additionally this will apply force mode as per the hsotg->dr_mode
- * parameter.
- */
-int dwc2_core_reset_and_force_dr_mode(struct dwc2_hsotg *hsotg)
-{
-   int retval;
-
-   retval = dwc2_core_reset(hsotg);
-   if (retval)
-   return retval;
-
-   dwc2_force_dr_mode(hsotg);
-   return 0;
-}
-
 /**
  * dwc2_dump_host_registers() - Prints the host registers
  *
@@ -1418,22 +1404,6 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
dwc2_set_param_hibernation(hsotg, params->hibernation);
 }
 
-/*
- * Forces either host or device mode if the controller is not
- * currently in that mode.
- *
- * Returns true if the mode was forced.
- */
-static bool dwc2_force_mode_if_needed(struct dwc2_hsotg *hsotg, bool host)
-{
-   if (host && dwc2_is_host_mode(hsotg))
-   return false;
-   else if (!host && dwc2_is_device_mode(hsotg))
-   return false;
-
-   return dwc2_force_mode(hsotg, host);
-}
-
 /*
  * Gets host hardware parameters. Forces host mode if not currently in
  * host mode. Should be called immediately after a core soft reset in
@@ -1444,21 +1414,17 @@ static void dwc2_get_host_hwparams(struct dwc2_hsotg 

[RFT PATCH v2 1/4] usb: dwc2: gadget: Only initialize device if in device mode

[John Youn]

In dwc2_hsotg_udc_start(), don't initialize the controller for device
mode unless we are actually in device mode.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index af46adf..358ba5a 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3475,8 +3475,11 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
*gadget,
otg_set_peripheral(hsotg->uphy->otg, >gadget);
 
spin_lock_irqsave(>lock, flags);
-   dwc2_hsotg_init(hsotg);
-   dwc2_hsotg_core_init_disconnected(hsotg, false);
+   if (dwc2_hw_is_device(hsotg)) {
+   dwc2_hsotg_init(hsotg);
+   dwc2_hsotg_core_init_disconnected(hsotg, false);
+   }
+
hsotg->enabled = 0;
spin_unlock_irqrestore(>lock, flags);
 
-- 
2.9.0

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Greg KH]

On Wed, Aug 24, 2016 at 11:29:51AM +0200, Rafał Miłecki wrote:
> On 24 August 2016 at 11:22, Greg KH  wrote:
> > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> >> +static ssize_t ports_show(struct device *dev, struct device_attribute 
> >> *attr,
> >> +   char *buf)
> >> +{
> >> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> >> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data;
> >> + struct usbport_trig_port *port;
> >> + ssize_t ret = 0;
> >> + int len;
> >> +
> >> + list_for_each_entry(port, _data->ports, list) {
> >> + len = sprintf(buf + ret, "%s\n", port->name);
> >> + if (len >= 0)
> >> + ret += len;
> >> + }
> >> +
> >> + return ret;
> >> +}
> >
> > sysfs is "one value per file", here you are listing a bunch of things in
> > one sysfs file.  Please don't do that.
> 
> OK. What do you think about creating "ports" subdirectory and creating
> file-per-port in it? Then I'd need to bring back something like
> "new_port" and "remove_port". Does it sound OK?

Maybe, I don't know.  Why is "USB" somehow unique here?  Why isn't this
the same for PCI slots/ports?  pccard?  sdcard?  thunderbolt?

thanks,

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Greg KH]

On Wed, Aug 24, 2016 at 11:26:27AM +0200, Rafał Miłecki wrote:
> On 24 August 2016 at 11:21, Greg KH  wrote:
> > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> >> From: Rafał Miłecki 
> >>
> >> This commit adds a new trigger responsible for turning on LED when USB
> >> device gets connected to the specified USB port. This can can useful for
> >> various home routers that have USB port(s) and a proper LED telling user
> >> a device is connected.
> >>
> >> The trigger gets its documentation file but basically it just requires
> >> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> >>
> >> During work on this trigger there was a plan to add DT bindings for it,
> >> but there wasn't an agreement on the format yet. This can be worked on
> >> later, a sysfs interface is needed anyway for platforms not using DT.
> >>
> >> Signed-off-by: Rafał Miłecki 
> >> ---
> >> V2: Trying to add DT support, idea postponed as it will take more time
> >> to discuss the bindings.
> >> V3: Fix typos in commit and Documentation (thanks Jacek!)
> >> Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> >> Check if there is USB device connected after adding new USB port
> >> Fix memory leak or two
> >>
> >> Felipe: I'd like to ask for your Ack before having this patch pushed.
> >> ---
> >>  Documentation/leds/ledtrig-usbport.txt |  49 +++
> >>  drivers/leds/trigger/Kconfig   |   8 ++
> >>  drivers/leds/trigger/Makefile  |   1 +
> >>  drivers/leds/trigger/ledtrig-usbport.c | 253 
> >> +
> >>  4 files changed, 311 insertions(+)
> >>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
> >>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c
> >
> > You are adding sysfs files without adding a Documentation/ABI/ entry,
> > please never do that.
> 
> This is the way all LED triggers are documented, I just blindly
> assumed it's OK. Could you be a bit more helpful and help me to
> understand further steps? Should I drop
> Documentation/leds/ledtrig-usbport.txt and add proper entry in
> Documentation/ABI/testing? Or should I keep both files? What about
> current sysfs of other triggers? Should they be moved/documented in
> ABI?

In the end, all should be moved into Documentation/ABI/  But for you,
might as well start documenting them in there for any new ones so you
don't have to just move them later.

thanks,

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

Re: [PACTH,v6,1/2] usb: xhci: plat: Enable runtime PM

[Robert Foss]

On 2016-08-22 11:23 PM, Brian Norris wrote:

+ others

Hi Robert and Felipe,

I have a few questions for one or both of you. I'm not really an expert
on runtime PM, so please take my questions with a grain of salt.

On Wed, Aug 10, 2016 at 04:32:15PM -0400, robert.f...@collabora.com wrote:

From: Robert Foss 

Enable runtime PM for the xhci-plat device so that the parent device
may implement runtime PM.

Signed-off-by: Robert Foss 

Tested-by: Robert Foss 
---
 drivers/usb/host/xhci-plat.c | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..ba4efe7 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;

+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+


How does it help to enable PM runtime like this, if you don't have any
kind of runtime_{suspend,resume}() callbacks?


Andrew, I think you understand the inner workings of this code better 
than me, maybe you could give a short summary?




I suspect that this patch set was derived from the Chromium OS kernel
tree, where we were supporting a Tegra XHCI chipset:

https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.10/drivers/usb/host/xhci-tegra.c#1920

It looks like the driver was refactored to not use xhci-plat.c before it
was upstreamed (and runtime PM support was dropped along the way).

So, I'm wondering how I might actually use this? Particularly, I'm
looking at trying out runtime suspend for a DWC3 controller in host
mode, and it looks like I'd have to do some layer-violating calls to
xhci_suspend()/xhci_resume() from the parent dwc3 device, or else
rewrite drivers/usb/dwc3/host.c to avoid using xhci-plat.c.

(I also see that Baolin, CC'd here, was interested in dwc3 [1].)

Or possibly an enlightening question for me: if you don't mind, how are
you utilizing runtime PM in conjunction with xhci-plat.c, Robert?
Presumably some other parent device/driver is doing some additional
management of the XHCI core?

Regards,
Brian

[1] [PATCH 4/4] usb: dwc3: core: Support the dwc3 host suspend/resume
https://lkml.org/lkml/2016/7/15/181
https://patchwork.kernel.org/patch/9231417/


return 0;


@@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;

+   pm_runtime_disable(>dev);
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);

@@ -292,6 +297,13 @@ static int xhci_plat_suspend(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;
+
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0) {
+   pm_runtime_put(dev);
+   return ret;
+   }

/*
 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
@@ -301,15 +313,28 @@ static int xhci_plat_suspend(struct device *dev)
 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
 * also applies to runtime suspend.
 */
-   return xhci_suspend(xhci, device_may_wakeup(dev));
+   ret = xhci_suspend(xhci, device_may_wakeup(dev));
+   pm_runtime_put(dev);
+
+   return ret;
 }

 static int xhci_plat_resume(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;

-   return xhci_resume(xhci, 0);
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0) {
+   pm_runtime_put(dev);
+   return ret;
+   }
+
+   ret = xhci_resume(xhci, 0);
+   pm_runtime_put(dev);
+
+   return ret;
 }

 static const struct dev_pm_ops xhci_plat_pm_ops = {

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

Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

On 24 August 2016 at 20:48, Bjørn Mork  wrote:
> Rafał Miłecki  writes:
>
>> The last big missing thing is Documentation update (this is why I'm
>> sending RFC). Greg pointed out we should have some entries in
>> Documentation/ABI, but it seems none of triggers have it.
>
> There's a lot missing, but there is at least one exception:
> The "inverted" attribute of the  gpio and backlight triggers is
> documented as part of Documentation/ABI/testing/sysfs-class-led
>
>> Any idea why is that?
>
> Manual enforcement fails from time to time? The requirement was less
> strict in the early days of sysfs? Does it matter?
>
>> Do we need to change it? Or is it required for new code only?
>
> The lack of ABI docs is a bug. Don't add new code with known bugs. Old
> code should be fixed, but there is no immediate *need* to fix it.

Don't get me wrong. I care about code quality and documentation. I
mean to follow kernel rules. It's just some things may be not clear to
ppl and it would be nice to get any explanation/help. That said thanks
a lot of your e-mail, I'll try to expand pointed Documentation.

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

Re: [PATCH] usb: musb: Fix locking errors for host only mode

[Bin Liu]

On Wed, Aug 24, 2016 at 12:29:26PM -0700, Tony Lindgren wrote:
> * Tony Lindgren  [160824 12:17]:
> > * Bin Liu  [160824 11:47]:
> > > Hi,
> > > 
> > > On Thu, Aug 18, 2016 at 03:40:38PM -0700, Tony Lindgren wrote:
> > > > If we have USB gadgets disabled and USB_MUSB_HOST set, we get
> > > > errors "possible irq lock inverssion dependency detected"
> > > > errors during boot.
> > > 
> > > On which platform was this issue found? I am trying to replicate the
> > > issue on am335x, but have no luck yet - musb-hdrc does not load at all
> > > when usb gadget support is disabled.
> > 
> > It seems to be with a built-in MUSB and gadgets in the .config,
> > I'll forward you the one I got earlier from Ladis.
> 
> And probably specifically CONFIG_USB_MUSB_HOST=y.

yes, I had gadget support disabled and CONFIG_USB_MUSB_HOST=y, but
MUSB_HDRC=m. I will try your .config once I have time.

Thanks,
-Bin.

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

Re: [PATCH] usb: musb: Fix locking errors for host only mode

[Tony Lindgren]

* Tony Lindgren  [160824 12:17]:
> * Bin Liu  [160824 11:47]:
> > Hi,
> > 
> > On Thu, Aug 18, 2016 at 03:40:38PM -0700, Tony Lindgren wrote:
> > > If we have USB gadgets disabled and USB_MUSB_HOST set, we get
> > > errors "possible irq lock inverssion dependency detected"
> > > errors during boot.
> > 
> > On which platform was this issue found? I am trying to replicate the
> > issue on am335x, but have no luck yet - musb-hdrc does not load at all
> > when usb gadget support is disabled.
> 
> It seems to be with a built-in MUSB and gadgets in the .config,
> I'll forward you the one I got earlier from Ladis.

And probably specifically CONFIG_USB_MUSB_HOST=y.

Regards,

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

Re: [PATCH] usb: musb: Fix locking errors for host only mode

[Tony Lindgren]

* Bin Liu  [160824 11:47]:
> Hi,
> 
> On Thu, Aug 18, 2016 at 03:40:38PM -0700, Tony Lindgren wrote:
> > If we have USB gadgets disabled and USB_MUSB_HOST set, we get
> > errors "possible irq lock inverssion dependency detected"
> > errors during boot.
> 
> On which platform was this issue found? I am trying to replicate the
> issue on am335x, but have no luck yet - musb-hdrc does not load at all
> when usb gadget support is disabled.

It seems to be with a built-in MUSB and gadgets in the .config,
I'll forward you the one I got earlier from Ladis.

Regards,

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

Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

[Bjørn Mork]

Rafał Miłecki  writes:

> The last big missing thing is Documentation update (this is why I'm
> sending RFC). Greg pointed out we should have some entries in
> Documentation/ABI, but it seems none of triggers have it.

There's a lot missing, but there is at least one exception:
The "inverted" attribute of the  gpio and backlight triggers is
documented as part of Documentation/ABI/testing/sysfs-class-led

> Any idea why is that?

Manual enforcement fails from time to time? The requirement was less
strict in the early days of sysfs? Does it matter?

> Do we need to change it? Or is it required for new code only?

The lack of ABI docs is a bug. Don't add new code with known bugs. Old
code should be fixed, but there is no immediate *need* to fix it.


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

Re: usb: chipidea: hdc: kernel panic during shutdown

[Alan Stern]

On Wed, 24 Aug 2016, Stefan Wahren wrote:

> Hi,
> 
> [add Li Jun to CC]
> 
> > Alan Stern  hat am 24. August 2016 um 15:45
> > geschrieben:
> > 
> > 
> > On Wed, 24 Aug 2016, Peter Chen wrote:
> > 
> > > On Tue, Aug 23, 2016 at 09:17:02PM +0200, Stefan Wahren wrote:
> > > > Hi,
> > > > 
> > > > i'm using a iMX233-OLinuXino board and the kernel panics during shutdown
> > > > with
> > > > 4.8.0-rc2-next-20160819:
> > > > 
> > > > [  420.04] ci_hdrc ci_hdrc.0: remove, state 1
> > > > [  420.05] usb usb1: USB disconnect, device number 1
> > > > [  420.06] usb 1-1: USB disconnect, device number 2
> > > > [  420.06] usb 1-1.1: USB disconnect, device number 3
> > > > [  420.09] smsc95xx 1-1.1:1.0 eth0: unregister 'smsc95xx'
> > > > usb-ci_hdrc.0-1.1,
> > > > smsc95xx USB 2.0 Ethernet
> > > > [  420.29] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> > > > [  420.30] Unable to handle kernel NULL pointer dereference at 
> > > > virtual
> > > > address 0118
> > > > [  420.30] pgd = c2ea4000
> > > > [  420.30] [0118] *pgd=
> > > > [  420.30] Internal error: Oops: 5 [#1] ARM
> > > > [  420.30] Modules linked in:
> > > > [  420.30] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted
> > > > 4.8.0-rc2-next-20160819 #1
> > > > [  420.30] Hardware name: Freescale MXS (Device Tree)
> > > > [  420.30] task: c349 task.stack: c348e000
> > > > [  420.30] PC is at usb_hcd_irq+0x0/0x34
> > > > [  420.30] LR is at ci_irq+0x58/0x12c
> > 
> > > I am afraid the hcd is freed before the interrupt triggered. Would you
> > > please try below changes:
> > > 
> > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > > index 96ae695..61237a9 100644
> > > --- a/drivers/usb/chipidea/host.c
> > > +++ b/drivers/usb/chipidea/host.c
> > > @@ -103,7 +103,7 @@ static const struct ehci_driver_overrides
> > > ehci_ci_overrides = {
> > > static irqreturn_t host_irq(struct ci_hdrc *ci)
> > > {
> > > -   return usb_hcd_irq(ci->irq, ci->hcd);
> > > +   return ci->hcd ? usb_hcd_irq(ci->irq, ci->hcd) : IRQ_NONE;
> > > }
> > 
> > This should not be needed.  Instead, the driver should make sure that 
> > the interrupt handler has been fully unregistered before the hcd is 
> > deallocated.
> 
> according to ci_hdrc_probe() from the chipidea core the IRQ seems to be
> requested via devm_request_irq() with the flag IRQF_SHARED. 
> 
> I have the suspicion the following commit triggers the kernel panic:
> 
> 43a404577a93 ("usb: chipidea: host: set host to be null after hcd is freed")

No, that's not the cause.  Without that commit, you would try to access 
deallocated memory instead of trying to dereference a NULL pointer, but 
the kernel would still oops.

Instead, how about setting ci->role to CI_ROLE_END and then calling
synchronize_irq(ci->irq) in host_stop(), before the usb_put_hcd()?

Alan Stern

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

Re: [PATCH] usb: musb: Fix locking errors for host only mode

[Bin Liu]

Hi,

On Thu, Aug 18, 2016 at 03:40:38PM -0700, Tony Lindgren wrote:
> If we have USB gadgets disabled and USB_MUSB_HOST set, we get
> errors "possible irq lock inverssion dependency detected"
> errors during boot.

On which platform was this issue found? I am trying to replicate the
issue on am335x, but have no luck yet - musb-hdrc does not load at all
when usb gadget support is disabled.

Regards,
-Bin.

> 
> Let's fix the issue by adding start_musb flag and start
> the controller after we're out of the spinlock protected
> section.
> 
> Reported-by: Ladislav Michl 
> Tested-by: Ladislav Michl 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/usb/musb/musb_virthub.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
> index 192248f..fe08e77 100644
> --- a/drivers/usb/musb/musb_virthub.c
> +++ b/drivers/usb/musb/musb_virthub.c
> @@ -290,6 +290,7 @@ int musb_hub_control(
>   u32 temp;
>   int retval = 0;
>   unsigned long   flags;
> + boolstart_musb = false;
>  
>   spin_lock_irqsave(>lock, flags);
>  
> @@ -390,7 +391,7 @@ int musb_hub_control(
>* logic relating to VBUS power-up.
>*/
>   if (!hcd->self.is_b_host && musb_has_gadget(musb))
> - musb_start(musb);
> + start_musb = true;
>   break;
>   case USB_PORT_FEAT_RESET:
>   musb_port_reset(musb, true);
> @@ -451,5 +452,9 @@ error:
>   retval = -EPIPE;
>   }
>   spin_unlock_irqrestore(>lock, flags);
> +
> + if (start_musb)
> + musb_start(musb);
> +
>   return retval;
>  }
> -- 
> 2.8.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: pwc over musb: 100% frame drop (lost) on high resolution stream

[Matwey V. Kornilov]

2016-08-22 11:32 GMT+03:00 Matwey V. Kornilov :
> 2016-08-22 1:00 GMT+03:00 Alan Stern :
>> On Sun, 21 Aug 2016, Matwey V. Kornilov wrote:
>>
>>> In both cases (with or without HCD_BH), usb_hcd_giveback_urb is called
>>> every 0.01 sec. It is not clear why behavior is so different.
>>
>> What behavior are you asking about?  The difference between HCD_BH set
>> and not set?
>>
>
> The difference between HCD_BH set and not set is that when it is not
> set then usb_hcd_giveback_urb() receive zero-length URBs. And this
> breaks my pwc webcam. And the question is how to fix it.
> As far as I can see, usb_hcd_giveback_urb is being called with the
> same rate in both cases, so zero-length URBs are probably supposed to
> be data-carrying.
>

I can't understand what makes the difference. What I found to this
moment is the following:

1) isoc transfer works in two empirical modes or regimes. I called
them 'normal' one and 'broken'.
1a) In the 'normal' mode, every package is 956 bytes long and
c->desc->pd2 (see cppi41_irq) is 149a
1b) In the 'broken' mode, every package is 0 bytes long and
c->desc->pd2 (see cppi41_irq) is 1408009a
2) In each mode cppi41_irq is invoked every 1 ms.
2a) When the time lag between two subsequent calls of cppi41_irq is
greater (up to 2 ms) or less (0.3 ms) than 1 ms then the mode is
switched. It can happen inside single URB without calling complete().
So, the data are flowing in large bulks of either empty or full packages.
3) When HCD_BH is not set, then this two regimes are being flipped
constantly breaking internal pwc logic. When HCD_BH is set, then first
dozens packages are empty, then there is a pause between cppi41_irq
and the rest packages are fine.


>> Alan Stern
>>
>
>
>
> --
> With best regards,
> Matwey V. Kornilov.
> Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
> 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v3 0/2] Type-C Port Manager

[Guenter Roeck]

Hello Heikki,

On Wed, Aug 24, 2016 at 5:52 AM, Heikki Krogerus
 wrote:
> Hi Guenter,
>
> On Tue, Aug 23, 2016 at 02:10:49PM -0700, Guenter Roeck wrote:
>> The following series of patches implements a USB Type-C Port Manager
>> using the pending USB Type-C class code as basis. The code is still WIP,
>> but I think it is important to get feedback from the community at this point.
>>
>> There are two patches in the series. The first patch implements the Type-C
>> Port Manager state machine. The forth patch is an interface between the
>> Type-C Port Manager and a TCPCI (Type-C Port Controller Interface) compliant
>> USB Type-C Port Controller.
>>
>> Patch 2/2 (the interface to a TCPCI compliant chip) is currently untested
>> since I don't have the necessary hardware available. The port manager code
>> was tested connecting to an Embedded Controller on a Chromebook, bypassing
>> the Port Manager implementation in the EC.
>>
>> Both Source and Sink operation was tested with various Type-C chargers, 
>> docks,
>> and connectors. Alternate mode support is partially implemented (Alternate 
>> mode
>> support is requested from the partner), but alternate modes are not actually
>> selected. Implementing this will require more thought, since the actual
>> alternate mode support has to be implemented elsewhere, such as in a 
>> dedicated
>> Phy driver. It should be possible to implement the interface between phy 
>> driver
>> and Type-C Port Controller driver using extcon, but I have not further 
>> explored
>> the possibilities, and other options might be possible and/or better.
>>
>> v3:
>> - Improve TCPM state machine resiliency if there are spurious CC line changes
>>   while the state machine is in a transient change (waiting for a timeout)
>> - Update current limit after CC voltage level changes on a port which is not
>>   PD capable.
>> - Applies to v6 of "USB Type-C Connector class" patch series.
>>
>> v2:
>> - Class code no longer uses locking, so the patch to remove it is no longer
>>   necessary.
>> - tcpm: Only update polarity if setting it was successful
>>   If setting the CC line polarity in the driver was not successful,
>>   don't update the internal polarity state.
>> - tcpm: All PD messages are little endian; convert to and from CPU 
>> endianness.
>> - tcpm: Avoid comparisons against NULL.
>> - tcpm: Use u8/u16/u32 instead of uint8_t/uint16_t/uint32_t consistently.
>> - tcpm/tcpc: Callbacks into tcpm need to be lockless to avoid timing problems
>>   in low level drivers.
>> - tcpm/tcpc: Simplify callbacks; tcpm can request the current state of 
>> cc/vbus
>>   when it is ready to use it.
>> - Applies to v5 of "USB Type-C Connector class" patch series.
>
> I have not reviewed these yet completely, but so far the series looks
> really good. Nice job!
>
> There are a lot of things that look generic and not tied to TCPCs, for
> example the USB PD message handling. Couldn't those already be
> separated from TCPM code or made otherwise available for non-TCPC
> PHYs? The struct tcpc_dev looks to me like it would be usable for also
> non-TCPC PHYs as is and enough for most cases.
>
Makes sense, if not for anything else to reduce file size. I'll see if
I can separate it.

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

[PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

From: Rafał Miłecki 

This commit adds a new trigger responsible for turning on LED when USB
device gets connected to the specified USB port. This can can useful for
various home routers that have USB port(s) and a proper LED telling user
a device is connected.

The trigger gets its documentation file but basically it just requires
specifying USB port in a Linux format (e.g. echo 1-1 > new_port).

During work on this trigger there was a plan to add DT bindings for it,
but there wasn't an agreement on the format yet. This can be worked on
later, a sysfs interface is needed anyway for platforms not using DT.

Signed-off-by: Rafał Miłecki 
---
V2: Trying to add DT support, idea postponed as it will take more time
to discuss the bindings.
V3: Fix typos in commit and Documentation (thanks Jacek!)
Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
Check if there is USB device connected after adding new USB port
Fix memory leak or two
V3.5: Fix e-mail address (thanks Matthias)
  Simplify conditions in usbport_trig_notify (thx Matthias)
  Make "ports" a subdirectory with file per port, to match one value
  per file sysfs rule (thanks Greg)
  As "ports" couldn't be used for adding and removing ports anymore,
  there are now "new_port" and "remove_port". Having them makes this
  API also common with e.g. pci and usb buses.

The last big missing thing is Documentation update (this is why I'm
sending RFC). Greg pointed out we should have some entries in
Documentation/ABI, but it seems none of triggers have it. Any idea why
is that? Do we need to change it? Or is it required for new code only?
If so, should I care about Documentation/leds/ledtrig-usbport.txt at
all in this patch?

For now I didn't update Documentation/leds/ledtrig-usbport.txt with the
new new_port and remove_port API, until I get a clue how to proceed.
---
 Documentation/leds/ledtrig-usbport.txt |  49 ++
 drivers/leds/trigger/Kconfig   |   8 +
 drivers/leds/trigger/Makefile  |   1 +
 drivers/leds/trigger/ledtrig-usbport.c | 309 +
 4 files changed, 367 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-usbport.txt
 create mode 100644 drivers/leds/trigger/ledtrig-usbport.c

diff --git a/Documentation/leds/ledtrig-usbport.txt 
b/Documentation/leds/ledtrig-usbport.txt
new file mode 100644
index 000..fa42227
--- /dev/null
+++ b/Documentation/leds/ledtrig-usbport.txt
@@ -0,0 +1,49 @@
+USB port LED trigger
+
+
+This LED trigger can be used for signalling to the user a presence of USB 
device
+in a given port. It simply turns on LED when device appears and turns it off
+when it disappears.
+
+It requires specifying a list of USB ports that should be observed. Used format
+matches Linux kernel format and consists of a root hub number and a hub port
+separated by a dash (e.g. 3-1).
+
+It is also possible to handle devices with internal hubs (that are always
+connected to the root hub). User can simply specify internal hub ports then
+(e.g. 1-1.1, 1-1.2, etc.).
+
+Please note that this trigger allows assigning multiple USB ports to a single
+LED. This can be useful in two cases:
+
+1) Device with single USB LED and few physical ports
+
+In such a case LED will be turned on as long as there is at least one connected
+USB device.
+
+2) Device with a physical port handled by few controllers
+
+Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 physical
+port may be handled by ohci-platform, ehci-platform and xhci-hcd. If there is
+only one LED user will most likely want to assign ports from all 3 hubs.
+
+
+This trigger can be activated from user space on led class devices as shown
+below:
+
+  echo usbport > trigger
+
+This adds the following sysfs attributes to the LED:
+
+  ports - Reading it lists all USB ports assigned to the trigger. Writing USB
+ port number to it will make this driver start observing it. It's also
+ possible to remove USB port from observable list by writing it with a
+ "-" prefix.
+
+Example use-case:
+
+  echo usbport > trigger
+  echo 4-1 > ports
+  echo 2-1 > ports
+  echo -4-1 > ports
+  cat ports
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 3f9ddb9..bdd6fd2 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC
  a different trigger.
  If unsure, say Y.
 
+config LEDS_TRIGGER_USBPORT
+   tristate "USB port LED trigger"
+   depends on LEDS_TRIGGERS && USB
+   help
+ This allows LEDs to be controlled by USB events. Enabling this option
+ allows specifying list of USB ports that should turn on LED when some
+ USB device gets connected.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index a72c43c..56e1741 100644
--- 

Re: usb: chipidea: hdc: kernel panic during shutdown

[Stefan Wahren]

Hi,

[add Li Jun to CC]

> Alan Stern  hat am 24. August 2016 um 15:45
> geschrieben:
> 
> 
> On Wed, 24 Aug 2016, Peter Chen wrote:
> 
> > On Tue, Aug 23, 2016 at 09:17:02PM +0200, Stefan Wahren wrote:
> > > Hi,
> > > 
> > > i'm using a iMX233-OLinuXino board and the kernel panics during shutdown
> > > with
> > > 4.8.0-rc2-next-20160819:
> > > 
> > > [  420.04] ci_hdrc ci_hdrc.0: remove, state 1
> > > [  420.05] usb usb1: USB disconnect, device number 1
> > > [  420.06] usb 1-1: USB disconnect, device number 2
> > > [  420.06] usb 1-1.1: USB disconnect, device number 3
> > > [  420.09] smsc95xx 1-1.1:1.0 eth0: unregister 'smsc95xx'
> > > usb-ci_hdrc.0-1.1,
> > > smsc95xx USB 2.0 Ethernet
> > > [  420.29] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> > > [  420.30] Unable to handle kernel NULL pointer dereference at virtual
> > > address 0118
> > > [  420.30] pgd = c2ea4000
> > > [  420.30] [0118] *pgd=
> > > [  420.30] Internal error: Oops: 5 [#1] ARM
> > > [  420.30] Modules linked in:
> > > [  420.30] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted
> > > 4.8.0-rc2-next-20160819 #1
> > > [  420.30] Hardware name: Freescale MXS (Device Tree)
> > > [  420.30] task: c349 task.stack: c348e000
> > > [  420.30] PC is at usb_hcd_irq+0x0/0x34
> > > [  420.30] LR is at ci_irq+0x58/0x12c
> 
> > I am afraid the hcd is freed before the interrupt triggered. Would you
> > please try below changes:
> > 
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 96ae695..61237a9 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -103,7 +103,7 @@ static const struct ehci_driver_overrides
> > ehci_ci_overrides = {
> > static irqreturn_t host_irq(struct ci_hdrc *ci)
> > {
> > -   return usb_hcd_irq(ci->irq, ci->hcd);
> > +   return ci->hcd ? usb_hcd_irq(ci->irq, ci->hcd) : IRQ_NONE;
> > }
> 
> This should not be needed.  Instead, the driver should make sure that 
> the interrupt handler has been fully unregistered before the hcd is 
> deallocated.

according to ci_hdrc_probe() from the chipidea core the IRQ seems to be
requested via devm_request_irq() with the flag IRQF_SHARED. 

I have the suspicion the following commit triggers the kernel panic:

43a404577a93 ("usb: chipidea: host: set host to be null after hcd is freed")

I will validate this.

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

Re: [PATCH] bluetooth: bcm203x: don't print error when allocating urb fails

[Marcel Holtmann]

Hi Wolfram,

> kmalloc will print enough information in case of failure.
> 
> Signed-off-by: Wolfram Sang 
> ---
> drivers/bluetooth/bcm203x.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

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

Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM

[Alban Bedel]

On Wed, 24 Aug 2016 16:30:39 +0200
Oliver Neukum  wrote:

> On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote:
> > Implement the .set_eeprom callback to allow setting the MAC address
> > as well as a few other parameters. Note that the EEPROM must have a
> > correct PID/VID checksum set otherwise the SROM is used and reads
> > return the SROM content.
> > 
> > Signed-off-by: Alban Bedel 
> > ---
> >  drivers/net/usb/ax88179_178a.c | 57
> > ++
> >  1 file changed, 57 insertions(+)
> > 
> > diff --git a/drivers/net/usb/ax88179_178a.c
> > b/drivers/net/usb/ax88179_178a.c
> > index e6338c16081a..e6a986303dad 100644
> > --- a/drivers/net/usb/ax88179_178a.c
> > +++ b/drivers/net/usb/ax88179_178a.c
> > @@ -28,6 +28,7 @@
> >  
> >  #define AX88179_PHY_ID 0x03
> >  #define AX_EEPROM_LEN  0x100
> > +#define AX_EEPROM_BLOCK0x40u
> >  #define AX88179_EEPROM_MAGIC   0x17900b95
> >  #define AX_MCAST_FLTSIZE   8
> >  #define AX_MAX_MCAST   64
> > @@ -43,6 +44,7 @@
> >  #define AX_ACCESS_PHY  0x02
> >  #define AX_ACCESS_EEPROM   0x04
> >  #define AX_ACCESS_EFUS 0x05
> > +#define AX_RELOAD_EEPROM   0x06
> >  #define AX_PAUSE_WATERLVL_HIGH 0x54
> >  #define AX_PAUSE_WATERLVL_LOW  0x55
> >  
> > @@ -620,6 +622,60 @@ ax88179_get_eeprom(struct net_device *net, struct
> > ethtool_eeprom *eeprom,
> > return 0;
> >  }
> >  
> > +static int
> > +ax88179_set_eeprom(struct net_device *net, struct ethtool_eeprom
> > *eeprom,
> > +  u8 *data)
> > +{
> > +   struct usbnet *dev = netdev_priv(net);
> > +   unsigned int offset = eeprom->offset;
> > +   unsigned int len = eeprom->len;
> > +   int i, err = 0;
> > +   u8 *block;
> > +
> > +   /* The EEPROM data must be aligned on blocks of 64 bytes */
> > +   if ((offset % AX_EEPROM_BLOCK) || (len % AX_EEPROM_BLOCK)) {
> > +   offset = eeprom->offset / AX_EEPROM_BLOCK *
> > AX_EEPROM_BLOCK;
> > +   len = eeprom->len + eeprom->offset - offset;
> > +   len = DIV_ROUND_UP(len, AX_EEPROM_BLOCK) *
> > AX_EEPROM_BLOCK;
> > +
> > +   block = kmalloc(len, GFP_KERNEL);
> > +   if (!block)
> > +   return -ENOMEM;
> > +
> > +   /* Copy the current data, we could skip some but KISS
> > */
> > +   for (i = 0; i < len; i += AX_EEPROM_BLOCK) {
> > +   err = __ax88179_read_cmd(dev,
> > AX_ACCESS_EEPROM,
> > +(offset + i) >> 1,
> > +AX_EEPROM_BLOCK >> 1,
> > +AX_EEPROM_BLOCK,
> > +[i], 0);
> > +   if (err < 0) {
> > +   kfree(block);
> > +   return err;
> > +   }
> > +   }
> > +   memcpy(block + eeprom->offset - offset, data,
> > eeprom->len);
> > +   } else {
> > +   block = data;
> > +   }
> > +
> > +   for (i = 0; err >= 0 && i < len; i += AX_EEPROM_BLOCK) {
> > +   err = ax88179_write_cmd(dev, AX_ACCESS_EEPROM,
> > +   (offset + i) >> 1,
> > +   AX_EEPROM_BLOCK >> 1,
> > +   AX_EEPROM_BLOCK, [i]);
> > +   }
> > +
> > +   if (block != data)
> > +   kfree(block);  
> 
> And if block == dta, what frees the memory?

In this case this function didn't allocate any memory, so there is
nothing to free.

Alban 



pgpI1v3WKESy_.pgp
Description: OpenPGP digital signature

Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM

[Oliver Neukum]

On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote:
> Implement the .set_eeprom callback to allow setting the MAC address
> as well as a few other parameters. Note that the EEPROM must have a
> correct PID/VID checksum set otherwise the SROM is used and reads
> return the SROM content.
> 
> Signed-off-by: Alban Bedel 
> ---
>  drivers/net/usb/ax88179_178a.c | 57
> ++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/net/usb/ax88179_178a.c
> b/drivers/net/usb/ax88179_178a.c
> index e6338c16081a..e6a986303dad 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -28,6 +28,7 @@
>  
>  #define AX88179_PHY_ID 0x03
>  #define AX_EEPROM_LEN  0x100
> +#define AX_EEPROM_BLOCK0x40u
>  #define AX88179_EEPROM_MAGIC   0x17900b95
>  #define AX_MCAST_FLTSIZE   8
>  #define AX_MAX_MCAST   64
> @@ -43,6 +44,7 @@
>  #define AX_ACCESS_PHY  0x02
>  #define AX_ACCESS_EEPROM   0x04
>  #define AX_ACCESS_EFUS 0x05
> +#define AX_RELOAD_EEPROM   0x06
>  #define AX_PAUSE_WATERLVL_HIGH 0x54
>  #define AX_PAUSE_WATERLVL_LOW  0x55
>  
> @@ -620,6 +622,60 @@ ax88179_get_eeprom(struct net_device *net, struct
> ethtool_eeprom *eeprom,
> return 0;
>  }
>  
> +static int
> +ax88179_set_eeprom(struct net_device *net, struct ethtool_eeprom
> *eeprom,
> +  u8 *data)
> +{
> +   struct usbnet *dev = netdev_priv(net);
> +   unsigned int offset = eeprom->offset;
> +   unsigned int len = eeprom->len;
> +   int i, err = 0;
> +   u8 *block;
> +
> +   /* The EEPROM data must be aligned on blocks of 64 bytes */
> +   if ((offset % AX_EEPROM_BLOCK) || (len % AX_EEPROM_BLOCK)) {
> +   offset = eeprom->offset / AX_EEPROM_BLOCK *
> AX_EEPROM_BLOCK;
> +   len = eeprom->len + eeprom->offset - offset;
> +   len = DIV_ROUND_UP(len, AX_EEPROM_BLOCK) *
> AX_EEPROM_BLOCK;
> +
> +   block = kmalloc(len, GFP_KERNEL);
> +   if (!block)
> +   return -ENOMEM;
> +
> +   /* Copy the current data, we could skip some but KISS
> */
> +   for (i = 0; i < len; i += AX_EEPROM_BLOCK) {
> +   err = __ax88179_read_cmd(dev,
> AX_ACCESS_EEPROM,
> +(offset + i) >> 1,
> +AX_EEPROM_BLOCK >> 1,
> +AX_EEPROM_BLOCK,
> +[i], 0);
> +   if (err < 0) {
> +   kfree(block);
> +   return err;
> +   }
> +   }
> +   memcpy(block + eeprom->offset - offset, data,
> eeprom->len);
> +   } else {
> +   block = data;
> +   }
> +
> +   for (i = 0; err >= 0 && i < len; i += AX_EEPROM_BLOCK) {
> +   err = ax88179_write_cmd(dev, AX_ACCESS_EEPROM,
> +   (offset + i) >> 1,
> +   AX_EEPROM_BLOCK >> 1,
> +   AX_EEPROM_BLOCK, [i]);
> +   }
> +
> +   if (block != data)
> +   kfree(block);

And if block == dta, what frees the memory?

Regards
Oliver


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

Re: Hotplug not working for USB 3.1 docking station on Dell XPS 13

[Oliver Neukum]

On Wed, 2016-08-24 at 14:43 +0100, Richard van der Hoff wrote:
> On 24/08/16 13:17, Oliver Neukum wrote:
> > On Wed, 2016-08-24 at 11:27 +0100, Richard van der Hoff wrote:
> >> I'm having problems with a Plugable USB-C docking station, with my
> >> laptop, a Dell XPS 13 (9350). If the docking station is plugged in at
> >> boot, it works correctly; however, when I hotplug it after boot, the
> >> USB
> >> devices are not detected until I force a rescan of the PCI bus.
> >>
> >> ...
> >
> > It looks like your initial diagnosis was correct. This is a PCI
> > issue.
> 
> Right. Do you happen to know if the bugzilla is the best place to pursue 
> it, or if there is an alternative mailing list?

linux-...@vger.kernel.org

HTH
Oliver


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

Re: Hotplug not working for USB 3.1 docking station on Dell XPS 13

[Richard van der Hoff]

On 24/08/16 15:17, Oliver Neukum wrote:

On Wed, 2016-08-24 at 14:43 +0100, Richard van der Hoff wrote:

It looks like your initial diagnosis was correct. This is a PCI
issue.


Right. Do you happen to know if the bugzilla is the best place to pursue
it, or if there is an alternative mailing list?


linux-...@vger.kernel.org


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

Re: Bug 153551: Kernel panic on Nexus 5X USB unplug while tethering

[Alan Stern]

On Wed, 24 Aug 2016, Mathias Nyman wrote:

> 
> >> Reference counting is supposed to keep everything important from being
> >> deallocated until all the releases are finished.  In particular, if the
> >> hcd structure was already deallocated when usb_put_hcd() was called
> >> then there is a refcounting bug somewhere.
> >
> > That seems like a possible cause, I'll look into the refcouting.
> >
> > This helps a lot. I was getting stuck with that bug.
> > Thanks
> 
> Just to update on this,
> It appears to be caused by the same address and bandwidth mutex free bug that 
> you
> already fixed in ab2a4bf USB: don't free bandwidth_mutex too early

Yeah, that's the commit which resets the peer->primary_hcd pointer 
when a shared hcd is released.

> The sleep() worked as it delayed freeing the primary hcd, changing the
> order to first release usb3 hcd and then usb2 hcd.

Does this mean that the patch you already posted is the proper fix?

Alan Stern

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

Re: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Alan Stern]

On Wed, 24 Aug 2016, Ritesh Raj Sarraf wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA512
> 
> On Tue, 2016-08-23 at 15:14 -0400, Alan Stern wrote:
> > Okay, good.  The "Driver=rtsx_usb" is what I wanted to see.  Something
> > funny is going on with that driver -- it claims to support autosuspend 
> > but it doesn't ever call usb_mark_last_busy() or 
> > usb_autopm_get_interface().
> > 
> > In the meantime, can you post the output from "lsmod"?
> > 
> > Also, I'd like to see the output from the following (do this before 
> > the device disappears):
> > 
> > cd /sys/bus/pci/devices/:00:14.0/usb?/usb?-4/
> > ls -lR
> > 
> 
> Please find the asked output at the following links:
> 
> https://people.debian.org/~rrs/tmp/usb-ls-lR.txt
> https://people.debian.org/~rrs/tmp/lsmod-usb.txt

Do you happen to know which driver is being used: the memstick
(rtsx_usb_ms) or mmc (rtsx_usb_sdmmc) driver?  I suppose this may 
depend on what type of card you insert in the reader.

> I have a question though. For power saving, I use Laptop Mode Tools. Based on
> power state (AC or BATT), it enables/disables power saving knobs.
> 
> https://github.com/rickysarraf/laptop-mode-tools
> 
> Since I suspected the same problem (incapable of LPM) I have had this device
> blacklisted in Laptop Mode Tools. Here are the attributes for this particular
> device, while on battery.
> 
> rrs@learner:/sys/bus/usb/devices/1-4$ cat power/autosuspend
> 0
> 2016-08-24 / 14:47:07 ♒♒♒  ☺  
> rrs@learner:/sys/bus/usb/devices/1-4$ cat power/control 
> on
> 2016-08-24 / 14:47:34 ♒♒♒  ☺  
> rrs@learner:/sys/bus/usb/devices/1-4$ cat power/level 
> on
> 2016-08-24 / 14:47:38 ♒♒♒  ☺  
> 
> 
> While I write this, the bug hasn't triggered again. I think what may have
> happened is this:
> 
> * OS Boots
> * Goes on battery
> * LMT sets OS to power saving mode, but blacklists device 1-4
> * Eventually, the device resets (device/driver bug?)
> * Upon reset, driver re-initializes the power saving values to default (?).
>   At this time LMT will not re-apply any settings, because it only gets 
> invoked
> when a power_supply state change is sensed, i.e. ON_BATT or ON_AC.

That sounds like a bug in LMT.  It ought to handle hotplug events.  You 
could report that to the authors.

As you mentioned above, there's another aspect to power management 
besides runtime PM, namely Link Power Management.  Perhaps the device 
can't handle LPM.

You can test this by editing the usb_device_supports_lpm() routine in 
drivers/usb/core/hub.c.  If you make it always return 0 immediately, 
that will disable LPM for all USB devices.  If the spontaneous 
disconnects don't reappear, we'll have the answer.

Alan Stern

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

Re: [PATCHv6 1/3] usb: USB Type-C connector class

[Vincent Palatin]

Sorry if I'm making redundant comments with previous discussions, I
might have missed a few threads.


On Mon, Aug 22, 2016 at 2:05 PM, Heikki Krogerus
 wrote:
> The purpose of USB Type-C connector class is to provide
> unified interface for the user space to get the status and
> basic information about USB Type-C connectors on a system,
> control over data role swapping, and when the port supports
> USB Power Delivery, also control over power role swapping
> and Alternate Modes.
>
> Signed-off-by: Heikki Krogerus 
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  199 +
>  Documentation/usb/typec.txt |  103 +++
>  MAINTAINERS |9 +
>  drivers/usb/Kconfig |2 +
>  drivers/usb/Makefile|2 +
>  drivers/usb/typec/Kconfig   |7 +
>  drivers/usb/typec/Makefile  |1 +
>  drivers/usb/typec/typec.c   | 1090 
> +++
>  include/linux/usb/typec.h   |  260 +++
>  9 files changed, 1673 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
>  create mode 100644 Documentation/usb/typec.txt
>  create mode 100644 drivers/usb/typec/Kconfig
>  create mode 100644 drivers/usb/typec/Makefile
>  create mode 100644 drivers/usb/typec/typec.c
>  create mode 100644 include/linux/usb/typec.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-typec 
> b/Documentation/ABI/testing/sysfs-class-typec
> new file mode 100644
> index 000..e6179d3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,199 @@
> +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> +
> +What:  /sys/class/typec//current_data_role
> +Date:  June 2016
> +Contact:   Heikki Krogerus 
> +Description:
> +   The current USB data role the port is operating in. This
> +   attribute can be used for requesting data role swapping on the
> +   port.

role swapping is sometimes a long operation. maybe we need to say
explicitly whether the 'write' is synchronous and returns when the
swap has succeeded / failed or asynchronous (and requires polling
current_data_role afterwards to know the result ?)


> +
> +   Valid values:
> +   - host
> +   - device

the USB workgroup has settled for DFP/UFP rather than host/device ?


> +
> +What:  /sys/class/typec//current_power_role
> +Date:  June 2016
> +Contact:   Heikki Krogerus 
> +Description:
> +   The current power role of the port. This attribute can be used
> +   to request power role swap on the port when the port supports

ditto


> +   USB Power Delivery.
> +
> +   Valid values:
> +   - source
> +   - sink
> +
> +What:  /sys/class/typec//current_vconn_role
> +Date:  June 2016
> +Contact:   Heikki Krogerus 
> +Description:
> +   Shows the current VCONN role of the port. This attribute can 
> be
> +   used to request VCONN role swap on the port when the port
> +   supports USB Power Delivery.
> +
> +   Valid values are:
> +   - source
> +   - sink


either we are currently sourcing vconn or not, but even if you are
not, you are probably not a vconn sink either (ie only vconn-powered
accessory are, your usual linux-powered laptop/phone is probably not)


> +
> +What:  /sys/class/typec//power_operation_mode
> +Date:  June 2016
> +Contact:   Heikki Krogerus 
> +Description:
> +   Shows the current power operational mode the port is in.
> +
> +   Valid values:
> +   - USB - Normal power levels defined in USB specifications
> +   - BC1.2 - Power levels defined in Battery Charging 
> Specification
> + v1.2
> +   - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C
> +   specification.
> +   - USB Type-C 3.0A - Higher 3A current defined in USB Type-C
> +   specification.
> +- USB Power Delivery - The voltages and currents defined in 
> USB
> +  Power Delivery specification
> +
> +What:  /sys/class/typec//preferred_role
> +Date:  June 2016
> +Contact:   Heikki Krogerus 
> +Description:
> +   The user space can notify the driver about the preferred role.
> +   It should be handled as enabling of Try.SRC or Try.SNK, as
> +   defined in USB Type-C specification, in the port drivers. 

Re: Bug 153551: Kernel panic on Nexus 5X USB unplug while tethering

[Mathias Nyman]

Reference counting is supposed to keep everything important from being
deallocated until all the releases are finished.  In particular, if the
hcd structure was already deallocated when usb_put_hcd() was called
then there is a refcounting bug somewhere.


That seems like a possible cause, I'll look into the refcouting.

This helps a lot. I was getting stuck with that bug.
Thanks


Just to update on this,
It appears to be caused by the same address and bandwidth mutex free bug that 
you
already fixed in ab2a4bf USB: don't free bandwidth_mutex too early

The sleep() worked as it delayed freeing the primary hcd, changing the
order to first release usb3 hcd and then usb2 hcd.

-Mathias


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

[PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM

[Alban Bedel]

Implement the .set_eeprom callback to allow setting the MAC address
as well as a few other parameters. Note that the EEPROM must have a
correct PID/VID checksum set otherwise the SROM is used and reads
return the SROM content.

Signed-off-by: Alban Bedel 
---
 drivers/net/usb/ax88179_178a.c | 57 ++
 1 file changed, 57 insertions(+)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index e6338c16081a..e6a986303dad 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -28,6 +28,7 @@
 
 #define AX88179_PHY_ID 0x03
 #define AX_EEPROM_LEN  0x100
+#define AX_EEPROM_BLOCK0x40u
 #define AX88179_EEPROM_MAGIC   0x17900b95
 #define AX_MCAST_FLTSIZE   8
 #define AX_MAX_MCAST   64
@@ -43,6 +44,7 @@
 #define AX_ACCESS_PHY  0x02
 #define AX_ACCESS_EEPROM   0x04
 #define AX_ACCESS_EFUS 0x05
+#define AX_RELOAD_EEPROM   0x06
 #define AX_PAUSE_WATERLVL_HIGH 0x54
 #define AX_PAUSE_WATERLVL_LOW  0x55
 
@@ -620,6 +622,60 @@ ax88179_get_eeprom(struct net_device *net, struct 
ethtool_eeprom *eeprom,
return 0;
 }
 
+static int
+ax88179_set_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom,
+  u8 *data)
+{
+   struct usbnet *dev = netdev_priv(net);
+   unsigned int offset = eeprom->offset;
+   unsigned int len = eeprom->len;
+   int i, err = 0;
+   u8 *block;
+
+   /* The EEPROM data must be aligned on blocks of 64 bytes */
+   if ((offset % AX_EEPROM_BLOCK) || (len % AX_EEPROM_BLOCK)) {
+   offset = eeprom->offset / AX_EEPROM_BLOCK * AX_EEPROM_BLOCK;
+   len = eeprom->len + eeprom->offset - offset;
+   len = DIV_ROUND_UP(len, AX_EEPROM_BLOCK) * AX_EEPROM_BLOCK;
+
+   block = kmalloc(len, GFP_KERNEL);
+   if (!block)
+   return -ENOMEM;
+
+   /* Copy the current data, we could skip some but KISS */
+   for (i = 0; i < len; i += AX_EEPROM_BLOCK) {
+   err = __ax88179_read_cmd(dev, AX_ACCESS_EEPROM,
+(offset + i) >> 1,
+AX_EEPROM_BLOCK >> 1,
+AX_EEPROM_BLOCK,
+[i], 0);
+   if (err < 0) {
+   kfree(block);
+   return err;
+   }
+   }
+   memcpy(block + eeprom->offset - offset, data, eeprom->len);
+   } else {
+   block = data;
+   }
+
+   for (i = 0; err >= 0 && i < len; i += AX_EEPROM_BLOCK) {
+   err = ax88179_write_cmd(dev, AX_ACCESS_EEPROM,
+   (offset + i) >> 1,
+   AX_EEPROM_BLOCK >> 1,
+   AX_EEPROM_BLOCK, [i]);
+   }
+
+   if (block != data)
+   kfree(block);
+
+   /* Reload the EEPROM */
+   if (err >= 0)
+   err = ax88179_write_cmd(dev, AX_RELOAD_EEPROM, 0, 0, 0, NULL);
+
+   return err < 0 ? err : 0;
+}
+
 static int ax88179_get_settings(struct net_device *net, struct ethtool_cmd 
*cmd)
 {
struct usbnet *dev = netdev_priv(net);
@@ -826,6 +882,7 @@ static const struct ethtool_ops ax88179_ethtool_ops = {
.set_wol= ax88179_set_wol,
.get_eeprom_len = ax88179_get_eeprom_len,
.get_eeprom = ax88179_get_eeprom,
+   .set_eeprom = ax88179_set_eeprom,
.get_settings   = ax88179_get_settings,
.set_settings   = ax88179_set_settings,
.get_eee= ax88179_get_eee,
-- 
2.9.3

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

Re: usb: chipidea: hdc: kernel panic during shutdown

[Alan Stern]

On Wed, 24 Aug 2016, Peter Chen wrote:

> On Tue, Aug 23, 2016 at 09:17:02PM +0200, Stefan Wahren wrote:
> > Hi,
> > 
> > i'm using a iMX233-OLinuXino board and the kernel panics during shutdown 
> > with
> > 4.8.0-rc2-next-20160819:
> > 
> > [  420.04] ci_hdrc ci_hdrc.0: remove, state 1
> > [  420.05] usb usb1: USB disconnect, device number 1
> > [  420.06] usb 1-1: USB disconnect, device number 2
> > [  420.06] usb 1-1.1: USB disconnect, device number 3
> > [  420.09] smsc95xx 1-1.1:1.0 eth0: unregister 'smsc95xx' 
> > usb-ci_hdrc.0-1.1,
> > smsc95xx USB 2.0 Ethernet
> > [  420.29] ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> > [  420.30] Unable to handle kernel NULL pointer dereference at virtual
> > address 0118
> > [  420.30] pgd = c2ea4000
> > [  420.30] [0118] *pgd=
> > [  420.30] Internal error: Oops: 5 [#1] ARM
> > [  420.30] Modules linked in:
> > [  420.30] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted
> > 4.8.0-rc2-next-20160819 #1
> > [  420.30] Hardware name: Freescale MXS (Device Tree)
> > [  420.30] task: c349 task.stack: c348e000
> > [  420.30] PC is at usb_hcd_irq+0x0/0x34
> > [  420.30] LR is at ci_irq+0x58/0x12c

> I am afraid the hcd is freed before the interrupt triggered. Would you
> please try below changes:
> 
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 96ae695..61237a9 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -103,7 +103,7 @@ static const struct ehci_driver_overrides 
> ehci_ci_overrides = {
> static irqreturn_t host_irq(struct ci_hdrc *ci)
> {
> -   return usb_hcd_irq(ci->irq, ci->hcd);
> +   return ci->hcd ? usb_hcd_irq(ci->irq, ci->hcd) : IRQ_NONE;
> }

This should not be needed.  Instead, the driver should make sure that 
the interrupt handler has been fully unregistered before the hcd is 
deallocated.

Alan Stern

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

Re: Hotplug not working for USB 3.1 docking station on Dell XPS 13

[Richard van der Hoff]

On 24/08/16 13:17, Oliver Neukum wrote:

On Wed, 2016-08-24 at 11:27 +0100, Richard van der Hoff wrote:

I'm having problems with a Plugable USB-C docking station, with my
laptop, a Dell XPS 13 (9350). If the docking station is plugged in at
boot, it works correctly; however, when I hotplug it after boot, the
USB
devices are not detected until I force a rescan of the PCI bus.

...


It looks like your initial diagnosis was correct. This is a PCI
issue.


Right. Do you happen to know if the bugzilla is the best place to pursue 
it, or if there is an alternative mailing list?



What did you do at 814s after boot?


Nothing, as far as I know. Possibly I switched to a text console? I 
don't *think* the error at that point is related.



Please don't just add a log without comments if you do something.
One cannot tell what happens in response to what.


Noted. The comments at 
https://bugzilla.kernel.org/show_bug.cgi?id=151261#c3 were my best 
attempt at explaining what I'd done, but I appreciate they might be 
insufficient.


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

Re: [RFC PATCH v3 0/2] Type-C Port Manager

[Heikki Krogerus]

Hi Guenter,

On Tue, Aug 23, 2016 at 02:10:49PM -0700, Guenter Roeck wrote:
> The following series of patches implements a USB Type-C Port Manager
> using the pending USB Type-C class code as basis. The code is still WIP,
> but I think it is important to get feedback from the community at this point.
> 
> There are two patches in the series. The first patch implements the Type-C
> Port Manager state machine. The forth patch is an interface between the
> Type-C Port Manager and a TCPCI (Type-C Port Controller Interface) compliant
> USB Type-C Port Controller.
> 
> Patch 2/2 (the interface to a TCPCI compliant chip) is currently untested
> since I don't have the necessary hardware available. The port manager code
> was tested connecting to an Embedded Controller on a Chromebook, bypassing
> the Port Manager implementation in the EC.
> 
> Both Source and Sink operation was tested with various Type-C chargers, docks,
> and connectors. Alternate mode support is partially implemented (Alternate 
> mode
> support is requested from the partner), but alternate modes are not actually
> selected. Implementing this will require more thought, since the actual
> alternate mode support has to be implemented elsewhere, such as in a dedicated
> Phy driver. It should be possible to implement the interface between phy 
> driver
> and Type-C Port Controller driver using extcon, but I have not further 
> explored
> the possibilities, and other options might be possible and/or better.
> 
> v3:
> - Improve TCPM state machine resiliency if there are spurious CC line changes
>   while the state machine is in a transient change (waiting for a timeout)
> - Update current limit after CC voltage level changes on a port which is not
>   PD capable.
> - Applies to v6 of "USB Type-C Connector class" patch series.
> 
> v2:
> - Class code no longer uses locking, so the patch to remove it is no longer
>   necessary.
> - tcpm: Only update polarity if setting it was successful
>   If setting the CC line polarity in the driver was not successful,
>   don't update the internal polarity state.
> - tcpm: All PD messages are little endian; convert to and from CPU endianness.
> - tcpm: Avoid comparisons against NULL.
> - tcpm: Use u8/u16/u32 instead of uint8_t/uint16_t/uint32_t consistently.
> - tcpm/tcpc: Callbacks into tcpm need to be lockless to avoid timing problems
>   in low level drivers.
> - tcpm/tcpc: Simplify callbacks; tcpm can request the current state of cc/vbus
>   when it is ready to use it.
> - Applies to v5 of "USB Type-C Connector class" patch series.

I have not reviewed these yet completely, but so far the series looks
really good. Nice job!

There are a lot of things that look generic and not tied to TCPCs, for
example the USB PD message handling. Couldn't those already be
separated from TCPM code or made otherwise available for non-TCPC
PHYs? The struct tcpc_dev looks to me like it would be usable for also
non-TCPC PHYs as is and enough for most cases.


Thanks,

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

Re: Hotplug not working for USB 3.1 docking station on Dell XPS 13

[Oliver Neukum]

On Wed, 2016-08-24 at 11:27 +0100, Richard van der Hoff wrote:
> I'm having problems with a Plugable USB-C docking station, with my 
> laptop, a Dell XPS 13 (9350). If the docking station is plugged in at 
> boot, it works correctly; however, when I hotplug it after boot, the
> USB 
> devices are not detected until I force a rescan of the PCI bus.
> 
> I originally assumed this was a problem with the PCI subsystem, so 
> reported it at https://bugzilla.kernel.org/show_bug.cgi?id=151261,
> but 
> was sent on from there to here. In any case, there are some dmesg
> traces 
> and lspci logs attached to that bug.

It looks like your initial diagnosis was correct. This is a PCI
issue. What did you do at 814s after boot?
Please don't just add a log without comments if you do something.
One cannot tell what happens in response to what.

Regards
Oliver


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

Re: [PATCH] USB: serial: option: add WeTelecom 0x6802 and 0x6803 products

[Johan Hovold]

On Wed, Aug 24, 2016 at 01:06:22PM +0300, Aleksandr Makarov wrote:
> These product IDs are listed in Windows driver.
> 0x6803 corresponds to WeTelecom WM-D300.
> 0x6802 name is unknown.
> 
> Signed-off-by: Aleksandr Makarov 

Now applied, thanks.

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Matthias Brugger]

On 24/08/16 13:02, Rafał Miłecki wrote:

On 24 August 2016 at 12:49, Matthias Brugger  wrote:

On 24/08/16 00:03, Rafał Miłecki wrote:


[...]


+static int usbport_trig_notify(struct notifier_block *nb, unsigned long
action,
+  void *data)
+{
+   struct usbport_trig_data *usbport_data =
+   container_of(nb, struct usbport_trig_data, nb);
+   struct led_classdev *led_cdev = usbport_data->led_cdev;
+
+   switch (action) {
+   case USB_DEVICE_ADD:
+   if (usbport_trig_usb_dev_observed(usbport_data, data)) {



Maybe we should switch this and fist see if the usbport is observed before
evaluating the action. Also cast data to "struct usb_device *" to make that
clear.


I'm aware there is one duplicated line of code, I did to first
evaluate very quick test (checking unsigned long value), then iterate
over ports & keep only 1 switch block. I could move
usbport_trig_usb_dev_observed call up, but it would be executed for
other actions as well (currently just USB_BUS_ADD and USB_BUS_REMOVE).



Ok. I'm a USB noop but from my understanding the notifier is only called 
when a device or a hub gets added/removed. So this shouldn't happen that 
much. Therefor it has no impact if we check if the usb device is in the 
observer list for all actions.





+   if (usbport_data->count++ == 0)



I'm a bit puzzled. I think:
if (++usbport_data->count > 0)
makes this more consistent with the remove case.


Your condition would be always true (as we don't use negative
numbers). The point is to enable LED only if it was disabled before.
So I need to increase counter unconditionally but enable LED only if
initial value (before increasing it) was 0.



Got it. My personal opinion is, that adding one line for 
incrementing/decrementing the counter would help to make this 
crystal-clear to everyone (at least to me :)


Cheers,
Matthias




+module_init(usbport_trig_init);
+module_exit(usbport_trig_exit);
+
+MODULE_AUTHOR("Rafał Miłecki ");



Nit: ra...@milecki.pl


Oops, thanks!


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

Re: [PATCH V5, 0/5] Add MediaTek USB3 DRD Driver

[Oliver Neukum]

On Wed, 2016-08-24 at 14:42 +0800, chunfeng yun wrote:
> Dear all,
> 
> Could you please help me to review the code? 

Is the structure

struct qmu_gpd

shared with the hardware? Do I read this correctly that
you do PIO to endpoint 0 but DMA to the others?

Could you resend the series?

Regards
Oliver


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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Matthias Brugger]

On 24/08/16 00:03, Rafał Miłecki wrote:

From: Rafał Miłecki 

This commit adds a new trigger responsible for turning on LED when USB
device gets connected to the specified USB port. This can can useful for
various home routers that have USB port(s) and a proper LED telling user
a device is connected.

The trigger gets its documentation file but basically it just requires
specifying USB port in a Linux format (e.g. echo 1-1 > new_port).

During work on this trigger there was a plan to add DT bindings for it,
but there wasn't an agreement on the format yet. This can be worked on
later, a sysfs interface is needed anyway for platforms not using DT.

Signed-off-by: Rafał Miłecki 
---
V2: Trying to add DT support, idea postponed as it will take more time
to discuss the bindings.
V3: Fix typos in commit and Documentation (thanks Jacek!)
Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
Check if there is USB device connected after adding new USB port
Fix memory leak or two

Felipe: I'd like to ask for your Ack before having this patch pushed.
---
 Documentation/leds/ledtrig-usbport.txt |  49 +++
 drivers/leds/trigger/Kconfig   |   8 ++
 drivers/leds/trigger/Makefile  |   1 +
 drivers/leds/trigger/ledtrig-usbport.c | 253 +
 4 files changed, 311 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-usbport.txt
 create mode 100644 drivers/leds/trigger/ledtrig-usbport.c

diff --git a/Documentation/leds/ledtrig-usbport.txt 
b/Documentation/leds/ledtrig-usbport.txt
new file mode 100644
index 000..fa42227
--- /dev/null
+++ b/Documentation/leds/ledtrig-usbport.txt
@@ -0,0 +1,49 @@
+USB port LED trigger
+
+
+This LED trigger can be used for signalling to the user a presence of USB 
device
+in a given port. It simply turns on LED when device appears and turns it off
+when it disappears.
+
+It requires specifying a list of USB ports that should be observed. Used format
+matches Linux kernel format and consists of a root hub number and a hub port
+separated by a dash (e.g. 3-1).
+
+It is also possible to handle devices with internal hubs (that are always
+connected to the root hub). User can simply specify internal hub ports then
+(e.g. 1-1.1, 1-1.2, etc.).
+
+Please note that this trigger allows assigning multiple USB ports to a single
+LED. This can be useful in two cases:
+
+1) Device with single USB LED and few physical ports
+
+In such a case LED will be turned on as long as there is at least one connected
+USB device.
+
+2) Device with a physical port handled by few controllers
+
+Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 physical
+port may be handled by ohci-platform, ehci-platform and xhci-hcd. If there is
+only one LED user will most likely want to assign ports from all 3 hubs.
+
+
+This trigger can be activated from user space on led class devices as shown
+below:
+
+  echo usbport > trigger
+
+This adds the following sysfs attributes to the LED:
+
+  ports - Reading it lists all USB ports assigned to the trigger. Writing USB
+ port number to it will make this driver start observing it. It's also
+ possible to remove USB port from observable list by writing it with a
+ "-" prefix.
+
+Example use-case:
+
+  echo usbport > trigger
+  echo 4-1 > ports
+  echo 2-1 > ports
+  echo -4-1 > ports
+  cat ports
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 3f9ddb9..bdd6fd2 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC
  a different trigger.
  If unsure, say Y.

+config LEDS_TRIGGER_USBPORT
+   tristate "USB port LED trigger"
+   depends on LEDS_TRIGGERS && USB
+   help
+ This allows LEDs to be controlled by USB events. Enabling this option
+ allows specifying list of USB ports that should turn on LED when some
+ USB device gets connected.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index a72c43c..56e1741 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o
 obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)   += ledtrig-transient.o
 obj-$(CONFIG_LEDS_TRIGGER_CAMERA)  += ledtrig-camera.o
 obj-$(CONFIG_LEDS_TRIGGER_PANIC)   += ledtrig-panic.o
+obj-$(CONFIG_LEDS_TRIGGER_USBPORT) += ledtrig-usbport.o
diff --git a/drivers/leds/trigger/ledtrig-usbport.c 
b/drivers/leds/trigger/ledtrig-usbport.c
new file mode 100644
index 000..7f5237c
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-usbport.c
@@ -0,0 +1,253 @@
+/*
+ * USB port LED trigger
+ *
+ * Copyright (C) 2016 Rafał Miłecki 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the 

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

On 24 August 2016 at 12:49, Matthias Brugger  wrote:
> On 24/08/16 00:03, Rafał Miłecki wrote:
>>
>> From: Rafał Miłecki 
>>
>> This commit adds a new trigger responsible for turning on LED when USB
>> device gets connected to the specified USB port. This can can useful for
>> various home routers that have USB port(s) and a proper LED telling user
>> a device is connected.
>>
>> The trigger gets its documentation file but basically it just requires
>> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
>>
>> During work on this trigger there was a plan to add DT bindings for it,
>> but there wasn't an agreement on the format yet. This can be worked on
>> later, a sysfs interface is needed anyway for platforms not using DT.
>>
>> Signed-off-by: Rafał Miłecki 
>> ---
>> V2: Trying to add DT support, idea postponed as it will take more time
>> to discuss the bindings.
>> V3: Fix typos in commit and Documentation (thanks Jacek!)
>> Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
>> Check if there is USB device connected after adding new USB port
>> Fix memory leak or two
>>
>> Felipe: I'd like to ask for your Ack before having this patch pushed.
>> ---
>>  Documentation/leds/ledtrig-usbport.txt |  49 +++
>>  drivers/leds/trigger/Kconfig   |   8 ++
>>  drivers/leds/trigger/Makefile  |   1 +
>>  drivers/leds/trigger/ledtrig-usbport.c | 253
>> +
>>  4 files changed, 311 insertions(+)
>>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
>>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c
>>
>> diff --git a/Documentation/leds/ledtrig-usbport.txt
>> b/Documentation/leds/ledtrig-usbport.txt
>> new file mode 100644
>> index 000..fa42227
>> --- /dev/null
>> +++ b/Documentation/leds/ledtrig-usbport.txt
>> @@ -0,0 +1,49 @@
>> +USB port LED trigger
>> +
>> +
>> +This LED trigger can be used for signalling to the user a presence of USB
>> device
>> +in a given port. It simply turns on LED when device appears and turns it
>> off
>> +when it disappears.
>> +
>> +It requires specifying a list of USB ports that should be observed. Used
>> format
>> +matches Linux kernel format and consists of a root hub number and a hub
>> port
>> +separated by a dash (e.g. 3-1).
>> +
>> +It is also possible to handle devices with internal hubs (that are always
>> +connected to the root hub). User can simply specify internal hub ports
>> then
>> +(e.g. 1-1.1, 1-1.2, etc.).
>> +
>> +Please note that this trigger allows assigning multiple USB ports to a
>> single
>> +LED. This can be useful in two cases:
>> +
>> +1) Device with single USB LED and few physical ports
>> +
>> +In such a case LED will be turned on as long as there is at least one
>> connected
>> +USB device.
>> +
>> +2) Device with a physical port handled by few controllers
>> +
>> +Some devices have e.g. one controller per PHY standard. E.g. USB 3.0
>> physical
>> +port may be handled by ohci-platform, ehci-platform and xhci-hcd. If
>> there is
>> +only one LED user will most likely want to assign ports from all 3 hubs.
>> +
>> +
>> +This trigger can be activated from user space on led class devices as
>> shown
>> +below:
>> +
>> +  echo usbport > trigger
>> +
>> +This adds the following sysfs attributes to the LED:
>> +
>> +  ports - Reading it lists all USB ports assigned to the trigger. Writing
>> USB
>> + port number to it will make this driver start observing it. It's
>> also
>> + possible to remove USB port from observable list by writing it
>> with a
>> + "-" prefix.
>> +
>> +Example use-case:
>> +
>> +  echo usbport > trigger
>> +  echo 4-1 > ports
>> +  echo 2-1 > ports
>> +  echo -4-1 > ports
>> +  cat ports
>> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
>> index 3f9ddb9..bdd6fd2 100644
>> --- a/drivers/leds/trigger/Kconfig
>> +++ b/drivers/leds/trigger/Kconfig
>> @@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC
>>   a different trigger.
>>   If unsure, say Y.
>>
>> +config LEDS_TRIGGER_USBPORT
>> +   tristate "USB port LED trigger"
>> +   depends on LEDS_TRIGGERS && USB
>> +   help
>> + This allows LEDs to be controlled by USB events. Enabling this
>> option
>> + allows specifying list of USB ports that should turn on LED when
>> some
>> + USB device gets connected.
>> +
>>  endif # LEDS_TRIGGERS
>> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
>> index a72c43c..56e1741 100644
>> --- a/drivers/leds/trigger/Makefile
>> +++ b/drivers/leds/trigger/Makefile
>> @@ -10,3 +10,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) +=
>> ledtrig-default-on.o
>>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)   += ledtrig-transient.o
>>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)  += ledtrig-camera.o
>>  obj-$(CONFIG_LEDS_TRIGGER_PANIC)   += ledtrig-panic.o
>> 

Hotplug not working for USB 3.1 docking station on Dell XPS 13

[Richard van der Hoff]

Hi,

I'm having problems with a Plugable USB-C docking station, with my 
laptop, a Dell XPS 13 (9350). If the docking station is plugged in at 
boot, it works correctly; however, when I hotplug it after boot, the USB 
devices are not detected until I force a rescan of the PCI bus.


I originally assumed this was a problem with the PCI subsystem, so 
reported it at https://bugzilla.kernel.org/show_bug.cgi?id=151261, but 
was sent on from there to here. In any case, there are some dmesg traces 
and lspci logs attached to that bug.


The lspci output suggests a chain of three PCI bridges (00:1c.0, 
01:00.0, 02:02.0) before a PCI->USB bridge (05:00.0 or 39:00.0). If I 
boot without the docking station plugged in, 01:00.0 and beyond do not 
appear, and nor do they appear on hotplug. However, if I hotplug the 
docking station and then force a rescan with "echo 1 > 
/sys/bus/pci/devices/:00:1c.0/rescan", the PCI bridges and USB 
devices appear and spring into life.


There are some ACPI errors in the dmesg at hotplug but they don't really 
mean much to me.


I'd really appreciate any suggestions on how to proceed with this.

Thanks

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

Re: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Ritesh Raj Sarraf]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

Hello Alan,

Please find some findings below.

On Wed, 2016-08-24 at 15:01 +0530, Ritesh Raj Sarraf wrote:
> I have a question though. For power saving, I use Laptop Mode Tools. Based on
> power state (AC or BATT), it enables/disables power saving knobs.
> 
> https://github.com/rickysarraf/laptop-mode-tools
> 
> Since I suspected the same problem (incapable of LPM) I have had this device
> blacklisted in Laptop Mode Tools. Here are the attributes for this particular
> device, while on battery.
> 
> rrs@learner:/sys/bus/usb/devices/1-4$ cat power/autosuspend
> 0
> 2016-08-24 / 14:47:07 ♒♒♒  ☺  
> rrs@learner:/sys/bus/usb/devices/1-4$ cat power/control 
> on
> 2016-08-24 / 14:47:34 ♒♒♒  ☺  
> rrs@learner:/sys/bus/usb/devices/1-4$ cat power/level 
> on
> 2016-08-24 / 14:47:38 ♒♒♒  ☺  
> 
> 
> While I write this, the bug hasn't triggered again. I think what may have
> happened is this:
> 
> * OS Boots
> * Goes on battery
> * LMT sets OS to power saving mode, but blacklists device 1-4
> * Eventually, the device resets (device/driver bug?)
> * Upon reset, driver re-initializes the power saving values to default (?).
>   At this time LMT will not re-apply any settings, because it only gets
> invoked
> when a power_supply state change is sensed, i.e. ON_BATT or ON_AC.

As I had suspected, that is exactly what happened.


rrs@learner:/sys/bus/usb/devices/1-4$ cat power/autosuspend
2
2016-08-24 / 15:44:08 ♒♒♒  ☺  

rrs@learner:/sys/bus/usb/devices/1-4$ cat power/control 
auto
2016-08-24 / 15:44:17 ♒♒♒  ☺  

rrs@learner:/sys/bus/usb/devices/1-4$ cat power/level 
auto
2016-08-24 / 15:44:25 ♒♒♒  ☺  


I picked these values after I noticed the device reset.

rrs@learner:/sys/bus/usb/devices/1-4$ dmesg | tail -n 20
[ 1730.712177] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 1731.434646] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 1731.523118] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 1733.983893] wlan0: authenticate with c4:6e:1f:d0:67:26
[ 1734.006873] wlan0: send auth to c4:6e:1f:d0:67:26 (try 1/3)
[ 1734.011307] wlan0: authenticated
[ 1734.012745] wlan0: associate with c4:6e:1f:d0:67:26 (try 1/3)
[ 1734.017849] wlan0: RX AssocResp from c4:6e:1f:d0:67:26 (capab=0x411 status=0
aid=4)
[ 1734.018228] wlan0: associated
[ 1734.018237] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
[ 1734.316964] systemd[1]: apt-daily.timer: Adding 1h 3min 47.910780s random
time.
[ 1734.455413] systemd[1]: apt-daily.timer: Adding 2h 14min 54.292999s random
time.
[ 3183.459386] WARNING! power/level is deprecated; use power/control instead
[ 6516.374635] usb 1-4: USB disconnect, device number 2
[ 6516.832012] usb 1-4: new high-speed USB device number 7 using xhci_hcd
[ 6517.005736] usb 1-4: New USB device found, idVendor=0bda, idProduct=0129
[ 6517.005740] usb 1-4: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 6517.005742] usb 1-4: Product: USB2.0-CRW
[ 6517.005743] usb 1-4: Manufacturer: Generic
[ 6517.005745] usb 1-4: SerialNumber: 2010020139600
2016-08-24 / 15:46:57 ♒♒♒  ☺  


- -- 
Ritesh Raj Sarraf
RESEARCHUT - http://www.researchut.com
"Necessity is the mother of invention."
-BEGIN PGP SIGNATURE-

iQIcBAEBCgAGBQJXvXRwAAoJEKY6WKPy4XVpmqQQAKMsDyX1iWLueQtiQOfId7l8
L3ah4BzTz027714URMzaM3WD0AfrjQy81AjMDp6cO9HEhqbTRwNhSk/uY4dyr0hz
uwmpm1hnMCtLEk6knvB5zsBN1JLd7vKGl1fGs+WG9+JyeNP/sWD7zumqTDhL8OD7
47ueP2xsV+j2PIB1cLCFPvTFdcLaAwK1oPh8+7lN/EBWkSr9deFS06gV09GAFQqB
5olbKSQDb8zo6n25VAHLxUAOIjJ5frEjT/GJMF6RfVuL4gCcc9mfYc7ZgVueDnVw
hHrNQK7dPRdH0kCZw/kTOpePafgbgYqX43F7j8T1REHVmC20fYhnYYpBAOqR+XZ9
Ynb9A4CDA3+wcHarL/th/BQXNsVlQeYm4NXn8yLCtf6jUTEjJgUSqIyw7yto4owv
3zOX3kCx2+LCdg7WwJu4aCJEjwozOI+3n9A2F1A4P6c578JV1pAsZybl1HB4ivZ9
+Wz2mW2yjPkh3/D8w0zdTFa/0nOlg2W0klPsHJJzNcyKUGP0ka4ygym0FH8N+IzQ
PVfLlsIOHVPabLkW/1WZvJTVyWwX29FoQ7Pq0ffeQc9aXp08+tyY8q7HbjmUKZYy
wArb0/eajDIb1D0nwcVtVNKCgNTqB2+xlvk5SqtvT1N/t1yQUdiJktMDLh4mIigc
4DV1MXqR9pxzzLXl8/VD
=rkLp
-END PGP SIGNATURE-

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

[PATCH] USB: serial: option: add WeTelecom 0x6802 and 0x6803 products

[Aleksandr Makarov]

These product IDs are listed in Windows driver.
0x6803 corresponds to WeTelecom WM-D300.
0x6802 name is unknown.

Signed-off-by: Aleksandr Makarov 
---
 drivers/usb/serial/option.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index bb6a711..9894e34 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -528,6 +528,8 @@ static void option_instat_callback(struct urb *urb);
 /* WeTelecom products */
 #define WETELECOM_VENDOR_ID0x22de
 #define WETELECOM_PRODUCT_WMD200   0x6801
+#define WETELECOM_PRODUCT_6802 0x6802
+#define WETELECOM_PRODUCT_WMD300   0x6803
 
 struct option_blacklist_info {
/* bitmask of interface numbers blacklisted for send_setup */
@@ -1996,6 +1998,8 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(INOVIA_VENDOR_ID, INOVIA_SEW858) },
{ USB_DEVICE(VIATELECOM_VENDOR_ID, VIATELECOM_PRODUCT_CDS7) },
{ USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, 
WETELECOM_PRODUCT_WMD200, 0xff, 0xff, 0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, 
WETELECOM_PRODUCT_6802, 0xff, 0xff, 0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, 
WETELECOM_PRODUCT_WMD300, 0xff, 0xff, 0xff) },
{ } /* Terminating entry */
 };
 MODULE_DEVICE_TABLE(usb, option_ids);
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xHCI problem? [was Re: Erratic USB device behavior and device loss]

[Ritesh Raj Sarraf]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On Tue, 2016-08-23 at 15:14 -0400, Alan Stern wrote:
> Okay, good.  The "Driver=rtsx_usb" is what I wanted to see.  Something
> funny is going on with that driver -- it claims to support autosuspend 
> but it doesn't ever call usb_mark_last_busy() or 
> usb_autopm_get_interface().
> 
> In the meantime, can you post the output from "lsmod"?
> 
> Also, I'd like to see the output from the following (do this before 
> the device disappears):
> 
> cd /sys/bus/pci/devices/:00:14.0/usb?/usb?-4/
> ls -lR
> 

Please find the asked output at the following links:

https://people.debian.org/~rrs/tmp/usb-ls-lR.txt
https://people.debian.org/~rrs/tmp/lsmod-usb.txt


> Finally, you can try applying this patch.  It should prevent the 
> constant runtime suspend & resume cycling, but it is not the proper 
> solution to your problem.  And it may not prevent the occasional 
> spontaneous disconnect.
> 

I am building the kernel right now. I'll post back results after trying out the
new kernel. 

I have a question though. For power saving, I use Laptop Mode Tools. Based on
power state (AC or BATT), it enables/disables power saving knobs.

https://github.com/rickysarraf/laptop-mode-tools

Since I suspected the same problem (incapable of LPM) I have had this device
blacklisted in Laptop Mode Tools. Here are the attributes for this particular
device, while on battery.

rrs@learner:/sys/bus/usb/devices/1-4$ cat power/autosuspend
0
2016-08-24 / 14:47:07 ♒♒♒  ☺  
rrs@learner:/sys/bus/usb/devices/1-4$ cat power/control 
on
2016-08-24 / 14:47:34 ♒♒♒  ☺  
rrs@learner:/sys/bus/usb/devices/1-4$ cat power/level 
on
2016-08-24 / 14:47:38 ♒♒♒  ☺  


While I write this, the bug hasn't triggered again. I think what may have
happened is this:

* OS Boots
* Goes on battery
* LMT sets OS to power saving mode, but blacklists device 1-4
* Eventually, the device resets (device/driver bug?)
* Upon reset, driver re-initializes the power saving values to default (?).
  At this time LMT will not re-apply any settings, because it only gets invoked
when a power_supply state change is sensed, i.e. ON_BATT or ON_AC.


I think your patch will clarify this once I've booted into it.

Thank you very much.

Ritesh

> Alan Stern
> 
> 
> 
> Index: usb-4.x/drivers/mfd/rtsx_usb.c
> ===
> --- usb-4.x.orig/drivers/mfd/rtsx_usb.c
> +++ usb-4.x/drivers/mfd/rtsx_usb.c
> @@ -780,7 +780,6 @@ static struct usb_driver rtsx_usb_driver
> .pre_reset  = rtsx_usb_pre_reset,
> .post_reset = rtsx_usb_post_reset,
> .id_table   = rtsx_usb_usb_ids,
> -   .supports_autosuspend   = 1,
> .soft_unbind= 1,
>  };
>  
- -- 
Ritesh Raj Sarraf
RESEARCHUT - http://www.researchut.com
"Necessity is the mother of invention."
-BEGIN PGP SIGNATURE-

iQIcBAEBCgAGBQJXvWmNAAoJEKY6WKPy4XVpQacP/Rcdtp010vEsOMibrn0RJbJM
J7uJ/dIUgYSJXTW138BcsRmkZ+D4+OVjaPYeIDNLd9+X1gw4GDozERoVPj9BzwJ5
rCNgIY8VhtrXOScrbqzJ3rJwngqCH5rYqZgAHhSxzZJGocWlHuMvhV893toRxMd0
A97PNZAWQv9GAyquIyQDqoKm1Tu6XdB2sP+jUtbh5l6GBA/G15InxqPtj/bYHq0a
PQ8me9WFxDvFnF6wF/pCDCJbCYkv+Z4HOYvj0sD3+/HaAL1RolZG4UbO0DYEYY4j
g9eqbtcTvs0OaFD/sUZj3rT6m488byenstnDqLHtkB+8hjYdTs/Cbt7HB5/AMjd+
Fo8i/4Tru1DpkNas6iQa9RsfH/0y0c4se98RCoPmuhxyYefdFHpRspgqw61lpdmo
hnac2fa1PfvT96Pywr6gv4haFBZc3vZQS1Xs8RceXB7ZBVeaLNEWNcL1IuqHP6yn
7F2J7bUYVOinmbVzEnc1VnbaMsQlXy7cWvVrAYRgf8hkQBgs6VkBjG4YEVo82jeR
T4qk34SFpqpSWO/3OAlu2GEY+JTHQ8VrUqS5lK+0ZbI4oA1IENnTI4HPE291yNyF
f5JCZ+Nip7WlXEK1tQ8WqyVrEGhxAcPaGKvr70mgten5MATo6fCSVpevn7aemA8k
OumrBZa1Wj8r7EG5xTuv
=CkGL
-END PGP SIGNATURE-

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

On 24 August 2016 at 11:22, Greg KH  wrote:
> On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
>> +static ssize_t ports_show(struct device *dev, struct device_attribute *attr,
>> +   char *buf)
>> +{
>> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data;
>> + struct usbport_trig_port *port;
>> + ssize_t ret = 0;
>> + int len;
>> +
>> + list_for_each_entry(port, _data->ports, list) {
>> + len = sprintf(buf + ret, "%s\n", port->name);
>> + if (len >= 0)
>> + ret += len;
>> + }
>> +
>> + return ret;
>> +}
>
> sysfs is "one value per file", here you are listing a bunch of things in
> one sysfs file.  Please don't do that.

OK. What do you think about creating "ports" subdirectory and creating
file-per-port in it? Then I'd need to bring back something like
"new_port" and "remove_port". Does it sound OK?

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

On 24 August 2016 at 11:21, Greg KH  wrote:
> On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>>
>> This commit adds a new trigger responsible for turning on LED when USB
>> device gets connected to the specified USB port. This can can useful for
>> various home routers that have USB port(s) and a proper LED telling user
>> a device is connected.
>>
>> The trigger gets its documentation file but basically it just requires
>> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
>>
>> During work on this trigger there was a plan to add DT bindings for it,
>> but there wasn't an agreement on the format yet. This can be worked on
>> later, a sysfs interface is needed anyway for platforms not using DT.
>>
>> Signed-off-by: Rafał Miłecki 
>> ---
>> V2: Trying to add DT support, idea postponed as it will take more time
>> to discuss the bindings.
>> V3: Fix typos in commit and Documentation (thanks Jacek!)
>> Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
>> Check if there is USB device connected after adding new USB port
>> Fix memory leak or two
>>
>> Felipe: I'd like to ask for your Ack before having this patch pushed.
>> ---
>>  Documentation/leds/ledtrig-usbport.txt |  49 +++
>>  drivers/leds/trigger/Kconfig   |   8 ++
>>  drivers/leds/trigger/Makefile  |   1 +
>>  drivers/leds/trigger/ledtrig-usbport.c | 253 
>> +
>>  4 files changed, 311 insertions(+)
>>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
>>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c
>
> You are adding sysfs files without adding a Documentation/ABI/ entry,
> please never do that.

This is the way all LED triggers are documented, I just blindly
assumed it's OK. Could you be a bit more helpful and help me to
understand further steps? Should I drop
Documentation/leds/ledtrig-usbport.txt and add proper entry in
Documentation/ABI/testing? Or should I keep both files? What about
current sysfs of other triggers? Should they be moved/documented in
ABI?

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Greg KH]

On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> +static ssize_t ports_show(struct device *dev, struct device_attribute *attr,
> +   char *buf)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data;
> + struct usbport_trig_port *port;
> + ssize_t ret = 0;
> + int len;
> +
> + list_for_each_entry(port, _data->ports, list) {
> + len = sprintf(buf + ret, "%s\n", port->name);
> + if (len >= 0)
> + ret += len;
> + }
> +
> + return ret;
> +}

sysfs is "one value per file", here you are listing a bunch of things in
one sysfs file.  Please don't do that.

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

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

[Greg KH]

On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> This commit adds a new trigger responsible for turning on LED when USB
> device gets connected to the specified USB port. This can can useful for
> various home routers that have USB port(s) and a proper LED telling user
> a device is connected.
> 
> The trigger gets its documentation file but basically it just requires
> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> 
> During work on this trigger there was a plan to add DT bindings for it,
> but there wasn't an agreement on the format yet. This can be worked on
> later, a sysfs interface is needed anyway for platforms not using DT.
> 
> Signed-off-by: Rafał Miłecki 
> ---
> V2: Trying to add DT support, idea postponed as it will take more time
> to discuss the bindings.
> V3: Fix typos in commit and Documentation (thanks Jacek!)
> Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> Check if there is USB device connected after adding new USB port
> Fix memory leak or two
> 
> Felipe: I'd like to ask for your Ack before having this patch pushed.
> ---
>  Documentation/leds/ledtrig-usbport.txt |  49 +++
>  drivers/leds/trigger/Kconfig   |   8 ++
>  drivers/leds/trigger/Makefile  |   1 +
>  drivers/leds/trigger/ledtrig-usbport.c | 253 
> +
>  4 files changed, 311 insertions(+)
>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c

You are adding sysfs files without adding a Documentation/ABI/ entry,
please never do that.

NAK.

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

Re: [PATCH v6 0/8] power: add power sequence library

[Peter Chen]

On Tue, Aug 23, 2016 at 04:02:48PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
> >Hi all,
> >
> >This is a follow-up for my last power sequence framework patch set [1].
> >According to Rob Herring and Ulf Hansson's comments[2], I use a generic
> >power sequence library for parsing the power sequence elements on DT,
> >and implement generic power sequence on library. The host driver
> >can allocate power sequence instance, and calls pwrseq APIs accordingly.
> >
> >In future, if there are special power sequence requirements, the special
> >power sequence library can be created.
> >
> >This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> >two hot-plug devices to simulate this use case, the related binding
> >change is updated at patch [1/6], The udoo board changes were tested
> >using my last power sequence patch set.[3]
> >
> >Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> >need to power on itself before it can be found by ULPI bus.
> >
> >[1] http://www.spinics.net/lists/linux-usb/msg142755.html
> >[2] http://www.spinics.net/lists/linux-usb/msg143106.html
> >[3] http://www.spinics.net/lists/linux-usb/msg142815.html
> (Please ignore my response on V2)
> 
> Sorry being so late in the discussion...
> 
> If I am not missing anything, then I am afraid to say that the
> generic library
> implementation in this patch series is not going to solve many of
> the custom
> requirement of power on, off, etc...
> I know you mentioned about adding another library when we come
> across such platforms, but should we not keep provision (or easy
> hooks/path)
> to enable that ?
> 
> Let me bring in the use case I am dealing with,
> 
> 
>   Host
>|
>V
>USB port
> 
>|
>V
>   USB HUB device (May need custom on/off seq)
>|
>V
>   =
>  | |
>  V V
>  Device-1   Device-2
> (Needs special power   (Needs special power
>  on/off sequence.   on/off sequence.
>  Also may need custom   Also, may need custom
>  sequence for   sequence for
>  suspend/resume)suspend/resume)
> 
> 
> Note: Both Devices are connected to HUB via HSIC and may differ
>   in terms of functionality, features they support.
> 
> In the above case, both Device-1 and Device-2, need separate
> power on/off sequence. So generic library currently we have in this
> patch series is not going to satisfy the need here.
> 
> I looked at all 6 revisions of this patch-series, went through the
> review comments, and looked at MMC power sequence code;
> what I can say here is, we need something similar to
> MMC power sequence here, where every device can have its own
> power sequence (if needed).
> 
> I know Rob is not in favor of creating platform device for
> this, and I understand his comment.
> If not platform device, but atleast we need mechanism to
> connect each device back to its of_node and its respective
> driver/library fns. For example, the Devices may support different
> boot modes, and platform driver needs to make sure that
> the right sequence is followed for booting.
> 
> Peter, My apologies for taking you back again on this series.
> I am OK, if you wish to address this in incremental addition,
> but my point is, we know that the current generic way is not
> enough for us, so I think we should try to fix it in initial phase only.
> 

Rob, it seems generic power sequence can't cover all cases.
Without information from DT, we can't know which power sequence
for which device.

-- 

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

[PATCH 1/4] usb: host: xhci: rcar: add a new firmware version for r8a7796

[Yoshihiro Shimoda]

This patch adds a new firmware version "V3" for r8a7796. This patch also
revises the explanation of each version's capability using a table.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/host/xhci-rcar.c | 17 -
 drivers/usb/host/xhci-rcar.h |  1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
index 0e4535e..723f575 100644
--- a/drivers/usb/host/xhci-rcar.c
+++ b/drivers/usb/host/xhci-rcar.c
@@ -19,13 +19,20 @@
 #include "xhci-rcar.h"
 
 /*
-* - The V2 firmware is possible to use on R-Car Gen2. However, the V2 causes
-*   performance degradation. So, this driver continues to use the V1 if R-Car
-*   Gen2.
-* - The V1 firmware is impossible to use on R-Car Gen3.
-*/
+ *|Gen2r8a7795 r8a7796
+ * ---+
+ * V3 |note1   NG  OK
+ * V2 |note1   OK  note1
+ * V1 |OK  NG  NG
+ *
+ * Gen2: r8a7790, r8a7791 and r8a7793
+ * OK: This firmware version works correctly on such SoC(s)
+ * NG: This firmware version is impossible to use on such SoC(s)
+ * note1: This firmware version causes performance degradation.
+ */
 MODULE_FIRMWARE(XHCI_RCAR_FIRMWARE_NAME_V1);
 MODULE_FIRMWARE(XHCI_RCAR_FIRMWARE_NAME_V2);
+MODULE_FIRMWARE(XHCI_RCAR_FIRMWARE_NAME_V3);
 
 /*** Register Offset ***/
 #define RCAR_USB3_INT_ENA  0x224   /* Interrupt Enable */
diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
index 2941a25..d2ffe20 100644
--- a/drivers/usb/host/xhci-rcar.h
+++ b/drivers/usb/host/xhci-rcar.h
@@ -13,6 +13,7 @@
 
 #define XHCI_RCAR_FIRMWARE_NAME_V1 "r8a779x_usb3_v1.dlmem"
 #define XHCI_RCAR_FIRMWARE_NAME_V2 "r8a779x_usb3_v2.dlmem"
+#define XHCI_RCAR_FIRMWARE_NAME_V3 "r8a779x_usb3_v3.dlmem"
 
 #if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
 void xhci_rcar_start(struct usb_hcd *hcd);
-- 
1.9.1

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

[PATCH 0/4] usb: host: xhci: plat: add support for Renesas r8a7796 SoC

[Yoshihiro Shimoda]

This patch is based on the latest Greg's usb.git / usb-next branch.
(commit id = 19acaea1dd2878d6c92b45e4c117ef425baf)

Yoshihiro Shimoda (4):
  usb: host: xhci: rcar: add a new firmware version for r8a7796
  usb: host: xhci: rcar: add a macro XHCI_PLAT_RENESAS_RCAR_PRIV
  usb: host: xhci: plat: use XHCI_PLAT_RENESAS_RCAR_PRIV macro for R-Car
  usb: host: xhci: plat: add support for Renesas r8a7796 SoC

 Documentation/devicetree/bindings/usb/usb-xhci.txt |  1 +
 drivers/usb/host/xhci-plat.c   | 17 ++---
 drivers/usb/host/xhci-rcar.c   | 18 +-
 drivers/usb/host/xhci-rcar.h   |  9 +
 4 files changed, 29 insertions(+), 16 deletions(-)

-- 
1.9.1

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

[PATCH 2/4] usb: host: xhci: rcar: add a macro XHCI_PLAT_RENESAS_RCAR_PRIV

[Yoshihiro Shimoda]

This patch adds a new macro "XHCI_PLAT_RENESAS_RCAR_PRIV" to make
struct xhci_plat_priv data for R-Car SoCs easily.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/host/xhci-rcar.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
index d2ffe20..78379e3 100644
--- a/drivers/usb/host/xhci-rcar.h
+++ b/drivers/usb/host/xhci-rcar.h
@@ -28,4 +28,12 @@ static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
return 0;
 }
 #endif
+
+#define XHCI_PLAT_RENESAS_RCAR_PRIV(soc, firmware) \
+static const struct xhci_plat_priv xhci_plat_renesas_rcar_##soc = {\
+   .firmware_name = firmware,  \
+   .init_quirk = xhci_rcar_init_quirk, \
+   .plat_start = xhci_rcar_start,  \
+}
+
 #endif /* _XHCI_RCAR_H */
-- 
1.9.1

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

[PATCH 3/4] usb: host: xhci: plat: use XHCI_PLAT_RENESAS_RCAR_PRIV macro for R-Car

[Yoshihiro Shimoda]

This patch uses XHCI_PLAT_RENESAS_RCAR_PRIV macro to make
the xhci_plat_priv data.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/host/xhci-plat.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..c1a6b5a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -88,17 +88,8 @@ static const struct xhci_plat_priv xhci_plat_marvell_armada 
= {
.init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
-static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
-   .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
-   .init_quirk = xhci_rcar_init_quirk,
-   .plat_start = xhci_rcar_start,
-};
-
-static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
-   .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
-   .init_quirk = xhci_rcar_init_quirk,
-   .plat_start = xhci_rcar_start,
-};
+XHCI_PLAT_RENESAS_RCAR_PRIV(gen2, XHCI_RCAR_FIRMWARE_NAME_V1);
+XHCI_PLAT_RENESAS_RCAR_PRIV(gen3, XHCI_RCAR_FIRMWARE_NAME_V2);
 
 static const struct of_device_id usb_xhci_of_match[] = {
{
-- 
1.9.1

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

[PATCH 4/4] usb: host: xhci: plat: add support for Renesas r8a7796 SoC

[Yoshihiro Shimoda]

This patch adds support for Renesas r8a7796 SoC. This SoC is not
compatible with r8a7795 because using firmware version differs.

Signed-off-by: Yoshihiro Shimoda 
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
 drivers/usb/host/xhci-plat.c   | 4 
 drivers/usb/host/xhci-rcar.c   | 1 +
 3 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 966885c..0b7d857 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -11,6 +11,7 @@ Required properties:
 - "renesas,xhci-r8a7791" for r8a7791 SoC
 - "renesas,xhci-r8a7793" for r8a7793 SoC
 - "renesas,xhci-r8a7795" for r8a7795 SoC
+- "renesas,xhci-r8a7796" for r8a7796 SoC
 - "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 compatible device
 - "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
 - "xhci-platform" (deprecated)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index c1a6b5a..3dc4bc4d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -90,6 +90,7 @@ static const struct xhci_plat_priv xhci_plat_marvell_armada = 
{
 
 XHCI_PLAT_RENESAS_RCAR_PRIV(gen2, XHCI_RCAR_FIRMWARE_NAME_V1);
 XHCI_PLAT_RENESAS_RCAR_PRIV(gen3, XHCI_RCAR_FIRMWARE_NAME_V2);
+XHCI_PLAT_RENESAS_RCAR_PRIV(r8a7796, XHCI_RCAR_FIRMWARE_NAME_V3);
 
 static const struct of_device_id usb_xhci_of_match[] = {
{
@@ -115,6 +116,9 @@ static const struct of_device_id usb_xhci_of_match[] = {
.compatible = "renesas,xhci-r8a7795",
.data = _plat_renesas_rcar_gen3,
}, {
+   .compatible = "renesas,xhci-r8a7796",
+   .data = _plat_renesas_rcar_r8a7796,
+   }, {
.compatible = "renesas,rcar-gen2-xhci",
.data = _plat_renesas_rcar_gen2,
}, {
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
index 723f575..49f7698 100644
--- a/drivers/usb/host/xhci-rcar.c
+++ b/drivers/usb/host/xhci-rcar.c
@@ -99,6 +99,7 @@ static int xhci_rcar_is_gen3(struct device *dev)
struct device_node *node = dev->of_node;
 
return of_device_is_compatible(node, "renesas,xhci-r8a7795") ||
+  of_device_is_compatible(node, "renesas,xhci-r8a7796") ||
of_device_is_compatible(node, "renesas,rcar-gen3-xhci");
 }
 
-- 
1.9.1

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

Re: [RESEND PATCH 3/4] usb: dwc2: assert phy reset when waking up in rk3288 platform

[Randy Li]

On 08/24/2016 04:46 AM, John Youn wrote:

On 8/21/2016 12:32 PM, Randy Li wrote:

On the rk3288 USB host-only port (the one that's not the OTG-enabled
port) the PHY can get into a bad state when a wakeup is asserted (not
just a wakeup from full system suspend but also a wakeup from
autosuspend).

We can get the PHY out of its bad state by asserting its "port reset",
but unfortunately that seems to assert a reset onto the USB bus so it
could confuse things if we don't actually deenumerate / reenumerate the
device.

We can also get the PHY out of its bad state by fully resetting it using
the reset from the CRU (clock reset unit) in chip, which does a more full
reset.  The CRU-based reset appears to actually cause devices on the bus
to be removed and reinserted, which fixes the problem (albeit in a hacky
way).

It's unfortunate that we need to do a full re-enumeration of devices at
wakeup time, but this is better than alternative of letting the bus get
wedged.

Signed-off-by: Randy Li 
---
 drivers/usb/dwc2/core_intr.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index d85c5c9..f57c48a 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -345,6 +345,7 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg 
*hsotg)
 static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
 {
int ret;
+   struct device_node *np = hsotg->dev->of_node;

/* Clear interrupt */
dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
@@ -379,6 +380,16 @@ static void dwc2_handle_wakeup_detected_intr(struct 
dwc2_hsotg *hsotg)
/* Restart the Phy Clock */
pcgcctl &= ~PCGCTL_STOPPCLK;
dwc2_writel(pcgcctl, hsotg->regs + PCGCTL);
+
+   /*
+* It is a quirk in Rockchip RK3288, causing by
+* a hardware bug. This will propagate out and
+* eventually we'll re-enumerate the device.
+* Not great but the best we can do


Remove the trailing whitespaces in this comment.

Run scripts/checkpatch.pl to catch these.

I see.



+*/
+   if (of_device_is_compatible(np, "rockchip,rk3288-usb"))
+   hsotg->phy->ops->reset(hsotg->phy);
+


You should probably check for NULL before calling the reset()
callback.

Sure.


Also, I'd rather see a generic quirk property that you set for your
platform.

Something like "phy_reset_on_wakeup_quirk".
But Rob Herring want me to implied by the SoC specific compatible 
string. I agree with him. It is certainly bug in RK3288 platform.

It is no found no the other platform.


Also, try to preserve the version tag in your subject for all the
patches so that we can easily identify the latest version of the
series, like:

[PATCH v5 3/4] ...

And, typically "RESEND" means there are no code change.

I see, I just make up my mind whether code style is new version
before. I won't again.

Thank you very much.


Regards,
John

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip



--
Randy Li
The third produce department

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

Re: chipidea: udc: kernel panic in isr_setup_status_phase

[Peter Chen]

On Tue, Aug 23, 2016 at 02:36:30AM +0200, Clemens Gruber wrote:
> Hi,
> 
> I am using an i.MX6Q embedded board, acting as a (ethernet) gadget with
> RNDIS function, connected over an USB OTG cable to a PC.
> Most of the time it works fine, but in some mysterious circumstances,
> a kernel panic occurs, just after attaching the OTG cable, connecting it
> to the other machine:
> 
> [   54.012989] Unable to handle kernel NULL pointer dereference at virtual 
> address 0020
> [   54.021099] pgd = 80004000
> [   54.023816] [0020] *pgd=
> [   54.027422] Internal error: Oops: 817 [#1] PREEMPT SMP ARM
> [   54.032915] Modules linked in:
> [   54.035998] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
> 4.8.0-rc3-00017-g336bc4a #315
> [   54.043662] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [   54.050196] task: 80b05f80 task.stack: 80b0
> [   54.054744] PC is at isr_setup_status_phase+0x1c/0x40
> [   54.059805] LR is at 0xbe570890
> [   54.062957] pc : [<804ac464>]lr : []psr: 200e0193
> [   54.062957] sp : 80b01e10  ip : be570570  fp : be570890
> [   54.074442] r10: be5eeebc  r9 : be570010  r8 : be5eeebc
> [   54.079673] r7 : be5708d0  r6 : be5eee80  r5 : be7fcf40  r4 : 0001
> [   54.086206] r3 : be571010  r2 : 804ab368  r1 :   r0 : be570010
> [   54.092742] Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment 
> none
> [   54.099972] Control: 10c5387d  Table: 4e34404a  DAC: 0051
> [   54.105723] Process swapper/0 (pid: 0, stack limit = 0x80b00210)
> (snip)
> [   54.247100] [<804ac464>] (isr_setup_status_phase) from [<804acbbc>] 
> (isr_tr_complete_handler+0x734/0x98c)
> [   54.256680] [<804acbbc>] (isr_tr_complete_handler) from [<804acfc0>] 
> (udc_irq+0x1ac/0x318)
> [   54.264964] [<804acfc0>] (udc_irq) from [<8018ba28>] 
> (__handle_irq_event_percpu+0x9c/0x128)
> [   54.273330] [<8018ba28>] (__handle_irq_event_percpu) from [<8018bae0>] 
> (handle_irq_event_percpu+0x2c/0x7c)
> [   54.282995] [<8018bae0>] (handle_irq_event_percpu) from [<8018bb68>] 
> (handle_irq_event+0x38/0x5c)
> [   54.291880] [<8018bb68>] (handle_irq_event) from [<8018f2cc>] 
> (handle_fasteoi_irq+0xd0/0x1bc)
> [   54.300418] [<8018f2cc>] (handle_fasteoi_irq) from [<8018afb0>] 
> (generic_handle_irq+0x24/0x34)
> [   54.309042] [<8018afb0>] (generic_handle_irq) from [<8018b2dc>] 
> (__handle_domain_irq+0x7c/0xec)
> [   54.317754] [<8018b2dc>] (__handle_domain_irq) from [<80101524>] 
> (gic_handle_irq+0x38/0x74)
> [   54.326119] [<80101524>] (gic_handle_irq) from [<8010ccb0>] 
> (__irq_svc+0x70/0xb0)
> (snip)
> 
> After looking through the isr_setup_status_phase disassembly, I found
> that ci->status must have been NULL and dereferencing it in
> ci->status->context = ci; triggered the panic.
> 
> The interrupt was a USBINT (UI bit was set) and isr_tr_complete_handler
> was called from udc_irq.
> In the IMX6DQRM I read about the UI bit: "This bit is also set by the
> Host/Device Controller when a short packet is detected." and about
> USBERRINT / UEI bit: "This bit is set along with the USBINT bit, if the
> TD on which the error interrupt occurred also had its interrupt on
> complete (IOC) bit set." (page 5494)
> 
> However, we do not check for UEI in udc_irq.
> Could this be the cause of this error?

UEI is an error interrupt, and software have not handled it, so it will
not affect ci->status.

> Should we only call isr_tr_complete_handler if UI && !UEI ?
> 
> Or would adding a check for ci->status == NULL in isr_setup-status_phase
> and returning an error code also be a good idea?

I agree with that.

> 
> Do you have an idea what's going on there and why ci->status is NULL?
> 

I can't understand it, the only possible is the last disconnect event
(see ci_udc_vbus_session->_gadget_stop_activity) has scheduled very late
due to vbus lowers very slow.

-- 

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

[PATCH] usb: renesas_usbhs: add a compatible string for r8a7796

[Yoshihiro Shimoda]

This patch adds support for r8a7796 (R-Car M3-W).

Signed-off-by: Yoshihiro Shimoda 
---
 Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 1 +
 drivers/usb/renesas_usbhs/common.c  | 4 
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index b604056..9e18e00 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -9,6 +9,7 @@ Required properties:
- "renesas,usbhs-r8a7793" for r8a7793 (R-Car M2-N) compatible device
- "renesas,usbhs-r8a7794" for r8a7794 (R-Car E2) compatible device
- "renesas,usbhs-r8a7795" for r8a7795 (R-Car H3) compatible device
+   - "renesas,usbhs-r8a7796" for r8a7796 (R-Car M3-W) compatible device
- "renesas,rcar-gen2-usbhs" for R-Car Gen2 compatible device
- "renesas,rcar-gen3-usbhs" for R-Car Gen3 compatible device
 
diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index ac67bab..012a37a 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -482,6 +482,10 @@ static const struct of_device_id usbhs_of_match[] = {
.data = (void *)USBHS_TYPE_RCAR_GEN3,
},
{
+   .compatible = "renesas,usbhs-r8a7796",
+   .data = (void *)USBHS_TYPE_RCAR_GEN3,
+   },
+   {
.compatible = "renesas,rcar-gen2-usbhs",
.data = (void *)USBHS_TYPE_RCAR_GEN2,
},
-- 
1.9.1

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

Re: [PATCH v2 03/22] usb: ulpi: Support device discovery via device properties

[Heikki Krogerus]

On Tue, Aug 23, 2016 at 12:58:07PM -0700, Stephen Boyd wrote:
> On Fri, Aug 5, 2016 at 2:27 PM, Stephen Boyd  wrote:
> > Quoting Peter Chen (2016-07-08 02:04:58)
> >> On Thu, Jul 07, 2016 at 03:20:54PM -0700, Stephen Boyd wrote:
> >> > @@ -39,6 +42,10 @@ static int ulpi_match(struct device *dev, struct 
> >> > device_driver *driver)
> >> >   struct ulpi *ulpi = to_ulpi_dev(dev);
> >> >   const struct ulpi_device_id *id;
> >> >
> >> > + /* Some ULPI devices don't have a product id so rely on OF match */
> >> > + if (ulpi->id.product == 0)
> >> > + return of_driver_match_device(dev, driver);
> >> > +
> >>
> >> How about using vendor id? It can't be 0, but pid may be 0.
> >> See: http://www.linux-usb.org/usb.ids
> >
> > Heikki suggested a product id of 0 would mean we need to use DT
> > matching. Should it be changed to vendor id instead?
> 
> Any comments here?

It makes sense. I don't have any problem with that.

Thanks,

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

[PATCH] usb: gadget: net2280: fix typo: "Inavlid" -> "Invalid"

[Colin King]

From: Colin Ian King 

trivial typo fix in dev_err message

Signed-off-by: Colin Ian King 
---
 drivers/usb/gadget/udc/net2280.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index 614ab951..d8c9ab4 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -589,7 +589,7 @@ static void net2280_free_request(struct usb_ep *_ep, struct 
usb_request *_req)
 
ep = container_of(_ep, struct net2280_ep, ep);
if (!_ep || !_req) {
-   dev_err(>dev->pdev->dev, "%s: Inavlid ep=%p or req=%p\n",
+   dev_err(>dev->pdev->dev, "%s: Invalid ep=%p or req=%p\n",
__func__, _ep, _req);
return;
}
-- 
2.9.3

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

Re: [PATCH V5, 0/5] Add MediaTek USB3 DRD Driver

[chunfeng yun]

Dear all,

Could you please help me to review the code? 

Thank you very much.

 
On Tue, 2016-08-09 at 16:23 +0800, Chunfeng Yun wrote:
> These patches introduce the MediaTek USB3 dual-role controller
> driver.
> 
> The driver can be configured as Dual-Role Device (DRD),
> Peripheral Only and Host Only (xHCI) modes. It works well
> with Mass Storage, RNDIS and g_zero on FS/HS and SS. And it is
> tested on MT8173 platform which only contains USB2.0 device IP,
> and on MT6290 platform which contains USB3.0 device IP.
> 
> Change in v5:
> 1. modify some comments
> 2. rename some unsuitable variables
> 3. add reg-names property for host node
> 4. add USB_MTU3_DEBUG to control debug messages
> 
> Change in v4:
> 1. fix build errors on non-mediatek platforms
> 2. provide manual dual-role switch via debugfs instead of sysfs
> 
> Change in v3:
> 1. fix some typo error
> 2. rename mtu3.txt to mt8173-mtu3.txt
> 
> Change in v2:
> 1. modify binding docs according to suggestions
> 2. modify some comments and remove some dummy blank lines
> 3. fix memory leakage
> 
> 
> Chunfeng Yun (5):
>   dt-bindings: mt8173-xhci: support host side of dual-role mode
>   dt-bindings: mt8173-mtu3: add devicetree bindings
>   usb: xhci-mtk: make IPPC register optional
>   usb: Add MediaTek USB3 DRD Driver
>   arm64: dts: mediatek: add USB3 DRD driver
> 
>  .../devicetree/bindings/usb/mt8173-mtu3.txt|   87 ++
>  .../devicetree/bindings/usb/mt8173-xhci.txt|   54 +-
>  arch/arm64/boot/dts/mediatek/mt8173-evb.dts|   46 +-
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi   |   29 +-
>  drivers/usb/Kconfig|2 +
>  drivers/usb/Makefile   |1 +
>  drivers/usb/host/xhci-mtk.c|   36 +-
>  drivers/usb/mtu3/Kconfig   |   54 ++
>  drivers/usb/mtu3/Makefile  |   19 +
>  drivers/usb/mtu3/mtu3.h|  422 ++
>  drivers/usb/mtu3/mtu3_core.c   |  874 +++
>  drivers/usb/mtu3/mtu3_dr.c |  375 +
>  drivers/usb/mtu3/mtu3_dr.h |  108 +++
>  drivers/usb/mtu3/mtu3_gadget.c |  731 
>  drivers/usb/mtu3/mtu3_gadget_ep0.c |  879 
> 
>  drivers/usb/mtu3/mtu3_host.c   |  294 +++
>  drivers/usb/mtu3/mtu3_hw_regs.h|  473 +++
>  drivers/usb/mtu3/mtu3_plat.c   |  490 +++
>  drivers/usb/mtu3/mtu3_qmu.c|  599 +
>  drivers/usb/mtu3/mtu3_qmu.h|   43 +
>  20 files changed, 5598 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
>  create mode 100644 drivers/usb/mtu3/Kconfig
>  create mode 100644 drivers/usb/mtu3/Makefile
>  create mode 100644 drivers/usb/mtu3/mtu3.h
>  create mode 100644 drivers/usb/mtu3/mtu3_core.c
>  create mode 100644 drivers/usb/mtu3/mtu3_dr.c
>  create mode 100644 drivers/usb/mtu3/mtu3_dr.h
>  create mode 100644 drivers/usb/mtu3/mtu3_gadget.c
>  create mode 100644 drivers/usb/mtu3/mtu3_gadget_ep0.c
>  create mode 100644 drivers/usb/mtu3/mtu3_host.c
>  create mode 100644 drivers/usb/mtu3/mtu3_hw_regs.h
>  create mode 100644 drivers/usb/mtu3/mtu3_plat.c
>  create mode 100644 drivers/usb/mtu3/mtu3_qmu.c
>  create mode 100644 drivers/usb/mtu3/mtu3_qmu.h
> 


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

Re: [PATCH V9 1/1] usb:serial: Add Fintek F81532/534 driver

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2016/8/23 下午 05:50 寫道:

On Tue, Aug 23, 2016 at 04:23:44PM +0800, Ji-Ze Hong (Peter Hong) wrote:

Hi Johan,

Johan Hovold 於 2016/8/22 下午 09:14 寫道:


I'd say it's not worth trying to avoid that extra allocation, and there
will be several further allocations done in the usb_control_msg path
anyway. What you have today (i.e. in v9) is fine.


Ok, I'll keep set/get register the same with V9.



+   tty_port_num = f81534_phy_to_logic_port(serial, phy_port_num);
+   port = serial->port[tty_port_num];
+
+   /*
+* The device will send back all information when we submitted
+* a read URB (MSR/DATA/TX_EMPTY). But it maybe get callback
+* before port_probe() or after port_remove().
+*
+* So we'll verify the pointer. If the pointer is NULL, it's
+* mean the port not init complete and the block will skip.
+*/
+   port_priv = usb_get_serial_port_data(port);


Check if the port has been opened here instead, no need to store MSR for
an unused port above.


It's useless for MSR & Receive data when port is closed, but we need
the URB to receive TX empty flag. We may not received TX empty flag
if we don't process when port is closed. It'll make the port not
workable.


But you explicitly clear the xmit fifo on open it seems?



The F81532/534 contains 2 blocks of H/W designs. One is a 16550A
compatible UART with 128 bytes FIFO, and another is a USB bridge with
DMA to access UART TX/RX FIFO and handle USB protocols.

The clear FIFO in f81534_open() is just clean UART TX/RX FIFO, not USB
bridge's RAM. So we must keep a read URB for get newest information via
USB bridge likes TX empty.

I'll try again to re-write the section as you mention, submit on first
open(), kill on last close() and test for some times. If had no other
issues, I'll apply to next patch, otherwise I'll preserve old method.

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