Re: [patch -target tree] usb: gadget: f_tcm: use after free

[Nicholas A. Bellinger]

On Wed, 2016-03-09 at 13:38 +0200, Felipe Balbi wrote:
> Hi,
> 
> "Nicholas A. Bellinger"  writes:
> > [ text/plain ]
> > Hi Felipe + usb-gadget folks,
> >
> > On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
> >> Dan Carpenter  writes:
> >> > We need to move the kfree() down a line so we don't dereference a freed
> >> > variable.
> >> >
> >> > Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to 
> >> > target_alloc_session')
> >> > Signed-off-by: Dan Carpenter 
> >> 
> >> It's okay to take this via target:
> >> 
> >> Signed-off-by: Felipe Balbi 
> >> 
> >
> > Note this specific patch is only a mechanical change, and we still need
> > reviews for the more interesting conversions:
> >
> > usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> > http://www.spinics.net/lists/target-devel/msg11777.html
> >
> > usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> > http://www.spinics.net/lists/target-devel/msg11782.html
> >
> I don't have either patch in my inbox apparently. Care to resend ?
> 
> sorry
> 

A -v4 is going on this week, and will make sure they hit your inbox.

--
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 -target tree] usb: gadget: f_tcm: use after free

[Nicholas A. Bellinger]

Hi Andrzej,

On Wed, 2016-03-09 at 13:53 +0100, Andrzej Pietrasiewicz wrote:
> Hi Nicholas,
> 
> W dniu 05.03.2016 o 08:26, Nicholas A. Bellinger pisze:
> > Hi Felipe + usb-gadget folks,
> >
> > On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
> 
> 
> 
> >
> > usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> > http://www.spinics.net/lists/target-devel/msg11777.html
> >
> > usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> > http://www.spinics.net/lists/target-devel/msg11782.html
> >
> > Felipe, Sebastian, & Andrezj, would you be so kind to review and test
> > usb-gadget using target-pending/for-next code..?
> >
> >
> 
> Actually I have noticed a problem at
> 8b00965 "target: Convert demo-mode only drivers to target_alloc_session"
> 
> I get:
> 
> [ 1698.406304] configfs-gadget gadget: high-speed config #1: c
> [ 1698.410547] Using the BOT protocol
> [ 1698.414163] Missing nexus, ignoring command
> [ 1698.417944] bot_cmd_complete(309): -22
> 
> I think the third message is from f_tcm. It is because
> tpg->tpg_nexus is not set anymore, because the line
> 
>   tpg->tpg_nexus = tv_nexus;
> 
> is removed by the commit mentioned above.
> 
> Restoring this line fixes the problem.

Thanks for testing!

Applying the following patch to re-add the missing assingment
as a proper alloc_session callback.

diff --git a/drivers/usb/gadget/function/f_tcm.c 
b/drivers/usb/gadget/function/f_tcm.c
index e352a31..348140c 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1570,6 +1570,16 @@ out:
return ret;
 }
 
+static int usbg_alloc_sess_cb(struct se_portal_group *se_tpg,
+ struct se_session *se_sess, void *p)
+{
+   struct usbg_tpg *tpg = container_of(se_tpg,
+   struct usbg_tpg, se_tpg);
+
+   tpg->tpg_nexus = p;
+   return 0;
+}
+
 static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
 {
struct tcm_usbg_nexus *tv_nexus;
@@ -1591,7 +1601,7 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char 
*name)
tv_nexus->tvn_se_sess = target_alloc_session(>se_tpg, 128,
 sizeof(struct usbg_cmd),
 TARGET_PROT_NORMAL, name,
-tv_nexus, NULL);
+tv_nexus, 
usbg_alloc_sess_cb);
if (IS_ERR(tv_nexus->tvn_se_sess)) {
 #define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
pr_debug(MAKE_NEXUS_MSG, name);

> 
> Then percpu_ida commit produces the below result
> (it does not happen immediately, but a while after
> running the script).
> I did not bisect, though; I only checked the commits
> which alter drivers/usb/gadget/function/f_tcm.c.
> The last one which (almost) works is:
> 
> 8b00965 "target: Convert demo-mode only drivers to target_alloc_session"
> 
> AP
> 
> I used the following script:
> 
> 8<
> 
> #!/bin/bash
> 
> mount -t configfs none /sys/kernel/config
> modprobe libcomposite
> cd /sys/kernel/config/usb_gadget
> mkdir tcm
> cd tcm
> mkdir functions/tcm.0
> cd /sys/kernel/config/target/
> mkdir usb_gadget
> cd usb_gadget
> mkdir naa.0123456789abcdef
> cd naa.0123456789abcdef
> mkdir tpgt_1
> cd tpgt_1
> echo naa.01234567890abcdef > nexus
> echo 1 > enable
> cd /sys/kernel/config/usb_gadget/tcm
> mkdir configs/c.1
> ln -s functions/tcm.0 configs/c.1
> echo 0x0525 > idVendor
> echo 0x1234 > idProduct
> echo 1240.dwc3 > UDC
> 
> 8<
> 
> # [   45.510513] ERR bot_status_complete(73)
> [   45.671921] configfs-gadget gadget: high-speed config #1: c
> [   45.676158] Using the BOT protocol
> [   45.679860] [ cut here ]
> [   45.683981] kernel BUG at drivers/target/target_core_transport.c:1476!

Mmmm, usbg_get_cmd() was missing an explicit memset() after tag lookup.

How about the following..?

diff --git a/drivers/usb/gadget/function/f_tcm.c 
b/drivers/usb/gadget/function/f_tcm.c
index e352a31..d4e8a91 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1078,6 +1078,7 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
return ERR_PTR(-ENOMEM);
 
cmd = &((struct usbg_cmd *)se_sess->sess_cmd_map)[tag];
+   memset(cmd, 0, sizeof(*cmd));
cmd->se_cmd.map_tag = tag;
cmd->se_cmd.tag = cmd->tag = scsi_tag;
cmd->fu = fu;

--
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 2/2] usb:dwc3: pass arch data to xhci-hcd child

[Thang Q. Nguyen]

Hi,
I would like to ask if I need to update anything else for this change?

Thanks,
Thang Q. Nguyen -
-Original Message-
From: Thang Q. Nguyen [mailto:tqngu...@apm.com]
Sent: Monday, January 25, 2016 9:26 PM
To: Felipe Balbi; Greg Kroah-Hartman; linux-usb@vger.kernel.org;
linux-o...@vger.kernel.org; linux-ker...@vger.kernel.org;
linux-...@lists.infradead.org
Cc: Thang Nguyen; Phong Vo; Loc Ho; patc...@apm.com
Subject: [PATCH v2 2/2] usb:dwc3: pass arch data to xhci-hcd child

The xhci-hcd child node needs to inherit archdata attribute to use dma_ops
functions and attributes. This patch enables the USB DWC3 driver to pass
archdata attributes to its xhci-hcd child node.

Signed-off-by: Thang Q. Nguyen 
---
Changes from v1:
- None

 drivers/usb/dwc3/host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index
c679f63..661fbae 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -37,6 +37,7 @@ int dwc3_host_init(struct dwc3 *dwc)
xhci->dev.parent= dwc->dev;
xhci->dev.dma_mask  = dwc->dev->dma_mask;
xhci->dev.dma_parms = dwc->dev->dma_parms;
+   xhci->dev.archdata  = dwc->dev->archdata;

dwc->xhci = xhci;

--
2.2.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] watchdog: add driver for StreamLabs USB watchdog device

[kbuild test robot]

Hi Alexey,

[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on v4.5-rc7 next-20160309]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Alexey-Klimov/watchdog-add-driver-for-StreamLabs-USB-watchdog-device/20160310-103523
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
hwmon-next
config: sparc64-allmodconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

>> drivers/watchdog/streamlabs_wdt.c:233:2: error: unknown field 'ref' 
>> specified in initializer
 .ref = usb_streamlabs_wdt_ref,
 ^
>> drivers/watchdog/streamlabs_wdt.c:233:2: warning: initialization from 
>> incompatible pointer type
   drivers/watchdog/streamlabs_wdt.c:233:2: warning: (near initialization for 
'usb_streamlabs_wdt_ops.get_timeleft')
>> drivers/watchdog/streamlabs_wdt.c:234:2: error: unknown field 'unref' 
>> specified in initializer
 .unref = usb_streamlabs_wdt_unref,
 ^
   drivers/watchdog/streamlabs_wdt.c:234:2: warning: initialization from 
incompatible pointer type
   drivers/watchdog/streamlabs_wdt.c:234:2: warning: (near initialization for 
'usb_streamlabs_wdt_ops.restart')

vim +/ref +233 drivers/watchdog/streamlabs_wdt.c

   227  
   228  static struct watchdog_ops usb_streamlabs_wdt_ops = {
   229  .owner  = THIS_MODULE,
   230  .start  = usb_streamlabs_wdt_start,
   231  .stop   = usb_streamlabs_wdt_stop,
   232  .set_timeout= usb_streamlabs_wdt_settimeout,
 > 233  .ref= usb_streamlabs_wdt_ref,
 > 234  .unref  = usb_streamlabs_wdt_unref,
   235  };
   236  
   237  static int usb_streamlabs_wdt_probe(struct usb_interface *intf,

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


.config.gz
Description: Binary data

RE: [PATCH v2 1/2] usb:dwc3: Enable support for 64-bit system

[Thang Q. Nguyen]

Hi,
I would like to ask if I need to update anything else for this change?

Thanks,
Thang Q. Nguyen -
-Original Message-
From: Thang Q. Nguyen [mailto:tqngu...@apm.com]
Sent: Monday, January 25, 2016 9:26 PM
To: Felipe Balbi; Greg Kroah-Hartman; linux-usb@vger.kernel.org;
linux-o...@vger.kernel.org; linux-ker...@vger.kernel.org;
linux-...@lists.infradead.org
Cc: Thang Nguyen; Phong Vo; Loc Ho; patc...@apm.com
Subject: [PATCH v2 1/2] usb:dwc3: Enable support for 64-bit system

Add 64-bit DMA operation support to the USB DWC3 driver.
First attempt to set the coherent DMA mask for 64-bit DMA.
If that failed, attempt again with 32-bit DMA.

Signed-off-by: Thang Q. Nguyen 
---
Changes from v1:
- Remove WARN_ON if dma_mask is NULL

 drivers/usb/dwc3/core.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
de5e01f..2479c24 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->mem = mem;
dwc->dev = dev;

+   /* Try to set 64-bit DMA first */
+   if (!pdev->dev.dma_mask)
+   /* Platform did not initialize dma_mask */
+   ret = dma_coerce_mask_and_coherent(>dev,
+  DMA_BIT_MASK(64));
+   else
+   ret = dma_set_mask_and_coherent(>dev,
DMA_BIT_MASK(64));
+
+   /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask
*/
+   if (ret) {
+   ret = dma_set_mask_and_coherent(>dev,
DMA_BIT_MASK(32));
+   if (ret)
+   return ret;
+   }
+
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res) {
dev_err(dev, "missing IRQ\n");
--
2.2.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] watchdog: add driver for StreamLabs USB watchdog device

[Guenter Roeck]

On 03/09/2016 06:29 PM, Alexey Klimov wrote:

This patch creates new driver that supports StreamLabs usb watchdog
device. This device plugs into 9-pin usb header and connects to
reset pin and reset button on common PC.

USB commands used to communicate with device were reverse
engineered using usbmon.

Signed-off-by: Alexey Klimov 
---
  drivers/watchdog/Kconfig  |  15 ++
  drivers/watchdog/Makefile |   1 +
  drivers/watchdog/streamlabs_wdt.c | 370 ++
  3 files changed, 386 insertions(+)
  create mode 100644 drivers/watchdog/streamlabs_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 80825a7..95d8f72 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1705,4 +1705,19 @@ config USBPCWATCHDOG

  Most people will say N.

+config USB_STREAMLABS_WATCHDOG
+   tristate "StreamLabs USB watchdog driver"
+   depends on USB
+   ---help---
+ This is the driver for the USB Watchdog dongle from StreamLabs.
+ If you correctly connect reset pins to motherboard Reset pin and
+ to Reset button then this device will simply watch your kernel to make
+ sure it doesn't freeze, and if it does, it reboots your computer
+ after a certain amount of time.
+
+
+ To compile this driver as a module, choose M here: the
+ module will be called streamlabs_wdt.
+
+ Most people will say N. Say yes or M if you want to use such usb 
device.
  endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f6a6a38..d54fd31 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_WDTPCI) += wdt_pci.o

  # USB-based Watchdog Cards
  obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
+obj-$(CONFIG_USB_STREAMLABS_WATCHDOG) += streamlabs_wdt.o

  # ALPHA Architecture

diff --git a/drivers/watchdog/streamlabs_wdt.c 
b/drivers/watchdog/streamlabs_wdt.c
new file mode 100644
index 000..031dbc35
--- /dev/null
+++ b/drivers/watchdog/streamlabs_wdt.c
@@ -0,0 +1,370 @@
+/*
+ * StreamLabs USB Watchdog driver
+ *
+ * Copyright (c) 2016 Alexey Klimov 
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * USB Watchdog device from Streamlabs
+ * http://www.stream-labs.com/products/devices/watchdog/
+ *
+ * USB commands have been reverse engineered using usbmon.
+ */
+
+#define DRIVER_AUTHOR "Alexey Klimov "
+#define DRIVER_DESC "StreamLabs USB watchdog driver"
+#define DRIVER_NAME "usb_streamlabs_wdt"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
+#define USB_STREAMLABS_WATCHDOG_VENDOR 0x13c0
+#define USB_STREAMLABS_WATCHDOG_PRODUCT0x0011
+
+/* one buffer is used for communication, however transmitted message is only
+ * 32 bytes long */


/*
 * Please use proper multi-line comments throughout.
 */


+#define BUFFER_TRANSFER_LENGTH 32
+#define BUFFER_LENGTH  64
+#define USB_TIMEOUT350
+
+#define STREAMLABS_CMD_START   0
+#define STREAMLABS_CMD_STOP1
+
+#define STREAMLABS_WDT_MIN_TIMEOUT 1
+#define STREAMLABS_WDT_MAX_TIMEOUT 46
+
+struct streamlabs_wdt {
+   struct watchdog_device wdt_dev;
+   struct usb_device *usbdev;
+   struct usb_interface *intf;
+
+   struct kref kref;
+   struct mutex lock;
+   u8 *buffer;
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static int usb_streamlabs_wdt_validate_response(u8 *buf)
+{
+   /* If watchdog device understood the command it will acknowledge
+* with values 1,2,3,4 at indexes 10, 11, 12, 13 in response message.
+*/
+   if (buf[10] != 1 || buf[11] != 2 || buf[12] != 3 || buf[13] != 4)
+   return -EINVAL;
+
+   return 0;
+}
+
+static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev, int cmd)
+{
+   struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
+   int retval;
+   int size;
+   unsigned long timeout_msec;
+   int retry_counter = 10; /* how many times to re-send stop cmd */
+
+   mutex_lock(_wdt->lock);
+
+   timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
+
+   /* Prepare message that will be sent to device.
+* This buffer is allocated by kzalloc(). Only initialize required
+* fields.


But only once, and 

[PATCH v2 1/2] usb: host: xhci: add a new quirk XHCI_CLEAR_AC64

[Yoshihiro Shimoda]

On some xHCI controllers (e.g. R-Car SoCs), the AC64 bit (bit 0) of
HCCPARAMS1 is set to 1. However, the xHCs don't support 64-bit
address memory pointers actually. So, in this case, this driver should
call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in xhci_gen_setup().
Otherwise, the xHCI controller will be died after a usb device is
connected if it runs on above 4GB physical memory environment.

So, this patch adds a new quirk XHCI_CLEAR_AC64 to resolve such an issue.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/host/xhci.c | 10 ++
 drivers/usb/host/xhci.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d51ee0c..4b5e979 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4948,6 +4948,16 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
return retval;
xhci_dbg(xhci, "Reset complete\n");
 
+   /*
+* On some xHCI controllers (e.g. R-Car SoCs), the AC64 bit (bit 0)
+* of HCCPARAMS1 is set to 1. However, the xHCs don't support 64-bit
+* address memory pointers actually. So, this driver clears the AC64
+* bit of xhci->hcc_params to call dma_set_coherent_mask(dev,
+* DMA_BIT_MASK(32)) in this xhci_gen_setup().
+*/
+   if (xhci->quirks & XHCI_CLEAR_AC64)
+   xhci->hcc_params &= ~BIT(0);
+
/* Set dma_mask and coherent_dma_mask to 64-bits,
 * if xHC supports 64-bit addressing */
if (HCC_64BIT_ADDR(xhci->hcc_params) &&
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e293e09..1963148 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1641,6 +1641,7 @@ struct xhci_hcd {
 #define XHCI_PME_STUCK_QUIRK   (1 << 20)
 #define XHCI_MTK_HOST  (1 << 21)
 #define XHCI_SSIC_PORT_UNUSED  (1 << 22)
+#define XHCI_CLEAR_AC64(1 << 23)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
-- 
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 v2 2/2] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys

[Yoshihiro Shimoda]

This patch fixes an issue that cannot work if R-Car Gen2/3 run on
above 4GB physical memory environment to use a quirk XHCI_CLEAR_AC64.

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

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5c15e9b..4dbd56f 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -39,12 +39,19 @@ static const struct xhci_driver_overrides 
xhci_plat_overrides __initconst = {
 
 static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
+   struct usb_hcd *hcd = xhci_to_hcd(xhci);
+
/*
 * As of now platform drivers don't provide MSI support so we ensure
 * here that the generic code does not try to make a pci_dev from our
 * dev struct in order to setup MSI
 */
xhci->quirks |= XHCI_PLAT;
+
+   /* Please refer to the xhci.c about the detail of this quirk */
+   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
+   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
+   xhci->quirks |= XHCI_CLEAR_AC64;
 }
 
 /* called during probe() after chip reset completes */
-- 
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 v2 0/2] usb: host: xhci: add a new quirk and fix an issue for R-Car Gen2/3

[Yoshihiro Shimoda]

This patch set is based on the latest usb.git / usb-next branch.
(commit id = ace53bfc4374cada8b645765e2b4ad5831e760932)

Changes from v1:
 - Add a new quirk "XHCI_CLEAR_AC64" for using it on some PCIe card.
   http://thread.gmane.org/gmane.linux.kernel.renesas-soc/858/focus=1694

Yoshihiro Shimoda (2):
  usb: host: xhci: add a new quirk XHCI_CLEAR_AC64
  usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB
phys

 drivers/usb/host/xhci-plat.c |  7 +++
 drivers/usb/host/xhci.c  | 10 ++
 drivers/usb/host/xhci.h  |  1 +
 3 files changed, 18 insertions(+)

-- 
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 v2 2/2] usb: renesas_usbhs: disable TX IRQ before starting TX DMAC transfer

[Yoshihiro Shimoda]

This patch adds a code to surely disable TX IRQ of the pipe before
starting TX DMAC transfer. Otherwise, a lot of unnecessary TX IRQs
may happen in rare cases when DMAC is used.

Fixes: e73a989 ("usb: renesas_usbhs: add DMAEngine support")
Cc:  # v3.1+
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/fifo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 0c25c01..000f975 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -890,6 +890,7 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
 
pkt->trans = len;
 
+   usbhsf_tx_irq_ctrl(pipe, 0);
INIT_WORK(>work, xfer_work);
schedule_work(>work);
 
-- 
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 v2 0/2] usb: renesas_usbhs: fix to avoid unexpected condition

[Yoshihiro Shimoda]

This patch set is based on the latest Felipe's usb.git / testing/next branch.
(commit id = ac5706631325ae3695bfa1527101ab2b2f64859f)

Changes from v1:
 - Add "Fixes" and "Cc: .
 - Revise the commit log of patch 2.
  - consistent words: tx -> TX, irq -> IRQ
  - typo: heppen -> happen

Yoshihiro Shimoda (2):
  usb: renesas_usbhs: avoid NULL pointer derefernce in
usbhsf_pkt_handler()
  usb: renesas_usbhs: disable TX IRQ before starting TX DMAC transfer

 drivers/usb/renesas_usbhs/fifo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
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 v2 1/2] usb: renesas_usbhs: avoid NULL pointer derefernce in usbhsf_pkt_handler()

[Yoshihiro Shimoda]

When unexpected situation happened (e.g. tx/rx irq happened while
DMAC is used), the usbhsf_pkt_handler() was possible to cause NULL
pointer dereference like the followings:

Unable to handle kernel NULL pointer dereference at virtual address 
pgd = c0004000
[] *pgd=
Internal error: Oops: 8007 [#1] SMP ARM
Modules linked in: usb_f_acm u_serial g_serial libcomposite
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.5.0-rc6-00842-gac57066-dirty #63
Hardware name: Generic R8A7790 (Flattened Device Tree)
task: c0729c00 ti: c0724000 task.ti: c0724000
PC is at 0x0
LR is at usbhsf_pkt_handler+0xac/0x118
pc : [<>]lr : []psr: 6193
sp : c0725db8  ip :   fp : c0725df4
r10: 0001  r9 : 0193  r8 : ef3ccab4
r7 : ef3cca10  r6 : eea4586c  r5 :   r4 : ef19ceb4
r3 :   r2 : 009c  r1 : c0725dc4  r0 : ef19ceb4

This patch adds a condition to avoid the dereference.

Fixes: e73a989 ("usb: renesas_usbhs: add DMAEngine support")
Cc:  # v3.1+
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/fifo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index b4de70e..0c25c01 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -190,7 +190,8 @@ static int usbhsf_pkt_handler(struct usbhs_pipe *pipe, int 
type)
goto __usbhs_pkt_handler_end;
}
 
-   ret = func(pkt, _done);
+   if (likely(func))
+   ret = func(pkt, _done);
 
if (is_done)
__usbhsf_pkt_del(pkt);
-- 
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] watchdog: add driver for StreamLabs USB watchdog device

[Alexey Klimov]

This patch creates new driver that supports StreamLabs usb watchdog
device. This device plugs into 9-pin usb header and connects to
reset pin and reset button on common PC.

USB commands used to communicate with device were reverse
engineered using usbmon.

Signed-off-by: Alexey Klimov 
---
 drivers/watchdog/Kconfig  |  15 ++
 drivers/watchdog/Makefile |   1 +
 drivers/watchdog/streamlabs_wdt.c | 370 ++
 3 files changed, 386 insertions(+)
 create mode 100644 drivers/watchdog/streamlabs_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 80825a7..95d8f72 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1705,4 +1705,19 @@ config USBPCWATCHDOG
 
  Most people will say N.
 
+config USB_STREAMLABS_WATCHDOG
+   tristate "StreamLabs USB watchdog driver"
+   depends on USB
+   ---help---
+ This is the driver for the USB Watchdog dongle from StreamLabs.
+ If you correctly connect reset pins to motherboard Reset pin and
+ to Reset button then this device will simply watch your kernel to make
+ sure it doesn't freeze, and if it does, it reboots your computer
+ after a certain amount of time.
+
+
+ To compile this driver as a module, choose M here: the
+ module will be called streamlabs_wdt.
+
+ Most people will say N. Say yes or M if you want to use such usb 
device.
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f6a6a38..d54fd31 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_WDTPCI) += wdt_pci.o
 
 # USB-based Watchdog Cards
 obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
+obj-$(CONFIG_USB_STREAMLABS_WATCHDOG) += streamlabs_wdt.o
 
 # ALPHA Architecture
 
diff --git a/drivers/watchdog/streamlabs_wdt.c 
b/drivers/watchdog/streamlabs_wdt.c
new file mode 100644
index 000..031dbc35
--- /dev/null
+++ b/drivers/watchdog/streamlabs_wdt.c
@@ -0,0 +1,370 @@
+/*
+ * StreamLabs USB Watchdog driver
+ *
+ * Copyright (c) 2016 Alexey Klimov 
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * USB Watchdog device from Streamlabs
+ * http://www.stream-labs.com/products/devices/watchdog/
+ *
+ * USB commands have been reverse engineered using usbmon.
+ */
+
+#define DRIVER_AUTHOR "Alexey Klimov "
+#define DRIVER_DESC "StreamLabs USB watchdog driver"
+#define DRIVER_NAME "usb_streamlabs_wdt"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
+#define USB_STREAMLABS_WATCHDOG_VENDOR 0x13c0
+#define USB_STREAMLABS_WATCHDOG_PRODUCT0x0011
+
+/* one buffer is used for communication, however transmitted message is only
+ * 32 bytes long */
+#define BUFFER_TRANSFER_LENGTH 32
+#define BUFFER_LENGTH  64
+#define USB_TIMEOUT350
+
+#define STREAMLABS_CMD_START   0
+#define STREAMLABS_CMD_STOP1
+
+#define STREAMLABS_WDT_MIN_TIMEOUT 1
+#define STREAMLABS_WDT_MAX_TIMEOUT 46
+
+struct streamlabs_wdt {
+   struct watchdog_device wdt_dev;
+   struct usb_device *usbdev;
+   struct usb_interface *intf;
+
+   struct kref kref;
+   struct mutex lock;
+   u8 *buffer;
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static int usb_streamlabs_wdt_validate_response(u8 *buf)
+{
+   /* If watchdog device understood the command it will acknowledge
+* with values 1,2,3,4 at indexes 10, 11, 12, 13 in response message.
+*/
+   if (buf[10] != 1 || buf[11] != 2 || buf[12] != 3 || buf[13] != 4)
+   return -EINVAL;
+
+   return 0;
+}
+
+static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev, int cmd)
+{
+   struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
+   int retval;
+   int size;
+   unsigned long timeout_msec;
+   int retry_counter = 10; /* how many times to re-send stop cmd */
+
+   mutex_lock(_wdt->lock);
+
+   timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
+
+   /* Prepare message that will be sent to device.
+* This buffer is allocated by kzalloc(). Only initialize required
+* fields.
+*/
+   if (cmd == STREAMLABS_CMD_START) {
+   streamlabs_wdt->buffer[0] = 0xcc;
+   

hello,

[jack]

My name is Jack, from the US. I'm in Syria right now fighting IS. I want to get 
to know you better, if I may be so bold. I consider myself an easy-going man, 
and I am currently looking for a relationship in which I feel loved.

Please tell me more about yourself, if you don't mind.
--
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 2/2] usb: renesas_usbhs: disable tx irq before starting TX DMAC transfer

[Yoshihiro Shimoda]

Hello,

> Hello.
> 
> On 3/8/2016 8:58 AM, Yoshihiro Shimoda wrote:
> 
> > This patch adds a code to surely disable tx irq of the pipe before
> 
> TX IRQ.
> 
> > starting TX DMAC transfer. Otherwise, a lot of unnecessary tx irqs
> 
> TX IRQs. Mixing "TX" and "tx" on a single line is... inconsistent. :-)
> 
> > may heppen in rare cases when DMAC is used.
> 
> Happen.

Thank you for the comment.
I will fix them in v2 patch.

Best regards,
Yoshihiro Shimoda

> >
> > Signed-off-by: Yoshihiro Shimoda 
> [...]
> 
> MBR, Sergei

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

RE: [PATCH 1/2] usb: renesas_usbhs: avoid NULL pointer derefernce in usbhsf_pkt_handler()

[Yoshihiro Shimoda]

Hi,

> 
> Yoshihiro Shimoda  writes:
> >> Yoshihiro Shimoda  writes:
> >> > This patch adds a condition to avoid the dereference.
> >> >
> >> > Signed-off-by: Yoshihiro Shimoda 
> >>
> >> is this a regression fix ? Do we need it in current -rc (it's getting
> >> late for that, actually), do we need a Cc:  here ?
> >>
> >> Same questions are valid for the other patch in this series.
> >
> > Thank you for the review.
> 
> no problem :-)
> 
> > This is a potential problem fix. This issue is possible to cause the
> > first DMA supporting (e73a989 usb: renesas_usbhs: add DMAEngine
> > support) at 2011, I think.
> 
> okay, so according to git describe:
> 
> $ git describe e73a989
> v3.0-rc2-22-ge73a9891b3a1
> 
> this entered mainline on v3.1. This means you need to add:
> 
> Fixes: e73a989 ("usb: renesas_usbhs: add DMAEngine support")
> Cc:  # v3.1+
> 
> to your commit log, right before your Signed-off-by.
> 
> > We don't need in current -rc because I also think this is too late for
> > it.  I'm not sure we need a CC here.
> 
> okay, so I'll queue this once v4.6-rc1 is merged. But please resend with
> the changes above. Let me know if you need any further clarification.

Thank you very much for the detail.
I will send v2 patches with the changes above.

Best regards,
Yoshihiro Shimoda

> --
> balbi
--
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: Minor xhci issues (failed to peer) on Dell XPS 13 9350 (Skylake)

[Mario Limonciello]

On 03/07/2016 02:42 PM, Andy Lutomirski wrote:
> On Mon, Mar 7, 2016 at 12:13 PM, Greg Kroah-Hartman
>  wrote:
> I'm adding a couple Dell people.
>
> Hi, Dell people-
>
> Your latest DSDT has this blatantly buggy method:
>
> Method (_Q66, 0, NotSerialized)  // _Qxx: EC Query
> {
> Acquire (PATM, 0x0064)
> If ((ECRD != One))
> {
> Return (Zero)
> }
>
> NEVT ()
> Release (PATM)
> Return (Zero)
> }
>
> At some point during boot (presumably), this runs with ECRD == 0,
> causing PATM to be acquired and never released.  Later on, something
> involved in USB-C hotplug (I think -- I can occasionally trigger
> errors during hotplug) breaks when it can't acquire PATM.
>
> Could you ask your BIOS team to please add the obviously missing Release 
> (PATM)?
>
> I don't know if fixing this bug will solve all the USB-C issues, but
> it seems unlikely to hurt.  I'm going to try to get it to work with a
> custom method, but I may or may not succeed.
>
> --Andy
>
>
Andy,

Sorry I haven't gotten back.  I was hoping to have an answer when I
responded but not yet so just wanted to let you know the message wasn't
missed.
I'm inquiring to my team about this.  I'm unsure the circumstances that
the EC isn't ready (race condition?) so it might take some time to get
answer.

The most likely thing to resolve USB-C issues is to update your alpine
ridge firmware.  A lot of the same USB-C issues are present on Windows
but fixed in AR firmware updates.
This unfortunately can't be done yet in Linux or even in an EFI
environment like we do with UEFI capsules.

I know that sucks, and I am working to solve that problem separately.
--
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: chipidea: Configure DMA properties and ops from DT

[Li Yang]

On Tue, Mar 8, 2016 at 9:40 PM, Bjorn Andersson
 wrote:
> On Tue, Mar 8, 2016 at 11:52 AM, Li Yang  wrote:
>> On Wed, Mar 2, 2016 at 4:59 PM, Li Yang  wrote:
>>> On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson
>>>  wrote:
 On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote:

>
>
> On 22/02/16 05:32, Bjorn Andersson wrote:
> >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set
> >to be able to do DMA allocations, so use the of_dma_configure() helper
> >to populate the dma properties and assign an appropriate dma_ops.
> [..]
> None of the drivers call of_dma_configure() explicitly, which makes me 
> feel
> that we are doing something wrong. TBH, this should be handled in more
> generic way rather than driver like this having an explicit call to
> of_dma_configure().
>

 I agree, trying to figure out if it should be inherited or something.
>>>
>>> I also agree.  We need address it in a more generic way.  I did a
>>> search for platform_device_add()/platform_device_register() in the
>>> kernel source code.  I found a lot of them and many could be also
>>> doing DMA.  Looks like it is still too early to assume every device is
>>> already getting dma_ops set through bus probe.  Otherwise, many
>>> drivers are potentially broken by this assumption.
>>
>> Any further comment on this topic?  I added the linux-arm mailing list
>> which was missing from previous discussion.
>>
>
> I had the chance to go through this with Arnd and the verdict is that
> devices not described in DT should not do DMA (or allocate buffers for
> doing DMA).
>
> So I believe the solution is to fall back on Peter's description; the
> chipidea driver is the core driver and the Qualcomm code should just
> be a platform layer.
>
> My suggestion is that we turn the chipidea core into a set of APIs
> that can called by the platform specific pieces. That way we will have
> the chipidea core be the device described in the DT.

But like I said, this problem is not just existing for chipidea
driver.  We already found that the dwc3 driver is also suffering from
the same issue.  I don't know how many other drivers are impacted by
this change, but I suspect there will be some. A grep of
platform_device_add() in driver/ directory returns many possible
drivers to be impacted.  As far as I know, the
drivers/net/ethernet/freescale/fman/mac.c is registering child
ethernet devices that definitely will do dma.   If you want to do this
kind of rework to all these drivers, it will be a really big effort.

Regards,
Leo
--
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: gadget: f_midi: Fixed a bug when buflen was smaller than wMaxPacketSize

[Steve Calfee]

On Wed, Mar 9, 2016 at 11:39 AM, Felipe F. Tonello  
wrote:
> buflen by default (256) is smaller than wMaxPacketSize (512) in high-speed
> devices.
>
> That caused the OUT endpoint to freeze if the host send any data packet of
> length greater than 256 bytes.
>
> This is an example dump of what happended on that enpoint:
> HOST:   [DATA][Length=260][...]
> DEVICE: [NAK]
> HOST:   [PING]
> DEVICE: [NAK]
> HOST:   [PING]
> DEVICE: [NAK]
> ...
> HOST:   [PING]
> DEVICE: [NAK]
>
> This patch fixes this problem by setting the minimum usb_request's buffer size
> for the OUT endpoint as its wMaxPacketSize.
>
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/function/f_midi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index 8475e3dc82d4..826ba641f29d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -366,7 +366,9 @@ static int f_midi_set_alt(struct usb_function *f, 
> unsigned intf, unsigned alt)
> /* allocate a bunch of read buffers and queue them all at once. */
> for (i = 0; i < midi->qlen && err == 0; i++) {
> struct usb_request *req =
> -   midi_alloc_ep_req(midi->out_ep, midi->buflen);
> +   midi_alloc_ep_req(midi->out_ep,
> +   max_t(unsigned, midi->buflen,
> +   bulk_out_desc.wMaxPacketSize));
> if (req == NULL)
> return -ENOMEM;
>
Won't you get a buffer overrun if midi->buflen is less than wMaxPacketSize?

Regards, Steve
--
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 host port {,performance} issues with Beaglebone

[Bin Liu]

Hi,

On Tue, Mar 08, 2016 at 10:57:41AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Victor Dodon  writes:
> > [ text/plain ]
> > Hi Felipe,
> >
> > On Mon, Mar 7, 2016 at 11:13 PM, Felipe Balbi  wrote:
> >>
> >> Hi,
> >>
> >> Victor Dodon  writes:
> >>> [ text/plain ]
> >>> Sorry, I accidentally pressed Send
> >>>
> >>> On Mon, Mar 7, 2016 at 7:35 PM, Victor Dodon  
> >>> wrote:
>  Hi all,
> 
>  I have some performance issues with the host port on a Beaglebone
>  board. I tested with kernel 3.8.13, 3.14.55 and 4.1.18 and the issue
>  still persists. Running a fio test with 64k random reads from a USB
>  flash drive yields a maximum of 14402.01 KiB/s (115216.08 Kb/s). The
>  3.14 and 4.1 kernels where build with CONFIG_TI_CPPI41_DMA=y. I was
>  able to get a much better performance on the client USB port by
>  enabling fifo double buffering. Iperf over a gigabit connection and a 
>  Ethernet
>  to USB adapter plugged in the host port gives a maximum of 180Mbit/s 
>  with fifo
>  double buffering enabled for the ep1 and ep2.
> 
>  Are there any known performance issues in the musb driver? For my use
>  case I need
>  a higher bandwidth and I would like to improve the host controller,
>  but I'm a beginner in Kernel hacking and I would appreciate some help,
>  tips or any cues to start.
> 
>  I also found a few problems with the host port. For example:
>  Using the setup described above (gigabit connection and a Ethernet
>  to USB adapter plugged in the host port and with a running iSCSI
>  initiator on the BB,
>  in usb/musb/musb_core.c if I change mode_4_cfg to enable double
>  buffering, and I restart the board while doing a dd from the disk
>  mounted with iSCSI, the kernel stops at:
> >>>
> >>> *if I enable double buffering for both RX and TX for only for ep 1
> >>> then the kernel stops at:
> >>>
> >>> [  233.930764] blk_update_request: I/O error, dev sda, sector 1620736
> >>> [  234.451076] musb-hdrc musb-hdrc.1.auto: remove, state 1
> >>> [  234.469702] usb usb1: USB disconnect, device number 1
> >>> [  234.492716] init: iscsid main process (466) killed by TERM signal
> >>> [  234.510663] usb 1-1: USB disconnect, device number 2
> >>> [  234.533235] usb 1-1.1: USB disconnect, device number 3
> >>> [  234.555153] usb 1-1.3: USB disconnect, device number 4
> >>> [  234.586962] musb-hdrc musb-hdrc.1.auto: USB bus 1 deregistered
> >>> [  234.606098] reboot: Restarting system
> >>>
> >>> if I enable double buffering for RX and TX for ep 1 and 2, only when
> >>
> >> you mean you're using FIFO_RXTX ? IIRC, that's only for Isochronous
> >> endpoints.
> >
> > No, I use:
> > static struct musb_fifo_cfg mode_4_cfg[] = {
> > { .hw_ep_num = 1, .style = FIFO_TX, .maxpacket = 512, .mode = BUF_DOUBLE, },
> > { .hw_ep_num = 1, .style = FIFO_RX, .maxpacket = 512, .mode = BUF_DOUBLE, },
> > { .hw_ep_num = 2, .style = FIFO_TX, .maxpacket = 512,  },
> > { .hw_ep_num = 2, .style = FIFO_RX, .maxpacket = 512, },
> > ...
> >
> > in the first case, and:
> >
> > static struct musb_fifo_cfg mode_4_cfg[] = {
> > { .hw_ep_num = 1, .style = FIFO_TX, .maxpacket = 512, .mode = BUF_DOUBLE, },
> > { .hw_ep_num = 1, .style = FIFO_RX, .maxpacket = 512, .mode = BUF_DOUBLE, },
> > { .hw_ep_num = 2, .style = FIFO_TX, .maxpacket = 512,  .mode = BUF_DOUBLE, 
> > },
> > { .hw_ep_num = 2, .style = FIFO_RX, .maxpacket = 512, .mode = BUF_DOUBLE, },
> >
> > in the second, with a few changes in the last endpoint to fit in 16k.
> 
> oh okay. I have some vague memory of some weird FIFO access bugs on
> MUSB. Maybe you're falling into one of them. Hopefully Bin has more info.

am335x hasn't been validated with double buffering, at least in last few
years, and I am not sure any other musb devices use double buffering
too, but the musb driver has great refactoring in last few years
though, so there is no surprise if double buffering is broken in musb
driver.

But I tried double buffering on am335x recently with kernel v4.1.x .
With my limited test I didn't see any problem. On host port, I tried W/R
to/from a MSC device; while on device port, I tried g_zero and
g_mess_storage. I didn't try ethernet gongle though.

Regards,
-Bin.

--
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 ID: 114111

[Peter Maciejko]

The problem is a bit complicated, that is why I put a video containing
a description of the problem on kernel bug tracker.

Motherboard: ASRock H87 PRO 4
Processor: Intel Core i5 4570 Haswell

If USB 3.0 controller is enabled in UEFI Setup then three, maybe four
seconds after shutdown, my computer starts boot process by itself.
After reboot, which is initiated by me through command line or
graphical environment - it doesnt matter - poweroff works fine but
only once. Besides, even if poweroff works fine, until my monitor is
enabled after proper shutdown, I can start boot process by clicking
left mouse button/right mouse button/scroll. If I disable my monitor
and then enable it again, then obviously I cant boot computer by
clicking. All "Power on" options are disabled in UEFI.

If USB 3.0 controller is disabled in UEFI Setup then poweroff works
fine, but after proper shutdown, until monitor is enabled, I can light
up my mouse's and keyboard's leds by clicking same as I described
above.


My mouse and keyboard are connected to the computer through monitor's USB ports.


Monitor: Dell U2414H
Mouse: SteelSeries Rival
Keyboard: CM Storm Quickfire Rapid-I

Sry for my bad english, I hope you will understand me, if not - then
watch movie pls.

Link to the movie:
https://www.dropbox.com/s/isr9i0yojgnlb6n/20160309_133125.mp4?dl=0
lsusb: http://pastebin.com/uhGAnCLU
lsusb -t: http://pastebin.com/iY1HZrER
lspci: http://pastebin.com/phz6if1U

I have tried:
xhci_hcd.quirks=270336 //XHCI_SPURIOUS_WAKEUP and XHCI_SPURIOUS_REBOOT
xhci_hcd.quirks=8192 //XHCI_SPURIOUS_REBOOT
xhci_hcd.quirks=262144 //XHCI_SPURIOUS_WAKEUP

Nothing helped, computer keeps rebooting after shutdown. I cant
remember when my rig works fine last time - it is very frustrating, I
am a little bit tired of this :)
I have now Fedora 23 with 4.4.3 kernel, but I have also tried
branded/vanilla 4.4.4 and obviously it didnt help.
On Ubuntu 14.04 and the latest 3.16, bug persists too - my girlfriend
has same rig and same issue.

There is two situations when poweroff works ALMOST fine:
1. Keyboard/Remote Power On and Mouse Power On is enabled in uefi, but
then poweroff process hangs for few seconds and I can boot computer by
clicking, until monitor is enabled after shutdown.
2. USB 3.0 Controller is disabled (still can light up
keyboard's/mouse's leds until monitor is enabled after shutdown).

Best regards guys and thanks for your effort,
PM

On Wed, Mar 9, 2016 at 6:40 PM, Felipe Ferreri Tonello
 wrote:
> Hi,
>
> On 09/03/16 17:12, Greg KH wrote:
>> On Wed, Mar 09, 2016 at 05:47:00PM +0100, Peter Maciejko wrote:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=114111
>>>
>>> Bug ID: 114111
>>> Summary: Reboot after shutdown, when USB 3.0 controller is enabled
>>
>> Please send us the full details so we don't have to dig them out on the
>> website...
>
> There is absolutely no description of the bug besides the Summary, which
> by itself is not saying much.
>
> Peter, can you describe exactly was is going on?
>
> --
> Felipe
--
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 5/5 v9] usb: musb: da8xx: Add DT support for the DA8xx driver

[Bin Liu]

Hi,

On Wed, Mar 09, 2016 at 10:25:27AM +0100, Petr Kulhavy wrote:
> This adds DT support for TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver
> 
> Signed-off-by: Petr Kulhavy 
> Tested-by: Petr Kulhavy 
> ---
> v1: 
> 
> v2:
>  - using standard MUSB properties "dr_mode", "mentor,power", 
> "mentor,num-eps", "mentor,multipoint", "mentor,ram-bits"
>  - using "ti," prefix instead of "da8xx," for specific property names
>  - no wilcards in compatibility string
>  - using CFGCHIP2_USB2PHYCLKMUX_SHIFT instead of CFGCHIP2_USB2PHYCLKMUX_OFFSET
> 
> v3:
>  - DMA mask initialization corrected
>  - removed extra #ifdef CONFIG_OF
> 
> v4:
>  - compatibility string set to "ti,da830-musb"
>  - "mentor,num-eps", "mentor,multipoint", "mentor,ram-bits" properties 
> removed and hardcoded
>  - "ti,phy20-clkmux-cfg" renamed to "ti,phy20-clkmux-pll" and changed to 
> boolean
>  - removed use of the DA8XX_SYSCFG0_VIRT macro
> 
> v5:
>  - using CFGCHIP2_REFFREQ_ in get_phy_refclk_cfg()
>  - simplified initialization of the hard coded config parameters
>  - optimization CFGCHIP2 register update
> 
> v6:
>  - using "ti,usb2-phy-" prefix instead of "ti,phy20-" for the specific 
> properties
>  - optimization CFGCHIP2 register update
>  
> v7:
>  - pdata::power hard coded to 500mA
> 
> v8:
>  - USB maximum power modelled via a regulator "vbus-supply"
>  - "ti,usb2-phy-refclock-frequency" renamed to "ti,usb2-phy-refclock-hz"
>  - "ti,usb2-phy-clkmux-pll" changed to "ti,usb2-phy-clkmux-refclkin" and the 
> boolean meaning inverted to reflect the more common case
> 
> v9:
>  - power regulator used only to get the current
> +
>  drivers/usb/musb/da8xx.c | 106 
> +++
>  1 file changed, 106 insertions(+)
> 
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index b03d3b8..aabd33a 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -6,6 +6,9 @@
>   * Based on the DaVinci "glue layer" code.
>   * Copyright (C) 2005-2006 by Texas Instruments
>   *
> + * DT support
> + * Copyright (c) 2016 Petr Kulhavy, Barix AG 
> + *
>   * This file is part of the Inventra Controller Driver for Linux.
>   *
>   * The Inventra Controller Driver for Linux is free software; you
> @@ -33,9 +36,11 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> +#include 
>  
>  #include "musb_core.h"
>  
> @@ -134,6 +139,55 @@ static inline void phy_off(void)
>   __raw_writel(cfgchip2, CFGCHIP2);
>  }
>  
> +static inline int get_phy_refclk_cfg(struct device_node *np)
> +{
> + u32 freq;
> +
> + if (of_property_read_u32(np, "ti,usb2-phy-refclock-hz", ))
> + return -EINVAL;
> +
> + switch (freq) {
> + case 1200:
> + return CFGCHIP2_REFFREQ_12MHZ;
> + case 1300:
> + return CFGCHIP2_REFFREQ_13MHZ;
> + case 1920:
> + return CFGCHIP2_REFFREQ_19_2MHZ;
> + case 2000:
> + return CFGCHIP2_REFFREQ_20MHZ;
> + case 2400:
> + return CFGCHIP2_REFFREQ_24MHZ;
> + case 2600:
> + return CFGCHIP2_REFFREQ_26MHZ;
> + case 3840:
> + return CFGCHIP2_REFFREQ_38_4MHZ;
> + case 4000:
> + return CFGCHIP2_REFFREQ_40MHZ;
> + case 4800:
> + return CFGCHIP2_REFFREQ_48MHZ;
> + default:
> + return -EINVAL;
> + }
> +}

Should this be wrapped with CONFIG_OF? Have you tried to build it with
OF disabled?

Regards,
-Bin.

> +
> +static inline u8 get_vbus_power(struct device *dev)
> +{
> + struct regulator *vbus_supply;
> + int current_uA;
> +
> + vbus_supply = regulator_get(dev, "vbus");
> + if (IS_ERR(vbus_supply))
> + return 255;
> +
> + current_uA = regulator_get_current_limit(vbus_supply);
> + regulator_put(vbus_supply);
> +
> + if (current_uA <= 0 || current_uA > 51)
> + return 255;
> +
> + return current_uA / 1000;
> +}
> +
>  /*
>   * Because we don't set CTRL.UINT, it's "important" to:
>   *   - not read/write INTRUSB/INTRUSBE (except during
> @@ -482,6 +536,12 @@ static const struct platform_device_info da8xx_dev_info 
> = {
>   .dma_mask   = DMA_BIT_MASK(32),
>  };
>  
> +static const struct musb_hdrc_config da8xx_config = {
> + .ram_bits = 10,
> + .num_eps = 5,
> + .multipoint = 1,
> +};
> +
>  static int da8xx_probe(struct platform_device *pdev)
>  {
>   struct resource musb_resources[2];
> @@ -490,6 +550,7 @@ static int da8xx_probe(struct platform_device *pdev)
>   struct da8xx_glue   *glue;
>   struct platform_device_info pinfo;
>   struct clk  *clk;
> + struct device_node  *np = pdev->dev.of_node;
>  
>   int ret = -ENOMEM;
>  
> @@ -515,6 +576,42 @@ static int da8xx_probe(struct platform_device *pdev)
>   glue->dev 

Re: usb: musb: tx fifo flush warning

[Bin Liu]

Hi,

On Fri, Mar 04, 2016 at 11:51:19AM +0100, John Ogness wrote:
> On 2016-03-04, John Ogness  wrote:
> > Using v4.5-rc6, I modified musb_h_tx_flush_fifo() to allow infinite
> > looping and kept a log of the number of loops that were executed. For
> > my test I am regularly seeing 3000-3200 loops (with a max so far of
> > 3289). Since there used to be an msleep() in the loop, I never hit the
> > warnings before.
> >
> > With my board, 3200 loops takes about 950us. Perhaps the msleep()
> > should be reinserted, but with a retry count of only 3 before aborting
> > with the warning.
> 
> Sorry, since musb_h_tx_flush_fifo() can run in interrupt context,
> obviously an msleep() cannot be used. It seems increasing the retry
> count may be the only option here. Maybe 5000?

If the problem with the latest kernel is that only kernel dump happens
but usb still works, I'd prefer to not try to fix it by increasing the
retries. There are cases in which multiple tx urbs (100+) are queued,
increasing the retries could causes minutes of delay in device disconnect.

Regards,
-Bin.
--
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: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Doug Anderson]

Stefan,

On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren  wrote:
>
>> Doug Anderson  hat am 7. März 2016 um 22:30
>> geschrieben:
>>
>>
>> Stefan,
>>
>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren  
>> wrote:
>> > Hi Doug,
>> >
>> >> Douglas Anderson  hat am 4. März 2016 um 19:23
>> >> geschrieben:
>> >>
>> >>
>> >> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on
>> >> bcm2835") now that we've found the root cause. See the change
>> >> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()").
>> >
>> > adding a delay of 10 ms after a core reset might be a idea, but applying
>> > both
>> > patches breaks USB support on RPi :-(
>> >
>> > I'm getting the wrong register values ...
>>
>> Ugh. :(
>>
>> Just out of curiosity, if you loop and time long it takes for the
>> registers to get to the right state after reset, what do you get?
>> AKA, pick:
>>
>> https://chromium-review.googlesource.com/331260
>>
>> ...and let me know what it prints out.
>
> On my Raspberry Pi B i get the following:
>
> [2.084411] dwc2 2098.usb: mapped PA 2098 to VA cc88
> [2.084461] dwc2 2098.usb: cannot get otg clock
> [2.084549] dwc2 2098.usb: registering common handler for irq33
> [2.084713] dwc2 2098.usb: Configuration mismatch. dr_mode forced to 
> host
> [2.153965] dwc2 2098.usb: Waited 49996 us, 0x00201000 => 0x01001000,
> 0x => 0x02002000
> [2.174930] dwc2 2098.usb: Forcing mode to host
>
> So i changed the delay in patch #1 to msleep(50) and then both patches work 
> like
> a charm.

Great news!  :-)

John: it's pretty clear that there's something taking almost exactly
10ms on my system and almost exactly 50ms on Stefan's system.  Is
there some register we could poll to see when this process is done?
...or can we look at the dwc2 revision number / feature register and
detect how long to delay?
--
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: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

> Doug Anderson  hat am 7. März 2016 um 22:30
> geschrieben:
>
>
> Stefan,
>
> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren  wrote:
> > Hi Doug,
> >
> >> Douglas Anderson  hat am 4. März 2016 um 19:23
> >> geschrieben:
> >>
> >>
> >> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on
> >> bcm2835") now that we've found the root cause. See the change
> >> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()").
> >
> > adding a delay of 10 ms after a core reset might be a idea, but applying
> > both
> > patches breaks USB support on RPi :-(
> >
> > I'm getting the wrong register values ...
>
> Ugh. :(
>
> Just out of curiosity, if you loop and time long it takes for the
> registers to get to the right state after reset, what do you get?
> AKA, pick:
>
> https://chromium-review.googlesource.com/331260
>
> ...and let me know what it prints out. 

On my Raspberry Pi B i get the following:

[2.084411] dwc2 2098.usb: mapped PA 2098 to VA cc88
[2.084461] dwc2 2098.usb: cannot get otg clock
[2.084549] dwc2 2098.usb: registering common handler for irq33
[2.084713] dwc2 2098.usb: Configuration mismatch. dr_mode forced to host
[2.153965] dwc2 2098.usb: Waited 49996 us, 0x00201000 => 0x01001000,
0x => 0x02002000
[2.174930] dwc2 2098.usb: Forcing mode to host

So i changed the delay in patch #1 to msleep(50) and then both patches work like
a charm.

Stefan
--
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 ID: 114111

[Felipe Ferreri Tonello]

Hi,

On 09/03/16 17:12, Greg KH wrote:
> On Wed, Mar 09, 2016 at 05:47:00PM +0100, Peter Maciejko wrote:
>> https://bugzilla.kernel.org/show_bug.cgi?id=114111
>>
>> Bug ID: 114111
>> Summary: Reboot after shutdown, when USB 3.0 controller is enabled
> 
> Please send us the full details so we don't have to dig them out on the
> website...

There is absolutely no description of the bug besides the Summary, which
by itself is not saying much.

Peter, can you describe exactly was is going on?

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: Bug ID: 114111

[Greg KH]

On Wed, Mar 09, 2016 at 05:47:00PM +0100, Peter Maciejko wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=114111
> 
> Bug ID: 114111
> Summary: Reboot after shutdown, when USB 3.0 controller is enabled

Please send us the full details so we don't have to dig them out on the
website...
--
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: gadget: f_midi: added spinlock on transmit function

[Felipe Ferreri Tonello]

Hi,

On 09/03/16 16:17, Felipe F. Tonello wrote:
> Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
> potentially cause a race condition between both calls because f_midi_transmit
> is not reentrant nor thread-safe. This is due to an implementation detail that
> the transmit function looks for the next available usb request from the fifo
> and only enqueues it if there is data to send, otherwise just re-uses it. So,
> if both ALSA and USB frameworks calls this function at the same time,
> kfifo_seek() will return the same usb_request, which will cause a race
> condition.
> 
> To solve this problem a syncronization mechanism is necessary. In this case it
> is used a spinlock since f_midi_transmit is also called by 
> usb_request->complete
> callback in interrupt context.
> 
> Cc:  # v4.5+
> Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
> Signed-off-by: Felipe F. Tonello 

I'm sorry but I forgot to add v2 to the subject prefix.

Anyway, the changes from the previous patch is that this patch is on top
of Linus' v4.5-rc7 tag.

> ---
>  drivers/usb/gadget/function/f_midi.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index fb1fe96d3215..ecb46d6abf0e 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -92,6 +93,7 @@ struct f_midi {
>   /* This fifo is used as a buffer ring for pre-allocated IN usb_requests 
> */
>   DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
>   unsigned int in_last_port;
> + spinlock_t transmit_lock;
>  };
>  
>  static inline struct f_midi *func_to_midi(struct usb_function *f)
> @@ -535,12 +537,15 @@ static void f_midi_drop_out_substreams(struct f_midi 
> *midi)
>  static void f_midi_transmit(struct f_midi *midi)
>  {
>   struct usb_ep *ep = midi->in_ep;
> + unsigned long flags;
>   bool active;
>  
>   /* We only care about USB requests if IN endpoint is enabled */
>   if (!ep || !ep->enabled)
>   goto drop_out;
>  
> + spin_lock_irqsave(>transmit_lock, flags);
> +
>   do {
>   struct usb_request *req = NULL;
>   unsigned int len, i;
> @@ -552,14 +557,17 @@ static void f_midi_transmit(struct f_midi *midi)
>   len = kfifo_peek(>in_req_fifo, );
>   if (len != 1) {
>   ERROR(midi, "%s: Couldn't get usb request\n", __func__);
> + spin_unlock_irqrestore(>transmit_lock, flags);
>   goto drop_out;
>   }
>  
>   /* If buffer overrun, then we ignore this transmission.
>* IMPORTANT: This will cause the user-space rawmidi device to 
> block until a) usb
>* requests have been completed or b) snd_rawmidi_write() times 
> out. */
> - if (req->length > 0)
> + if (req->length > 0) {
> + spin_unlock_irqrestore(>transmit_lock, flags);
>   return;
> + }
>  
>   for (i = midi->in_last_port; i < MAX_PORTS; i++) {
>   struct gmidi_in_port *port = midi->in_port[i];
> @@ -611,6 +619,8 @@ static void f_midi_transmit(struct f_midi *midi)
>   }
>   } while (active);
>  
> + spin_unlock_irqrestore(>transmit_lock, flags);
> +
>   return;
>  
>  drop_out:
> @@ -1216,6 +1226,8 @@ static struct usb_function *f_midi_alloc(struct 
> usb_function_instance *fi)
>   if (status)
>   goto setup_fail;
>  
> + spin_lock_init(>transmit_lock);
> +
>   ++opts->refcnt;
>   mutex_unlock(>lock);
>  
> 

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Bug ID: 114111

[Peter Maciejko]

https://bugzilla.kernel.org/show_bug.cgi?id=114111

Bug ID: 114111
Summary: Reboot after shutdown, when USB 3.0 controller is enabled

PM
--
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: f_midi: added spinlock on transmit function

[Felipe F. Tonello]

Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
potentially cause a race condition between both calls because f_midi_transmit
is not reentrant nor thread-safe. This is due to an implementation detail that
the transmit function looks for the next available usb request from the fifo
and only enqueues it if there is data to send, otherwise just re-uses it. So,
if both ALSA and USB frameworks calls this function at the same time,
kfifo_seek() will return the same usb_request, which will cause a race
condition.

To solve this problem a syncronization mechanism is necessary. In this case it
is used a spinlock since f_midi_transmit is also called by usb_request->complete
callback in interrupt context.

Cc:  # v4.5+
Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index fb1fe96d3215..ecb46d6abf0e 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -92,6 +93,7 @@ struct f_midi {
/* This fifo is used as a buffer ring for pre-allocated IN usb_requests 
*/
DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
unsigned int in_last_port;
+   spinlock_t transmit_lock;
 };
 
 static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -535,12 +537,15 @@ static void f_midi_drop_out_substreams(struct f_midi 
*midi)
 static void f_midi_transmit(struct f_midi *midi)
 {
struct usb_ep *ep = midi->in_ep;
+   unsigned long flags;
bool active;
 
/* We only care about USB requests if IN endpoint is enabled */
if (!ep || !ep->enabled)
goto drop_out;
 
+   spin_lock_irqsave(>transmit_lock, flags);
+
do {
struct usb_request *req = NULL;
unsigned int len, i;
@@ -552,14 +557,17 @@ static void f_midi_transmit(struct f_midi *midi)
len = kfifo_peek(>in_req_fifo, );
if (len != 1) {
ERROR(midi, "%s: Couldn't get usb request\n", __func__);
+   spin_unlock_irqrestore(>transmit_lock, flags);
goto drop_out;
}
 
/* If buffer overrun, then we ignore this transmission.
 * IMPORTANT: This will cause the user-space rawmidi device to 
block until a) usb
 * requests have been completed or b) snd_rawmidi_write() times 
out. */
-   if (req->length > 0)
+   if (req->length > 0) {
+   spin_unlock_irqrestore(>transmit_lock, flags);
return;
+   }
 
for (i = midi->in_last_port; i < MAX_PORTS; i++) {
struct gmidi_in_port *port = midi->in_port[i];
@@ -611,6 +619,8 @@ static void f_midi_transmit(struct f_midi *midi)
}
} while (active);
 
+   spin_unlock_irqrestore(>transmit_lock, flags);
+
return;
 
 drop_out:
@@ -1216,6 +1226,8 @@ static struct usb_function *f_midi_alloc(struct 
usb_function_instance *fi)
if (status)
goto setup_fail;
 
+   spin_lock_init(>transmit_lock);
+
++opts->refcnt;
mutex_unlock(>lock);
 
-- 
2.7.2

--
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: gadget: f_midi: added spinlock on transmit function

[Felipe Ferreri Tonello]

Hi Balbi,

On 09/03/16 10:33, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello  writes:
>> [ text/plain ]
>> Hi Balbi,
>>
>> On 09/03/16 07:20, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> "Felipe F. Tonello"  writes:
 [ text/plain ]
 Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
 potentially cause a race condition between both calls because 
 f_midi_transmit
 is not reentrant nor thread-safe. This is due to an implementation detail 
 that
 the transmit function looks for the next available usb request from the 
 fifo
 and only enqueues it if there is data to send, otherwise just re-uses it. 
 So,
 if both ALSA and USB frameworks calls this function at the same time,
 kfifo_seek() will return the same usb_request, which will cause a race
 condition.

 To solve this problem a syncronization mechanism is necessary. In this 
 case it
 is used a spinlock since f_midi_transmit is also called by 
 usb_request->complete
 callback in interrupt context.

 Cc:  # v4.4+
>>>
>>> this should be v4.5+
>>>
>>> $ git describe e1e3d7ec5da3
>>> v4.4-rc5-59-ge1e3d7ec5da3
>>>
 @@ -651,17 +653,22 @@ static void f_midi_transmit(struct f_midi *midi)
  {
struct usb_ep *ep = midi->in_ep;
int ret;
 +  unsigned long flags;
  
/* We only care about USB requests if IN endpoint is enabled */
if (!ep || !ep->enabled)
goto drop_out;
  
 +  spin_lock_irqsave(>transmit_lock, flags);
 +
do {
ret = f_midi_do_transmit(midi, ep);
>>>
>>> you wrote this commit on top of 'next' right ? I know that because of
>>> this call to f_midi_do_transmit() here which is introduced by commit
>>> aac887442d5e ("usb: gadget: f_midi: move some of f_midi_transmit to
>>> separate func") which is not in Linus' tree yet.
>>>
>>> This prevents me from taking this patch during current -rc.
>>
>> Yes, but then what should I do? Because if I patch on Linus' tree, then
>> the patch won't apply to your tree.
> 
> it should apply to my "fixes" branch where fixes for current -rc should
> go :-) Note that v4.5-rc7 doesn't have f_midi_do_transmit()

Right, but then you will have to fix the merge conflicts by hand. :(

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH] usb: gadget: f_midi: refactor state machine

[Felipe Ferreri Tonello]

Hi Balbi,

On 09/03/16 07:04, Felipe Balbi wrote:
> 
> Hi,
> 
> "Felipe F. Tonello"  writes:
>> [ text/plain ]
>> This refactor results in a cleaner state machine code and promotes
>> consistency, readability, and maintanability of this driver.
>>
>> Signed-off-by: Felipe F. Tonello 
> 
> while working on this driver ...
> 
>> ---
>>  drivers/usb/gadget/function/f_midi.c | 204 
>> ++-
>>  1 file changed, 129 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>> b/drivers/usb/gadget/function/f_midi.c
>> index 84c0ee5ebd1e..3cdb0741f3f8 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -50,6 +50,19 @@ static const char f_midi_longname[] = "MIDI Gadget";
>>   */
>>  #define MAX_PORTS 16
>>  
>> +/* MIDI message states */
>> +enum {
>> +STATE_INITIAL = 0,  /* pseudo state */
>> +STATE_1PARAM,
>> +STATE_2PARAM_1,
>> +STATE_2PARAM_2,
>> +STATE_SYSEX_0,
>> +STATE_SYSEX_1,
>> +STATE_SYSEX_2,
>> +STATE_REAL_TIME,
>> +STATE_FINISHED, /* pseudo state */
>> +};
>> +
>>  /*
>>   * This is a gadget, and the IN/OUT naming is from the host's perspective.
>>   * USB -> OUT endpoint -> rawmidi
>> @@ -60,13 +73,6 @@ struct gmidi_in_port {
>>  int active;
>>  uint8_t cable;
>>  uint8_t state;
>> -#define STATE_UNKNOWN   0
>> -#define STATE_1PARAM1
>> -#define STATE_2PARAM_1  2
>> -#define STATE_2PARAM_2  3
>> -#define STATE_SYSEX_0   4
>> -#define STATE_SYSEX_1   5
>> -#define STATE_SYSEX_2   6
>>  uint8_t data[2];
>>  };
>>  
>> @@ -400,118 +406,166 @@ static int f_midi_snd_free(struct snd_device *device)
>>  return 0;
>>  }
>>  
>> -static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
>> -uint8_t p1, uint8_t p2, uint8_t p3)
>> -{
>> -unsigned length = req->length;
>> -u8 *buf = (u8 *)req->buf + length;
>> -
>> -buf[0] = p0;
>> -buf[1] = p1;
>> -buf[2] = p2;
>> -buf[3] = p3;
>> -req->length = length + 4;
>> -}
>> -
>>  /*
>>   * Converts MIDI commands to USB MIDI packets.
>>   */
>>  static void f_midi_transmit_byte(struct usb_request *req,
>>   struct gmidi_in_port *port, uint8_t b)
> 
> ... could you get rid of the userspace types (as a separate patch, of
> course) ?

userspace types?

> 
>>  {
>> -uint8_t p0 = port->cable << 4;
>> +uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
>> +uint8_t next_state = STATE_INITIAL;
> 
> and here too. I know you're going for consistency, but maybe it makes
> sense to clean up the types before you cleanup the state machine.

What do you mean by types? Do you mean to convert the state macros to an
enum?

> 
> [...]
> 
>> +/* States where we have to write into the USB request */
>> +if (next_state == STATE_FINISHED ||
>> +port->state == STATE_SYSEX_2 ||
>> +port->state == STATE_1PARAM ||
>> +port->state == STATE_2PARAM_2 ||
>> +port->state == STATE_REAL_TIME) {
>> +
>> +unsigned length = req->length;
>> +u8 *buf = (u8 *)req->buf + length;
>> +
>> +memcpy(buf, p, sizeof(p));
>> +req->length = length + sizeof(p);
>> +
>> +if (next_state == STATE_FINISHED) {
>> +next_state = STATE_INITIAL;
>> +port->data[0] = port->data[1] = 0;
>> +}
>>  }
>> +
>> +port->state = next_state;
> 
> okay, so this function will be called from ->complete() which is called
> without controller lock held but with IRQs disabled. I wonder if you're
> racing port->state here with this change. Could race on SMP, no ?

Yes it will race because this function is not thread-safe. BTW, it would
race anyway even without my patch.

That's why we need that spin lock patch.
It goes like this:

USB request complete() or ALSA transmit():
  f_midi_transmit()
spin lock with irq
  call this refactored function()
spin unlock with irq
  return
return

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH 2/2] usb: renesas_usbhs: disable tx irq before starting TX DMAC transfer

[Sergei Shtylyov]

Hello.

On 3/8/2016 8:58 AM, Yoshihiro Shimoda wrote:


This patch adds a code to surely disable tx irq of the pipe before


   TX IRQ.


starting TX DMAC transfer. Otherwise, a lot of unnecessary tx irqs


   TX IRQs. Mixing "TX" and "tx" on a single line is... inconsistent. :-)


may heppen in rare cases when DMAC is used.


   Happen.



Signed-off-by: Yoshihiro Shimoda 

[...]

MBR, Sergei

--
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 7/7] USB: usbatm: avoid fragile and inefficient snprintf use

[Sergei Shtylyov]

Hello.

On 3/8/2016 11:40 PM, Rasmus Villemoes wrote:


Passing overlapping source and destination is fragile, and in this
case we can even simplify the code and avoid the huge stack buffer by
using the %p extension for printing a small hex dump.

Signed-off-by: Rasmus Villemoes 
---
  drivers/usb/atm/usbatm.c | 11 ---
  1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
index db322d9ccb6e..fb47f9883056 100644
--- a/drivers/usb/atm/usbatm.c
+++ b/drivers/usb/atm/usbatm.c
@@ -1331,15 +1331,12 @@ MODULE_VERSION(DRIVER_VERSION);
  static int usbatm_print_packet(struct usbatm_data *instance,
   const unsigned char *data, int len)
  {
-   unsigned char buffer[256];
-   int i = 0, j = 0;
+   int i, j;

for (i = 0; i < len;) {
-   buffer[0] = '\0';
-   sprintf(buffer, "%.3d :", i);
-   for (j = 0; (j < 16) && (i < len); j++, i++)
-   sprintf(buffer, "%s %2.2x", buffer, data[i]);
-   dev_dbg(>usb_intf->dev, "%s", buffer);
+   j = min(16, len-i);


   Kernel style assumes spaces on either side of the operators, like below, no?


+   dev_dbg(>usb_intf->dev, "%.3d : %*ph", i, j, data + 
i);
+   i += j;
}
return i;
  }


MBR, Sergei

--
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 -target tree] usb: gadget: f_tcm: use after free

[Andrzej Pietrasiewicz]

Hi Nicholas,

W dniu 05.03.2016 o 08:26, Nicholas A. Bellinger pisze:

Hi Felipe + usb-gadget folks,

On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:






usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
http://www.spinics.net/lists/target-devel/msg11777.html

usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
http://www.spinics.net/lists/target-devel/msg11782.html

Felipe, Sebastian, & Andrezj, would you be so kind to review and test
usb-gadget using target-pending/for-next code..?




Actually I have noticed a problem at
8b00965 "target: Convert demo-mode only drivers to target_alloc_session"

I get:

[ 1698.406304] configfs-gadget gadget: high-speed config #1: c
[ 1698.410547] Using the BOT protocol
[ 1698.414163] Missing nexus, ignoring command
[ 1698.417944] bot_cmd_complete(309): -22

I think the third message is from f_tcm. It is because
tpg->tpg_nexus is not set anymore, because the line

tpg->tpg_nexus = tv_nexus;

is removed by the commit mentioned above.

Restoring this line fixes the problem.

Then percpu_ida commit produces the below result
(it does not happen immediately, but a while after
running the script).
I did not bisect, though; I only checked the commits
which alter drivers/usb/gadget/function/f_tcm.c.
The last one which (almost) works is:

8b00965 "target: Convert demo-mode only drivers to target_alloc_session"

AP

I used the following script:

8<

#!/bin/bash

mount -t configfs none /sys/kernel/config
modprobe libcomposite
cd /sys/kernel/config/usb_gadget
mkdir tcm
cd tcm
mkdir functions/tcm.0
cd /sys/kernel/config/target/
mkdir usb_gadget
cd usb_gadget
mkdir naa.0123456789abcdef
cd naa.0123456789abcdef
mkdir tpgt_1
cd tpgt_1
echo naa.01234567890abcdef > nexus
echo 1 > enable
cd /sys/kernel/config/usb_gadget/tcm
mkdir configs/c.1
ln -s functions/tcm.0 configs/c.1
echo 0x0525 > idVendor
echo 0x1234 > idProduct
echo 1240.dwc3 > UDC

8<

# [   45.510513] ERR bot_status_complete(73)
[   45.671921] configfs-gadget gadget: high-speed config #1: c
[   45.676158] Using the BOT protocol
[   45.679860] [ cut here ]
[   45.683981] kernel BUG at drivers/target/target_core_transport.c:1476!
[   45.690480] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[   45.696285] Modules linked in: usb_f_tcm libcomposite
[   45.701316] CPU: 2 PID: 1221 Comm: kworker/2:1 Not tainted 4.5.0-rc5+ #171
[   45.708156] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   45.714258] Workqueue: tcm_usb_gadget bot_cmd_work [usb_f_tcm]
[   45.720030] task: edc1f6c0 ti: ee2d6000 task.ti: ee2d6000
[   45.725410] PC is at target_submit_cmd_map_sgls+0x1fc/0x20c
[   45.730947] LR is at 0xeea71400
[   45.734070] pc : []lr : []psr: a013
[   45.734070] sp : ee2d7e50  ip :   fp : c07f8100
[   45.745505] r10:   r9 : eefa25c0  r8 : ed593988
[   45.750705] r7 : ed593b8c  r6 : ee00d830  r5 :   r4 : ed5939e0
[   45.757204] r3 : bf0144d8  r2 : ed593988  r1 : eea71400  r0 : ed5939e0
[   45.763705] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   45.770809] Control: 10c5387d  Table: 6d44806a  DAC: 0051
[   45.776528] Process kworker/2:1 (pid: 1221, stack limit = 0xee2d6210)
[   45.782940] Stack: (0xee2d7e50 to 0xee2d8000)
[   45.787276] 7e40: 002293a6  
 
[   45.795423] 7e60:   0003 0020  c0326a60 
 
[   45.803568] 7e80:  0020 0003    
 
[   45.811714] 7ea0:   ed5939cc 0020 ed5939e0 edf6b580 
 bf012358
[   45.819860] 7ec0:    0020 0003  
eefa2ac0 ee30da00
[   45.828006] 7ee0: ed5939cc eefa25c0 eefaaa00 c00369bc edc1f6c0 ee30da00 
0001 ee30da00
[   45.836151] 7f00: eefa25e4 0001 eefa25c0 ee30da18 eefa25c0 0008 
c07f8100 c0036c24
[   45.844297] 7f20: edc1f6c0 eeafefc0  eeafefc0  ee30da00 
c0036bf8 
[   45.852442] 7f40:    c003bdf8 fffe  
ee2d7f70 ee30da00
[   45.860587] 7f60:   dead4ead   ee2d7f74 
ee2d7f74 
[   45.868733] 7f80:  dead4ead   ee2d7f90 ee2d7f90 
ee2d7fac eeafefc0
[   45.876878] 7fa0: c003bd20   c000f8b8   
 
[   45.885023] 7fc0:       
 
[   45.893169] 7fe0:     0013  
 
[   45.901324] [] (target_submit_cmd_map_sgls) from [] 
(target_submit_cmd+0x50/0x58)
[   45.910514] [] (target_submit_cmd) from [] 
(bot_cmd_work+0x74/0x100 [usb_f_tcm])
[   45.919623] [] (bot_cmd_work [usb_f_tcm]) from [] 
(process_one_work+0x120/0x324)
[   45.928703] [] (process_one_work) from [] 
(worker_thread+0x2c/0x4b4)
[   45.936760] 

RE: [PATCH 1/2] usb: renesas_usbhs: avoid NULL pointer derefernce in usbhsf_pkt_handler()

[Felipe Balbi]

Hi,

Yoshihiro Shimoda  writes:
>> Yoshihiro Shimoda  writes:
>> > [ text/plain ]
>> > When unexpected situation happened (e.g. tx/rx irq happened while
>> > DMAC is used), the usbhsf_pkt_handler() was possible to cause NULL
>> > pointer dereference like the followings:
>> >
>> > Unable to handle kernel NULL pointer dereference at virtual address 
>> > 
>> > pgd = c0004000
>> > [] *pgd=
>> > Internal error: Oops: 8007 [#1] SMP ARM
>> > Modules linked in: usb_f_acm u_serial g_serial libcomposite
>> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.5.0-rc6-00842-gac57066-dirty 
>> > #63
>> > Hardware name: Generic R8A7790 (Flattened Device Tree)
>> > task: c0729c00 ti: c0724000 task.ti: c0724000
>> > PC is at 0x0
>> > LR is at usbhsf_pkt_handler+0xac/0x118
>> > pc : [<>]lr : []psr: 6193
>> > sp : c0725db8  ip :   fp : c0725df4
>> > r10: 0001  r9 : 0193  r8 : ef3ccab4
>> > r7 : ef3cca10  r6 : eea4586c  r5 :   r4 : ef19ceb4
>> > r3 :   r2 : 009c  r1 : c0725dc4  r0 : ef19ceb4
>> >
>> > This patch adds a condition to avoid the dereference.
>> >
>> > Signed-off-by: Yoshihiro Shimoda 
>> 
>> is this a regression fix ? Do we need it in current -rc (it's getting
>> late for that, actually), do we need a Cc:  here ?
>> 
>> Same questions are valid for the other patch in this series.
>
> Thank you for the review.

no problem :-)

> This is a potential problem fix. This issue is possible to cause the
> first DMA supporting (e73a989 usb: renesas_usbhs: add DMAEngine
> support) at 2011, I think.

okay, so according to git describe:

$ git describe e73a989
v3.0-rc2-22-ge73a9891b3a1

this entered mainline on v3.1. This means you need to add:

Fixes: e73a989 ("usb: renesas_usbhs: add DMAEngine support")
Cc:  # v3.1+

to your commit log, right before your Signed-off-by.

> We don't need in current -rc because I also think this is too late for
> it.  I'm not sure we need a CC here.

okay, so I'll queue this once v4.6-rc1 is merged. But please resend with
the changes above. Let me know if you need any further clarification.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys

[Yoshihiro Shimoda]

Hi Felipe,

> Sent: Wednesday, March 09, 2016 8:00 PM
< snip >
> >> On xHCI controllers of R-Car Gen2 and Gen3, the AC64 bit (bit 0) of
> >> HCCPARAMS1 is set to 1. However, these SoCs don't support 64-bit address
> >> memory pointers. So, this driver should call dma_set_coherent_mask(dev,
> >> DMA_BIT_MASK(32)) in xhci_gen_setup(). Otherwise, the xHCI controller
> >> will be died after a usb device is connected if R-Car Gen2/3 run on above
> >> 4GB physical memory environment.
> >>
> >> This patch adds clearing the AC64 bit of xhci->hcc_params in the
> >> xhci_plat_quirks() to fix the issue.
> >>
> >> Signed-off-by: Yoshihiro Shimoda 
> >> ---
> >>  drivers/usb/host/xhci-plat.c | 13 +
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> >> index d39d6bf..047fb6a 100644
> >> --- a/drivers/usb/host/xhci-plat.c
> >> +++ b/drivers/usb/host/xhci-plat.c
> >> @@ -39,12 +39,25 @@ static const struct xhci_driver_overrides 
> >> xhci_plat_overrides __initconst = {
> >>
> >>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> >>  {
> >> +  struct usb_hcd *hcd = xhci_to_hcd(xhci);
> >> +
> >>/*
> >> * As of now platform drivers don't provide MSI support so we ensure
> >> * here that the generic code does not try to make a pci_dev from our
> >> * dev struct in order to setup MSI
> >> */
> >>xhci->quirks |= XHCI_PLAT;
> >> +
> >> +  /*
> >> +   * On R-Car Gen2 and Gen3, the AC64 bit (bit 0) of HCCPARAMS1 is set
> >> +   * to 1. However, these SoCs don't support 64-bit address memory
> >> +   * pointers. So, this driver clears the AC64 bit of xhci->hcc_params
> >> +   * to call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in
> >> +   * xhci_gen_setup().
> >> +   */
> >> +  if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> >> +  xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
> >> +  xhci->hcc_params &= ~BIT(0);/* clear HCC_64BIT_ADDR
> >>  */
> 
> Mathias, of course, has the final saying; but I feel that, just like any
> other quirk, this should be setting a flag in xhci->quirks and xhci core
> would clear hcc_params. The reason is that if we ever need this on some
> PCIe card, we won't have to shuffle code around.

Thank you for the comment. I agree with you.
So, I will make such a code and send v2 patch tomorrow.

Best regards,
Yoshihiro Shimoda

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

RE: [PATCH 1/2] usb: renesas_usbhs: avoid NULL pointer derefernce in usbhsf_pkt_handler()

[Yoshihiro Shimoda]

Hi Felipe,

> Hi Yoshihiro,
> 
> Yoshihiro Shimoda  writes:
> > [ text/plain ]
> > When unexpected situation happened (e.g. tx/rx irq happened while
> > DMAC is used), the usbhsf_pkt_handler() was possible to cause NULL
> > pointer dereference like the followings:
> >
> > Unable to handle kernel NULL pointer dereference at virtual address 
> > pgd = c0004000
> > [] *pgd=
> > Internal error: Oops: 8007 [#1] SMP ARM
> > Modules linked in: usb_f_acm u_serial g_serial libcomposite
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.5.0-rc6-00842-gac57066-dirty #63
> > Hardware name: Generic R8A7790 (Flattened Device Tree)
> > task: c0729c00 ti: c0724000 task.ti: c0724000
> > PC is at 0x0
> > LR is at usbhsf_pkt_handler+0xac/0x118
> > pc : [<>]lr : []psr: 6193
> > sp : c0725db8  ip :   fp : c0725df4
> > r10: 0001  r9 : 0193  r8 : ef3ccab4
> > r7 : ef3cca10  r6 : eea4586c  r5 :   r4 : ef19ceb4
> > r3 :   r2 : 009c  r1 : c0725dc4  r0 : ef19ceb4
> >
> > This patch adds a condition to avoid the dereference.
> >
> > Signed-off-by: Yoshihiro Shimoda 
> 
> is this a regression fix ? Do we need it in current -rc (it's getting
> late for that, actually), do we need a Cc:  here ?
> 
> Same questions are valid for the other patch in this series.

Thank you for the review.
This is a potential problem fix. This issue is possible to cause the first
DMA supporting (e73a989 usb: renesas_usbhs: add DMAEngine support) at 2011, I 
think.

We don't need in current -rc because I also think this is too late for it.
I'm not sure we need a CC here.

Best regards,
Yoshihiro Shimoda

> --
> balbi
--
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 5/5 v9] usb: musb: da8xx: Add DT support for the DA8xx driver

[Sergei Shtylyov]

Hello.

On 3/9/2016 12:25 PM, Petr Kulhavy wrote:


This adds DT support for TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver

Signed-off-by: Petr Kulhavy 
Tested-by: Petr Kulhavy 

[...]


diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b03d3b8..aabd33a 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c

[...]

@@ -134,6 +139,55 @@ static inline void phy_off(void)
__raw_writel(cfgchip2, CFGCHIP2);
  }

+static inline int get_phy_refclk_cfg(struct device_node *np)
+{
+   u32 freq;
+
+   if (of_property_read_u32(np, "ti,usb2-phy-refclock-hz", ))
+   return -EINVAL;


Shouldn't the frequency be retrieved thru the clk API, like we do it for 
the power, BTW?



+
+   switch (freq) {
+   case 1200:
+   return CFGCHIP2_REFFREQ_12MHZ;
+   case 1300:
+   return CFGCHIP2_REFFREQ_13MHZ;
+   case 1920:
+   return CFGCHIP2_REFFREQ_19_2MHZ;
+   case 2000:
+   return CFGCHIP2_REFFREQ_20MHZ;
+   case 2400:
+   return CFGCHIP2_REFFREQ_24MHZ;
+   case 2600:
+   return CFGCHIP2_REFFREQ_26MHZ;
+   case 3840:
+   return CFGCHIP2_REFFREQ_38_4MHZ;
+   case 4000:
+   return CFGCHIP2_REFFREQ_40MHZ;
+   case 4800:
+   return CFGCHIP2_REFFREQ_48MHZ;
+   default:
+   return -EINVAL;
+   }
+}
+
+static inline u8 get_vbus_power(struct device *dev)
+{
+   struct regulator *vbus_supply;
+   int current_uA;
+
+   vbus_supply = regulator_get(dev, "vbus");
+   if (IS_ERR(vbus_supply))
+   return 255;
+
+   current_uA = regulator_get_current_limit(vbus_supply);
+   regulator_put(vbus_supply);
+
+   if (current_uA <= 0 || current_uA > 51)
+   return 255;
+
+   return current_uA / 1000;
+}


   Wait, the platform data expects that in 2 mA units, you forgot to divide by 
2!

[...]

MBR, Sergei

--
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 2/5 v9] usb: musb: core: added helper function for parsing DT

[Sergei Shtylyov]

Hello.

On 3/9/2016 12:25 PM, Petr Kulhavy wrote:


This adds the function musb_get_mode() to get the DT property "dr_mode"

Signed-off-by: Petr Kulhavy 


Acked-by: Sergei Shtylyov 

MBR, Sergei

--
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 -target tree] usb: gadget: f_tcm: use after free

[Felipe Balbi]

Hi,

"Nicholas A. Bellinger"  writes:
> [ text/plain ]
> Hi Felipe + usb-gadget folks,
>
> On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
>> Dan Carpenter  writes:
>> > We need to move the kfree() down a line so we don't dereference a freed
>> > variable.
>> >
>> > Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to 
>> > target_alloc_session')
>> > Signed-off-by: Dan Carpenter 
>> 
>> It's okay to take this via target:
>> 
>> Signed-off-by: Felipe Balbi 
>> 
>
> Note this specific patch is only a mechanical change, and we still need
> reviews for the more interesting conversions:
>
> usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> http://www.spinics.net/lists/target-devel/msg11777.html
>
> usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> http://www.spinics.net/lists/target-devel/msg11782.html
>
I don't have either patch in my inbox apparently. Care to resend ?

sorry

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/2] usb: renesas_usbhs: avoid NULL pointer derefernce in usbhsf_pkt_handler()

[Felipe Balbi]

Hi Yoshihiro,

Yoshihiro Shimoda  writes:
> [ text/plain ]
> When unexpected situation happened (e.g. tx/rx irq happened while
> DMAC is used), the usbhsf_pkt_handler() was possible to cause NULL
> pointer dereference like the followings:
>
> Unable to handle kernel NULL pointer dereference at virtual address 
> pgd = c0004000
> [] *pgd=
> Internal error: Oops: 8007 [#1] SMP ARM
> Modules linked in: usb_f_acm u_serial g_serial libcomposite
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.5.0-rc6-00842-gac57066-dirty #63
> Hardware name: Generic R8A7790 (Flattened Device Tree)
> task: c0729c00 ti: c0724000 task.ti: c0724000
> PC is at 0x0
> LR is at usbhsf_pkt_handler+0xac/0x118
> pc : [<>]lr : []psr: 6193
> sp : c0725db8  ip :   fp : c0725df4
> r10: 0001  r9 : 0193  r8 : ef3ccab4
> r7 : ef3cca10  r6 : eea4586c  r5 :   r4 : ef19ceb4
> r3 :   r2 : 009c  r1 : c0725dc4  r0 : ef19ceb4
>
> This patch adds a condition to avoid the dereference.
>
> Signed-off-by: Yoshihiro Shimoda 

is this a regression fix ? Do we need it in current -rc (it's getting
late for that, actually), do we need a Cc:  here ?

Same questions are valid for the other patch in this series.

-- 
balbi


signature.asc
Description: PGP signature

[ANNOUNCE] permanent email

[Felipe Balbi]

Hi guys,

so this will be the email I'll actually be using in the list. It's okay
to continue to send emails to ba...@kernel.org as my notmuch searches
will include that account, plus I don't want to update MAINTAINERS yet
again :-)

If you want to update your contact list and send me emails to
felipe.ba...@linux.intel.com that's also okay; whatever works best for
you guys. Either ba...@kernel.org or felipe.ba...@linux.intel.com will
work just as well.

Thanks

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys

[Felipe Balbi]

Hi,

Yoshihiro Shimoda  writes:
> [ text/plain ]
> Hi Mathias,
>
> Would you review this patch?
> This patch could be applied into the latest usb.git / usb-next branch.
>
> Best regards,
> Yoshihiro Shimoda
>
>> -Original Message-
>> From: Yoshihiro Shimoda
>> Sent: Tuesday, February 16, 2016 12:26 PM
>> To: mathias.ny...@intel.com; gre...@linuxfoundation.org
>> Cc: linux-usb@vger.kernel.org; linux-renesas-...@vger.kernel.org; Yoshihiro 
>> Shimoda 
>> Subject: [PATCH] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run 
>> on above 4GB phys
>> 
>> On xHCI controllers of R-Car Gen2 and Gen3, the AC64 bit (bit 0) of
>> HCCPARAMS1 is set to 1. However, these SoCs don't support 64-bit address
>> memory pointers. So, this driver should call dma_set_coherent_mask(dev,
>> DMA_BIT_MASK(32)) in xhci_gen_setup(). Otherwise, the xHCI controller
>> will be died after a usb device is connected if R-Car Gen2/3 run on above
>> 4GB physical memory environment.
>> 
>> This patch adds clearing the AC64 bit of xhci->hcc_params in the
>> xhci_plat_quirks() to fix the issue.
>> 
>> Signed-off-by: Yoshihiro Shimoda 
>> ---
>>  drivers/usb/host/xhci-plat.c | 13 +
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index d39d6bf..047fb6a 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -39,12 +39,25 @@ static const struct xhci_driver_overrides 
>> xhci_plat_overrides __initconst = {
>> 
>>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>>  {
>> +struct usb_hcd *hcd = xhci_to_hcd(xhci);
>> +
>>  /*
>>   * As of now platform drivers don't provide MSI support so we ensure
>>   * here that the generic code does not try to make a pci_dev from our
>>   * dev struct in order to setup MSI
>>   */
>>  xhci->quirks |= XHCI_PLAT;
>> +
>> +/*
>> + * On R-Car Gen2 and Gen3, the AC64 bit (bit 0) of HCCPARAMS1 is set
>> + * to 1. However, these SoCs don't support 64-bit address memory
>> + * pointers. So, this driver clears the AC64 bit of xhci->hcc_params
>> + * to call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in
>> + * xhci_gen_setup().
>> + */
>> +if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
>> +xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
>> +xhci->hcc_params &= ~BIT(0);/* clear HCC_64BIT_ADDR
>>  */

Mathias, of course, has the final saying; but I feel that, just like any
other quirk, this should be setting a flag in xhci->quirks and xhci core
would clear hcc_params. The reason is that if we ever need this on some
PCIe card, we won't have to shuffle code around.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on above 4GB phys

[Yoshihiro Shimoda]

Hi Mathias,

Would you review this patch?
This patch could be applied into the latest usb.git / usb-next branch.

Best regards,
Yoshihiro Shimoda

> -Original Message-
> From: Yoshihiro Shimoda
> Sent: Tuesday, February 16, 2016 12:26 PM
> To: mathias.ny...@intel.com; gre...@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; linux-renesas-...@vger.kernel.org; Yoshihiro 
> Shimoda 
> Subject: [PATCH] usb: host: xhci-plat: fix cannot work if R-Car Gen2/3 run on 
> above 4GB phys
> 
> On xHCI controllers of R-Car Gen2 and Gen3, the AC64 bit (bit 0) of
> HCCPARAMS1 is set to 1. However, these SoCs don't support 64-bit address
> memory pointers. So, this driver should call dma_set_coherent_mask(dev,
> DMA_BIT_MASK(32)) in xhci_gen_setup(). Otherwise, the xHCI controller
> will be died after a usb device is connected if R-Car Gen2/3 run on above
> 4GB physical memory environment.
> 
> This patch adds clearing the AC64 bit of xhci->hcc_params in the
> xhci_plat_quirks() to fix the issue.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/usb/host/xhci-plat.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index d39d6bf..047fb6a 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -39,12 +39,25 @@ static const struct xhci_driver_overrides 
> xhci_plat_overrides __initconst = {
> 
>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>  {
> + struct usb_hcd *hcd = xhci_to_hcd(xhci);
> +
>   /*
>* As of now platform drivers don't provide MSI support so we ensure
>* here that the generic code does not try to make a pci_dev from our
>* dev struct in order to setup MSI
>*/
>   xhci->quirks |= XHCI_PLAT;
> +
> + /*
> +  * On R-Car Gen2 and Gen3, the AC64 bit (bit 0) of HCCPARAMS1 is set
> +  * to 1. However, these SoCs don't support 64-bit address memory
> +  * pointers. So, this driver clears the AC64 bit of xhci->hcc_params
> +  * to call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in
> +  * xhci_gen_setup().
> +  */
> + if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> + xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
> + xhci->hcc_params &= ~BIT(0);/* clear HCC_64BIT_ADDR */
>  }
> 
>  /* called during probe() after chip reset completes */
> --
> 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] usb: gadget: f_midi: added spinlock on transmit function

[Felipe Balbi]

Hi,

Felipe Ferreri Tonello  writes:
> [ text/plain ]
> Hi Balbi,
>
> On 09/03/16 07:20, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> "Felipe F. Tonello"  writes:
>>> [ text/plain ]
>>> Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
>>> potentially cause a race condition between both calls because 
>>> f_midi_transmit
>>> is not reentrant nor thread-safe. This is due to an implementation detail 
>>> that
>>> the transmit function looks for the next available usb request from the fifo
>>> and only enqueues it if there is data to send, otherwise just re-uses it. 
>>> So,
>>> if both ALSA and USB frameworks calls this function at the same time,
>>> kfifo_seek() will return the same usb_request, which will cause a race
>>> condition.
>>>
>>> To solve this problem a syncronization mechanism is necessary. In this case 
>>> it
>>> is used a spinlock since f_midi_transmit is also called by 
>>> usb_request->complete
>>> callback in interrupt context.
>>>
>>> Cc:  # v4.4+
>> 
>> this should be v4.5+
>> 
>> $ git describe e1e3d7ec5da3
>> v4.4-rc5-59-ge1e3d7ec5da3
>> 
>>> @@ -651,17 +653,22 @@ static void f_midi_transmit(struct f_midi *midi)
>>>  {
>>> struct usb_ep *ep = midi->in_ep;
>>> int ret;
>>> +   unsigned long flags;
>>>  
>>> /* We only care about USB requests if IN endpoint is enabled */
>>> if (!ep || !ep->enabled)
>>> goto drop_out;
>>>  
>>> +   spin_lock_irqsave(>transmit_lock, flags);
>>> +
>>> do {
>>> ret = f_midi_do_transmit(midi, ep);
>> 
>> you wrote this commit on top of 'next' right ? I know that because of
>> this call to f_midi_do_transmit() here which is introduced by commit
>> aac887442d5e ("usb: gadget: f_midi: move some of f_midi_transmit to
>> separate func") which is not in Linus' tree yet.
>> 
>> This prevents me from taking this patch during current -rc.
>
> Yes, but then what should I do? Because if I patch on Linus' tree, then
> the patch won't apply to your tree.

it should apply to my "fixes" branch where fixes for current -rc should
go :-) Note that v4.5-rc7 doesn't have f_midi_do_transmit()

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: f_midi: added spinlock on transmit function

[Felipe Ferreri Tonello]

Hi Balbi,

On 09/03/16 07:20, Felipe Balbi wrote:
> 
> Hi,
> 
> "Felipe F. Tonello"  writes:
>> [ text/plain ]
>> Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
>> potentially cause a race condition between both calls because f_midi_transmit
>> is not reentrant nor thread-safe. This is due to an implementation detail 
>> that
>> the transmit function looks for the next available usb request from the fifo
>> and only enqueues it if there is data to send, otherwise just re-uses it. So,
>> if both ALSA and USB frameworks calls this function at the same time,
>> kfifo_seek() will return the same usb_request, which will cause a race
>> condition.
>>
>> To solve this problem a syncronization mechanism is necessary. In this case 
>> it
>> is used a spinlock since f_midi_transmit is also called by 
>> usb_request->complete
>> callback in interrupt context.
>>
>> Cc:  # v4.4+
> 
> this should be v4.5+
> 
> $ git describe e1e3d7ec5da3
> v4.4-rc5-59-ge1e3d7ec5da3
> 
>> @@ -651,17 +653,22 @@ static void f_midi_transmit(struct f_midi *midi)
>>  {
>>  struct usb_ep *ep = midi->in_ep;
>>  int ret;
>> +unsigned long flags;
>>  
>>  /* We only care about USB requests if IN endpoint is enabled */
>>  if (!ep || !ep->enabled)
>>  goto drop_out;
>>  
>> +spin_lock_irqsave(>transmit_lock, flags);
>> +
>>  do {
>>  ret = f_midi_do_transmit(midi, ep);
> 
> you wrote this commit on top of 'next' right ? I know that because of
> this call to f_midi_do_transmit() here which is introduced by commit
> aac887442d5e ("usb: gadget: f_midi: move some of f_midi_transmit to
> separate func") which is not in Linus' tree yet.
> 
> This prevents me from taking this patch during current -rc.

Yes, but then what should I do? Because if I patch on Linus' tree, then
the patch won't apply to your tree.

-- 
Felipe


0x92698E6A.asc
Description: application/pgp-keys

[PATCH 4/5 v9] ARM: davinci: defined missing CFGCHIP2_REFFREQ_* macros for MUSB PHY

[Petr Kulhavy]

Only few MUSB PHY reference clock  frequencies were defined.
This patch defines macros for the missing frequencies:
19.2MHz, 38.4MHz, 13MHz, 26MHz, 20MHz, 40MHz

Signed-off-by: Petr Kulhavy 
Acked-by: Sergei Shtylyov 
Signed-off-by: Bin Liu 
---
v5: 
v6: 
v7: 
v8: 
v9: 

 include/linux/platform_data/usb-davinci.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/platform_data/usb-davinci.h 
b/include/linux/platform_data/usb-davinci.h
index e0bc4ab..961d3dc 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -33,6 +33,12 @@
 #define CFGCHIP2_REFFREQ_12MHZ (1 << 0)
 #define CFGCHIP2_REFFREQ_24MHZ (2 << 0)
 #define CFGCHIP2_REFFREQ_48MHZ (3 << 0)
+#define CFGCHIP2_REFFREQ_19_2MHZ   (4 << 0)
+#define CFGCHIP2_REFFREQ_38_4MHZ   (5 << 0)
+#define CFGCHIP2_REFFREQ_13MHZ (6 << 0)
+#define CFGCHIP2_REFFREQ_26MHZ (7 << 0)
+#define CFGCHIP2_REFFREQ_20MHZ (8 << 0)
+#define CFGCHIP2_REFFREQ_40MHZ (9 << 0)
 
 struct da8xx_ohci_root_hub;
 
-- 
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 5/5 v9] usb: musb: da8xx: Add DT support for the DA8xx driver

[Petr Kulhavy]

This adds DT support for TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver

Signed-off-by: Petr Kulhavy 
Tested-by: Petr Kulhavy 
---
v1: 

v2:
 - using standard MUSB properties "dr_mode", "mentor,power", "mentor,num-eps", 
"mentor,multipoint", "mentor,ram-bits"
 - using "ti," prefix instead of "da8xx," for specific property names
 - no wilcards in compatibility string
 - using CFGCHIP2_USB2PHYCLKMUX_SHIFT instead of CFGCHIP2_USB2PHYCLKMUX_OFFSET

v3:
 - DMA mask initialization corrected
 - removed extra #ifdef CONFIG_OF

v4:
 - compatibility string set to "ti,da830-musb"
 - "mentor,num-eps", "mentor,multipoint", "mentor,ram-bits" properties removed 
and hardcoded
 - "ti,phy20-clkmux-cfg" renamed to "ti,phy20-clkmux-pll" and changed to boolean
 - removed use of the DA8XX_SYSCFG0_VIRT macro

v5:
 - using CFGCHIP2_REFFREQ_ in get_phy_refclk_cfg()
 - simplified initialization of the hard coded config parameters
 - optimization CFGCHIP2 register update

v6:
 - using "ti,usb2-phy-" prefix instead of "ti,phy20-" for the specific 
properties
 - optimization CFGCHIP2 register update
 
v7:
 - pdata::power hard coded to 500mA

v8:
 - USB maximum power modelled via a regulator "vbus-supply"
 - "ti,usb2-phy-refclock-frequency" renamed to "ti,usb2-phy-refclock-hz"
 - "ti,usb2-phy-clkmux-pll" changed to "ti,usb2-phy-clkmux-refclkin" and the 
boolean meaning inverted to reflect the more common case

v9:
 - power regulator used only to get the current
+
 drivers/usb/musb/da8xx.c | 106 +++
 1 file changed, 106 insertions(+)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b03d3b8..aabd33a 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -6,6 +6,9 @@
  * Based on the DaVinci "glue layer" code.
  * Copyright (C) 2005-2006 by Texas Instruments
  *
+ * DT support
+ * Copyright (c) 2016 Petr Kulhavy, Barix AG 
+ *
  * This file is part of the Inventra Controller Driver for Linux.
  *
  * The Inventra Controller Driver for Linux is free software; you
@@ -33,9 +36,11 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
+#include 
 
 #include "musb_core.h"
 
@@ -134,6 +139,55 @@ static inline void phy_off(void)
__raw_writel(cfgchip2, CFGCHIP2);
 }
 
+static inline int get_phy_refclk_cfg(struct device_node *np)
+{
+   u32 freq;
+
+   if (of_property_read_u32(np, "ti,usb2-phy-refclock-hz", ))
+   return -EINVAL;
+
+   switch (freq) {
+   case 1200:
+   return CFGCHIP2_REFFREQ_12MHZ;
+   case 1300:
+   return CFGCHIP2_REFFREQ_13MHZ;
+   case 1920:
+   return CFGCHIP2_REFFREQ_19_2MHZ;
+   case 2000:
+   return CFGCHIP2_REFFREQ_20MHZ;
+   case 2400:
+   return CFGCHIP2_REFFREQ_24MHZ;
+   case 2600:
+   return CFGCHIP2_REFFREQ_26MHZ;
+   case 3840:
+   return CFGCHIP2_REFFREQ_38_4MHZ;
+   case 4000:
+   return CFGCHIP2_REFFREQ_40MHZ;
+   case 4800:
+   return CFGCHIP2_REFFREQ_48MHZ;
+   default:
+   return -EINVAL;
+   }
+}
+
+static inline u8 get_vbus_power(struct device *dev)
+{
+   struct regulator *vbus_supply;
+   int current_uA;
+
+   vbus_supply = regulator_get(dev, "vbus");
+   if (IS_ERR(vbus_supply))
+   return 255;
+
+   current_uA = regulator_get_current_limit(vbus_supply);
+   regulator_put(vbus_supply);
+
+   if (current_uA <= 0 || current_uA > 51)
+   return 255;
+
+   return current_uA / 1000;
+}
+
 /*
  * Because we don't set CTRL.UINT, it's "important" to:
  * - not read/write INTRUSB/INTRUSBE (except during
@@ -482,6 +536,12 @@ static const struct platform_device_info da8xx_dev_info = {
.dma_mask   = DMA_BIT_MASK(32),
 };
 
+static const struct musb_hdrc_config da8xx_config = {
+   .ram_bits = 10,
+   .num_eps = 5,
+   .multipoint = 1,
+};
+
 static int da8xx_probe(struct platform_device *pdev)
 {
struct resource musb_resources[2];
@@ -490,6 +550,7 @@ static int da8xx_probe(struct platform_device *pdev)
struct da8xx_glue   *glue;
struct platform_device_info pinfo;
struct clk  *clk;
+   struct device_node  *np = pdev->dev.of_node;
 
int ret = -ENOMEM;
 
@@ -515,6 +576,42 @@ static int da8xx_probe(struct platform_device *pdev)
glue->dev   = >dev;
glue->clk   = clk;
 
+   if (IS_ENABLED(CONFIG_OF) && np) {
+   int refclk_cfg;
+   u32 cfgchip2;
+
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata) {
+   ret = -ENOMEM;
+   goto err5;
+   }
+
+   

[PATCH 3/5 v9] usb: musb: core: added missing const qualifier to musb_hdrc_platform_data::config

[Petr Kulhavy]

The musb_hdrc_platform_data::config was defined as a non-const pointer.
However some drivers (e.g. the ux500) set up this pointer to point to a
static structure, which is potentially dangerous. Since the musb core
uses the pointer in a read-only manner the const qualifier was added to
protect the content of the config.

Signed-off-by: Petr Kulhavy 
Acked-by: Sergei Shtylyov 
Signed-off-by: Bin Liu 
---
v5: 

v6:
 - whitespace formatting corrected

v7: 
v8: 
v9: 

 drivers/usb/musb/musb_core.c | 2 +-
 drivers/usb/musb/musb_core.h | 2 +-
 include/linux/usb/musb.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 6a2d6d3..b0be1a9 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1920,7 +1920,7 @@ static void musb_recover_from_babble(struct musb *musb)
  */
 
 static struct musb *allocate_instance(struct device *dev,
-   struct musb_hdrc_config *config, void __iomem *mbase)
+   const struct musb_hdrc_config *config, void __iomem *mbase)
 {
struct musb *musb;
struct musb_hw_ep   *ep;
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 3f7a325..a062185 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -438,7 +438,7 @@ struct musb {
 */
unsigneddouble_buffer_not_ok:1;
 
-   struct musb_hdrc_config *config;
+   const struct musb_hdrc_config *config;
 
int xceiv_old_state;
 #ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 96ddfb7..0b3da40 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -124,7 +124,7 @@ struct musb_hdrc_platform_data {
int (*set_power)(int state);
 
/* MUSB configuration-specific details */
-   struct musb_hdrc_config *config;
+   const struct musb_hdrc_config *config;
 
/* Architecture specific board data */
void*board_data;
-- 
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/5 v9] usb: musb: core: added helper function for parsing DT

[Petr Kulhavy]

This adds the function musb_get_mode() to get the DT property "dr_mode"

Signed-off-by: Petr Kulhavy 
---
v4: 
 - created musb_get_dr_mode()

v5:
 - musb_get_dr_mode() renamed to musb_get_mode()
 - added musb_get_power()

v6:
 - musb_get_power() : added missing boundary check for the maximum value 510mA
 - formatting
 - added empty implementation of musb_get_power()

v7:
 - removed empty implementation of musb_get_power()
 - musb_get_mode() returns MUSB_OTG if the property is not found

v8:
 - musb_get_power() removed

v9: 

 drivers/usb/musb/musb_core.c | 19 +++
 drivers/usb/musb/musb_core.h |  5 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index c3791a0..6a2d6d3 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -100,6 +100,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "musb_core.h"
 
@@ -129,6 +130,24 @@ static inline struct musb *dev_to_musb(struct device *dev)
return dev_get_drvdata(dev);
 }
 
+enum musb_mode musb_get_mode(struct device *dev)
+{
+   enum usb_dr_mode mode;
+
+   mode = usb_get_dr_mode(dev);
+   switch (mode) {
+   case USB_DR_MODE_HOST:
+   return MUSB_HOST;
+   case USB_DR_MODE_PERIPHERAL:
+   return MUSB_PERIPHERAL;
+   case USB_DR_MODE_OTG:
+   case USB_DR_MODE_UNKNOWN:
+   default:
+   return MUSB_OTG;
+   }
+}
+EXPORT_SYMBOL_GPL(musb_get_mode);
+
 /*-*/
 
 #ifndef CONFIG_BLACKFIN
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index fd215fb..3f7a325 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -614,4 +614,9 @@ static inline void musb_platform_post_root_reset_end(struct 
musb *musb)
musb->ops->post_root_reset_end(musb);
 }
 
+/* gets the "dr_mode" property from DT and converts it into musb_mode
+ * if the property is not found or not recognized returns MUSB_OTG
+ */
+extern enum musb_mode musb_get_mode(struct device *dev);
+
 #endif /* __MUSB_CORE_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 1/5 v9] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.

Signed-off-by: Petr Kulhavy 
---
v1: 

v2:
 - using standard properties "dr_mode", "mentor,power", "mentor,num-eps", 
"mentor,multipoint", "mentor,ram-bits"
 - using "ti," prefix instead of "da8xx," for specific property names
 - no wildcards in compatibility string

v3:
 - added "reg", "interrupts" and "interrupt-names" properties
 - wildcards in compatibility string

v4:
 - compatibility string set to "ti,da830-musb"
 - "mentor,num-eps", "mentor,multipoint", "mentor,ram-bits" properties removed 
and hardcoded
 - "ti,phy20-clkmux-cfg" renamed to "ti,phy20-clkmux-pll" and changed to boolean
 - removed "ti,hwmods"

v5:
 - "ti,phy20-refclock-frequency" property made mandatory

v6:
 - using "ti,usb2-phy-" prefix instead of "ti,phy20-" for the specific 
properties

v7:
 - removed the "mentor,power" property; hard coded to 500mA in the code

v8:
 - "ti,usb2-phy-refclock-frequency" renamed to "ti,usb2-phy-refclock-hz" and 
description amended
 - "ti,usb2-phy-clkmux-pll" changed to "ti,usb2-phy-clkmux-refclkin" to reflect 
the more common case
 - USB maximum power modelled via a regulator "vbus-supply"

v9: 

 .../devicetree/bindings/usb/da8xx-usb.txt  | 45 ++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt 
b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
new file mode 100644
index 000..a6eda5b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -0,0 +1,45 @@
+TI DA8xx MUSB
+~
+For DA8xx/OMAP-L1x/AM17xx/AM18xx platforms.
+
+Required properties:
+
+ - compatible : Should be set to "ti,da830-musb".
+
+ - reg: Offset and length of the USB controller register set.
+
+ - interrupts: The USB interrupt number.
+
+ - interrupt-names: Should be set to "mc".
+
+ - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or 
"otg".
+
+ - vbus-supply: Phandle to a regulator providing the USB bus power.
+
+ - ti,usb2-phy-refclock-hz : Integer. Frequency in Hz of the USB 2.0 PHY 
reference clock,
+ either provided by the internal PLL or an external source.
+ The supported values are: 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 
38.4MHz, 40MHz, 48MHz.
+
+
+Optional properties:
+
+ - ti,usb2-phy-clkmux-refclkin: Boolean. Defines the USB 2.0 PHY reference 
clock source.
+ If present the the external USB_REFCLKIN pin is used as a clock source, 
otherwise
+ the internal PLL is used.
+
+Example:
+
+   usb20: usb@1e0 {
+   compatible = "ti,da830-musb";
+   reg =   <0x0020 0x1>;
+   interrupt-parent = <>;
+   interrupts = <58>;
+   interrupt-names = "mc";
+
+   dr_mode = "host";
+   vbus-supply = <_vbus>;
+
+   ti,usb2-phy-refclock-hz = <2400>;
+
+   status = "okay";
+   };
-- 
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 1/1] dm9601: enable EP3 interrupt and enhance eeprom functions

[Peter Korsgaard]

> "Joseph" == Joseph CHANG  writes:

Hi Joseph,

 > Enable chip's EP3 interrupt to get the link-up notify soon
 > immediately.

 > Add to maintain variant eeprom adapters which may have not right
 > dm9620x's format.

 > Add function dm9601_set_eeprom which tested good with ethtool
 > utility.

Thanks for the patch. This sounds like 3 seperate changes, could you
please restructure it as 3 patches each doing one of the changes?

-- 
Bye, Peter Korsgaard
--
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/1] dm9601: enable EP3 interrupt and enhance eeprom functions

[Joseph CHANG]

Enable chip's EP3 interrupt to get the link-up notify soon
immediately.

Add to maintain variant eeprom adapters which may have not right
dm9620x's format.

Add function dm9601_set_eeprom which tested good with ethtool
utility.

Signed-off-by: Joseph CHANG 
---
 drivers/net/usb/dm9601.c | 127 +--
 1 file changed, 123 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index 0b4bdd3..3027a60 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -46,6 +46,9 @@
 #define DM_GPR_DATA0x1f
 #define DM_CHIP_ID 0x2c
 #define DM_MODE_CTRL   0x91/* only on dm9620 */
+#defineDM_USB_CTRL 0xf4
+
+#defineUSB_CTRL_EP3ACK 0x20
 
 /* chip id values */
 #define ID_DM9601  0
@@ -57,6 +60,9 @@
 #define DM_TX_OVERHEAD 2   /* 2 byte header */
 #define DM_RX_OVERHEAD 7   /* 3 byte header + 4 byte crc tail */
 #define DM_TIMEOUT 1000
+#defineDM_EP3I_VAL 0x07
+#defineDM_CONF_INC 126
+#defineMD96XX_EEPROM_MAGIC 0x9620
 
 static int dm_read(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
@@ -146,13 +152,14 @@ static int dm_read_shared_word(struct usbnet *dev, int 
phy, u8 reg, __le16 *valu
return ret;
 }
 
-static int dm_write_shared_word(struct usbnet *dev, int phy, u8 reg, __le16 
value)
+static int dm_write_shared_word(struct usbnet *dev, int phy, u8 reg,
+   __le16 *value)
 {
int ret, i;
 
mutex_lock(>phy_mutex);
 
-   ret = dm_write(dev, DM_SHARED_DATA, 2, );
+   ret = dm_write(dev, DM_SHARED_DATA, 2, value);
if (ret < 0)
goto out;
 
@@ -190,6 +197,77 @@ static int dm_read_eeprom_word(struct usbnet *dev, u8 
offset, void *value)
return dm_read_shared_word(dev, 0, offset, value);
 }
 
+static void dm_write_eeprom_word(struct usbnet *dev, u8 offset, void *data)
+{
+   dm_write_shared_word(dev, 0, offset, data);
+}
+
+static int dm_render_write(struct usbnet *dev, u8 wordoffset, u16 mask_word,
+  u16 orset_word, u16 *pmd)
+{
+   u16 srom;
+
+   dm_read_eeprom_word(dev, wordoffset, );
+   if ((srom & mask_word) == orset_word)
+   return 0;
+
+   srom &= ~mask_word;
+   srom |= orset_word;
+
+   dm_write_eeprom_word(dev, wordoffset, );
+   *pmd = srom;
+   return 1;
+}
+
+static void dm_render_report(struct usbnet *dev, u8 wordoffset, u16 md)
+{
+   int i = 0;
+   u16 srom;
+
+   do {
+   i++;
+   usleep_range(100, 200);
+   dm_read_eeprom_word(dev, wordoffset, );
+   if (i == 8)
+   break;
+   } while (srom != md);
+
+   if (srom == md) {
+   netdev_info(dev->net, "set eeprom word%d 0x%04x\n",
+   wordoffset, md);
+   } else {
+   netdev_info(dev->net, "warning - set eeprom word%d 0x%04x\n",
+   wordoffset, md);
+   netdev_info(dev->net, "warning - fail as eeprom word%d 
0x%04x\n",
+   wordoffset, srom);
+   }
+}
+
+static void dm_eeprom_render(struct usbnet *dev, u8 wordoffset,
+u16 orset_word, u16 mask_word)
+{
+   u16 m;
+
+   if (!dm_render_write(dev, wordoffset, mask_word, orset_word, ))
+   return;
+
+   dm_render_report(dev, wordoffset, m);
+}
+
+static void dm_render_begin(struct usbnet *dev)
+{
+   /* Render eeprom if need, WORD3 render, set D[15:14] 01b */
+   dm_eeprom_render(dev, 3, 0x4000, 0xc000);
+   /* Render eeprom if need, WORD7 render, clear D[10] */
+   dm_eeprom_render(dev, 7, 0x, 0x0400);
+   /* Render eeprom if need, WORD11 render, need 0x005a */
+   dm_eeprom_render(dev, 11, 0x005a, 0x);
+   /* Render eeprom if need, WORD12 render, need 0x0007 */
+   dm_eeprom_render(dev, 12, DM_EP3I_VAL, 0x);
+   /* Render eeprom if need, WORD15 render, need E.g.126 (0x007e) */
+   dm_eeprom_render(dev, 15, (u16)(DM_CONF_INC << 8), 0x);
+}
+
 
 
 static int dm9601_get_eeprom_len(struct net_device *dev)
@@ -216,6 +294,41 @@ static int dm9601_get_eeprom(struct net_device *net,
return 0;
 }
 
+static int dm9601_set_eeprom(struct net_device *net,
+struct ethtool_eeprom *eeprom, u8 *data)
+{
+   struct usbnet *dev = netdev_priv(net);
+   int offset = eeprom->offset;
+   int len = eeprom->len;
+   int done;
+
+   if (eeprom->magic != MD96XX_EEPROM_MAGIC) {
+   netdev_dbg(dev->net, "EEPROM: magic value mismatch, magic = 
0x%x",
+  eeprom->magic);
+   return -EINVAL;
+   }
+
+   while (len > 0) {
+   if (len & 1 || offset & 1) {
+   int which = offset & 1;
+   u8 tmp[2];
+
+