[PATCH v3 1/7] extcon: usb-gpio: add device binding for platform device

[Lu Baolu]

This is needed to handle the GPIO connected USB ID pin found on
Intel Baytrail devices.

Signed-off-by: Lu Baolu 
Reviewed-by: Felipe Balbi 
Acked-by: Chanwoo Choi 
---
 drivers/extcon/extcon-usb-gpio.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index 2b2fecf..af9c8b0 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -206,6 +206,12 @@ static const struct of_device_id usb_extcon_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, usb_extcon_dt_match);
 
+static const struct platform_device_id usb_extcon_platform_ids[] = {
+   { .name = "extcon-usb-gpio", },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, usb_extcon_platform_ids);
+
 static struct platform_driver usb_extcon_driver = {
.probe  = usb_extcon_probe,
.remove = usb_extcon_remove,
@@ -214,6 +220,7 @@ static struct platform_driver usb_extcon_driver = {
.pm = _extcon_pm_ops,
.of_match_table = usb_extcon_dt_match,
},
+   .id_table = usb_extcon_platform_ids,
 };
 
 module_platform_driver(usb_extcon_driver);
-- 
2.1.4

--
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 v3 5/7] usb: mux: add driver for Intel drcfg controlled port mux

[Lu Baolu]

Several Intel PCHs and SOCs have an internal mux that is used to
share one USB port between device controller and host controller.
The mux is handled through the Dual Role Configuration Register.

Signed-off-by: Heikki Krogerus 
Signed-off-by: Lu Baolu 
Signed-off-by: Wu Hao 
Reviewed-by: Felipe Balbi 
---
 MAINTAINERS   |   1 +
 drivers/usb/mux/Kconfig   |   7 ++
 drivers/usb/mux/Makefile  |   1 +
 drivers/usb/mux/intel-mux-drcfg.c | 161 ++
 4 files changed, 170 insertions(+)
 create mode 100644 drivers/usb/mux/intel-mux-drcfg.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ab876eb..399cefe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11407,6 +11407,7 @@ S:  Supported
 F: drivers/usb/mux/intel-mux.c
 F: include/linux/usb/intel-mux.h
 F: drivers/usb/mux/intel-mux-gpio.c
+F: drivers/usb/mux/intel-mux-drcfg.c
 
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index 060bf5c..775d5da 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -15,4 +15,11 @@ config INTEL_MUX_GPIO
  Say Y here to enable support for Intel dual role port mux
  controlled by GPIOs.
 
+config INTEL_MUX_DRCFG
+   tristate "Intel dual role port mux controlled by register"
+   depends on X86
+   select INTEL_USB_MUX
+   help
+ Say Y here to enable support for Intel dual role port mux
+ controlled by the Dual Role Configuration Register.
 endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index 66f765c..3179909 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -3,3 +3,4 @@
 #
 obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
 obj-$(CONFIG_INTEL_MUX_GPIO)   += intel-mux-gpio.o
+obj-$(CONFIG_INTEL_MUX_DRCFG)  += intel-mux-drcfg.o
diff --git a/drivers/usb/mux/intel-mux-drcfg.c 
b/drivers/usb/mux/intel-mux-drcfg.c
new file mode 100644
index 000..11db826
--- /dev/null
+++ b/drivers/usb/mux/intel-mux-drcfg.c
@@ -0,0 +1,161 @@
+/**
+ * intel-mux-drcfg.c - Driver for Intel USB mux via register
+ *
+ * Copyright (C) 2016 Intel Corporation
+ * Author: Heikki Krogerus 
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define INTEL_MUX_CFG0 0x00
+#define INTEL_MUX_CFG1 0x04
+#define CFG0_SW_IDPIN  BIT(20)
+#define CFG0_SW_IDPIN_EN   BIT(21)
+#define CFG0_SW_VBUS_VALID BIT(24)
+#define CFG1_SW_MODE   BIT(29)
+#define CFG1_POLL_TIMEOUT  1000
+
+struct intel_usb_mux {
+   struct intel_mux_dev umdev;
+   void __iomem *regs;
+};
+
+static inline int intel_mux_drcfg_switch(struct intel_mux_dev *umdev, bool 
host)
+{
+   struct intel_usb_mux *mux;
+   unsigned long timeout;
+   u32 data;
+
+   mux = container_of(umdev, struct intel_usb_mux, umdev);
+
+   /* Check and set mux to SW controlled mode */
+   data = readl(mux->regs + INTEL_MUX_CFG0);
+   if (!(data & CFG0_SW_IDPIN_EN)) {
+   data |= CFG0_SW_IDPIN_EN;
+   writel(data, mux->regs + INTEL_MUX_CFG0);
+   }
+
+   /*
+* Configure CFG0 to switch the mux and VBUS_VALID bit is
+* required for device mode.
+*/
+   data = readl(mux->regs + INTEL_MUX_CFG0);
+   if (host)
+   data &= ~(CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
+   else
+   data |= (CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
+   writel(data, mux->regs + INTEL_MUX_CFG0);
+
+   /*
+* Polling CFG1 for safety, most case it takes about 600ms
+* to finish mode switching, set TIMEOUT long enough.
+*/
+   timeout = jiffies + msecs_to_jiffies(CFG1_POLL_TIMEOUT);
+
+   /* Polling on CFG1 register to confirm mode switch. */
+   while (!time_after(jiffies, timeout)) {
+   data = readl(mux->regs + INTEL_MUX_CFG1);
+   if (!(host ^ (data & CFG1_SW_MODE)))
+   return 0;
+   /* interval for polling is set to about 5ms */
+   usleep_range(5000, 5100);
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int intel_mux_drcfg_cable_set(struct intel_mux_dev *umdev)
+{
+   dev_dbg(umdev->dev, "drcfg mux switch to HOST\n");
+
+   return intel_mux_drcfg_switch(umdev, true);
+}
+
+static int intel_mux_drcfg_cable_unset(struct intel_mux_dev *umdev)
+{
+   dev_dbg(umdev->dev, "drcfg mux switch to DEVICE\n");
+
+   return intel_mux_drcfg_switch(umdev, false);
+}
+
+static int intel_mux_drcfg_probe(struct 

[PATCH v3 4/7] usb: mux: add driver for Intel gpio controlled port mux

[Lu Baolu]

In some Intel platforms, a single usb port is shared between USB host
and device controller. The shared port is under control of GPIO pins.

This patch adds the support for USB GPIO controlled port mux.

Signed-off-by: David Cohen 
Signed-off-by: Lu Baolu 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Felipe Balbi 
Signed-off-by: Fengguang Wu 
---
 MAINTAINERS  |   1 +
 drivers/usb/mux/Kconfig  |   9 +++
 drivers/usb/mux/Makefile |   1 +
 drivers/usb/mux/intel-mux-gpio.c | 125 +++
 4 files changed, 136 insertions(+)
 create mode 100644 drivers/usb/mux/intel-mux-gpio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a93d26a..ab876eb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11406,6 +11406,7 @@ L:  linux-usb@vger.kernel.org
 S: Supported
 F: drivers/usb/mux/intel-mux.c
 F: include/linux/usb/intel-mux.h
+F: drivers/usb/mux/intel-mux-gpio.c
 
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index 8fedc4f..060bf5c 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -6,4 +6,13 @@ config INTEL_USB_MUX
select EXTCON
def_bool n
 
+config INTEL_MUX_GPIO
+   tristate "Intel dual role port mux controlled by GPIOs"
+   depends on GPIOLIB
+   depends on X86 && ACPI
+   select INTEL_USB_MUX
+   help
+ Say Y here to enable support for Intel dual role port mux
+ controlled by GPIOs.
+
 endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index 84f0ae8..66f765c 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -2,3 +2,4 @@
 # Makefile for USB port mux drivers
 #
 obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
+obj-$(CONFIG_INTEL_MUX_GPIO)   += intel-mux-gpio.o
diff --git a/drivers/usb/mux/intel-mux-gpio.c b/drivers/usb/mux/intel-mux-gpio.c
new file mode 100644
index 000..adcb496
--- /dev/null
+++ b/drivers/usb/mux/intel-mux-gpio.c
@@ -0,0 +1,125 @@
+/*
+ * USB Dual Role Port Mux driver controlled by gpios
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ * Author: David Cohen 
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct vuport {
+   struct intel_mux_dev umdev;
+   struct gpio_desc *gpio_vbus_en;
+   struct gpio_desc *gpio_usb_mux;
+};
+
+/*
+ * id == 0, HOST connected, USB port should be set to peripheral
+ * id == 1, HOST disconnected, USB port should be set to host
+ *
+ * Peripheral: set USB mux to peripheral and disable VBUS
+ * Host: set USB mux to host and enable VBUS
+ */
+static inline int vuport_set_port(struct intel_mux_dev *umdev, int id)
+{
+   struct vuport *vup = container_of(umdev, struct vuport, umdev);
+
+   dev_dbg(umdev->dev, "USB PORT ID: %s\n", id ? "HOST" : "PERIPHERAL");
+
+   gpiod_set_value_cansleep(vup->gpio_usb_mux, !id);
+   gpiod_set_value_cansleep(vup->gpio_vbus_en, id);
+
+   return 0;
+}
+
+static int vuport_cable_set(struct intel_mux_dev *umdev)
+{
+   return vuport_set_port(umdev, 1);
+}
+
+static int vuport_cable_unset(struct intel_mux_dev *umdev)
+{
+   return vuport_set_port(umdev, 0);
+}
+
+static int vuport_probe(struct platform_device *pdev)
+{
+   struct intel_mux_dev *umdev;
+   struct device *dev = >dev;
+   struct vuport *vup;
+
+   vup = devm_kzalloc(dev, sizeof(*vup), GFP_KERNEL);
+   if (!vup)
+   return -ENOMEM;
+
+   /* retrieve vbus and mux gpios */
+   vup->gpio_vbus_en = devm_gpiod_get_optional(dev,
+   "vbus_en", GPIOD_ASIS);
+   if (IS_ERR(vup->gpio_vbus_en))
+   return PTR_ERR(vup->gpio_vbus_en);
+
+   vup->gpio_usb_mux = devm_gpiod_get_optional(dev,
+   "usb_mux", GPIOD_ASIS);
+   if (IS_ERR(vup->gpio_usb_mux))
+   return PTR_ERR(vup->gpio_usb_mux);
+
+   /* populate the mux generic structure */
+   umdev = >umdev;
+   umdev->dev = dev;
+   umdev->cable_name = "USB-HOST";
+   umdev->cable_set_cb = vuport_cable_set;
+   umdev->cable_unset_cb = vuport_cable_unset;
+
+   return intel_usb_mux_register(umdev);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+   return intel_usb_mux_unregister(>dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * In case a micro A cable was plugged in while device was sleeping,
+ * we missed the interrupt. We need to poll usb id gpio when waking the
+ * driver to detect the missed event.
+ * We use 'complete' callback to 

[PATCH v3 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Lu Baolu]

In some Intel platforms, a single usb port is shared between USB host
and device controllers. The shared port is under control of a switch
which is defined in the Intel vendor defined extended capability for
xHCI.

This patch adds the support to detect and create the platform device
for the port mux switch.

Signed-off-by: Lu Baolu 
Reviewed-by: Felipe Balbi 
---
 drivers/usb/host/pci-quirks.c| 47 ++--
 drivers/usb/host/xhci-ext-caps.h |  2 ++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 35af362..6a737cf 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,10 +16,11 @@
 #include 
 #include 
 #include 
+#include 
+
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
-
 #define UHCI_USBLEGSUP 0xc0/* legacy support */
 #define UHCI_USBCMD0   /* command register */
 #define UHCI_USBINTR   4   /* interrupt register */
@@ -78,6 +79,9 @@
 #define USB_INTEL_USB3_PSSEN   0xD8
 #define USB_INTEL_USB3PRM  0xDC
 
+#define DEVICE_ID_INTEL_CHERRYVIEW_XHCI0x22b5
+#define DEVICE_ID_INTEL_BROXTON_M_XHCI 0x0aa8
+
 /*
  * amd_chipset_gen values represent AMD different chipset generations
  */
@@ -956,6 +960,41 @@ void usb_disable_xhci_ports(struct pci_dev *xhci_pdev)
 }
 EXPORT_SYMBOL_GPL(usb_disable_xhci_ports);
 
+static void create_intel_usb_mux_device(struct pci_dev *xhci_pdev,
+   void __iomem *base)
+{
+   struct platform_device *plat_dev;
+   struct property_set pset;
+   int ret;
+
+   struct property_entry pentry[] = {
+   PROPERTY_ENTRY_U64("reg-start",
+   pci_resource_start(xhci_pdev, 0) + 0x80d8),
+   PROPERTY_ENTRY_U64("reg-size", 8),
+   { },
+   };
+
+   if (!xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_INTEL_USB_MUX))
+   return;
+
+   plat_dev = platform_device_alloc("intel-mux-drcfg",
+   PLATFORM_DEVID_AUTO);
+   if (!plat_dev)
+   return;
+
+   plat_dev->dev.parent = _pdev->dev;
+   pset.properties = pentry;
+   platform_device_add_properties(plat_dev, );
+
+   ret = platform_device_add(plat_dev);
+   if (ret) {
+   dev_warn(_pdev->dev,
+   "failed to create mux device with error %d",
+   ret);
+   platform_device_put(plat_dev);
+   }
+}
+
 /**
  * PCI Quirks for xHCI.
  *
@@ -1022,8 +1061,12 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
 
 hc_init:
-   if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
usb_enable_intel_xhci_ports(pdev);
+   if (pdev->device == DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+   pdev->device == DEVICE_ID_INTEL_BROXTON_M_XHCI)
+   create_intel_usb_mux_device(pdev, base);
+   }
 
op_reg_base = base + XHCI_HC_LENGTH(readl(base));
 
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index e0244fb..e368ccb 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -51,6 +51,8 @@
 #define XHCI_EXT_CAPS_ROUTE5
 /* IDs 6-9 reserved */
 #define XHCI_EXT_CAPS_DEBUG10
+/* Vendor defined 192-255 */
+#define XHCI_EXT_CAPS_INTEL_USB_MUX192
 /* USB Legacy Support Capability - section 7.1.1 */
 #define XHCI_HC_BIOS_OWNED (1 << 16)
 #define XHCI_HC_OS_OWNED   (1 << 24)
-- 
2.1.4

--
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 v3 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver

[Lu Baolu]

Some Intel platforms have an USB port mux controlled by GPIOs.
There's a single ACPI platform device that provides both USB ID
extcon device and a USB port mux device. This MFD driver will
split the 2 devices for their respective drivers.

Signed-off-by: Lu Baolu 
Suggested-by: David Cohen 
Reviewed-by: Felipe Balbi 
Signed-off-by: Fengguang Wu 
---
 MAINTAINERS|  6 
 drivers/mfd/Kconfig|  8 +
 drivers/mfd/Makefile   |  1 +
 drivers/mfd/intel-vuport.c | 74 ++
 4 files changed, 89 insertions(+)
 create mode 100644 drivers/mfd/intel-vuport.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 399cefe..1486c80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5599,6 +5599,12 @@ L:   linux...@vger.kernel.org
 S: Supported
 F: drivers/cpufreq/intel_pstate.c
 
+INTEL VIRTUAL USB PORT DRIVER
+M: Lu Baolu 
+L: linux-usb@vger.kernel.org
+S: Supported
+F: drivers/mfd/intel-vuport.c
+
 INTEL FRAMEBUFFER DRIVER (excluding 810 and 815)
 M: Maik Broemme 
 L: linux-fb...@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9ca66de..48933d4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1534,5 +1534,13 @@ config MFD_VEXPRESS_SYSREG
  System Registers are the platform configuration block
  on the ARM Ltd. Versatile Express board.
 
+config MFD_INTEL_VUPORT
+   tristate "Intel virtual USB port controller"
+   select MFD_CORE
+   depends on X86 && ACPI
+   help
+ Say Y here to enable support for Intel's dual role port mux
+ controlled by GPIOs.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0f230a6..0ccd107 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -198,3 +198,4 @@ intel-soc-pmic-objs := intel_soc_pmic_core.o 
intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)   += intel-soc-pmic.o
 obj-$(CONFIG_MFD_MT6397)   += mt6397-core.o
+obj-$(CONFIG_MFD_INTEL_VUPORT) += intel-vuport.o
diff --git a/drivers/mfd/intel-vuport.c b/drivers/mfd/intel-vuport.c
new file mode 100644
index 000..a07920f
--- /dev/null
+++ b/drivers/mfd/intel-vuport.c
@@ -0,0 +1,74 @@
+/*
+ * MFD driver for Intel virtual USB port
+ *
+ * Copyright(c) 2016 Intel Corporation.
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* ACPI GPIO Mappings */
+static const struct acpi_gpio_params id_gpio = { 0, 0, false };
+static const struct acpi_gpio_params vbus_gpio = { 1, 0, false };
+static const struct acpi_gpio_params mux_gpio = { 2, 0, false };
+static const struct acpi_gpio_mapping acpi_usb_gpios[] = {
+   { "id-gpios", _gpio, 1 },
+   { "vbus_en-gpios", _gpio, 1 },
+   { "usb_mux-gpios", _gpio, 1 },
+   { },
+};
+
+static const struct mfd_cell intel_vuport_mfd_cells[] = {
+   { .name = "extcon-usb-gpio", },
+   { .name = "intel-mux-gpio", },
+};
+
+static int vuport_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   int ret;
+
+   ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), acpi_usb_gpios);
+   if (ret)
+   return ret;
+
+   return mfd_add_devices(>dev, 0, intel_vuport_mfd_cells,
+   ARRAY_SIZE(intel_vuport_mfd_cells), NULL, 0,
+   NULL);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+   mfd_remove_devices(>dev);
+   acpi_dev_remove_driver_gpios(ACPI_COMPANION(>dev));
+
+   return 0;
+}
+
+static struct acpi_device_id vuport_acpi_match[] = {
+   { "INT3496" },
+   { }
+};
+MODULE_DEVICE_TABLE(acpi, vuport_acpi_match);
+
+static struct platform_driver vuport_driver = {
+   .driver = {
+   .name = "intel-vuport",
+   .acpi_match_table = ACPI_PTR(vuport_acpi_match),
+   },
+   .probe = vuport_probe,
+   .remove = vuport_remove,
+};
+
+module_platform_driver(vuport_driver);
+
+MODULE_AUTHOR("Lu Baolu ");
+MODULE_DESCRIPTION("Intel virtual USB port");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

--
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 v3 3/7] usb: mux: add common code for Intel dual role port mux

[Lu Baolu]

Several Intel PCHs and SOCs have an internal mux that is used to
share one USB port between device controller and host controller.

A usb port mux could be abstracted as the following elements:
1) mux state: HOST or PERIPHERAL;
2) an extcon cable which triggers the change of mux state between
   HOST and PERIPHERAL;
3) The required action to do the real port switch.

This patch adds the common code to handle usb port mux. With this
common code, the individual mux driver, which always is platform
dependent, could focus on the real operation of mux switch.

Signed-off-by: Lu Baolu 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Felipe Balbi 
---
 Documentation/ABI/testing/sysfs-bus-platform |  15 +++
 MAINTAINERS  |   7 ++
 drivers/usb/Kconfig  |   2 +
 drivers/usb/Makefile |   1 +
 drivers/usb/mux/Kconfig  |   9 ++
 drivers/usb/mux/Makefile |   4 +
 drivers/usb/mux/intel-mux.c  | 166 +++
 include/linux/usb/intel-mux.h|  47 
 8 files changed, 251 insertions(+)
 create mode 100644 drivers/usb/mux/Kconfig
 create mode 100644 drivers/usb/mux/Makefile
 create mode 100644 drivers/usb/mux/intel-mux.c
 create mode 100644 include/linux/usb/intel-mux.h

diff --git a/Documentation/ABI/testing/sysfs-bus-platform 
b/Documentation/ABI/testing/sysfs-bus-platform
index 5172a61..a2261cb 100644
--- a/Documentation/ABI/testing/sysfs-bus-platform
+++ b/Documentation/ABI/testing/sysfs-bus-platform
@@ -18,3 +18,18 @@ Description:
devices to opt-out of driver binding using a driver_override
name such as "none".  Only a single driver may be specified in
the override, there is no support for parsing delimiters.
+
+What:  /sys/bus/platform/devices/.../intel_mux
+Date:  Febuary 2016
+Contact:   Lu Baolu 
+Description:
+   In some platforms, a single USB port is shared between a USB 
host
+   controller and a device controller. A USB mux driver is needed 
to
+   handle the port mux. intel_mux attribute shows and stores the 
mux
+   state.
+   For read:
+   'peripheral' - mux switched to PERIPHERAL controller;
+   'host'   - mux switched to HOST controller.
+   For write:
+   'peripheral' - mux will be switched to PERIPHERAL controller;
+   'host'   - mux will be switched to HOST controller.
diff --git a/MAINTAINERS b/MAINTAINERS
index c55b37e..a93d26a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11400,6 +11400,13 @@ T: git 
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
 S: Maintained
 F: drivers/usb/phy/
 
+USB PORT MUX DRIVER
+M: Lu Baolu 
+L: linux-usb@vger.kernel.org
+S: Supported
+F: drivers/usb/mux/intel-mux.c
+F: include/linux/usb/intel-mux.h
+
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451d..dbd6620 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -149,6 +149,8 @@ endif # USB
 
 source "drivers/usb/phy/Kconfig"
 
+source "drivers/usb/mux/Kconfig"
+
 source "drivers/usb/gadget/Kconfig"
 
 config USB_LED_TRIG
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..9a92338 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -6,6 +6,7 @@
 
 obj-$(CONFIG_USB)  += core/
 obj-$(CONFIG_USB_SUPPORT)  += phy/
+obj-$(CONFIG_USB_SUPPORT)  += mux/
 
 obj-$(CONFIG_USB_DWC3) += dwc3/
 obj-$(CONFIG_USB_DWC2) += dwc2/
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
new file mode 100644
index 000..8fedc4f
--- /dev/null
+++ b/drivers/usb/mux/Kconfig
@@ -0,0 +1,9 @@
+#
+# USB port mux driver configuration
+#
+menu "USB Port MUX drivers"
+config INTEL_USB_MUX
+   select EXTCON
+   def_bool n
+
+endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
new file mode 100644
index 000..84f0ae8
--- /dev/null
+++ b/drivers/usb/mux/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for USB port mux drivers
+#
+obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
diff --git a/drivers/usb/mux/intel-mux.c b/drivers/usb/mux/intel-mux.c
new file mode 100644
index 000..b243758
--- /dev/null
+++ b/drivers/usb/mux/intel-mux.c
@@ -0,0 +1,166 @@
+/**
+ * mux.c - USB Port Mux support
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include 

[PATCH v3 0/7] usb: add support for Intel dual role port mux

[Lu Baolu]

Intel SOC chips are featured with USB dual role. The host role is
provided by Intel xHCI IP, and the gadget role is provided by IP
from designware. Tablet platform designs always share a single
port for both host and gadget controllers. There is a mux to
switch the port to the right controller according to the cable
type. OS needs to provide the callback to control the mux when
a plug-in event raises. The method to control the mux is platform
dependent. At least three types of implementation can be found
across current devices. 1) GPIO pins; 2) a unit which can be
controlled by memory mapped registers; 3) ACPI ASL code.

This patch series adds supports for Intel dual role port mux.
It includes:
(1) A helper layer on top of extcon for individual mux driver.
It listens to the USB-HOST extcon cable and call the switch
call-back when the cable state changes.
(2) Drivers for GPIO controlled port mux which could be found on
Baytrail devices. A mfd driver is used to split the GPIOs into
USB gpio extcon device and a USB mux device. Driver for USB
gpio extcon device is already in upstream Linux. This patch
series includes a driver for GPIO USB mux.
(3) Drivers for USB port mux controlled through memory mapped
registers and the logic to create the mux device. This type
of dual role port mux could be found in Cherry Trail and
Broxton devices.

Lu Baolu (7):
  extcon: usb-gpio: add device binding for platform device
  extcon: usb-gpio: add support for ACPI gpio interface
  usb: mux: add common code for Intel dual role port mux
  usb: mux: add driver for Intel gpio controlled port mux
  usb: mux: add driver for Intel drcfg controlled port mux
  usb: pci-quirks: add Intel USB drcfg mux device
  mfd: intel_vuport: Add Intel virtual USB port MFD Driver

 Documentation/ABI/testing/sysfs-bus-platform |  15 +++
 MAINTAINERS  |  15 +++
 drivers/extcon/extcon-usb-gpio.c |  10 +-
 drivers/mfd/Kconfig  |   8 ++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/intel-vuport.c   |  74 
 drivers/usb/Kconfig  |   2 +
 drivers/usb/Makefile |   1 +
 drivers/usb/host/pci-quirks.c|  47 +++-
 drivers/usb/host/xhci-ext-caps.h |   2 +
 drivers/usb/mux/Kconfig  |  25 
 drivers/usb/mux/Makefile |   6 +
 drivers/usb/mux/intel-mux-drcfg.c| 161 ++
 drivers/usb/mux/intel-mux-gpio.c | 125 
 drivers/usb/mux/intel-mux.c  | 166 +++
 include/linux/usb/intel-mux.h|  47 
 16 files changed, 702 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mfd/intel-vuport.c
 create mode 100644 drivers/usb/mux/Kconfig
 create mode 100644 drivers/usb/mux/Makefile
 create mode 100644 drivers/usb/mux/intel-mux-drcfg.c
 create mode 100644 drivers/usb/mux/intel-mux-gpio.c
 create mode 100644 drivers/usb/mux/intel-mux.c
 create mode 100644 include/linux/usb/intel-mux.h

Change log:
v2->v3:
 - uvport mfd driver got reviewed by Lee Jones, the following
   changes were made accordingly.
 - seperate uvport driver from the mux drivers in MAINTAINERS file
 - refine the description in Kconfig
 - refine the mfd_cell structure data

v1->v2:
 - move mux driver from drivers/usb/misc to drivers/usb/mux;
 - replace debugfs with sysfs for user level mux control;
 - remove unnecessary register restore if mux registeration failed;
 - Add "Acked-by: Chanwoo Choi " to extcon changes;
 - Make the file names and exported function names more specific;
 - Remove the usb_mux_get_dev() interface;
 - Move "struct intel_usb_mux" from .h to .c file;
 - Fix various kbuild robot warnings.
-- 
2.1.4

--
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 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver

[Lu Baolu]

On 03/08/2016 12:40 PM, Lee Jones wrote:
> On Mon, 07 Mar 2016, Lu Baolu wrote:
>
>> Some Intel platforms have an USB port mux controlled by GPIOs.
>> There's a single ACPI platform device that provides both USB ID
>> extcon device and a USB port mux device. This MFD driver will
>> split the 2 devices for their respective drivers.
>>
>> Signed-off-by: Lu Baolu 
>> Suggested-by: David Cohen 
>> Reviewed-by: Felipe Balbi 
>> Signed-off-by: Fengguang Wu 
>> ---
>>  MAINTAINERS|  1 +
>>  drivers/mfd/Kconfig|  8 +
>>  drivers/mfd/Makefile   |  1 +
>>  drivers/mfd/intel-vuport.c | 78 
>> ++
>>  4 files changed, 88 insertions(+)
>>  create mode 100644 drivers/mfd/intel-vuport.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 399cefe..1671a4d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11408,6 +11408,7 @@ F:   drivers/usb/mux/intel-mux.c
>>  F:  include/linux/usb/intel-mux.h
>>  F:  drivers/usb/mux/intel-mux-gpio.c
>>  F:  drivers/usb/mux/intel-mux-drcfg.c
>> +F:  drivers/mfd/intel-vuport.c
> Separate patch please.

Sure.

>
>>  USB PRINTER DRIVER (usblp)
>>  M:  Pete Zaitcev 
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 9ca66de..8dd1bf3 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -1534,5 +1534,13 @@ config MFD_VEXPRESS_SYSREG
>>System Registers are the platform configuration block
>>on the ARM Ltd. Versatile Express board.
>>  
>> +config MFD_INTEL_VUPORT
>> +tristate "Intel virtual USB port controller"
>> +select MFD_CORE
>> +depends on X86 && ACPI
>> +help
>> +  Say Y here to enable support for Intel dual role port mux
> Either "Intel's" or "the Intel".

I will change it to "Intel's".

>
>> +  controlled by 3 GPIOs.
>> +
>>  endmenu
>>  endif
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 0f230a6..0ccd107 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -198,3 +198,4 @@ intel-soc-pmic-objs  := 
>> intel_soc_pmic_core.o intel_soc_pmic_crc.o
>>  intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)  += intel_soc_pmic_bxtwc.o
>>  obj-$(CONFIG_INTEL_SOC_PMIC)+= intel-soc-pmic.o
>>  obj-$(CONFIG_MFD_MT6397)+= mt6397-core.o
>> +obj-$(CONFIG_MFD_INTEL_VUPORT)  += intel-vuport.o
>> diff --git a/drivers/mfd/intel-vuport.c b/drivers/mfd/intel-vuport.c
>> new file mode 100644
>> index 000..1371099
>> --- /dev/null
>> +++ b/drivers/mfd/intel-vuport.c
>> @@ -0,0 +1,78 @@
>> +/*
>> + * MFD driver for Intel virtual USB port
>> + *
>> + * Copyright(c) 2016 Intel Corporation.
>> + * Author: Lu Baolu 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* ACPI GPIO Mappings */
>> +static const struct acpi_gpio_params id_gpio = { 0, 0, false };
>> +static const struct acpi_gpio_params vbus_gpio = { 1, 0, false };
>> +static const struct acpi_gpio_params mux_gpio = { 2, 0, false };
>> +static const struct acpi_gpio_mapping acpi_usb_gpios[] = {
>> +{ "id-gpios", _gpio, 1 },
>> +{ "vbus_en-gpios", _gpio, 1 },
>> +{ "usb_mux-gpios", _gpio, 1 },
>> +{ },
>> +};
>> +
>> +static const struct mfd_cell intel_vuport_mfd_cells[] = {
>> +{
>> +.name = "extcon-usb-gpio",
>> +},
>> +{
>> +.name = "intel-mux-gpio",
>> +},
>> +};
> Please place single line entries on the same line as the '{' and '}'.
>
> As you have above.

Sure.

>
>> +static int vuport_probe(struct platform_device *pdev)
>> +{
>> +struct device *dev = >dev;
>> +int ret;
>> +
>> +ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), acpi_usb_gpios);
>> +if (ret)
>> +return ret;
>> +
>> +return mfd_add_devices(>dev, 0, intel_vuport_mfd_cells,
>> +ARRAY_SIZE(intel_vuport_mfd_cells), NULL, 0,
>> +NULL);
>> +}
>> +
>> +static int vuport_remove(struct platform_device *pdev)
>> +{
>> +mfd_remove_devices(>dev);
>> +acpi_dev_remove_driver_gpios(ACPI_COMPANION(>dev));
>> +
>> +return 0;
>> +}
>> +
>> +static struct acpi_device_id vuport_acpi_match[] = {
>> +{ "INT3496" },
>> +{ }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, vuport_acpi_match);
>> +
>> +static struct platform_driver vuport_driver = {
>> +.driver = {
>> +.name = "intel-vuport",
>> +.acpi_match_table = ACPI_PTR(vuport_acpi_match),
>> +},
>> +.probe = vuport_probe,
>> +.remove = vuport_remove,
>> +};
>> +
>> +module_platform_driver(vuport_driver);
>> +
>> +MODULE_AUTHOR("Lu Baolu ");
>> 

Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

[Felipe Balbi]

Hi,

Felipe Ferreri Tonello  writes:
>>> On March 5, 2016 4:28:45 PM GMT+00:00, Michal Nazarewicz 
>>>  wrote:
>> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>>> @@ -16,7 +16,7 @@
>>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>>   *   Ben Williamson 
>>>   *
>>> - * Licensed under the GPL-2 or later.
>>> + * Licensed under the GPLv2.

> On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
  wrote:
>> Any particular reason to do that?

 On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
> Because the kernel is v2 only and not later. 

 Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
 parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
 copyright noticed clear unless you explicitly want your contribution be
 GPLv2 only which brings the whole file GPLv2 only.

> I just tried to make this driver more consistent with the coding
 style
> used across the kernel. That's it.

 Column alignment of field names or RHS of assignment operators is quite
 inconsistent already within drivers/usb/gadget/ which is why I’m
 concerned whether this is really helping.

 Anyway, I actually don’t care much, just adding my two rappen.
>>>
>>> Right, I am ok with Balbi completely ignoring this patch. But I prefer
>>> to have at least this driver consistent than nothing. Of course I'll
>>> remove the license change I made.
>> 
>> consistent in what way ?
>
> Source-code.
>
> The goal of this patch is to update this driver coding style to promote
> consistency, readability, and maintainability based on the Linux coding
> style.
>
> If this patch does not achieving that or if that is not necessary, than
> just ignore this patch.

yeah, I don't think that's what you're doing here.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes

[Felipe Balbi]

Hi,

Felipe Ferreri Tonello  writes:
>>> @@ -63,6 +63,14 @@ static unsigned int out_ports = 1;
>>>  module_param(out_ports, uint, S_IRUGO);
>>>  MODULE_PARM_DESC(out_ports, "Number of MIDI output ports");
>>>  
>>> +static unsigned int bmAttributes = USB_CONFIG_ATT_ONE;
>>> +module_param(bmAttributes, uint, S_IRUGO);
>>> +MODULE_PARM_DESC(bmAttributes, "Configuration Descriptor's
>> bmAttributes parameter");
>>> +
>>> +static unsigned int MaxPower = CONFIG_USB_GADGET_VBUS_DRAW;
>>> +module_param(MaxPower, uint, S_IRUGO);
>>> +MODULE_PARM_DESC(MaxPower, "Used to calculate Configuration
>> Descriptor's bMaxPower parameter");
>>
>> you didn't run checkpatch, did you ? Also, are you sure you will need
>> to
>> change this by simply reloading the module ? I'm not convinced.
>
> Yes I always run checkpatch :)

 do you really ? Why do you have a 98-character line, then ?

btw, you didn't reply this ^^

>>> @@ -119,8 +127,8 @@ static struct usb_configuration midi_config = {
>>> .label  = "MIDI Gadget",
>>> .bConfigurationValue = 1,
>>> /* .iConfiguration = DYNAMIC */
>>> -   .bmAttributes   = USB_CONFIG_ATT_ONE,
>>
>> nack, nackety nack nack nack :-)
>>
>> USB_CONFIG_ATT_ONE *must* always be set. With your module parameter you
>> give users the oportunity to violate USB spec.
>
> You are right. It needs to check the value before it assigns to
> bmAttributes.

 why check ? You can just unconditionally or USB_CONFIG_ATT_ONE. In any
 case, I'm not convinced this is necessary at all.
>>>
>>> Right.
>>>
>>> This is necessary because this driver is actually wrong in which is
>>> asking for the host to power itself. This is not specified on USB-MIDI
>>> specification, neither makes any sense since this configuration is
>>> device specific.
>>>
>>> What is your suggestion to make it configurable? Maybe at compile-time?
>>> I really don't know what is the best solution if this is not something
>>> you like it.
>> 
>> well, you could use our configfs-based gadget interface. You don't
>> really need to use gmidi.ko at all. In fact, we wanna do away with any
>> static modules and rely only on configfs. If configfs doesn't let you
>> change what you want/need, then we can talk about adding support for
>> those.
>> 
>> bMaxPower and bmAttributes sound like good things to have configurable
>> over configfs but beware of what the USB specification says about them,
>> we cannot let users violate the spec by passing bogus values on these
>> fields.
>
> I agree that we should move to configfs, but the truth is that these
> legacy devices are still useful. They just do one thing, mostly, but

yes, they are useful as they are. They don't need to be changed to be
useful. Plus, you can have a gadget built with configfs that does only
one thing. And you can do that with a simple shell script.

> its easy and simple to setup and use. So I think before we have some

so is configfs.

> sort of preset library of configfs-based gadget drivers, we still need
> these modules.

there is already a library called libusbg.

> Any suggestions on that?
>
> Do you want to support what I am proposing for gmidi.ko or just ignore
> it and move on to configfs?

I prefer to not touch these gadget drivers if at all necessary. If you
fixing a bug, then sure we must fix bugs. But you're not fixing a bug
and, on top of that, you're adding regressions and violating the USB
spec. This shows that you're writing these patches without knowing
(and/or even caring about) the specification at all.

Here's some enlightening presentation you probably wanna watch:

https://www.youtube.com/watch?v=fMeH7wqOwXA

TL;DR : this project is large and you need to convince me we need your
code/patch.

-- 
balbi


signature.asc
Description: PGP signature

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

[Felipe Balbi]

Hi,

Felipe Ferreri Tonello  writes:
> Since f_midi_transmit is called by both ALSA and USB frameworks, it
 can
> potentially cause a race condition between both calls. This is bad
 because the
> way f_midi_transmit is implemented can't handle concurrent calls.
 This is due
> to the fact that the usb request fifo looks for the next element and
 only if
> it has data to process it enqueues the request, otherwise re-uses it.
 If both
> (ALSA and USB) frameworks calls this function at the same time, the
> 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.
>
> On benchmarks realized by me, spinlocks were more efficient then
 scheduling
> the f_midi_transmit tasklet in process context and using a mutex
> to synchronize. Also it performs better then previous
> implementation
 that
> allocated a usb_request for every new transmit made.

 behaves better in what way ? Also, previous implementation would not
 suffer from this concurrency problem, right ?
>>>
>>> The spin lock is faster than allocating usb requests all the time,
>>> even if the udc uses da for it.
>> 
>> did you measure ? Is the extra speed really necessary ? How did you
>> benchmark this ?
>
> Yes I did measure and it was not that significant. This is not about
> speed. There was a bug in that approach that I already explained on

you have very confusing statements. When I mentioned that previous code
wouldn't have the need for the spinlock you replied that spinlock was
faster.

When I asked you about benchmarks you reply saying it's not about the
speed.

Make up your mind dude. What are you trying to achieve ?

> that patch, which was approved and applied BTW.

patches can be reverted if we realise we're better off without
them. Don't get cocky, please.

> Any way, this spinlock should've been there since that patch but I
> couldn't really trigger this problem without a stress test.

which tells me you sent me patches without properly testing. How much
time did it take to trigger this ? How did you trigger this situation ?

> So, this patch fixes a bug in the current implementation.

fixes a regression introduced by you, true. I'm trying to figure out if
we're better off without the original patch; to make a good decision I
need to know if the extra "speed" we get from not allocating requests on
demand are really that important.

So, how much faster did you get and is that extra "speed" really
important ?

-- 
balbi


signature.asc
Description: PGP signature

Re: USB host port {,performance} issues with Beaglebone

[Felipe Balbi]

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.

I'm adding Bin (current MUSB maintainer) to see if he has anything to
say.

ps: 4.1 is still too old, can you try v4.4 or v4.5-rc6 ?

> logging in to the target, the network connectivity fails, I after some
> time I got:
>
> [   92.176028] udevd[98]: worker [803] /devices/platform/hos
> t0/session1/target0:0:0/0:0:0:0/block/sda/sda1 timeout; kill it
> [   92.190702] udevd[98]: seq 1480
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda1'
> killed
> [   92.202237] udevd[98]: worker [805]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a2
> timeout; kill it
> [   92.216125] udevd[98]: seq 1481
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda2'
> killed
> [   92.226088] udevd[98]: worker [806]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a3
> timeout; kill it
> [   92.240727] udevd[98]: seq 1482
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda3'
> killed
> [   92.252076] udevd[98]: worker [807]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a4
> timeout; kill it
> [   92.265958] udevd[98]: seq 1483
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda4'
> killed
> [   92.276066] udevd[98]: worker [811]
> /devices/platform/host0/session1/target0:0:0/0:0:0:1/block/sdb ti
> meout; kill it
> [   92.290420] udevd[98]: seq 1477
> '/devices/platform/host0/session1/target0:0:0/0:0:0:1/block/sdb' kill
> ed
> [   92.299911] udevd[98]: worker [813]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a5
> timeout; kill it
> [   92.314794] udevd[98]: seq 1484
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda5'
> killed
> [   92.326211] udevd[98]: worker [814]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a6
> timeout; kill it
> [   92.340026] udevd[98]: seq 1485
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda6'
> killed
> [   92.350116] udevd[98]: worker [815]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a7
> timeout; kill it
> [   92.364315] udevd[98]: seq 1486
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda7'
> killed
> [   92.374220] udevd[98]: worker [816]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a8
> timeout; kill it
> [   92.388688] udevd[98]: seq 1487
> '/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda8'
> killed
> [   92.399953] udevd[98]: worker [817]
> /devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a9
> timeout; kill it
> [   92.413449] udevd[98]: seq 1488
> 

[PATCH 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)

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 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 heppen in rare cases when DMAC is used.

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 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.

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

Re: [PATCH v2 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver

[Lee Jones]

On Mon, 07 Mar 2016, Lu Baolu wrote:

> Some Intel platforms have an USB port mux controlled by GPIOs.
> There's a single ACPI platform device that provides both USB ID
> extcon device and a USB port mux device. This MFD driver will
> split the 2 devices for their respective drivers.
> 
> Signed-off-by: Lu Baolu 
> Suggested-by: David Cohen 
> Reviewed-by: Felipe Balbi 
> Signed-off-by: Fengguang Wu 
> ---
>  MAINTAINERS|  1 +
>  drivers/mfd/Kconfig|  8 +
>  drivers/mfd/Makefile   |  1 +
>  drivers/mfd/intel-vuport.c | 78 
> ++
>  4 files changed, 88 insertions(+)
>  create mode 100644 drivers/mfd/intel-vuport.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 399cefe..1671a4d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11408,6 +11408,7 @@ F:drivers/usb/mux/intel-mux.c
>  F:   include/linux/usb/intel-mux.h
>  F:   drivers/usb/mux/intel-mux-gpio.c
>  F:   drivers/usb/mux/intel-mux-drcfg.c
> +F:   drivers/mfd/intel-vuport.c

Separate patch please.

>  USB PRINTER DRIVER (usblp)
>  M:   Pete Zaitcev 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 9ca66de..8dd1bf3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1534,5 +1534,13 @@ config MFD_VEXPRESS_SYSREG
> System Registers are the platform configuration block
> on the ARM Ltd. Versatile Express board.
>  
> +config MFD_INTEL_VUPORT
> + tristate "Intel virtual USB port controller"
> + select MFD_CORE
> + depends on X86 && ACPI
> + help
> +   Say Y here to enable support for Intel dual role port mux

Either "Intel's" or "the Intel".

> +   controlled by 3 GPIOs.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 0f230a6..0ccd107 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -198,3 +198,4 @@ intel-soc-pmic-objs   := 
> intel_soc_pmic_core.o intel_soc_pmic_crc.o
>  intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)   += intel_soc_pmic_bxtwc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
>  obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
> +obj-$(CONFIG_MFD_INTEL_VUPORT)   += intel-vuport.o
> diff --git a/drivers/mfd/intel-vuport.c b/drivers/mfd/intel-vuport.c
> new file mode 100644
> index 000..1371099
> --- /dev/null
> +++ b/drivers/mfd/intel-vuport.c
> @@ -0,0 +1,78 @@
> +/*
> + * MFD driver for Intel virtual USB port
> + *
> + * Copyright(c) 2016 Intel Corporation.
> + * Author: Lu Baolu 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* ACPI GPIO Mappings */
> +static const struct acpi_gpio_params id_gpio = { 0, 0, false };
> +static const struct acpi_gpio_params vbus_gpio = { 1, 0, false };
> +static const struct acpi_gpio_params mux_gpio = { 2, 0, false };
> +static const struct acpi_gpio_mapping acpi_usb_gpios[] = {
> + { "id-gpios", _gpio, 1 },
> + { "vbus_en-gpios", _gpio, 1 },
> + { "usb_mux-gpios", _gpio, 1 },
> + { },
> +};
> +
> +static const struct mfd_cell intel_vuport_mfd_cells[] = {
> + {
> + .name = "extcon-usb-gpio",
> + },
> + {
> + .name = "intel-mux-gpio",
> + },
> +};

Please place single line entries on the same line as the '{' and '}'.

As you have above.

> +static int vuport_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + int ret;
> +
> + ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), acpi_usb_gpios);
> + if (ret)
> + return ret;
> +
> + return mfd_add_devices(>dev, 0, intel_vuport_mfd_cells,
> + ARRAY_SIZE(intel_vuport_mfd_cells), NULL, 0,
> + NULL);
> +}
> +
> +static int vuport_remove(struct platform_device *pdev)
> +{
> + mfd_remove_devices(>dev);
> + acpi_dev_remove_driver_gpios(ACPI_COMPANION(>dev));
> +
> + return 0;
> +}
> +
> +static struct acpi_device_id vuport_acpi_match[] = {
> + { "INT3496" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, vuport_acpi_match);
> +
> +static struct platform_driver vuport_driver = {
> + .driver = {
> + .name = "intel-vuport",
> + .acpi_match_table = ACPI_PTR(vuport_acpi_match),
> + },
> + .probe = vuport_probe,
> + .remove = vuport_remove,
> +};
> +
> +module_platform_driver(vuport_driver);
> +
> +MODULE_AUTHOR("Lu Baolu ");
> +MODULE_DESCRIPTION("Intel virtual USB port");
> +MODULE_LICENSE("GPL v2");

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | 

Re: USB host port {,performance} issues with Beaglebone

[Rajaram R]

Victor


On Tue, Mar 8, 2016 at 9:05 AM, 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.

Do you have answer for the following

- What is the raw throughput of musb driver ? ( With no class driver)
- What is the performance you are expecting ? performance of MSC
depends on storage medium as well. May be you should use high
performance storage device
- For Ethernet, did you try NCM ? I believe that gives better results

With little bit of experience in musb, I have seen only similar
performances in 3.0 Kernel. Any tweaks for improvement would help the
community.


>
> 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:
>
>
> Kind regards,
> Victor Dodon.
> --
> 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
--
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

[Victor Dodon]

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
logging in to the target, the network connectivity fails, I after some
time I got:

[   92.176028] udevd[98]: worker [803] /devices/platform/hos
t0/session1/target0:0:0/0:0:0:0/block/sda/sda1 timeout; kill it
[   92.190702] udevd[98]: seq 1480
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda1'
killed
[   92.202237] udevd[98]: worker [805]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a2
timeout; kill it
[   92.216125] udevd[98]: seq 1481
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda2'
killed
[   92.226088] udevd[98]: worker [806]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a3
timeout; kill it
[   92.240727] udevd[98]: seq 1482
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda3'
killed
[   92.252076] udevd[98]: worker [807]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a4
timeout; kill it
[   92.265958] udevd[98]: seq 1483
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda4'
killed
[   92.276066] udevd[98]: worker [811]
/devices/platform/host0/session1/target0:0:0/0:0:0:1/block/sdb ti
meout; kill it
[   92.290420] udevd[98]: seq 1477
'/devices/platform/host0/session1/target0:0:0/0:0:0:1/block/sdb' kill
ed
[   92.299911] udevd[98]: worker [813]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a5
timeout; kill it
[   92.314794] udevd[98]: seq 1484
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda5'
killed
[   92.326211] udevd[98]: worker [814]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a6
timeout; kill it
[   92.340026] udevd[98]: seq 1485
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda6'
killed
[   92.350116] udevd[98]: worker [815]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a7
timeout; kill it
[   92.364315] udevd[98]: seq 1486
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda7'
killed
[   92.374220] udevd[98]: worker [816]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a8
timeout; kill it
[   92.388688] udevd[98]: seq 1487
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda8'
killed
[   92.399953] udevd[98]: worker [817]
/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sd a9
timeout; kill it
[   92.413449] udevd[98]: seq 1488
'/devices/platform/host0/session1/target0:0:0/0:0:0:0/block/sda/sda9'
killed
[  110.497428] [ cut here ]
[  110.501976] WARNING: CPU: 0 PID: 842 at
../../../../../tmp/portage/sys-kernel/kernel-beaglebone-4_1-4
.1.18/work/kernel-beaglebone-4_1-4.1.18/drivers/dma/cppi41.c:611
cppi41_stop_chan+0x26c/0x2d0()
[  110.518903] Modules linked in: r8152 iscsi_tcp libiscsi_tcp
libiscsi scsi_transport_iscsi uinput omap _hwspinlock hwspinlock_core
cfg80211 fuse nf_conntrack_ipv6 nf_defrag_ipv6 

USB host port {,performance} issues with Beaglebone

[Victor Dodon]

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:


Kind regards,
Victor Dodon.
--
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: high interrupt latency due to usb_sg_wait()

[Alan Stern]

On Mon, 7 Mar 2016, David Mosberger wrote:

> We are seeing relatively high interrupt latencies due to usb_sg_wait()
> calling usb_submit_urb() with interrupts disabled (as a result of its
> spin_lock_irq() call).
> 
> As far as I can see, io->lock protects io->status and it's not really
> necessary to hold the lock (or disable interrupts) during the
> usb_submit_urb() call.

It looks like you're right.  The important races here are against 
sg_complete() and usb_sg_cancel().

On the other hand, it looks like io->urb[i]->dev does need to be 
protected by the spinlock, which it isn't in usb_sg_cancel() or 
sg_complete().

> Note that the current code doesn't protect against multiple concurrent
> calls to usb_sg_wait(): a second caller of usb_sg_wait() could acquire
> io->lock when the first caller drops it after calling usb_submit_urb()
> and then it could resubmit the same URBs before the first caller gets
> a chance to update io->status.  This is perhaps an outright bug?

There can't be multiple concurrent calls to usb_sg_wait().  Or, if 
there are, it's already a bug.

> Am I missing something?

I think you're basically right.  Want to submit a patch?

Alan Stern

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

Re: Possible double-free in the usbnet driver

[Oliver Neukum]

On Mon, 2016-03-07 at 22:50 +0300, Andrey Konovalov wrote:
> Could you also add:
> Reported-by: Andrey Konovalov 

Well, the exact bug you reported is fixed in Bjorn's
patch the way Linus suggested. I'm fixing just a further
race that would require an error condition on top
of what you have seen.
So I don't think your Reported-by would be totally
appropriate.

Regards
Oliver


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

Re: Unable to enumerate USB Device, error -110 Kernel 3.19-0.49-lowlatency netboot

[Alan Stern]

On Mon, 7 Mar 2016, Devon Ash wrote:

> Attached is dmesg.txt and usbmon.txt.

For what kernel version?  And what's with all those netconsole error 
messages popping up every 5 minutes?

>  This is the only usbmon I could
> capture, from 0u, 5u and 6u don't give anything, I've tried to plug
> them in and out of every usb port.

It looks like you traced 2u.  That's not going to help much if the 
error messages are for bus 5 or 6.

Tried to plug _what_ in and out of every USB port?

> Yes, the filepath from above is the current filepath I'm using for usbmon.

What filepath above?

> Note, I had enabled dynamic debugging for this.
> 
> On Mon, Mar 7, 2016 at 3:29 PM, Devon Ash  wrote:
> > Running kernel 4.4.4 is no good.. with the same setup as above but
> > only changing out vmlinuz and initrd.img, I run into a kernel panic.
> >
> > "Kernel panic - not syncing: attempted to kill init!"
> >
> > There was an error that I hadn't seen before about
> >
> > "systemd-udevd: could not open builtin file
> > /lib/modules/4.4.4/modules.builtin.bin".

It sounds like you didn't install the new kernel properly.

> > Before the kernel panic, I got the same "device not accepting address,
> > error -110" as I did on 3.19.

There wasn't anything about a -110 error in the dmesg log you attached.  
Or in the usbmon trace.

> > To focus on the problem at hand, should I be using kernel 3.19 source
> > configured with the options above?

What options above?

Alan Stern

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Doug Anderson]

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 system I see:

[1.990743] dwc2 ff54.usb: Waited 31 us, 0x04000400 =>
0x04000400, 0x02000800 => 0x02000800
[2.119677] dwc2 ff58.usb: Waited 9997 us, 0x00100400 =>
0x04000400, 0x => 0x02000800

I believe the difference in behavior is because of the two different
types of USB controllers (one is OTG and the other is host only).


-Doug
--
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] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind

[David Miller]

From: Bjørn Mork 
Date: Mon, 7 Mar 2016 21:15:36 +0100

> usbnet_link_change will call schedule_work and should be
> avoided if bind is failing. Otherwise we will end up with
> scheduled work referring to a netdev which has gone away.
> 
> Instead of making the call conditional, we can just defer
> it to usbnet_probe, using the driver_info flag made for
> this purpose.
> 
> Fixes: 8a34b0ae8778 ("usbnet: cdc_ncm: apply usbnet_link_change")
> Reported-by: Andrey Konovalov 
> Suggested-by: Linus Torvalds 
> Signed-off-by: Bjørn Mork 
...
> Even with Oliver's generic fix we should still fix the inconsistency
> in cdc_ncm, as pointed out by Linus.
> 
> This is a slightly different approach than the patch proposed by Linus.
> When I started looking at this I couldn't figure out why we were doing
> this differently in this driver from all the other usbnet drivers
> disabling the link at probe time.  So let's make it consistent.  Then at
> least we get consistent bugs :)

Fair enough, applied and queued up for -stable.

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

[PATCH] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind

[Bjørn Mork]

usbnet_link_change will call schedule_work and should be
avoided if bind is failing. Otherwise we will end up with
scheduled work referring to a netdev which has gone away.

Instead of making the call conditional, we can just defer
it to usbnet_probe, using the driver_info flag made for
this purpose.

Fixes: 8a34b0ae8778 ("usbnet: cdc_ncm: apply usbnet_link_change")
Reported-by: Andrey Konovalov 
Suggested-by: Linus Torvalds 
Signed-off-by: Bjørn Mork 
---

David Miller  writes:
> From: Andrey Konovalov 
>
>> Could you also add:
>> Reported-by: Andrey Konovalov 
>> ?
>
> Sorry it's already committed to my tree and I can't redo the commit message
> once that happens since my tree has static history.

Even with Oliver's generic fix we should still fix the inconsistency
in cdc_ncm, as pointed out by Linus.

This is a slightly different approach than the patch proposed by Linus.
When I started looking at this I couldn't figure out why we were doing
this differently in this driver from all the other usbnet drivers
disabling the link at probe time.  So let's make it consistent.  Then at
least we get consistent bugs :)


Bjørn


 drivers/net/usb/cdc_ncm.c | 20 +---
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index be927964375b..86ba30ba35e8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -988,8 +988,6 @@ EXPORT_SYMBOL_GPL(cdc_ncm_select_altsetting);
 
 static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-   int ret;
-
/* MBIM backwards compatible function? */
if (cdc_ncm_select_altsetting(intf) != CDC_NCM_COMM_ALTSETTING_NCM)
return -ENODEV;
@@ -998,16 +996,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct 
usb_interface *intf)
 * Additionally, generic NCM devices are assumed to accept arbitrarily
 * placed NDP.
 */
-   ret = cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0);
-
-   /*
-* We should get an event when network connection is "connected" or
-* "disconnected". Set network connection in "disconnected" state
-* (carrier is OFF) during attach, so the IP network stack does not
-* start IPv6 negotiation and more.
-*/
-   usbnet_link_change(dev, 0, 0);
-   return ret;
+   return cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0);
 }
 
 static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t 
remainder, size_t max)
@@ -1590,7 +1579,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb 
*urb)
 
 static const struct driver_info cdc_ncm_info = {
.description = "CDC NCM",
-   .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
+   .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+   | FLAG_LINK_INTR,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.manage_power = usbnet_manage_power,
@@ -1603,7 +1593,7 @@ static const struct driver_info cdc_ncm_info = {
 static const struct driver_info wwan_info = {
.description = "Mobile Broadband Network Device",
.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
-   | FLAG_WWAN,
+   | FLAG_LINK_INTR | FLAG_WWAN,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.manage_power = usbnet_manage_power,
@@ -1616,7 +1606,7 @@ static const struct driver_info wwan_info = {
 static const struct driver_info wwan_noarp_info = {
.description = "Mobile Broadband Network Device (NO ARP)",
.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
-   | FLAG_WWAN | FLAG_NOARP,
+   | FLAG_LINK_INTR | FLAG_WWAN | FLAG_NOARP,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.manage_power = usbnet_manage_power,
-- 
2.1.4
--
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)

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 12:13 PM, Greg Kroah-Hartman
 wrote:
> On Mon, Mar 07, 2016 at 11:09:36AM -0800, Andy Lutomirski wrote:
>> Quick ping here: this is still busted on 4.5-rc6.
>>
>> On Wed, Feb 17, 2016 at 9:40 AM, Andy Lutomirski  wrote:
>> > On Wed, Feb 17, 2016 at 8:18 AM, Mathias Nyman
>> >  wrote:
>> >> On 17.02.2016 07:19, Andy Lutomirski wrote:
>> >>>
>> >>> On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
>> >>>  wrote:
>> 
>>  On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
>> >
>> > I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I
>> > see:
>> >
>> >> ...
>> >>
>> > [1.061036] hub 1-0:1.0: power on to power good time: 20ms
>> > [1.061109] hub 1-0:1.0: local power source is good
>> > [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
>> > according to platform information.
>> > [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
>> > according to platform information.
>> > [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
>> > according to platform information.
>> >>
>> >> ...
>> >
>> > [1.085684] usb usb2-port1: peered to usb1-port1
>> > [1.086356] usb usb2-port2: peered to usb1-port2
>> > [1.087004] usb usb2-port3: peered to usb1-port6
>> > [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
>> > location (usb2-port4:none) (usb1-port6:usb2-port3)
>> > [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
>> > [1.087716] usb: port power management may be unreliable
>> > [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
>> > location (usb2-port5:none) (usb1-port6:usb2-port3)
>> > [1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
>> > [1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
>> > location (usb2-port6:none) (usb1-port6:usb2-port3)
>> > [1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)
>> 
>> 
>>  Other than your internal hub not liking to be a peer (which seems like a
>>  BIOS/ACPI issue, right Mathias), I don't see an error here.  Are things
>>  working properly?
>> >
>> > No, actually.
>> >
>> > The USB 3.0 ports seem to work fine, although I haven't tried
>> > suspending with a device inserted.
>> >
>> > The USB Type C port is presently screwed up.  I have a Realtek USB C
>> > to Ethernet adapter.  If I plug it in, nothing shows up in the logs at
>> > all.  If I suspend while it's plugged in, the screen turns off and the
>> > system freezes hard without going to sleep.
>> >
>> > The Realtek adapter used to work, but that was on an older kernel
>> > (4.3?) and a different BIOS version.
>> >
>> >>
>> >>
>> >> Probably.
>> >> The "by location" message in the log reveals that the peering
>> >> is based on ACPI DSDT table _PLD entries (Physical Device Location).
>> >>
>> >> usb2 ports 3-6 are all peered with usb1 port6.
>> >> First two ports appear to be sane (usb1_port1 <->usb2_port1)
>> >>
>> >> How many physical ports does the DELL XPS have visible and connectable?
>> >
>> > There are:
>> >
>> >  - Two Super Speed ports.  (If I'm understanding correctly, one is 1-1
>> > and one is 1-2.)
>> >  - One USB Type C port (Alpine Ridge)
>> >  - Two internal M.2 ports
>> >
>> > One of the M.2 ports has an Intel 7265 plugged in, and that's the
>> > Bluetooth device.  The other one has an nvme device plugged in, and I
>> > have no idea whether USB is wired up.
>> >
>> > The Intel 7265 is "removable" in the sense that I personally inserted
>> > it into the port (with the power off, of course).
>> >
>> >>
>> >> The ACPI DSDt _UPC entry describes if a port is hardwired or hotplug, and
>> >> the
>> >> hub descriptor DeviceRemovable field are also involved in all this.
>> >> The log also shows changing the built in bluetooth and webcam to removable
>> >> which
>> >> is a  bit suspicious
>> >>
>> >>
>> >>>
>> >>>
>> >>> I think so, but I haven't tested with a USB device plugged in across
>> >>> suspend/resume.  I do get this on resume, though:
>> >>>
>> >>> [  +0.008245] ACPI: Waking up from system sleep state S3
>> >>> [  +0.137335] xhci_hcd :00:14.0: System wakeup disabled by ACPI
>> >>> [  +0.000117] PM: noirq resume of devices complete after 12.890 msecs
>> >>> [  +0.011969] PM: early resume of devices complete after 11.890 msecs
>> >>> [  +0.000345] usb usb1: root hub lost power or was reset
>> >>> [  +0.05] usb usb2: root hub lost power or was reset
>> >>> [  +0.005946] rtc_cmos 00:01: System wakeup disabled by ACPI
>> >>> [  +0.364455] usb 1-3: reset full-speed USB device number 2 using 
>> >>> xhci_hcd
>> >>> [  +0.318243] usb 1-5: reset high-speed USB device number 3 using 
>> >>> xhci_hcd
>> >>> [  +0.208028] PM: resume of devices complete after 896.974 msecs
>> >>> [  

Re: Unable to enumerate USB Device, error -110 Kernel 3.19-0.49-lowlatency netboot

[Devon Ash]

Running kernel 4.4.4 is no good.. with the same setup as above but
only changing out vmlinuz and initrd.img, I run into a kernel panic.

"Kernel panic - not syncing: attempted to kill init!"

There was an error that I hadn't seen before about

"systemd-udevd: could not open builtin file
/lib/modules/4.4.4/modules.builtin.bin".


Before the kernel panic, I got the same "device not accepting address,
error -110" as I did on 3.19.


To focus on the problem at hand, should I be using kernel 3.19 source
configured with the options above?



On Fri, Mar 4, 2016 at 4:47 PM, Alan Stern  wrote:
> On Fri, 4 Mar 2016, Devon Ash wrote:
>
>> Failing that, can you at least provide a usbmon trace showing what
>> happens when you plug a device into one of the bad ports?
>>
>> usbmon trace:
>>
>> I'm unable to get anything from doing "cat 0u && cat 5u && cat 6u"
>> (which are all of the offending devices locations)
>
> Is CONFIG_USB_MON set?  If it is set to M, have you loaded the usbmon
> module?
>
>> And also a
>> dmesg log with USB debugging enabled?
>>
>> dmesg shows nothing. I htink I'm missing something - to enable USB
>> debugging all that needs to be done is mount the debugfs right?
>
> No, that's not enough.  I don't remember how 3.19 does it, but with
> more modern kernels you have to enable dynamic debugging by doing:
>
> echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control
>
> That's assuming CONFIG_DYNAMIC_DEBUG is set, which it should be in a
> typical distribution kernel.
>
>> Regarding Kernel 4.2.0-30-lowlatency,
>>
>> system-udevd is now giving me the outputs on boot:
>>
>> seq 1106 '/devices/pci:00/:00:1a.0/usb5 killed (same for usb6)
>> reason being ; a timeout
>>
>> Then I start to get
>>
>> usb 5-1: device not accepting address 5, error -110
>> usb usb5-port1; unable to enumerate USB device
>>
>> and the same for usb6.
>>
>> There is also:
>>
>> system-udevd: worker terminated by signal 9, and then the system waits
>> for 2-5 minutes, finishing with a kernel hang and a stack trace "not
>> tainted, blocked for more than 120 seconds"
>
> Can you post the actual dmesg log?
>
>> here is the output of:
>>
>> sudo mount -t usbfs none /proc/bus/usb
>>
>> mount: mounting none on /proc/bus/usb failed: no such file or directory
>
> There no longer is any such thing as a usbfs filesystem.  Instead
> there are USB device nodes under /dev/bus/usb.
>
>> cat /proc/bus/usb/devices: no such file or directory
>>
>> sudo mount -t debugfs none /sys/kernel/debug
>>
>> mount: muonting none on /sys/kernel/debug failed: Device or resource busy
>
> Probably because it's already mounted there.
>
>> As well, all usbmon outputs are returning me nothing when plugging
>> in/out the broken USB ports that are in question
>
> What is the full path of the files you are watching for usbmon?  They
> should be things like /sys/kernel/debug/usb/usbmon/5u -- which of
> course won't exist unless debugfs is properly mounted and usbmon is
> properly loaded.
>
> Alan Stern
>



-- 

Devon Ash
http://ca.linkedin.com/pub/devon-ash/48/478/981;
style="text-decoration:none;">https://static.licdn.com/scds/common/u/img/webpromo/btn_in_20x15.png;
width="20" height="15" alt="View Devon Ash's LinkedIn profile"
style="vertical-align:middle" border="0">View Devon Ash's
profile
--
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)

[Greg Kroah-Hartman]

On Mon, Mar 07, 2016 at 11:09:36AM -0800, Andy Lutomirski wrote:
> Quick ping here: this is still busted on 4.5-rc6.
> 
> On Wed, Feb 17, 2016 at 9:40 AM, Andy Lutomirski  wrote:
> > On Wed, Feb 17, 2016 at 8:18 AM, Mathias Nyman
> >  wrote:
> >> On 17.02.2016 07:19, Andy Lutomirski wrote:
> >>>
> >>> On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
> >>>  wrote:
> 
>  On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
> >
> > I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I
> > see:
> >
> >> ...
> >>
> > [1.061036] hub 1-0:1.0: power on to power good time: 20ms
> > [1.061109] hub 1-0:1.0: local power source is good
> > [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
> > according to platform information.
> > [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
> > according to platform information.
> > [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
> > according to platform information.
> >>
> >> ...
> >
> > [1.085684] usb usb2-port1: peered to usb1-port1
> > [1.086356] usb usb2-port2: peered to usb1-port2
> > [1.087004] usb usb2-port3: peered to usb1-port6
> > [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
> > location (usb2-port4:none) (usb1-port6:usb2-port3)
> > [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
> > [1.087716] usb: port power management may be unreliable
> > [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
> > location (usb2-port5:none) (usb1-port6:usb2-port3)
> > [1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
> > [1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
> > location (usb2-port6:none) (usb1-port6:usb2-port3)
> > [1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)
> 
> 
>  Other than your internal hub not liking to be a peer (which seems like a
>  BIOS/ACPI issue, right Mathias), I don't see an error here.  Are things
>  working properly?
> >
> > No, actually.
> >
> > The USB 3.0 ports seem to work fine, although I haven't tried
> > suspending with a device inserted.
> >
> > The USB Type C port is presently screwed up.  I have a Realtek USB C
> > to Ethernet adapter.  If I plug it in, nothing shows up in the logs at
> > all.  If I suspend while it's plugged in, the screen turns off and the
> > system freezes hard without going to sleep.
> >
> > The Realtek adapter used to work, but that was on an older kernel
> > (4.3?) and a different BIOS version.
> >
> >>
> >>
> >> Probably.
> >> The "by location" message in the log reveals that the peering
> >> is based on ACPI DSDT table _PLD entries (Physical Device Location).
> >>
> >> usb2 ports 3-6 are all peered with usb1 port6.
> >> First two ports appear to be sane (usb1_port1 <->usb2_port1)
> >>
> >> How many physical ports does the DELL XPS have visible and connectable?
> >
> > There are:
> >
> >  - Two Super Speed ports.  (If I'm understanding correctly, one is 1-1
> > and one is 1-2.)
> >  - One USB Type C port (Alpine Ridge)
> >  - Two internal M.2 ports
> >
> > One of the M.2 ports has an Intel 7265 plugged in, and that's the
> > Bluetooth device.  The other one has an nvme device plugged in, and I
> > have no idea whether USB is wired up.
> >
> > The Intel 7265 is "removable" in the sense that I personally inserted
> > it into the port (with the power off, of course).
> >
> >>
> >> The ACPI DSDt _UPC entry describes if a port is hardwired or hotplug, and
> >> the
> >> hub descriptor DeviceRemovable field are also involved in all this.
> >> The log also shows changing the built in bluetooth and webcam to removable
> >> which
> >> is a  bit suspicious
> >>
> >>
> >>>
> >>>
> >>> I think so, but I haven't tested with a USB device plugged in across
> >>> suspend/resume.  I do get this on resume, though:
> >>>
> >>> [  +0.008245] ACPI: Waking up from system sleep state S3
> >>> [  +0.137335] xhci_hcd :00:14.0: System wakeup disabled by ACPI
> >>> [  +0.000117] PM: noirq resume of devices complete after 12.890 msecs
> >>> [  +0.011969] PM: early resume of devices complete after 11.890 msecs
> >>> [  +0.000345] usb usb1: root hub lost power or was reset
> >>> [  +0.05] usb usb2: root hub lost power or was reset
> >>> [  +0.005946] rtc_cmos 00:01: System wakeup disabled by ACPI
> >>> [  +0.364455] usb 1-3: reset full-speed USB device number 2 using xhci_hcd
> >>> [  +0.318243] usb 1-5: reset high-speed USB device number 3 using xhci_hcd
> >>> [  +0.208028] PM: resume of devices complete after 896.974 msecs
> >>> [  +0.000109] usb 1-3:1.0: rebind failed: -517
> >>> [  +0.07] usb 1-3:1.1: rebind failed: -517
> >>
> >>
> >> was usb 1-3  the bluetooth device?
> >
> > [  +0.044127] usb 1-3: new full-speed USB device number 2 

Re: [PATCH RESEND] xhci:Remove unused marco definitions from the file xhci-hub.c

[Greg KH]

On Mon, Mar 07, 2016 at 02:20:03PM -0500, Nicholas Krause wrote:
> This removes the unneeded marco definitions for the marcos
> of XHCI_PORT_RW1S, XHCI_PORT_RW1C, XHCI_PORT_RWand
> XHCI_PORT_RZ due to no uses of these marcos in the file
> xhci-hub.c or any other related kernel source code file
> related to supporting xhci host controllers.
> 
> Signed-off-by: Nicholas Krause 
> ---
>  drivers/usb/host/xhci-hub.c | 23 ---
>  1 file changed, 23 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index e75c565..fd430de 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -196,29 +196,6 @@ static unsigned int xhci_port_speed(unsigned int 
> port_status)
>   * link state, port power, port indicator state, "wake on" enable state
>   */
>  #define XHCI_PORT_RWS((0xf<<5) | (1<<9) | (0x3<<14) | (0x7<<25))
> -/*
> - * These bits are RW; writing a 1 sets the bit, writing a 0 has no effect:
> - * bit 4 (port reset)
> - */
> -#define  XHCI_PORT_RW1S  ((1<<4))
> -/*
> - * These bits are RW; writing a 1 clears the bit, writing a 0 has no effect:
> - * bits 1, 17, 18, 19, 20, 21, 22, 23
> - * port enable/disable, and
> - * change bits: connect, PED, warm port reset changed (reserved zero for USB 
> 2.0 ports),
> - * over-current, reset, link state, and L1 change
> - */
> -#define XHCI_PORT_RW1CS  ((1<<1) | (0x7f<<17))
> -/*
> - * Bit 16 is RW, and writing a '1' to it causes the link state control to be
> - * latched in
> - */
> -#define  XHCI_PORT_RW((1<<16))
> -/*
> - * These bits are Reserved Zero (RsvdZ) and zero should be written to them:
> - * bits 2, 24, 28:31
> - */
> -#define  XHCI_PORT_RZ((1<<2) | (1<<24) | (0xf<<28))
>  
>  /*
>   * Given a port state, this function returns a value that would result in the

As I have stated numerous times in the past, please do not send any
future Linux kernel changes, I will not be accepting them.

Mathias, please ignore.

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

high interrupt latency due to usb_sg_wait()

[David Mosberger]

We are seeing relatively high interrupt latencies due to usb_sg_wait()
calling usb_submit_urb() with interrupts disabled (as a result of its
spin_lock_irq() call).

As far as I can see, io->lock protects io->status and it's not really
necessary to hold the lock (or disable interrupts) during the
usb_submit_urb() call.

Note that the current code doesn't protect against multiple concurrent
calls to usb_sg_wait(): a second caller of usb_sg_wait() could acquire
io->lock when the first caller drops it after calling usb_submit_urb()
and then it could resubmit the same URBs before the first caller gets
a chance to update io->status.  This is perhaps an outright bug?

Am I missing something?

 --david
-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768
--
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: Possible double-free in the usbnet driver

[David Miller]

From: Andrey Konovalov 
Date: Mon, 7 Mar 2016 22:50:41 +0300

> On Mon, Mar 7, 2016 at 10:11 PM, David Miller  wrote:
>> From: Linus Torvalds 
>> Date: Mon, 7 Mar 2016 10:13:09 -0800
>>
>>> On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork  wrote:


 Definitely.  The patch is so obviously correct that we can only wonder how 
 it was possible to miss it it the first place :)

 Will take a look to see if we could do a better job cleaning up in other 
 places.
>>>
>>> What should I do for 4.5? Will there be a pull request for this, or
>>> should I just commit my cdc_ncm_bind() patch directly?
>>
>> Yes I plan to send you a pull request today with Oliver's fix.
>>
>> Assuming this is what you guys are referring to:
>>
>> commit 1666984c8625b3db19a9abc298931d35ab7bc64b
>> Author: Oliver Neukum 
>> Date:   Mon Mar 7 11:31:10 2016 +0100
>>
>> usbnet: cleanup after bind() in probe()
>>
>> In case bind() works, but a later error forces bailing
>> in probe() in error cases work and a timer may be scheduled.
>> They must be killed. This fixes an error case related to
>> the double free reported in
>> http://www.spinics.net/lists/netdev/msg367669.html
>> and needs to go on top of Linus' fix to cdc-ncm.
>>
>> Signed-off-by: Oliver Neukum 
>> Signed-off-by: David S. Miller 
> 
> Could you also add:
> Reported-by: Andrey Konovalov 
> ?

Sorry it's already committed to my tree and I can't redo the commit message
once that happens since my tree has static history.
--
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: Possible double-free in the usbnet driver

[Andrey Konovalov]

On Mon, Mar 7, 2016 at 10:11 PM, David Miller  wrote:
> From: Linus Torvalds 
> Date: Mon, 7 Mar 2016 10:13:09 -0800
>
>> On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork  wrote:
>>>
>>>
>>> Definitely.  The patch is so obviously correct that we can only wonder how 
>>> it was possible to miss it it the first place :)
>>>
>>> Will take a look to see if we could do a better job cleaning up in other 
>>> places.
>>
>> What should I do for 4.5? Will there be a pull request for this, or
>> should I just commit my cdc_ncm_bind() patch directly?
>
> Yes I plan to send you a pull request today with Oliver's fix.
>
> Assuming this is what you guys are referring to:
>
> commit 1666984c8625b3db19a9abc298931d35ab7bc64b
> Author: Oliver Neukum 
> Date:   Mon Mar 7 11:31:10 2016 +0100
>
> usbnet: cleanup after bind() in probe()
>
> In case bind() works, but a later error forces bailing
> in probe() in error cases work and a timer may be scheduled.
> They must be killed. This fixes an error case related to
> the double free reported in
> http://www.spinics.net/lists/netdev/msg367669.html
> and needs to go on top of Linus' fix to cdc-ncm.
>
> Signed-off-by: Oliver Neukum 
> Signed-off-by: David S. Miller 

Could you also add:
Reported-by: Andrey Konovalov 
?

Thanks in advance.

>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 0b0ba7e..1079812 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1769,6 +1769,13 @@ out3:
> if (info->unbind)
> info->unbind (dev, udev);
>  out1:
> +   /* subdrivers must undo all they did in bind() if they
> +* fail it, but we may fail later and a deferred kevent
> +* may trigger an error resubmitting itself and, worse,
> +* schedule a timer. So we kill it all just in case.
> +*/
> +   cancel_work_sync(>kevent);
> +   del_timer_sync(>delay);
> free_netdev(net);
>  out:
> return status;
--
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 net,stable] cdc_ncm: toggle altsetting to force reset before setup

[David Miller]

From: Bjørn Mork 
Date: Thu,  3 Mar 2016 22:20:53 +0100

> Some devices will silently fail setup unless they are reset first.
> This is necessary even if the data interface is already in
> altsetting 0, which it will be when the device is probed for the
> first time.  Briefly toggling the altsetting forces a function
> reset regardless of the initial state.
> 
> This fixes a setup problem observed on a number of Huawei devices,
> appearing to operate in NTB-32 mode even if we explicitly set them
> to NTB-16 mode.
> 
> Signed-off-by: Bjørn Mork 

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

[PATCH] uas: Reduce can_queue to MAX_CMNDS

[Hans de Goede]

The uas driver can never queue more then MAX_CMNDS (- 1) tags and tags
are shared between luns, so there is no need to claim that we can_queue
some random large number.

Not claiming that we can_queue 65536 commands, fixes the uas driver
failing to initialize while allocating the tag map with a "Page allocation
failure (order 7)" error on systems which have been running for a while
and thus have fragmented memory.

Cc: sta...@vger.kernel.org
Reported-and-tested-by: Yves-Alexis Perez 
Signed-off-by: Hans de Goede 
---
 drivers/usb/storage/uas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 9ff9404..c90a7e4 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -812,7 +812,7 @@ static struct scsi_host_template uas_host_template = {
.slave_configure = uas_slave_configure,
.eh_abort_handler = uas_eh_abort_handler,
.eh_bus_reset_handler = uas_eh_bus_reset_handler,
-   .can_queue = 65536, /* Is there a limit on the _host_ ? */
+   .can_queue = MAX_CMNDS,
.this_id = -1,
.sg_tablesize = SG_NONE,
.skip_settle_delay = 1,
-- 
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] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 10:04:21 -0800
Andy Lutomirski  wrote:


> > Exactly. The compiler may get away with this in userspace (maybe), but
> > for the kernel, it is definitely a show stopper. Especially if it knows
> > that an asm() may be called.  
> 
> It's broken for user code that fiddles with AC, too.

Which is why I added the "(maybe)" ;-)

-- 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: Possible double-free in the usbnet driver

[David Miller]

From: Linus Torvalds 
Date: Mon, 7 Mar 2016 10:13:09 -0800

> On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork  wrote:
>>
>>
>> Definitely.  The patch is so obviously correct that we can only wonder how 
>> it was possible to miss it it the first place :)
>>
>> Will take a look to see if we could do a better job cleaning up in other 
>> places.
> 
> What should I do for 4.5? Will there be a pull request for this, or
> should I just commit my cdc_ncm_bind() patch directly?

Yes I plan to send you a pull request today with Oliver's fix.

Assuming this is what you guys are referring to:

commit 1666984c8625b3db19a9abc298931d35ab7bc64b
Author: Oliver Neukum 
Date:   Mon Mar 7 11:31:10 2016 +0100

usbnet: cleanup after bind() in probe()

In case bind() works, but a later error forces bailing
in probe() in error cases work and a timer may be scheduled.
They must be killed. This fixes an error case related to
the double free reported in
http://www.spinics.net/lists/netdev/msg367669.html
and needs to go on top of Linus' fix to cdc-ncm.

Signed-off-by: Oliver Neukum 
Signed-off-by: David S. Miller 

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 0b0ba7e..1079812 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1769,6 +1769,13 @@ out3:
if (info->unbind)
info->unbind (dev, udev);
 out1:
+   /* subdrivers must undo all they did in bind() if they
+* fail it, but we may fail later and a deferred kevent
+* may trigger an error resubmitting itself and, worse,
+* schedule a timer. So we kill it all just in case.
+*/
+   cancel_work_sync(>kevent);
+   del_timer_sync(>delay);
free_netdev(net);
 out:
return status;
--
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)

[Andy Lutomirski]

Quick ping here: this is still busted on 4.5-rc6.

On Wed, Feb 17, 2016 at 9:40 AM, Andy Lutomirski  wrote:
> On Wed, Feb 17, 2016 at 8:18 AM, Mathias Nyman
>  wrote:
>> On 17.02.2016 07:19, Andy Lutomirski wrote:
>>>
>>> On Tue, Feb 16, 2016 at 6:33 PM, Greg Kroah-Hartman
>>>  wrote:

 On Tue, Feb 16, 2016 at 10:01:13AM -0800, Andy Lutomirski wrote:
>
> I get some warnings at boot on all kernels I've tried.  On 4.5-rc4, I
> see:
>
>> ...
>>
> [1.061036] hub 1-0:1.0: power on to power good time: 20ms
> [1.061109] hub 1-0:1.0: local power source is good
> [1.084337] usb usb1-port3: DeviceRemovable is changed to 1
> according to platform information.
> [1.084339] usb usb1-port4: DeviceRemovable is changed to 1
> according to platform information.
> [1.084341] usb usb1-port5: DeviceRemovable is changed to 1
> according to platform information.
>>
>> ...
>
> [1.085684] usb usb2-port1: peered to usb1-port1
> [1.086356] usb usb2-port2: peered to usb1-port2
> [1.087004] usb usb2-port3: peered to usb1-port6
> [1.087713] usb: failed to peer usb2-port4 and usb1-port6 by
> location (usb2-port4:none) (usb1-port6:usb2-port3)
> [1.087715] usb usb2-port4: failed to peer to usb1-port6 (-16)
> [1.087716] usb: port power management may be unreliable
> [1.088377] usb: failed to peer usb2-port5 and usb1-port6 by
> location (usb2-port5:none) (usb1-port6:usb2-port3)
> [1.088379] usb usb2-port5: failed to peer to usb1-port6 (-16)
> [1.089017] usb: failed to peer usb2-port6 and usb1-port6 by
> location (usb2-port6:none) (usb1-port6:usb2-port3)
> [1.089018] usb usb2-port6: failed to peer to usb1-port6 (-16)


 Other than your internal hub not liking to be a peer (which seems like a
 BIOS/ACPI issue, right Mathias), I don't see an error here.  Are things
 working properly?
>
> No, actually.
>
> The USB 3.0 ports seem to work fine, although I haven't tried
> suspending with a device inserted.
>
> The USB Type C port is presently screwed up.  I have a Realtek USB C
> to Ethernet adapter.  If I plug it in, nothing shows up in the logs at
> all.  If I suspend while it's plugged in, the screen turns off and the
> system freezes hard without going to sleep.
>
> The Realtek adapter used to work, but that was on an older kernel
> (4.3?) and a different BIOS version.
>
>>
>>
>> Probably.
>> The "by location" message in the log reveals that the peering
>> is based on ACPI DSDT table _PLD entries (Physical Device Location).
>>
>> usb2 ports 3-6 are all peered with usb1 port6.
>> First two ports appear to be sane (usb1_port1 <->usb2_port1)
>>
>> How many physical ports does the DELL XPS have visible and connectable?
>
> There are:
>
>  - Two Super Speed ports.  (If I'm understanding correctly, one is 1-1
> and one is 1-2.)
>  - One USB Type C port (Alpine Ridge)
>  - Two internal M.2 ports
>
> One of the M.2 ports has an Intel 7265 plugged in, and that's the
> Bluetooth device.  The other one has an nvme device plugged in, and I
> have no idea whether USB is wired up.
>
> The Intel 7265 is "removable" in the sense that I personally inserted
> it into the port (with the power off, of course).
>
>>
>> The ACPI DSDt _UPC entry describes if a port is hardwired or hotplug, and
>> the
>> hub descriptor DeviceRemovable field are also involved in all this.
>> The log also shows changing the built in bluetooth and webcam to removable
>> which
>> is a  bit suspicious
>>
>>
>>>
>>>
>>> I think so, but I haven't tested with a USB device plugged in across
>>> suspend/resume.  I do get this on resume, though:
>>>
>>> [  +0.008245] ACPI: Waking up from system sleep state S3
>>> [  +0.137335] xhci_hcd :00:14.0: System wakeup disabled by ACPI
>>> [  +0.000117] PM: noirq resume of devices complete after 12.890 msecs
>>> [  +0.011969] PM: early resume of devices complete after 11.890 msecs
>>> [  +0.000345] usb usb1: root hub lost power or was reset
>>> [  +0.05] usb usb2: root hub lost power or was reset
>>> [  +0.005946] rtc_cmos 00:01: System wakeup disabled by ACPI
>>> [  +0.364455] usb 1-3: reset full-speed USB device number 2 using xhci_hcd
>>> [  +0.318243] usb 1-5: reset high-speed USB device number 3 using xhci_hcd
>>> [  +0.208028] PM: resume of devices complete after 896.974 msecs
>>> [  +0.000109] usb 1-3:1.0: rebind failed: -517
>>> [  +0.07] usb 1-3:1.1: rebind failed: -517
>>
>>
>> was usb 1-3  the bluetooth device?
>
> [  +0.044127] usb 1-3: new full-speed USB device number 2 using xhci_hcd
> [  +0.166395] usb 1-3: New USB device found, idVendor=8087, idProduct=0a2a
> [  +0.09] usb 1-3: New USB device strings: Mfr=0, Product=0, 
> SerialNumber=0
>
> So, yes, I think so.  Right now, 1-3:1.0 and 1-3:1.1 are both bound by btusb.
>
> I attached the DSDT.
>
> I have the 

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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 ...

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: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Linus Torvalds]

On Mon, Mar 7, 2016 at 10:07 AM, Alan Stern  wrote:
>
> Of course, there are other ways to save a single flag value (such as
> setz).  It's up to the compiler developers to decide what they think is
> best.

Using 'setcc' to save eflags somewhere is definitely the right thing to do.

Using pushf/popf in generated code is completely insane (unless done
very localized in a controlled area).

It is, in fact, insane and wrong even in user space, since eflags does
contain bits that user space itself might be modifying.

In fact, even IF may be modified with iopl 3 (thing old X server
setups), but ignoring that flag entirely, you have AC that acts in
very similar ways (system-wide alignment control) that user space
might be using to make sure it doesn't have unaligned accesses.

It's rare, yes. But still - this isn't really limited to just the kernel.

But perhaps more importantly, I suspect using pushf/popf isn't just
semantically the wrong thing to do, it's just plain stupid. It's
likely slower than the obvious 'setcc' model. Agner Fog's table shows
it "popf" as being 25-30 uops on several microarchitectures. Looks
like it's often microcode.

Now, pushf/popf may well be fairly cheap on *some* uarchitectures, but
it really sounds like a bad idea to use it when not absolutely
required. And that is completely independent of the fact that is
screws up the IF bit.

But yeah, for the kernel we at a minimum need a way to disable that
code generation, even if the clang guys might have some insane reason
to keep it for other cases.

  Linus
--
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: Possible double-free in the usbnet driver

[Linus Torvalds]

On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork  wrote:
>
>
> Definitely.  The patch is so obviously correct that we can only wonder how it 
> was possible to miss it it the first place :)
>
> Will take a look to see if we could do a better job cleaning up in other 
> places.

What should I do for 4.5? Will there be a pull request for this, or
should I just commit my cdc_ncm_bind() patch directly?

  Linus
--
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] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

On Mon, 7 Mar 2016, David Laight wrote:

> From: Sedat Dilek
> ...
> > Did someone look at the next/follow-ups in this thread?
> > For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?
> 
> LAHF and SAHF come with the following note:
> 
> This instruction executes as described above in compatibility mode and legacy 
> mode.
> It is valid in 64-bit mode only if CPUID.8001H:ECX.LAHF-SAHF[bit 0] = 1.
> 
> So I suspect they can't be used.

Of course, there are other ways to save a single flag value (such as
setz).  It's up to the compiler developers to decide what they think is
best.

Alan Stern

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

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Andy Lutomirski]

On Mon, Mar 7, 2016 at 9:30 AM, Steven Rostedt  wrote:
> On Mon, 7 Mar 2016 18:24:12 +0100 (CET)
> Jiri Kosina  wrote:
>
>> > So, if Clang is producing wrong X86 code here, is it possible to turn
>> > interrupts on/off manually? But, hmm that affects other places as well
>> > in the Linux sources, so.
>>
>> This issue needs to be handled in the compiler.
>>
>
> Exactly. The compiler may get away with this in userspace (maybe), but
> for the kernel, it is definitely a show stopper. Especially if it knows
> that an asm() may be called.

It's broken for user code that fiddles with AC, too.

--Andy
--
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 v8] usb: musb: da8xx: Add DT support for the DA8xx driver

[Sergei Shtylyov]

On 03/07/2016 07:08 PM, Petr Kulhavy wrote:

Hello

On 07.03.2016 14:29, Sergei Shtylyov wrote:

Hello.

On 3/7/2016 2:20 PM, Petr Kulhavy wrote:


@@ -544,17 +643,25 @@ static int da8xx_probe(struct platform_device *pdev)
  pinfo.data = pdata;
  pinfo.size_data = sizeof(*pdata);

+ret = regulator_enable(glue->vbus_supply);


   What does this achieve with a regulator that can't be explicitly controlled?



If this is a _real_ regulator, then it needs to be enabled by somebody,
doesn't it?



   Well, it's possible that Vbus is controlled via GPIO like on DaVinci 
DM644x. But so far we haven't seen this on DA8xx...



+if (ret) {
+dev_err(>dev, "failed to enable power: %d\n", ret);
+goto err6;
+}
+
  glue->musb = musb = platform_device_register_full();
  if (IS_ERR(musb)) {
  ret = PTR_ERR(musb);
  dev_err(>dev, "failed to register musb device: %d\n", ret);
-goto err6;
+goto err7;
  }

  return 0;
+err7:
+usb_phy_generic_unregister(glue->phy);

  err6:
-usb_phy_generic_unregister(glue->phy);
+regulator_disable(glue->vbus_supply);


   How's that? Aren't you envaling Vbus regulator *after* registering PHY?


No, as you can see just a few lines above in the patch.


   I'm seeing exactly opposite -- you're enabling the regulator right before 
registering the platform device, so after registering generic PHY. It's 
therefore not at all clear why you've exchanged the calls above. I can't ACK 
this patch.



[...]

@@ -582,11 +690,20 @@ static int da8xx_remove(struct platform_device *pdev)
  return 0;
  }

+static const struct of_device_id da8xx_id_table[] = {
+{
+.compatible = "ti,da830-musb",
+},
+{},
+};
+MODULE_DEVICE_TABLE(of, da8xx_id_table);
+
  static struct platform_driver da8xx_driver = {
  .probe= da8xx_probe,
  .remove= da8xx_remove,
  .driver= {
  .name= "musb-da8xx",
+.of_match_table = of_match_ptr(da8xx_id_table),


   Doesn't this cause a warning about in non-DT case?


No, the of_match_table is defined independent of the CONFIG_OF.


   But of_match_ptr() depends on it, IIRC.


Regards
Petr


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] usbnet: cleanup after bind() in probe()

[Oliver Neukum]

On Mon, 2016-03-07 at 11:41 -0500, David Miller wrote:
> From: Oliver Neukum 
> Date: Mon,  7 Mar 2016 11:31:10 +0100
> 
> > In case bind() works, but a later error forces bailing
> > in probe() in error cases work and a timer may be scheduled.
> > They must be killed. This fixes an error case related to
> > the double free reported in
> > http://www.spinics.net/lists/netdev/msg367669.html
> > and needs to go on top of Linus' fix to cdc-ncm.
> > 
> > Signed-off-by: Oliver Neukum 
> 
> Applied, thanks Oliver.
> 
> How far back does this bug exist?  I'm trying to figure out what
> -stable branchs to send it to.

It goes back to 3.12

HTH
Oliver



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

RE: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[David Laight]

From: Sedat Dilek
...
> Did someone look at the next/follow-ups in this thread?
> For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?

LAHF and SAHF come with the following note:

This instruction executes as described above in compatibility mode and legacy 
mode.
It is valid in 64-bit mode only if CPUID.8001H:ECX.LAHF-SAHF[bit 0] = 1.

So I suspect they can't be used.

David

N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 18:24:12 +0100 (CET)
Jiri Kosina  wrote:

> > So, if Clang is producing wrong X86 code here, is it possible to turn 
> > interrupts on/off manually? But, hmm that affects other places as well 
> > in the Linux sources, so.  
> 
> This issue needs to be handled in the compiler.
> 

Exactly. The compiler may get away with this in userspace (maybe), but
for the kernel, it is definitely a show stopper. Especially if it knows
that an asm() may be called.

-- 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: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Jiri Kosina]

On Mon, 7 Mar 2016, Sedat Dilek wrote:

> Did someone look at the next/follow-ups in this thread?
> For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?

Using LAHF/SAHF would "solve" it, as IF is at bit #9. The question is 
whether they need to play with flags here at all.

On Mon, 7 Mar 2016, Sedat Dilek wrote:

> So, if Clang is producing wrong X86 code here, is it possible to turn 
> interrupts on/off manually? But, hmm that affects other places as well 
> in the Linux sources, so.

This issue needs to be handled in the compiler.

-- 
Jiri Kosina
SUSE Labs

--
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] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Mar 7, 2016 at 5:41 PM, Alan Stern  wrote:
> On Mon, 7 Mar 2016, Sedat Dilek wrote:
>
>> Hmm, we are there where I was looking at...
>>
>> Please, read the reply of Jiri [1], we did some tweaking.
>> With CONFIG_FTRACE=n and CONFIG_PROVE_LOCKING=n !
>
> Yes, Jiri was looking more or less at the right place but his
> conclusions were wrong.
>
>> *** Part one: ObjectDump of hid-core.o ***
>>
>> $ objdump -D drivers/hid/usbhid/hid-core.o | awk '/<[^>]*>:$/ { p=0; }
>> /:/ { p=1; } { if (p) print $0; }' >
>> ../objdump-D_hid-core_o_usbhid_close_$(uname -r).txt
>
> This reveals the problem, at last...
>
>> $ cat ../objdump-D_hid-core_o_usbhid_close_4.4.4-1-iniza-small.txt
>> 02e0 :
>>  2e0:   55  push   %rbp
>>  2e1:   48 89 e5mov%rsp,%rbp
>>  2e4:   41 57   push   %r15
>>  2e6:   41 56   push   %r14
>>  2e8:   41 54   push   %r12
>>  2ea:   53  push   %rbx
>>  2eb:   49 89 ffmov%rdi,%r15
>>  2ee:   4d 8b b7 e8 1e 00 00mov0x1ee8(%r15),%r14
>>  2f5:   48 c7 c7 00 00 00 00mov$0x0,%rdi
>>  2fc:   31 f6   xor%esi,%esi
>>  2fe:   e8 00 00 00 00  callq  303 
>
> mutex_lock(_open_mut);
>
>>  303:   49 8d 9e 88 28 00 00lea0x2888(%r14),%rbx
>>  30a:   48 89 dfmov%rbx,%rdi
>>  30d:   e8 00 00 00 00  callq  312 
>
> spin_lock_irq(>lock);
>
>>  312:   41 ff 8f e4 1d 00 00decl   0x1de4(%r15)
>
> --hid->open
>
>>  319:   9c  pushfq
>>  31a:   41 5c   pop%r12
>>  31c:   48 89 dfmov%rbx,%rdi
>>  31f:   e8 00 00 00 00  callq  324 
>>  324:   41 54   push   %r12
>>  326:   9d  popfq
>
> spin_unlock_irq(>lock); while attempting to preserve the Z
> flag.  The problem is that this code sequence will also preserve the
> Interrupt Flag!
>
>>  327:   75 23   jne34c 
>
> if (!--hid->open), testing the Z flag from the decl.
>
>>  329:   4c 89 f7mov%r14,%rdi
>>  32c:   e8 3f 00 00 00  callq  370 
>
> But now hid_cancel_delayed_stuff(usbhid) gets called with interrupts
> disabled.
>
> It's hard to call this a compiler bug, but perhaps it is -- I don't
> know how programmers are supposed to tell CLANG that a subroutine
> modifies the Interrupt Flag in a way that the compiler shouldn't mess
> up.
>

So, if Clang is producing wrong X86 code here, is it possible to turn
interrupts on/off manually?
But, hmm that affects other places as well in the Linux sources, so.

- Sedat -
--
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] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Mar 7, 2016 at 6:05 PM, Jiri Kosina  wrote:
> On Mon, 7 Mar 2016, Alan Stern wrote:
>
>> >  319:   9c  pushfq
>> >  31a:   41 5c   pop%r12
>> >  31c:   48 89 dfmov%rbx,%rdi
>> >  31f:   e8 00 00 00 00  callq  324 
>> >  324:   41 54   push   %r12
>> >  326:   9d  popfq
>>
>> spin_unlock_irq(>lock); while attempting to preserve the Z
>> flag.  The problem is that this code sequence will also preserve the
>> Interrupt Flag!
>
> You are right Alan, thanks a lot, for reason I could not understand I
> completely missed the pushf/popf last time I was looking at the generated
> assembly!
>
> OK, a little bit of googling revealed related discussion on LLVM
> mailinglist:
>
> http://lists.llvm.org/pipermail/llvm-dev/2015-July/088780.html
>
> Seems like it has been reported already, but noone dared to fix it yet.
>
> This basically makes LLVM unusable for compiling the kernel.
>

OK, OK.

Did someone look at the next/follow-ups in this thread?
For example: D6629 "x86: Emit LAHF/SAHF instead of PUSHF/POPF" [2]?

- Sedat -

[1] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088874.html
[2] http://reviews.llvm.org/D6629
--
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] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 11:41:37 -0500 (EST)
Alan Stern  wrote:


> It's hard to call this a compiler bug, but perhaps it is -- I don't
> know how programmers are supposed to tell CLANG that a subroutine
> modifies the Interrupt Flag in a way that the compiler shouldn't mess
> up.


I would state that this is a compiler bug for any kernel development.
Because it's modifying a global variable (IF) that can be modified by
asm(). Clang is assuming that this is userspace where IF can't change.
But because this is kernel space, the IF can (and does here), which
makes this "feature" incompatible with any (Linux or otherwise) kernel
programming.

-- 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: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Jiri Kosina]

On Mon, 7 Mar 2016, Alan Stern wrote:

> >  319:   9c  pushfq
> >  31a:   41 5c   pop%r12
> >  31c:   48 89 dfmov%rbx,%rdi
> >  31f:   e8 00 00 00 00  callq  324 
> >  324:   41 54   push   %r12
> >  326:   9d  popfq
> 
> spin_unlock_irq(>lock); while attempting to preserve the Z
> flag.  The problem is that this code sequence will also preserve the
> Interrupt Flag!

You are right Alan, thanks a lot, for reason I could not understand I 
completely missed the pushf/popf last time I was looking at the generated 
assembly!

OK, a little bit of googling revealed related discussion on LLVM 
mailinglist:

http://lists.llvm.org/pipermail/llvm-dev/2015-July/088780.html

Seems like it has been reported already, but noone dared to fix it yet. 

This basically makes LLVM unusable for compiling the kernel.

-- 
Jiri Kosina
SUSE Labs

--
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] usbhid: Fix lockdep unannotated irqs-off warning

[Steven Rostedt]

On Mon, 7 Mar 2016 11:41:37 -0500 (EST)
Alan Stern  wrote:

> It's hard to call this a compiler bug, but perhaps it is -- I don't
> know how programmers are supposed to tell CLANG that a subroutine
> modifies the Interrupt Flag in a way that the compiler shouldn't mess
> up.


Really! This is what's is happening??

Clang takes this:

if (!--hid->open) {
spin_unlock_irq(X);
do_something();
} else {
spin_unlock_irq(X);
}

Thus it's basically doing:

FLAG = !--hid->open;
push flags;
spin_unlock_irq(X)
pop flags;
if (FLAG zero set) {
do_something();
}


OUCH!!! There's gotta be a way to turn that off, otherwise Clang can
not be used to compile the kernel.

Nice detective work.

-- 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: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Alan Stern]

On Mon, 7 Mar 2016, Sedat Dilek wrote:

> Hmm, we are there where I was looking at...
> 
> Please, read the reply of Jiri [1], we did some tweaking.
> With CONFIG_FTRACE=n and CONFIG_PROVE_LOCKING=n !

Yes, Jiri was looking more or less at the right place but his
conclusions were wrong.

> *** Part one: ObjectDump of hid-core.o ***
> 
> $ objdump -D drivers/hid/usbhid/hid-core.o | awk '/<[^>]*>:$/ { p=0; }
> /:/ { p=1; } { if (p) print $0; }' >
> ../objdump-D_hid-core_o_usbhid_close_$(uname -r).txt

This reveals the problem, at last...

> $ cat ../objdump-D_hid-core_o_usbhid_close_4.4.4-1-iniza-small.txt
> 02e0 :
>  2e0:   55  push   %rbp
>  2e1:   48 89 e5mov%rsp,%rbp
>  2e4:   41 57   push   %r15
>  2e6:   41 56   push   %r14
>  2e8:   41 54   push   %r12
>  2ea:   53  push   %rbx
>  2eb:   49 89 ffmov%rdi,%r15
>  2ee:   4d 8b b7 e8 1e 00 00mov0x1ee8(%r15),%r14
>  2f5:   48 c7 c7 00 00 00 00mov$0x0,%rdi
>  2fc:   31 f6   xor%esi,%esi
>  2fe:   e8 00 00 00 00  callq  303 

mutex_lock(_open_mut);

>  303:   49 8d 9e 88 28 00 00lea0x2888(%r14),%rbx
>  30a:   48 89 dfmov%rbx,%rdi
>  30d:   e8 00 00 00 00  callq  312 

spin_lock_irq(>lock);

>  312:   41 ff 8f e4 1d 00 00decl   0x1de4(%r15)

--hid->open

>  319:   9c  pushfq
>  31a:   41 5c   pop%r12
>  31c:   48 89 dfmov%rbx,%rdi
>  31f:   e8 00 00 00 00  callq  324 
>  324:   41 54   push   %r12
>  326:   9d  popfq

spin_unlock_irq(>lock); while attempting to preserve the Z
flag.  The problem is that this code sequence will also preserve the
Interrupt Flag!

>  327:   75 23   jne34c 

if (!--hid->open), testing the Z flag from the decl.

>  329:   4c 89 f7mov%r14,%rdi
>  32c:   e8 3f 00 00 00  callq  370 

But now hid_cancel_delayed_stuff(usbhid) gets called with interrupts
disabled.

It's hard to call this a compiler bug, but perhaps it is -- I don't
know how programmers are supposed to tell CLANG that a subroutine
modifies the Interrupt Flag in a way that the compiler shouldn't mess
up.

Alan Stern

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

Re: [PATCH] usbnet: cleanup after bind() in probe()

[David Miller]

From: Oliver Neukum 
Date: Mon,  7 Mar 2016 11:31:10 +0100

> In case bind() works, but a later error forces bailing
> in probe() in error cases work and a timer may be scheduled.
> They must be killed. This fixes an error case related to
> the double free reported in
> http://www.spinics.net/lists/netdev/msg367669.html
> and needs to go on top of Linus' fix to cdc-ncm.
> 
> Signed-off-by: Oliver Neukum 

Applied, thanks Oliver.

How far back does this bug exist?  I'm trying to figure out what
-stable branchs to send it to.

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

Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Mon, Mar 7, 2016 at 4:59 PM, Sedat Dilek  wrote:
> On Sun, Mar 6, 2016 at 6:23 PM, Alan Stern  wrote:
>> On Sat, 5 Mar 2016, Sedat Dilek wrote:
>>
>>> On Fri, Mar 4, 2016 at 5:04 PM, Alan Stern  
>>> wrote:
>>> > On Wed, 2 Mar 2016, Sedat Dilek wrote:
>>> >
>>> >> On 3/1/16, Alan Stern  wrote:
>>> >> > On Tue, 1 Mar 2016, Sedat Dilek wrote:
>>> >> >
>>> >> >> On Tue, Oct 13, 2015 at 2:57 AM, Steven Rostedt 
>>> >> >> wrote:
>>> >> >> > On Sat, 3 Oct 2015 12:05:42 +0200
>>> >> >> > Sedat Dilek  wrote:
>>> >> >> >
>>> >> >> >> So, at the beginning... dunno WTF is causing the problems - no
>>> >> >> >> workaround for CLANG.
>>> >> >> >
>>> >> >> > Probably need to compile with gcc and with clang and look at the 
>>> >> >> > binary
>>> >> >> > differences. Or at least what objdump shows.
>>> >> >> >
>>> >> >>
>>> >> >> [ Hope to address this issue to the correct people - CCed some people
>>> >> >> I taped on their nerves ]
>>> >> >>
>>> >> >> Not sure if I should open a new thread?
>>> >> >> Please, some clear statements on this.
>>> >> >> Thanks.
>>> >> >>
>>> >> >> The issue is still visible and alive.
>>> >
>>> > I think it would be worthwhile to doublecheck the time at which
>>> > interrupts get disabled.  Sedat, please try your plug/unplug the USB
>>> > mouse test with the patch below.
>>> >
>>> > Alan Stern
>>> >
>>> >
>>> >
>>> > Index: usb-4.4/drivers/hid/usbhid/hid-core.c
>>> > ===
>>> > --- usb-4.4.orig/drivers/hid/usbhid/hid-core.c
>>> > +++ usb-4.4/drivers/hid/usbhid/hid-core.c
>>> > @@ -1393,8 +1393,11 @@ static void usbhid_disconnect(struct usb
>>> >
>>> >  static void hid_cancel_delayed_stuff(struct usbhid_device *usbhid)
>>> >  {
>>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled A\n");
>>> > del_timer_sync(>io_retry);
>>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled B\n");
>>> > cancel_work_sync(>reset_work);
>>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled C\n");
>>> >  }
>>> >
>>> >  static void hid_cease_io(struct usbhid_device *usbhid)
>>> >
>>>
>>> With your patch I get the dmesg attached.
>>
>>> [   22.234758] usbhid irqs disabled A
>>> [   22.234857] usbhid irqs disabled B
>>> [   22.234912] BUG: sleeping function called from invalid context 
>>> atkernel/workqueue.c:2688
>>
>> That's a smoking gun.  It means everyone has been looking in the wrong
>> place.  Can you provide an objdump listing of usbhid_close()?  The
>> routine starts like this:
>>
>> void usbhid_close(struct hid_device *hid)
>> {
>> struct usbhid_device *usbhid = hid->driver_data;
>>
>> mutex_lock(_open_mut);
>>
>> /* protecting hid->open to make sure we don't restart
>>  * data acquistion due to a resumption we no longer
>>  * care about
>>  */
>> spin_lock_irq(>lock);
>> if (!--hid->open) {
>> spin_unlock_irq(>lock);
>> hid_cancel_delayed_stuff(usbhid);
>>
>> It appears that the spin_unlock_irq() call isn't working.
>>
>> For extra thoroughness, try putting one of those raw_irqs_disabled()
>> checks just before and one just after the spin_lock_irq() line above.
>> Maybe also before the mutex_lock() line.
>>
>> Alan Stern
>>
>
> Hmm, we are there where I was looking at...
>
> Please, read the reply of Jiri [1], we did some tweaking.
> With CONFIG_FTRACE=n and CONFIG_PROVE_LOCKING=n !
>

Shall I enable CONFIG_TRACE_IRQFLAGS (CONFIG_PROVE_LOCKING=n disables it)?

- Sedat -
--
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 v8] usb: musb: da8xx: Add DT support for the DA8xx driver

[Petr Kulhavy]

Hello

On 07.03.2016 14:29, Sergei Shtylyov wrote:

Hello.

On 3/7/2016 2:20 PM, Petr Kulhavy wrote:

@@ -544,17 +643,25 @@ static int da8xx_probe(struct platform_device 
*pdev)

  pinfo.data = pdata;
  pinfo.size_data = sizeof(*pdata);

+ret = regulator_enable(glue->vbus_supply);


   What does this achieve with a regulator that can't be explicitly 
controlled?




If this is a _real_ regulator, then it needs to be enabled by somebody, 
doesn't it?



+if (ret) {
+dev_err(>dev, "failed to enable power: %d\n", ret);
+goto err6;
+}
+
  glue->musb = musb = platform_device_register_full();
  if (IS_ERR(musb)) {
  ret = PTR_ERR(musb);
  dev_err(>dev, "failed to register musb device: %d\n", 
ret);

-goto err6;
+goto err7;
  }

  return 0;
+err7:
+usb_phy_generic_unregister(glue->phy);

  err6:
-usb_phy_generic_unregister(glue->phy);
+regulator_disable(glue->vbus_supply);


   How's that? Aren't you envaling Vbus regulator *after* registering 
PHY?



No, as you can see just a few lines above in the patch.


[...]
@@ -582,11 +690,20 @@ static int da8xx_remove(struct platform_device 
*pdev)

  return 0;
  }

+static const struct of_device_id da8xx_id_table[] = {
+{
+.compatible = "ti,da830-musb",
+},
+{},
+};
+MODULE_DEVICE_TABLE(of, da8xx_id_table);
+
  static struct platform_driver da8xx_driver = {
  .probe= da8xx_probe,
  .remove= da8xx_remove,
  .driver= {
  .name= "musb-da8xx",
+.of_match_table = of_match_ptr(da8xx_id_table),


   Doesn't this cause a warning about in non-DT case?


No, the of_match_table is defined independent of the CONFIG_OF.

Regards
Petr
--
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] usbhid: Fix lockdep unannotated irqs-off warning

[Sedat Dilek]

On Sun, Mar 6, 2016 at 6:23 PM, Alan Stern  wrote:
> On Sat, 5 Mar 2016, Sedat Dilek wrote:
>
>> On Fri, Mar 4, 2016 at 5:04 PM, Alan Stern  wrote:
>> > On Wed, 2 Mar 2016, Sedat Dilek wrote:
>> >
>> >> On 3/1/16, Alan Stern  wrote:
>> >> > On Tue, 1 Mar 2016, Sedat Dilek wrote:
>> >> >
>> >> >> On Tue, Oct 13, 2015 at 2:57 AM, Steven Rostedt 
>> >> >> wrote:
>> >> >> > On Sat, 3 Oct 2015 12:05:42 +0200
>> >> >> > Sedat Dilek  wrote:
>> >> >> >
>> >> >> >> So, at the beginning... dunno WTF is causing the problems - no
>> >> >> >> workaround for CLANG.
>> >> >> >
>> >> >> > Probably need to compile with gcc and with clang and look at the 
>> >> >> > binary
>> >> >> > differences. Or at least what objdump shows.
>> >> >> >
>> >> >>
>> >> >> [ Hope to address this issue to the correct people - CCed some people
>> >> >> I taped on their nerves ]
>> >> >>
>> >> >> Not sure if I should open a new thread?
>> >> >> Please, some clear statements on this.
>> >> >> Thanks.
>> >> >>
>> >> >> The issue is still visible and alive.
>> >
>> > I think it would be worthwhile to doublecheck the time at which
>> > interrupts get disabled.  Sedat, please try your plug/unplug the USB
>> > mouse test with the patch below.
>> >
>> > Alan Stern
>> >
>> >
>> >
>> > Index: usb-4.4/drivers/hid/usbhid/hid-core.c
>> > ===
>> > --- usb-4.4.orig/drivers/hid/usbhid/hid-core.c
>> > +++ usb-4.4/drivers/hid/usbhid/hid-core.c
>> > @@ -1393,8 +1393,11 @@ static void usbhid_disconnect(struct usb
>> >
>> >  static void hid_cancel_delayed_stuff(struct usbhid_device *usbhid)
>> >  {
>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled A\n");
>> > del_timer_sync(>io_retry);
>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled B\n");
>> > cancel_work_sync(>reset_work);
>> > +   if (raw_irqs_disabled())  pr_info("usbhid irqs disabled C\n");
>> >  }
>> >
>> >  static void hid_cease_io(struct usbhid_device *usbhid)
>> >
>>
>> With your patch I get the dmesg attached.
>
>> [   22.234758] usbhid irqs disabled A
>> [   22.234857] usbhid irqs disabled B
>> [   22.234912] BUG: sleeping function called from invalid context 
>> atkernel/workqueue.c:2688
>
> That's a smoking gun.  It means everyone has been looking in the wrong
> place.  Can you provide an objdump listing of usbhid_close()?  The
> routine starts like this:
>
> void usbhid_close(struct hid_device *hid)
> {
> struct usbhid_device *usbhid = hid->driver_data;
>
> mutex_lock(_open_mut);
>
> /* protecting hid->open to make sure we don't restart
>  * data acquistion due to a resumption we no longer
>  * care about
>  */
> spin_lock_irq(>lock);
> if (!--hid->open) {
> spin_unlock_irq(>lock);
> hid_cancel_delayed_stuff(usbhid);
>
> It appears that the spin_unlock_irq() call isn't working.
>
> For extra thoroughness, try putting one of those raw_irqs_disabled()
> checks just before and one just after the spin_lock_irq() line above.
> Maybe also before the mutex_lock() line.
>
> Alan Stern
>

Hmm, we are there where I was looking at...

Please, read the reply of Jiri [1], we did some tweaking.
With CONFIG_FTRACE=n and CONFIG_PROVE_LOCKING=n !

*** Part one: ObjectDump of hid-core.o ***

$ objdump -D drivers/hid/usbhid/hid-core.o | awk '/<[^>]*>:$/ { p=0; }
/:/ { p=1; } { if (p) print $0; }' >
../objdump-D_hid-core_o_usbhid_close_$(uname -r).txt

$ cat ../objdump-D_hid-core_o_usbhid_close_4.4.4-1-iniza-small.txt
02e0 :
 2e0:   55  push   %rbp
 2e1:   48 89 e5mov%rsp,%rbp
 2e4:   41 57   push   %r15
 2e6:   41 56   push   %r14
 2e8:   41 54   push   %r12
 2ea:   53  push   %rbx
 2eb:   49 89 ffmov%rdi,%r15
 2ee:   4d 8b b7 e8 1e 00 00mov0x1ee8(%r15),%r14
 2f5:   48 c7 c7 00 00 00 00mov$0x0,%rdi
 2fc:   31 f6   xor%esi,%esi
 2fe:   e8 00 00 00 00  callq  303 
 303:   49 8d 9e 88 28 00 00lea0x2888(%r14),%rbx
 30a:   48 89 dfmov%rbx,%rdi
 30d:   e8 00 00 00 00  callq  312 
 312:   41 ff 8f e4 1d 00 00decl   0x1de4(%r15)
 319:   9c  pushfq
 31a:   41 5c   pop%r12
 31c:   48 89 dfmov%rbx,%rdi
 31f:   e8 00 00 00 00  callq  324 
 324:   41 54   push   %r12
 326:   9d  popfq
 327:   75 23   jne34c 

Re: [PATCH 5/5 v8] usb: musb: da8xx: Add DT support for the DA8xx driver

[Sergei Shtylyov]

Hello.

On 3/7/2016 2:20 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 
Acked-by: Sergei Shtylyov 

[...]


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

[...]

@@ -544,17 +643,25 @@ static int da8xx_probe(struct platform_device *pdev)
pinfo.data = pdata;
pinfo.size_data = sizeof(*pdata);

+   ret = regulator_enable(glue->vbus_supply);


   What does this achieve with a regulator that can't be explicitly controlled?


+   if (ret) {
+   dev_err(>dev, "failed to enable power: %d\n", ret);
+   goto err6;
+   }
+
glue->musb = musb = platform_device_register_full();
if (IS_ERR(musb)) {
ret = PTR_ERR(musb);
dev_err(>dev, "failed to register musb device: %d\n", 
ret);
-   goto err6;
+   goto err7;
}

return 0;
+err7:
+   usb_phy_generic_unregister(glue->phy);

  err6:
-   usb_phy_generic_unregister(glue->phy);
+   regulator_disable(glue->vbus_supply);


   How's that? Aren't you envaling Vbus regulator *after* registering PHY?

[...]

@@ -582,11 +690,20 @@ static int da8xx_remove(struct platform_device *pdev)
return 0;
  }

+static const struct of_device_id da8xx_id_table[] = {
+   {
+   .compatible = "ti,da830-musb",
+   },
+   {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_id_table);
+
  static struct platform_driver da8xx_driver = {
.probe  = da8xx_probe,
.remove = da8xx_remove,
.driver = {
.name   = "musb-da8xx",
+   .of_match_table = of_match_ptr(da8xx_id_table),


   Doesn't this cause a warning about in non-DT case?

[...]

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 v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad controller binding

[Thierry Reding]

On Fri, Mar 04, 2016 at 10:31:45PM -0600, Rob Herring wrote:
> On Fri, Mar 04, 2016 at 05:19:31PM +0100, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > The NVIDIA Tegra XUSB pad controller provides a set of pads, each with a
> > set of lanes that are used for PCIe, SATA and USB.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> > Changes in v10:
> > - clarify that the hardware documentation means something different when
> >   referring to a "port" (intra-SoC connectivity)
> > 
> > Changes in v9:
> > - rename UTMI -> USB2 to match hardware documentation
> > - reword according to suggestions by Stephen Warren
> > - make Tegra132 compatible string list consistent
> > - remove mailbox support
> > 
> >  .../bindings/phy/nvidia,tegra124-xusb-padctl.txt   | 376 
> > +
> >  .../pinctrl/nvidia,tegra124-xusb-padctl.txt|   5 +
> >  2 files changed, 381 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> 
> Without really understanding the h/w here, looks okay to me.
> 
> Acked-by: Rob Herring 
> 
> > +SoC include:
> > +
> > +   padctl@0,7009f000 {
> 
> Drop the comma. Commas should only be used if there are distinct fields.
> 
> If I get my dtc patch done, these are going to start warning, so you 
> might want to go fix dts files (assuming that's where this is coming 
> from).

I noticed that in today's next the updated DTC already complains about a
lot of things in existing DTS files. For Tegra that's primarily the
memory node, because it has a reg property but no unit name. Any hints
as to how to solve that? I think I remember from way back that memory
was supposed to be an exception, perhaps DTC needs to be taught that?

Thierry


signature.asc
Description: PGP signature

[PATCH 1/5 v8] 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"

 .../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

[PATCH 5/5 v8] 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 
Acked-by: Sergei Shtylyov 
---
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

 drivers/usb/musb/da8xx.c | 121 ++-
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b03d3b8..bbd8cac 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"
 
@@ -87,6 +92,7 @@ struct da8xx_glue {
struct platform_device  *musb;
struct platform_device  *phy;
struct clk  *clk;
+   struct regulator*vbus_supply;
 };
 
 /*
@@ -134,6 +140,48 @@ 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 regulator *reg)
+{
+   int current_uA;
+
+   current_uA = regulator_get_current_limit(reg);
+   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 +530,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 +544,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 +570,50 @@ 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 2/5 v8] 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

 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 4/5 v8] 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: 

 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 3/5 v8] 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: 

 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

Re: [PATCH 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes

[Felipe Ferreri Tonello]

Hi Balbi, how are you?

On 07/03/16 10:59, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello  writes:
> "Felipe F. Tonello"  writes:
>> [ text/plain ]
>> This gadget uses a bmAttributes and MaxPower that requires the USB
> bus to be
>> powered from the host, which is not correct because this
> configuration is device
>> specific, not a USB-MIDI requirement.
>>
>> This patch adds two modules parameters, bmAttributes and MaxPower,
> that allows
>> the user to set those flags. The default values are what the gadget
> used to use
>> for backward compatibility.
>>
>> Signed-off-by: Felipe F. Tonello 
>> ---
>>  drivers/usb/gadget/legacy/gmidi.c | 14 --
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/legacy/gmidi.c
> b/drivers/usb/gadget/legacy/gmidi.c
>> index fc2ac150f5ff..0553435cc360 100644
>> --- a/drivers/usb/gadget/legacy/gmidi.c
>> +++ b/drivers/usb/gadget/legacy/gmidi.c
>> @@ -63,6 +63,14 @@ static unsigned int out_ports = 1;
>>  module_param(out_ports, uint, S_IRUGO);
>>  MODULE_PARM_DESC(out_ports, "Number of MIDI output ports");
>>  
>> +static unsigned int bmAttributes = USB_CONFIG_ATT_ONE;
>> +module_param(bmAttributes, uint, S_IRUGO);
>> +MODULE_PARM_DESC(bmAttributes, "Configuration Descriptor's
> bmAttributes parameter");
>> +
>> +static unsigned int MaxPower = CONFIG_USB_GADGET_VBUS_DRAW;
>> +module_param(MaxPower, uint, S_IRUGO);
>> +MODULE_PARM_DESC(MaxPower, "Used to calculate Configuration
> Descriptor's bMaxPower parameter");
>
> you didn't run checkpatch, did you ? Also, are you sure you will need
> to
> change this by simply reloading the module ? I'm not convinced.

 Yes I always run checkpatch :)
>>>
>>> do you really ? Why do you have a 98-character line, then ?
>>>
 What do you mean by reloading the module?
>>>
>>> modprobe g_midi MaxPower=4
>>> modprobe -r g_midi
>>> modprobe g_midi MaxPower=10
>>>
>>> I'm not convinced this is a valid use-case. Specially since you can just
>>> provide several configurations and let the host choose the one it suits
>>> it best.
>>
>> Ok, I will further test it.
>>
>>>
>> @@ -119,8 +127,8 @@ static struct usb_configuration midi_config = {
>>  .label  = "MIDI Gadget",
>>  .bConfigurationValue = 1,
>>  /* .iConfiguration = DYNAMIC */
>> -.bmAttributes   = USB_CONFIG_ATT_ONE,
>
> nack, nackety nack nack nack :-)
>
> USB_CONFIG_ATT_ONE *must* always be set. With your module parameter you
> give users the oportunity to violate USB spec.

 You are right. It needs to check the value before it assigns to
 bmAttributes.
>>>
>>> why check ? You can just unconditionally or USB_CONFIG_ATT_ONE. In any
>>> case, I'm not convinced this is necessary at all.
>>
>> Right.
>>
>> This is necessary because this driver is actually wrong in which is
>> asking for the host to power itself. This is not specified on USB-MIDI
>> specification, neither makes any sense since this configuration is
>> device specific.
>>
>> What is your suggestion to make it configurable? Maybe at compile-time?
>> I really don't know what is the best solution if this is not something
>> you like it.
> 
> well, you could use our configfs-based gadget interface. You don't
> really need to use gmidi.ko at all. In fact, we wanna do away with any
> static modules and rely only on configfs. If configfs doesn't let you
> change what you want/need, then we can talk about adding support for
> those.
> 
> bMaxPower and bmAttributes sound like good things to have configurable
> over configfs but beware of what the USB specification says about them,
> we cannot let users violate the spec by passing bogus values on these
> fields.

I agree that we should move to configfs, but the truth is that these
legacy devices are still useful. They just do one thing, mostly, but its
easy and simple to setup and use. So I think before we have some sort of
preset library of configfs-based gadget drivers, we still need these
modules.

Any suggestions on that?

Do you want to support what I am proposing for gmidi.ko or just ignore
it and move on to configfs?

Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes

[Felipe Balbi]

Hi,

Felipe Ferreri Tonello  writes:
 "Felipe F. Tonello"  writes:
> [ text/plain ]
> This gadget uses a bmAttributes and MaxPower that requires the USB
 bus to be
> powered from the host, which is not correct because this
 configuration is device
> specific, not a USB-MIDI requirement.
>
> This patch adds two modules parameters, bmAttributes and MaxPower,
 that allows
> the user to set those flags. The default values are what the gadget
 used to use
> for backward compatibility.
>
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/legacy/gmidi.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/gmidi.c
 b/drivers/usb/gadget/legacy/gmidi.c
> index fc2ac150f5ff..0553435cc360 100644
> --- a/drivers/usb/gadget/legacy/gmidi.c
> +++ b/drivers/usb/gadget/legacy/gmidi.c
> @@ -63,6 +63,14 @@ static unsigned int out_ports = 1;
>  module_param(out_ports, uint, S_IRUGO);
>  MODULE_PARM_DESC(out_ports, "Number of MIDI output ports");
>  
> +static unsigned int bmAttributes = USB_CONFIG_ATT_ONE;
> +module_param(bmAttributes, uint, S_IRUGO);
> +MODULE_PARM_DESC(bmAttributes, "Configuration Descriptor's
 bmAttributes parameter");
> +
> +static unsigned int MaxPower = CONFIG_USB_GADGET_VBUS_DRAW;
> +module_param(MaxPower, uint, S_IRUGO);
> +MODULE_PARM_DESC(MaxPower, "Used to calculate Configuration
 Descriptor's bMaxPower parameter");

 you didn't run checkpatch, did you ? Also, are you sure you will need
 to
 change this by simply reloading the module ? I'm not convinced.
>>>
>>> Yes I always run checkpatch :)
>> 
>> do you really ? Why do you have a 98-character line, then ?
>> 
>>> What do you mean by reloading the module?
>> 
>> modprobe g_midi MaxPower=4
>> modprobe -r g_midi
>> modprobe g_midi MaxPower=10
>> 
>> I'm not convinced this is a valid use-case. Specially since you can just
>> provide several configurations and let the host choose the one it suits
>> it best.
>
> Ok, I will further test it.
>
>> 
> @@ -119,8 +127,8 @@ static struct usb_configuration midi_config = {
>   .label  = "MIDI Gadget",
>   .bConfigurationValue = 1,
>   /* .iConfiguration = DYNAMIC */
> - .bmAttributes   = USB_CONFIG_ATT_ONE,

 nack, nackety nack nack nack :-)

 USB_CONFIG_ATT_ONE *must* always be set. With your module parameter you
 give users the oportunity to violate USB spec.
>>>
>>> You are right. It needs to check the value before it assigns to
>>> bmAttributes.
>> 
>> why check ? You can just unconditionally or USB_CONFIG_ATT_ONE. In any
>> case, I'm not convinced this is necessary at all.
>
> Right.
>
> This is necessary because this driver is actually wrong in which is
> asking for the host to power itself. This is not specified on USB-MIDI
> specification, neither makes any sense since this configuration is
> device specific.
>
> What is your suggestion to make it configurable? Maybe at compile-time?
> I really don't know what is the best solution if this is not something
> you like it.

well, you could use our configfs-based gadget interface. You don't
really need to use gmidi.ko at all. In fact, we wanna do away with any
static modules and rely only on configfs. If configfs doesn't let you
change what you want/need, then we can talk about adding support for
those.

bMaxPower and bmAttributes sound like good things to have configurable
over configfs but beware of what the USB specification says about them,
we cannot let users violate the spec by passing bogus values on these
fields.

cheers

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usbnet: cleanup after bind() in probe()

[Oliver Neukum]

In case bind() works, but a later error forces bailing
in probe() in error cases work and a timer may be scheduled.
They must be killed. This fixes an error case related to
the double free reported in
http://www.spinics.net/lists/netdev/msg367669.html
and needs to go on top of Linus' fix to cdc-ncm.

Signed-off-by: Oliver Neukum 
---
 drivers/net/usb/usbnet.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 0b0ba7e..1079812 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1769,6 +1769,13 @@ out3:
if (info->unbind)
info->unbind (dev, udev);
 out1:
+   /* subdrivers must undo all they did in bind() if they
+* fail it, but we may fail later and a deferred kevent
+* may trigger an error resubmitting itself and, worse,
+* schedule a timer. So we kill it all just in case.
+*/
+   cancel_work_sync(>kevent);
+   del_timer_sync(>delay);
free_netdev(net);
 out:
return status;
-- 
2.1.4

--
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: musb: tx fifo flush warning

[David Laight]

From: John Ogness
> Sent: 04 March 2016 10:51
> 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?

Spinning for 1ms+ isn't great.
Sounds like some major refactoring is needed so that the flush can be
asynchronous.

David

--
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 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes

[Felipe Ferreri Tonello]

Hi Balbi,

On 07/03/16 07:34, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello  writes:
>> [ text/plain ]
>> Hi Balbi, 
>>
>> On March 4, 2016 7:16:42 AM GMT+00:00, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> "Felipe F. Tonello"  writes:
 [ text/plain ]
 This gadget uses a bmAttributes and MaxPower that requires the USB
>>> bus to be
 powered from the host, which is not correct because this
>>> configuration is device
 specific, not a USB-MIDI requirement.

 This patch adds two modules parameters, bmAttributes and MaxPower,
>>> that allows
 the user to set those flags. The default values are what the gadget
>>> used to use
 for backward compatibility.

 Signed-off-by: Felipe F. Tonello 
 ---
  drivers/usb/gadget/legacy/gmidi.c | 14 --
  1 file changed, 12 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/gadget/legacy/gmidi.c
>>> b/drivers/usb/gadget/legacy/gmidi.c
 index fc2ac150f5ff..0553435cc360 100644
 --- a/drivers/usb/gadget/legacy/gmidi.c
 +++ b/drivers/usb/gadget/legacy/gmidi.c
 @@ -63,6 +63,14 @@ static unsigned int out_ports = 1;
  module_param(out_ports, uint, S_IRUGO);
  MODULE_PARM_DESC(out_ports, "Number of MIDI output ports");
  
 +static unsigned int bmAttributes = USB_CONFIG_ATT_ONE;
 +module_param(bmAttributes, uint, S_IRUGO);
 +MODULE_PARM_DESC(bmAttributes, "Configuration Descriptor's
>>> bmAttributes parameter");
 +
 +static unsigned int MaxPower = CONFIG_USB_GADGET_VBUS_DRAW;
 +module_param(MaxPower, uint, S_IRUGO);
 +MODULE_PARM_DESC(MaxPower, "Used to calculate Configuration
>>> Descriptor's bMaxPower parameter");
>>>
>>> you didn't run checkpatch, did you ? Also, are you sure you will need
>>> to
>>> change this by simply reloading the module ? I'm not convinced.
>>
>> Yes I always run checkpatch :)
> 
> do you really ? Why do you have a 98-character line, then ?
> 
>> What do you mean by reloading the module?
> 
> modprobe g_midi MaxPower=4
> modprobe -r g_midi
> modprobe g_midi MaxPower=10
> 
> I'm not convinced this is a valid use-case. Specially since you can just
> provide several configurations and let the host choose the one it suits
> it best.

Ok, I will further test it.

> 
 @@ -119,8 +127,8 @@ static struct usb_configuration midi_config = {
.label  = "MIDI Gadget",
.bConfigurationValue = 1,
/* .iConfiguration = DYNAMIC */
 -  .bmAttributes   = USB_CONFIG_ATT_ONE,
>>>
>>> nack, nackety nack nack nack :-)
>>>
>>> USB_CONFIG_ATT_ONE *must* always be set. With your module parameter you
>>> give users the oportunity to violate USB spec.
>>
>> You are right. It needs to check the value before it assigns to
>> bmAttributes.
> 
> why check ? You can just unconditionally or USB_CONFIG_ATT_ONE. In any
> case, I'm not convinced this is necessary at all.

Right.

This is necessary because this driver is actually wrong in which is
asking for the host to power itself. This is not specified on USB-MIDI
specification, neither makes any sense since this configuration is
device specific.

What is your suggestion to make it configurable? Maybe at compile-time?
I really don't know what is the best solution if this is not something
you like it.

Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes

[Felipe Ferreri Tonello]

Hi Balbi,

On 07/03/16 07:35, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello  writes:
>> [ text/plain ]
>> Hi Michal, 
>>
>> On March 5, 2016 4:28:45 PM GMT+00:00, Michal Nazarewicz  
>> wrote:
> On Wed, Mar 02 2016, Felipe F. Tonello wrote:
>> @@ -16,7 +16,7 @@
>>   *   Copyright (C) 2006 Thumtronics Pty Ltd.
>>   *   Ben Williamson 
>>   *
>> - * Licensed under the GPL-2 or later.
>> + * Licensed under the GPLv2.
>>>
 On March 4, 2016 7:17:31 PM GMT+00:00, Michal Nazarewicz
>>>  wrote:
> Any particular reason to do that?
>>>
>>> On Fri, Mar 04 2016, Felipe Ferreri Tonello wrote:
 Because the kernel is v2 only and not later. 
>>>
>>> Linux as a whole is GPLv2 only, but that doesn’t necessarily mean that
>>> parts of it cannot be dual licensed (or GPLv2+).  It’s safer to leave
>>> copyright noticed clear unless you explicitly want your contribution be
>>> GPLv2 only which brings the whole file GPLv2 only.
>>>
 I just tried to make this driver more consistent with the coding
>>> style
 used across the kernel. That's it.
>>>
>>> Column alignment of field names or RHS of assignment operators is quite
>>> inconsistent already within drivers/usb/gadget/ which is why I’m
>>> concerned whether this is really helping.
>>>
>>> Anyway, I actually don’t care much, just adding my two rappen.
>>
>> Right, I am ok with Balbi completely ignoring this patch. But I prefer
>> to have at least this driver consistent than nothing. Of course I'll
>> remove the license change I made.
> 
> consistent in what way ?

Source-code.

The goal of this patch is to update this driver coding style to promote
consistency, readability, and maintainability based on the Linux coding
style.

If this patch does not achieving that or if that is not necessary, than
just ignore this patch.

Thanks,
Felipe


0x92698E6A.asc
Description: application/pgp-keys

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

[Felipe Ferreri Tonello]

Hi Balbi,

On 07/03/16 07:32, Felipe Balbi wrote:
> 
> Hi,
> 
> (please break your lines at 80-characters, have a look at
> Documentation/email-clients.txt if needed)
> 
> Felipe Ferreri Tonello  writes:
>> [ text/plain ]
>> Hi Balbi, 
>>
>> On March 4, 2016 7:20:10 AM GMT+00:00, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> "Felipe F. Tonello"  writes:
 [ text/plain ]
 Since f_midi_transmit is called by both ALSA and USB frameworks, it
>>> can
 potentially cause a race condition between both calls. This is bad
>>> because the
 way f_midi_transmit is implemented can't handle concurrent calls.
>>> This is due
 to the fact that the usb request fifo looks for the next element and
>>> only if
 it has data to process it enqueues the request, otherwise re-uses it.
>>> If both
 (ALSA and USB) frameworks calls this function at the same time, the
 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.

 On benchmarks realized by me, spinlocks were more efficient then
>>> scheduling
 the f_midi_transmit tasklet in process context and using a mutex to
 synchronize. Also it performs better then previous implementation
>>> that
 allocated a usb_request for every new transmit made.
>>>
>>> behaves better in what way ? Also, previous implementation would not
>>> suffer from this concurrency problem, right ?
>>
>> The spin lock is faster than allocating usb requests all the time,
>> even if the udc uses da for it.
> 
> did you measure ? Is the extra speed really necessary ? How did you
> benchmark this ?

Yes I did measure and it was not that significant. This is not about
speed. There was a bug in that approach that I already explained on that
patch, which was approved and applied BTW.

Any way, this spinlock should've been there since that patch but I
couldn't really trigger this problem without a stress test.

So, this patch fixes a bug in the current implementation.

Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH 5/5] usb: gadget: f_midi: updated copyright

[Felipe Ferreri Tonello]

Hi Balbi,

On 07/03/16 07:36, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello  writes:
>> [ text/plain ]
>> Hi Balbi, 
>>
>> On March 4, 2016 7:13:05 AM GMT+00:00, Felipe Balbi  wrote:
>>> "Felipe F. Tonello"  writes:
 [ text/plain ]
 Signed-off-by: Felipe F. Tonello 
>>>
>>> no commit log == no commit
>>
>> Got it. 
>>
>>>
 ---
  drivers/usb/gadget/function/f_midi.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/usb/gadget/function/f_midi.c
>>> b/drivers/usb/gadget/function/f_midi.c
 index 9a9e6112e224..5c7f5c780fda 100644
 --- a/drivers/usb/gadget/function/f_midi.c
 +++ b/drivers/usb/gadget/function/f_midi.c
 @@ -5,6 +5,9 @@
   * Developed for Thumtronics by Grey Innovation
   * Ben Williamson 
   *
 + * Copyright (C) 2015,2016 ROLI Ltd.
 + * Felipe F. Tonello 
>>>
>>> Did you check with your company's lawyer that your changes are enough
>>> to
>>> justify a copyright ?
>>
>> Yes. Specially if that state machine refractor gets approved. TBH I
>> can't see it won't.
> 
> okay, so did that same lawyer tell you to change the driver's license ?
> 

No. That was my bad call. TBH I really don't care about this copyright.
You can just ignore this patch and patch 4.

Thanks

Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: Possible double-free in the usbnet driver

[Dmitry Vyukov]

On Fri, Mar 4, 2016 at 11:43 PM, Linus Torvalds
 wrote:
> On Fri, Mar 4, 2016 at 2:26 PM, Andrey Konovalov  wrote:
>>
>> and when I run the vm and connect the device I get:
>>
>> [   23.672662] cdc_ncm 1-1:1.6: bind() failure
>> [   23.673447] usbnet_probe(): freeing netdev: 88006ab48000
>> [   23.675822] usbnet_probe(): freeing netdev: 88006ab48000
>>
>> So this seems to be a double-free (or at least a double free_netdev()
>> call), but the object gets freed twice from usbnet_probe() and not
>> from usbnet_disconnect(), so you're right that the latter doesn't get
>> called. I'm not sure how usbnet_probe() ends up being called twice.
>
> I still don't think it's a double free. I think the probe thing is
> called twice, and that's why to different allocations get free'd twice
> (and the reason it's the same pointer is that the same allocation got
> reused)


FYI, we have a KASAN patch in flight that adds "quarantine" for freed
memory (memory is reused only after a significant delay). It should
help to easily differentiate between use-after-free, double-free and
heap-out-of-bound. Yes, now it is confusing.


> But I don't know that driver, really.
>
>>> Which particular usbnet bind routine is this? There are multiple
>>> sub-drivers for usbnet that all do different things.
>>
>> cdc_ncm_bind()
>
> Ok, so that matches my theory.
>
> Does this attached stupid patch make the warning go away? Because from
> what I can tell, usbnet_link_change() really will start using that
> "dev" that simply isn't valid - and will be free'd - if the bind
> fails.
>
> So you have usbnet_defer_kevent() getting triggered, which in turn
> ends up using "usbnet->kevent"
>
> But somebody like Oliver is really the right person to check this. For
> example, it's entirely possible that we should just instead do
>
> cancel_work_sync(>kevent);
>
> before the "free_netdev(net)" in the "out1" label.
>
> And there might be other things that bind() can have touched than the
> kevent workqueue.
>
>Linus
--
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/7] usb: add support for Intel dual role port mux

[Lu Baolu]

Intel SOC chips are featured with USB dual role. The host role is
provided by Intel xHCI IP, and the gadget role is provided by IP
from designware. Tablet platform designs always share a single
port for both host and gadget controllers. There is a mux to
switch the port to the right controller according to the cable
type. OS needs to provide the callback to control the mux when
a plug-in event raises. The method to control the mux is platform
dependent. At least three types of implementation can be found
across current devices. 1) GPIO pins; 2) a unit which can be
controlled by memory mapped registers; 3) ACPI ASL code.

This patch series adds supports for Intel dual role port mux.
It includes:
(1) A helper layer on top of extcon for individual mux driver.
It listens to the USB-HOST extcon cable and call the switch
call-back when the cable state changes.
(2) Drivers for GPIO controlled port mux which could be found on
Baytrail devices. A mfd driver is used to split the GPIOs into
USB gpio extcon device and a USB mux device. Driver for USB
gpio extcon device is already in upstream Linux. This patch
series includes a driver for GPIO USB mux.
(3) Drivers for USB port mux controlled through memory mapped
registers and the logic to create the mux device. This type
of dual role port mux could be found in Cherry Trail and
Broxton devices.

Lu Baolu (7):
  extcon: usb-gpio: add device binding for platform device
  extcon: usb-gpio: add support for ACPI gpio interface
  usb: mux: add common code for Intel dual role port mux
  usb: mux: add driver for Intel gpio controlled port mux
  usb: mux: add driver for Intel drcfg controlled port mux
  usb: pci-quirks: add Intel USB drcfg mux device
  mfd: intel_vuport: Add Intel virtual USB port MFD Driver

 Documentation/ABI/testing/sysfs-bus-platform |  15 +++
 MAINTAINERS  |  10 ++
 drivers/extcon/extcon-usb-gpio.c |  10 +-
 drivers/mfd/Kconfig  |   8 ++
 drivers/mfd/Makefile |   1 +
 drivers/mfd/intel-vuport.c   |  78 +
 drivers/usb/Kconfig  |   2 +
 drivers/usb/Makefile |   1 +
 drivers/usb/host/pci-quirks.c|  47 +++-
 drivers/usb/host/xhci-ext-caps.h |   2 +
 drivers/usb/mux/Kconfig  |  25 
 drivers/usb/mux/Makefile |   6 +
 drivers/usb/mux/intel-mux-drcfg.c| 161 ++
 drivers/usb/mux/intel-mux-gpio.c | 125 
 drivers/usb/mux/intel-mux.c  | 166 +++
 include/linux/usb/intel-mux.h|  47 
 16 files changed, 701 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mfd/intel-vuport.c
 create mode 100644 drivers/usb/mux/Kconfig
 create mode 100644 drivers/usb/mux/Makefile
 create mode 100644 drivers/usb/mux/intel-mux-drcfg.c
 create mode 100644 drivers/usb/mux/intel-mux-gpio.c
 create mode 100644 drivers/usb/mux/intel-mux.c
 create mode 100644 include/linux/usb/intel-mux.h

Change log:

v1->v2:
 - move mux driver from drivers/usb/misc to drivers/usb/mux;
 - replace debugfs with sysfs for user level mux control;
 - remove unnecessary register restore if mux registeration failed;
 - Add "Acked-by: Chanwoo Choi " to extcon changes;
 - Make the file names and exported function names more specific;
 - Remove the usb_mux_get_dev() interface;
 - Move "struct intel_usb_mux" from .h to .c file;
 - Fix various kbuild robot warnings.

-- 
2.1.4

--
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 3/7] usb: mux: add common code for Intel dual role port mux

[Lu Baolu]

Several Intel PCHs and SOCs have an internal mux that is used to
share one USB port between device controller and host controller.

A usb port mux could be abstracted as the following elements:
1) mux state: HOST or PERIPHERAL;
2) an extcon cable which triggers the change of mux state between
   HOST and PERIPHERAL;
3) The required action to do the real port switch.

This patch adds the common code to handle usb port mux. With this
common code, the individual mux driver, which always is platform
dependent, could focus on the real operation of mux switch.

Signed-off-by: Lu Baolu 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Felipe Balbi 
---
 Documentation/ABI/testing/sysfs-bus-platform |  15 +++
 MAINTAINERS  |   7 ++
 drivers/usb/Kconfig  |   2 +
 drivers/usb/Makefile |   1 +
 drivers/usb/mux/Kconfig  |   9 ++
 drivers/usb/mux/Makefile |   4 +
 drivers/usb/mux/intel-mux.c  | 166 +++
 include/linux/usb/intel-mux.h|  47 
 8 files changed, 251 insertions(+)
 create mode 100644 drivers/usb/mux/Kconfig
 create mode 100644 drivers/usb/mux/Makefile
 create mode 100644 drivers/usb/mux/intel-mux.c
 create mode 100644 include/linux/usb/intel-mux.h

diff --git a/Documentation/ABI/testing/sysfs-bus-platform 
b/Documentation/ABI/testing/sysfs-bus-platform
index 5172a61..a2261cb 100644
--- a/Documentation/ABI/testing/sysfs-bus-platform
+++ b/Documentation/ABI/testing/sysfs-bus-platform
@@ -18,3 +18,18 @@ Description:
devices to opt-out of driver binding using a driver_override
name such as "none".  Only a single driver may be specified in
the override, there is no support for parsing delimiters.
+
+What:  /sys/bus/platform/devices/.../intel_mux
+Date:  Febuary 2016
+Contact:   Lu Baolu 
+Description:
+   In some platforms, a single USB port is shared between a USB 
host
+   controller and a device controller. A USB mux driver is needed 
to
+   handle the port mux. intel_mux attribute shows and stores the 
mux
+   state.
+   For read:
+   'peripheral' - mux switched to PERIPHERAL controller;
+   'host'   - mux switched to HOST controller.
+   For write:
+   'peripheral' - mux will be switched to PERIPHERAL controller;
+   'host'   - mux will be switched to HOST controller.
diff --git a/MAINTAINERS b/MAINTAINERS
index c55b37e..a93d26a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11400,6 +11400,13 @@ T: git 
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
 S: Maintained
 F: drivers/usb/phy/
 
+USB PORT MUX DRIVER
+M: Lu Baolu 
+L: linux-usb@vger.kernel.org
+S: Supported
+F: drivers/usb/mux/intel-mux.c
+F: include/linux/usb/intel-mux.h
+
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451d..dbd6620 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -149,6 +149,8 @@ endif # USB
 
 source "drivers/usb/phy/Kconfig"
 
+source "drivers/usb/mux/Kconfig"
+
 source "drivers/usb/gadget/Kconfig"
 
 config USB_LED_TRIG
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..9a92338 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -6,6 +6,7 @@
 
 obj-$(CONFIG_USB)  += core/
 obj-$(CONFIG_USB_SUPPORT)  += phy/
+obj-$(CONFIG_USB_SUPPORT)  += mux/
 
 obj-$(CONFIG_USB_DWC3) += dwc3/
 obj-$(CONFIG_USB_DWC2) += dwc2/
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
new file mode 100644
index 000..8fedc4f
--- /dev/null
+++ b/drivers/usb/mux/Kconfig
@@ -0,0 +1,9 @@
+#
+# USB port mux driver configuration
+#
+menu "USB Port MUX drivers"
+config INTEL_USB_MUX
+   select EXTCON
+   def_bool n
+
+endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
new file mode 100644
index 000..84f0ae8
--- /dev/null
+++ b/drivers/usb/mux/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for USB port mux drivers
+#
+obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
diff --git a/drivers/usb/mux/intel-mux.c b/drivers/usb/mux/intel-mux.c
new file mode 100644
index 000..b243758
--- /dev/null
+++ b/drivers/usb/mux/intel-mux.c
@@ -0,0 +1,166 @@
+/**
+ * mux.c - USB Port Mux support
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include 

[PATCH v2 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Lu Baolu]

In some Intel platforms, a single usb port is shared between USB host
and device controllers. The shared port is under control of a switch
which is defined in the Intel vendor defined extended capability for
xHCI.

This patch adds the support to detect and create the platform device
for the port mux switch.

Signed-off-by: Lu Baolu 
Reviewed-by: Felipe Balbi 
---
 drivers/usb/host/pci-quirks.c| 47 ++--
 drivers/usb/host/xhci-ext-caps.h |  2 ++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 35af362..6a737cf 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,10 +16,11 @@
 #include 
 #include 
 #include 
+#include 
+
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
-
 #define UHCI_USBLEGSUP 0xc0/* legacy support */
 #define UHCI_USBCMD0   /* command register */
 #define UHCI_USBINTR   4   /* interrupt register */
@@ -78,6 +79,9 @@
 #define USB_INTEL_USB3_PSSEN   0xD8
 #define USB_INTEL_USB3PRM  0xDC
 
+#define DEVICE_ID_INTEL_CHERRYVIEW_XHCI0x22b5
+#define DEVICE_ID_INTEL_BROXTON_M_XHCI 0x0aa8
+
 /*
  * amd_chipset_gen values represent AMD different chipset generations
  */
@@ -956,6 +960,41 @@ void usb_disable_xhci_ports(struct pci_dev *xhci_pdev)
 }
 EXPORT_SYMBOL_GPL(usb_disable_xhci_ports);
 
+static void create_intel_usb_mux_device(struct pci_dev *xhci_pdev,
+   void __iomem *base)
+{
+   struct platform_device *plat_dev;
+   struct property_set pset;
+   int ret;
+
+   struct property_entry pentry[] = {
+   PROPERTY_ENTRY_U64("reg-start",
+   pci_resource_start(xhci_pdev, 0) + 0x80d8),
+   PROPERTY_ENTRY_U64("reg-size", 8),
+   { },
+   };
+
+   if (!xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_INTEL_USB_MUX))
+   return;
+
+   plat_dev = platform_device_alloc("intel-mux-drcfg",
+   PLATFORM_DEVID_AUTO);
+   if (!plat_dev)
+   return;
+
+   plat_dev->dev.parent = _pdev->dev;
+   pset.properties = pentry;
+   platform_device_add_properties(plat_dev, );
+
+   ret = platform_device_add(plat_dev);
+   if (ret) {
+   dev_warn(_pdev->dev,
+   "failed to create mux device with error %d",
+   ret);
+   platform_device_put(plat_dev);
+   }
+}
+
 /**
  * PCI Quirks for xHCI.
  *
@@ -1022,8 +1061,12 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
 
 hc_init:
-   if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
usb_enable_intel_xhci_ports(pdev);
+   if (pdev->device == DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+   pdev->device == DEVICE_ID_INTEL_BROXTON_M_XHCI)
+   create_intel_usb_mux_device(pdev, base);
+   }
 
op_reg_base = base + XHCI_HC_LENGTH(readl(base));
 
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index e0244fb..e368ccb 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -51,6 +51,8 @@
 #define XHCI_EXT_CAPS_ROUTE5
 /* IDs 6-9 reserved */
 #define XHCI_EXT_CAPS_DEBUG10
+/* Vendor defined 192-255 */
+#define XHCI_EXT_CAPS_INTEL_USB_MUX192
 /* USB Legacy Support Capability - section 7.1.1 */
 #define XHCI_HC_BIOS_OWNED (1 << 16)
 #define XHCI_HC_OS_OWNED   (1 << 24)
-- 
2.1.4

--
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/7] extcon: usb-gpio: add support for ACPI gpio interface

[Lu Baolu]

GPIO resource could be retrieved through APCI as well.

Signed-off-by: Lu Baolu 
Reviewed-by: Felipe Balbi 
Acked-by: Chanwoo Choi 
---
 drivers/extcon/extcon-usb-gpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index af9c8b0..472c431 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define USB_GPIO_DEBOUNCE_MS   20  /* ms */
 
@@ -91,7 +92,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
struct usb_extcon_info *info;
int ret;
 
-   if (!np)
+   if (!np && !ACPI_HANDLE(dev))
return -EINVAL;
 
info = devm_kzalloc(>dev, sizeof(*info), GFP_KERNEL);
-- 
2.1.4

--
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 5/7] usb: mux: add driver for Intel drcfg controlled port mux

[Lu Baolu]

Several Intel PCHs and SOCs have an internal mux that is used to
share one USB port between device controller and host controller.
The mux is handled through the Dual Role Configuration Register.

Signed-off-by: Heikki Krogerus 
Signed-off-by: Lu Baolu 
Signed-off-by: Wu Hao 
Reviewed-by: Felipe Balbi 
---
 MAINTAINERS   |   1 +
 drivers/usb/mux/Kconfig   |   7 ++
 drivers/usb/mux/Makefile  |   1 +
 drivers/usb/mux/intel-mux-drcfg.c | 161 ++
 4 files changed, 170 insertions(+)
 create mode 100644 drivers/usb/mux/intel-mux-drcfg.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ab876eb..399cefe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11407,6 +11407,7 @@ S:  Supported
 F: drivers/usb/mux/intel-mux.c
 F: include/linux/usb/intel-mux.h
 F: drivers/usb/mux/intel-mux-gpio.c
+F: drivers/usb/mux/intel-mux-drcfg.c
 
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index 060bf5c..775d5da 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -15,4 +15,11 @@ config INTEL_MUX_GPIO
  Say Y here to enable support for Intel dual role port mux
  controlled by GPIOs.
 
+config INTEL_MUX_DRCFG
+   tristate "Intel dual role port mux controlled by register"
+   depends on X86
+   select INTEL_USB_MUX
+   help
+ Say Y here to enable support for Intel dual role port mux
+ controlled by the Dual Role Configuration Register.
 endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index 66f765c..3179909 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -3,3 +3,4 @@
 #
 obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
 obj-$(CONFIG_INTEL_MUX_GPIO)   += intel-mux-gpio.o
+obj-$(CONFIG_INTEL_MUX_DRCFG)  += intel-mux-drcfg.o
diff --git a/drivers/usb/mux/intel-mux-drcfg.c 
b/drivers/usb/mux/intel-mux-drcfg.c
new file mode 100644
index 000..11db826
--- /dev/null
+++ b/drivers/usb/mux/intel-mux-drcfg.c
@@ -0,0 +1,161 @@
+/**
+ * intel-mux-drcfg.c - Driver for Intel USB mux via register
+ *
+ * Copyright (C) 2016 Intel Corporation
+ * Author: Heikki Krogerus 
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define INTEL_MUX_CFG0 0x00
+#define INTEL_MUX_CFG1 0x04
+#define CFG0_SW_IDPIN  BIT(20)
+#define CFG0_SW_IDPIN_EN   BIT(21)
+#define CFG0_SW_VBUS_VALID BIT(24)
+#define CFG1_SW_MODE   BIT(29)
+#define CFG1_POLL_TIMEOUT  1000
+
+struct intel_usb_mux {
+   struct intel_mux_dev umdev;
+   void __iomem *regs;
+};
+
+static inline int intel_mux_drcfg_switch(struct intel_mux_dev *umdev, bool 
host)
+{
+   struct intel_usb_mux *mux;
+   unsigned long timeout;
+   u32 data;
+
+   mux = container_of(umdev, struct intel_usb_mux, umdev);
+
+   /* Check and set mux to SW controlled mode */
+   data = readl(mux->regs + INTEL_MUX_CFG0);
+   if (!(data & CFG0_SW_IDPIN_EN)) {
+   data |= CFG0_SW_IDPIN_EN;
+   writel(data, mux->regs + INTEL_MUX_CFG0);
+   }
+
+   /*
+* Configure CFG0 to switch the mux and VBUS_VALID bit is
+* required for device mode.
+*/
+   data = readl(mux->regs + INTEL_MUX_CFG0);
+   if (host)
+   data &= ~(CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
+   else
+   data |= (CFG0_SW_IDPIN | CFG0_SW_VBUS_VALID);
+   writel(data, mux->regs + INTEL_MUX_CFG0);
+
+   /*
+* Polling CFG1 for safety, most case it takes about 600ms
+* to finish mode switching, set TIMEOUT long enough.
+*/
+   timeout = jiffies + msecs_to_jiffies(CFG1_POLL_TIMEOUT);
+
+   /* Polling on CFG1 register to confirm mode switch. */
+   while (!time_after(jiffies, timeout)) {
+   data = readl(mux->regs + INTEL_MUX_CFG1);
+   if (!(host ^ (data & CFG1_SW_MODE)))
+   return 0;
+   /* interval for polling is set to about 5ms */
+   usleep_range(5000, 5100);
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int intel_mux_drcfg_cable_set(struct intel_mux_dev *umdev)
+{
+   dev_dbg(umdev->dev, "drcfg mux switch to HOST\n");
+
+   return intel_mux_drcfg_switch(umdev, true);
+}
+
+static int intel_mux_drcfg_cable_unset(struct intel_mux_dev *umdev)
+{
+   dev_dbg(umdev->dev, "drcfg mux switch to DEVICE\n");
+
+   return intel_mux_drcfg_switch(umdev, false);
+}
+
+static int intel_mux_drcfg_probe(struct 

[PATCH v2 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver

[Lu Baolu]

Some Intel platforms have an USB port mux controlled by GPIOs.
There's a single ACPI platform device that provides both USB ID
extcon device and a USB port mux device. This MFD driver will
split the 2 devices for their respective drivers.

Signed-off-by: Lu Baolu 
Suggested-by: David Cohen 
Reviewed-by: Felipe Balbi 
Signed-off-by: Fengguang Wu 
---
 MAINTAINERS|  1 +
 drivers/mfd/Kconfig|  8 +
 drivers/mfd/Makefile   |  1 +
 drivers/mfd/intel-vuport.c | 78 ++
 4 files changed, 88 insertions(+)
 create mode 100644 drivers/mfd/intel-vuport.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 399cefe..1671a4d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11408,6 +11408,7 @@ F:  drivers/usb/mux/intel-mux.c
 F: include/linux/usb/intel-mux.h
 F: drivers/usb/mux/intel-mux-gpio.c
 F: drivers/usb/mux/intel-mux-drcfg.c
+F: drivers/mfd/intel-vuport.c
 
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9ca66de..8dd1bf3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1534,5 +1534,13 @@ config MFD_VEXPRESS_SYSREG
  System Registers are the platform configuration block
  on the ARM Ltd. Versatile Express board.
 
+config MFD_INTEL_VUPORT
+   tristate "Intel virtual USB port controller"
+   select MFD_CORE
+   depends on X86 && ACPI
+   help
+ Say Y here to enable support for Intel dual role port mux
+ controlled by 3 GPIOs.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0f230a6..0ccd107 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -198,3 +198,4 @@ intel-soc-pmic-objs := intel_soc_pmic_core.o 
intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)   += intel-soc-pmic.o
 obj-$(CONFIG_MFD_MT6397)   += mt6397-core.o
+obj-$(CONFIG_MFD_INTEL_VUPORT) += intel-vuport.o
diff --git a/drivers/mfd/intel-vuport.c b/drivers/mfd/intel-vuport.c
new file mode 100644
index 000..1371099
--- /dev/null
+++ b/drivers/mfd/intel-vuport.c
@@ -0,0 +1,78 @@
+/*
+ * MFD driver for Intel virtual USB port
+ *
+ * Copyright(c) 2016 Intel Corporation.
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* ACPI GPIO Mappings */
+static const struct acpi_gpio_params id_gpio = { 0, 0, false };
+static const struct acpi_gpio_params vbus_gpio = { 1, 0, false };
+static const struct acpi_gpio_params mux_gpio = { 2, 0, false };
+static const struct acpi_gpio_mapping acpi_usb_gpios[] = {
+   { "id-gpios", _gpio, 1 },
+   { "vbus_en-gpios", _gpio, 1 },
+   { "usb_mux-gpios", _gpio, 1 },
+   { },
+};
+
+static const struct mfd_cell intel_vuport_mfd_cells[] = {
+   {
+   .name = "extcon-usb-gpio",
+   },
+   {
+   .name = "intel-mux-gpio",
+   },
+};
+
+static int vuport_probe(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   int ret;
+
+   ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), acpi_usb_gpios);
+   if (ret)
+   return ret;
+
+   return mfd_add_devices(>dev, 0, intel_vuport_mfd_cells,
+   ARRAY_SIZE(intel_vuport_mfd_cells), NULL, 0,
+   NULL);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+   mfd_remove_devices(>dev);
+   acpi_dev_remove_driver_gpios(ACPI_COMPANION(>dev));
+
+   return 0;
+}
+
+static struct acpi_device_id vuport_acpi_match[] = {
+   { "INT3496" },
+   { }
+};
+MODULE_DEVICE_TABLE(acpi, vuport_acpi_match);
+
+static struct platform_driver vuport_driver = {
+   .driver = {
+   .name = "intel-vuport",
+   .acpi_match_table = ACPI_PTR(vuport_acpi_match),
+   },
+   .probe = vuport_probe,
+   .remove = vuport_remove,
+};
+
+module_platform_driver(vuport_driver);
+
+MODULE_AUTHOR("Lu Baolu ");
+MODULE_DESCRIPTION("Intel virtual USB port");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

--
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 4/7] usb: mux: add driver for Intel gpio controlled port mux

[Lu Baolu]

In some Intel platforms, a single usb port is shared between USB host
and device controller. The shared port is under control of GPIO pins.

This patch adds the support for USB GPIO controlled port mux.

Signed-off-by: David Cohen 
Signed-off-by: Lu Baolu 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Felipe Balbi 
Signed-off-by: Fengguang Wu 
---
 MAINTAINERS  |   1 +
 drivers/usb/mux/Kconfig  |   9 +++
 drivers/usb/mux/Makefile |   1 +
 drivers/usb/mux/intel-mux-gpio.c | 125 +++
 4 files changed, 136 insertions(+)
 create mode 100644 drivers/usb/mux/intel-mux-gpio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a93d26a..ab876eb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11406,6 +11406,7 @@ L:  linux-usb@vger.kernel.org
 S: Supported
 F: drivers/usb/mux/intel-mux.c
 F: include/linux/usb/intel-mux.h
+F: drivers/usb/mux/intel-mux-gpio.c
 
 USB PRINTER DRIVER (usblp)
 M: Pete Zaitcev 
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index 8fedc4f..060bf5c 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -6,4 +6,13 @@ config INTEL_USB_MUX
select EXTCON
def_bool n
 
+config INTEL_MUX_GPIO
+   tristate "Intel dual role port mux controlled by GPIOs"
+   depends on GPIOLIB
+   depends on X86 && ACPI
+   select INTEL_USB_MUX
+   help
+ Say Y here to enable support for Intel dual role port mux
+ controlled by GPIOs.
+
 endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index 84f0ae8..66f765c 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -2,3 +2,4 @@
 # Makefile for USB port mux drivers
 #
 obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
+obj-$(CONFIG_INTEL_MUX_GPIO)   += intel-mux-gpio.o
diff --git a/drivers/usb/mux/intel-mux-gpio.c b/drivers/usb/mux/intel-mux-gpio.c
new file mode 100644
index 000..adcb496
--- /dev/null
+++ b/drivers/usb/mux/intel-mux-gpio.c
@@ -0,0 +1,125 @@
+/*
+ * USB Dual Role Port Mux driver controlled by gpios
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ * Author: David Cohen 
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct vuport {
+   struct intel_mux_dev umdev;
+   struct gpio_desc *gpio_vbus_en;
+   struct gpio_desc *gpio_usb_mux;
+};
+
+/*
+ * id == 0, HOST connected, USB port should be set to peripheral
+ * id == 1, HOST disconnected, USB port should be set to host
+ *
+ * Peripheral: set USB mux to peripheral and disable VBUS
+ * Host: set USB mux to host and enable VBUS
+ */
+static inline int vuport_set_port(struct intel_mux_dev *umdev, int id)
+{
+   struct vuport *vup = container_of(umdev, struct vuport, umdev);
+
+   dev_dbg(umdev->dev, "USB PORT ID: %s\n", id ? "HOST" : "PERIPHERAL");
+
+   gpiod_set_value_cansleep(vup->gpio_usb_mux, !id);
+   gpiod_set_value_cansleep(vup->gpio_vbus_en, id);
+
+   return 0;
+}
+
+static int vuport_cable_set(struct intel_mux_dev *umdev)
+{
+   return vuport_set_port(umdev, 1);
+}
+
+static int vuport_cable_unset(struct intel_mux_dev *umdev)
+{
+   return vuport_set_port(umdev, 0);
+}
+
+static int vuport_probe(struct platform_device *pdev)
+{
+   struct intel_mux_dev *umdev;
+   struct device *dev = >dev;
+   struct vuport *vup;
+
+   vup = devm_kzalloc(dev, sizeof(*vup), GFP_KERNEL);
+   if (!vup)
+   return -ENOMEM;
+
+   /* retrieve vbus and mux gpios */
+   vup->gpio_vbus_en = devm_gpiod_get_optional(dev,
+   "vbus_en", GPIOD_ASIS);
+   if (IS_ERR(vup->gpio_vbus_en))
+   return PTR_ERR(vup->gpio_vbus_en);
+
+   vup->gpio_usb_mux = devm_gpiod_get_optional(dev,
+   "usb_mux", GPIOD_ASIS);
+   if (IS_ERR(vup->gpio_usb_mux))
+   return PTR_ERR(vup->gpio_usb_mux);
+
+   /* populate the mux generic structure */
+   umdev = >umdev;
+   umdev->dev = dev;
+   umdev->cable_name = "USB-HOST";
+   umdev->cable_set_cb = vuport_cable_set;
+   umdev->cable_unset_cb = vuport_cable_unset;
+
+   return intel_usb_mux_register(umdev);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+   return intel_usb_mux_unregister(>dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * In case a micro A cable was plugged in while device was sleeping,
+ * we missed the interrupt. We need to poll usb id gpio when waking the
+ * driver to detect the missed event.
+ * We use 'complete' callback to 

[PATCH v2 1/7] extcon: usb-gpio: add device binding for platform device

[Lu Baolu]

This is needed to handle the GPIO connected USB ID pin found on
Intel Baytrail devices.

Signed-off-by: Lu Baolu 
Reviewed-by: Felipe Balbi 
Acked-by: Chanwoo Choi 
---
 drivers/extcon/extcon-usb-gpio.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index 2b2fecf..af9c8b0 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -206,6 +206,12 @@ static const struct of_device_id usb_extcon_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, usb_extcon_dt_match);
 
+static const struct platform_device_id usb_extcon_platform_ids[] = {
+   { .name = "extcon-usb-gpio", },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, usb_extcon_platform_ids);
+
 static struct platform_driver usb_extcon_driver = {
.probe  = usb_extcon_probe,
.remove = usb_extcon_remove,
@@ -214,6 +220,7 @@ static struct platform_driver usb_extcon_driver = {
.pm = _extcon_pm_ops,
.of_match_table = usb_extcon_dt_match,
},
+   .id_table = usb_extcon_platform_ids,
 };
 
 module_platform_driver(usb_extcon_driver);
-- 
2.1.4

--
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: Odd merge 3b30be3b6487 ("Merge tag 'v4.5-rc6' into next")

[Michael Niewoehner]

Hi,

Am 07.03.2016 um 08:26 schrieb Felipe Balbi :

> 
> Hi,
> 
> John Youn  writes:
>> [ text/plain ]
>> On 3/4/2016 6:14 PM, Doug Anderson wrote:
>>> Hi,
>>> 
>>> On Fri, Mar 4, 2016 at 5:56 PM, John Youn  wrote:
 On 3/4/2016 5:04 PM, Doug Anderson wrote:
> Felipe,
> 
> Michael pointed out that there's an odd merge that happened.
> Specifically it looks like commit bd84f4ae9986 ("usb: dwc2: Add extra
> delay when forcing dr_mode") got lost somehow in the merge commit
> 3b30be3b6487 ("Merge tag 'v4.5-rc6' into next")
> 
> Specifically:
> 
> $ git blame 3b30be3b6487 -- drivers/usb/dwc2/core.c | grep "This is 
> required"
> $ git blame 3b30be3b6487^2 -- drivers/usb/dwc2/core.c | grep "This is 
> required"
> bd84f4ae9986a drivers/usb/dwc2/core.c (John Youn
> 2016-02-15 15:30:20 -0800  624)   * NOTE: This is required for some
> rockchip soc based
> 
> 
> I think I'm about at the limit of my git-fu, so I'm a bit baffled.
> Any idea what happened?  Was anything else lost?
> 
 
 
 Felipe recently rebased his next branch without the merge commit
 referenced above. So maybe that's causing some issues.
 
 But everything seems ok to me. I see the commit and correct code on
 4.5-rc6, Felipe's next branch, and Greg's usb-next.
>>> 
>>> They are missing from from next-20160304 and next-20160303 (though in
>>> next-20160302) as found by Michael.
>>> 
>>> -Doug
>>> 
>> 
>> Ah okay I do see that too. Could be because of the rebase...
>> 
>> Maybe it'll all be sorted by the next next tree :)
> 
> It takes a day or so for next to update. Should be sorted out later
> today or tomorrow.
> 
> -- 
> balbi


just checked current next; everything looks fine now.

regards
Michael
--
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 v7 1/5] leds: core: add generic support for RGB LED's

[Jacek Anaszewski]

Hi Karl,

On 03/06/2016 09:55 PM, Karl Palsson wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Heiner Kallweit  wrote:

Add generic support for RGB LED's.

Basic idea is to use enum led_brightness also for the hue and
saturation color components.This allows to implement the color
extension w/o changes to struct led_classdev.

Select LEDS_RGB to enable building drivers using the RGB
extension.

Flag LED_SET_HUE_SAT allows to specify that hue / saturation
should be overridden even if the provided values are zero.

Some examples for writing values to
/sys/class/leds//brightness: (now also hex notation can be
used)

255 -> set full brightness and keep existing color if set 0 ->
switch LED off but keep existing color so that it can be
restored
  if the LED is switched on again later
0x100 -> switch LED off and set also hue and saturation to
0 0x00 -> set full brightness, full saturation and set hue
to 0 (red)




I admit I hadn't seen this earlier, and I didn't spend all day
looking at previous attempts, but what's the motivation for
putting all this overloaded syntax into the "brightness" file? I
thought the point was to have a single value in each file, one of
the references I did find was


With single value per file there would be problems with colour
components setting synchronization.


http://www.spinics.net/lists/linux-leds/msg02995.html Is there
some thread where this was decided as advantageous? Surely 0-255
for _brightness_ is what the brightness entry should do?


You can find a reference to the related discussion in [1].


I'd like to set the rgb colour of a led (or hsv, that can be it's
own file too) and separately ramp the brightness up and down. I
also think it's substantially simpler and easier to use from the
user's point of view, which is surely the place you want easy
right?


I'm also not very keen on overloading the brightness attribute
semantics. Nonetheless it seems impossible to add support for
setting three colour components otherwise than through a single
syscall, if we want to make it synchronous and compatible with
LED triggers.

HSV color scheme is also very convenient for adapting monochrome
brightness semantics to the RGB realm.


[1] http://www.spinics.net/lists/linux-leds/msg05477.html

--
Best regards,
Jacek Anaszewski
--
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