hello linux

[Joe Bryant]

good morning linux



http://recoval.be/truth.php?coast=b20hf5neea2af2





Joe
--
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 v8 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.

In this patch, it calls power sequence library APIs to finish
the power sequence events. It will do power on sequence at hub's
probe for all devices under this hub (includes root hub).
At hub_disconnect, it will do power off sequence which is at powered
on list.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
 drivers/usb/core/hub.c | 41 ++---
 drivers/usb/core/hub.h |  1 +
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cbb1467..acbbf0a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1695,6 +1696,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
 
+   of_pwrseq_off_list(>pwrseq_on_list);
mutex_lock(_port_peer_mutex);
 
/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1722,12 +1724,41 @@ static void hub_disconnect(struct usb_interface *intf)
kref_put(>kref, hub_release);
 }
 
+#ifdef CONFIG_OF
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+   struct device *parent;
+   struct usb_device *hdev = hub->hdev;
+   struct device_node *np;
+   int ret;
+
+   if (hdev->parent)
+   parent = >dev;
+   else
+   parent = bus_to_hcd(hdev->bus)->self.controller;
+
+   for_each_child_of_node(parent->of_node, np) {
+   ret = of_pwrseq_on_list(np, >pwrseq_on_list);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+#else
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+   return 0;
+}
+#endif
+
 static int hub_probe(struct usb_interface *intf, const struct usb_device_id 
*id)
 {
struct usb_host_interface *desc;
struct usb_endpoint_descriptor *endpoint;
struct usb_device *hdev;
struct usb_hub *hub;
+   int ret = -ENODEV;
 
desc = intf->cur_altsetting;
hdev = interface_to_usbdev(intf);
@@ -1832,6 +1863,7 @@ descriptor_error:
INIT_DELAYED_WORK(>leds, led_work);
INIT_DELAYED_WORK(>init_work, NULL);
INIT_WORK(>events, hub_event);
+   INIT_LIST_HEAD(>pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
 
@@ -1845,11 +1877,14 @@ descriptor_error:
if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
hub->quirk_check_port_auto_suspend = 1;
 
-   if (hub_configure(hub, endpoint) >= 0)
-   return 0;
+   if (hub_configure(hub, endpoint) >= 0) {
+   ret = hub_of_pwrseq_on(hub);
+   if (!ret)
+   return 0;
+   }
 
hub_disconnect(intf);
-   return -ENODEV;
+   return ret;
 }
 
 static int
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..cd86f91 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -78,6 +78,7 @@ struct usb_hub {
struct delayed_work init_work;
struct work_struct  events;
struct usb_port **ports;
+   struct list_headpwrseq_on_list; /* powered pwrseq node list */
 };
 
 /**
-- 
2.7.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 v8 5/8] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Peter Chen]

From: Peter Chen 

At device tree, we have no device node for chipidea core,
the glue layer's node is the parent node for host and udc
device. But in related driver, the parent device is chipidea
core. So, in order to let the common driver get parent's node,
we let the core's device node equals glue layer device node.

Signed-off-by: Peter Chen 
Tested-by: Maciej S. Szmigiero 
Tested-by Joshua Clayton 
---
 drivers/usb/chipidea/core.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..6839e19 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -927,6 +927,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
 
+   /*
+* At device tree, we have no device node for chipidea core,
+* the glue layer's node is the parent node for host and udc
+* device. But in related driver, the parent device is chipidea
+* core. So, in order to let the common driver get parent's node,
+* we let the core's device node equals glue layer's node.
+*/
+   if (dev->parent && dev->parent->of_node)
+   dev->of_node = dev->parent->of_node;
+
if (ci->platdata->phy) {
ci->phy = ci->platdata->phy;
} else if (ci->platdata->usb_phy) {
@@ -937,11 +947,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
/* if both generic PHY and USB PHY layers aren't enabled */
if (PTR_ERR(ci->phy) == -ENOSYS &&
-   PTR_ERR(ci->usb_phy) == -ENXIO)
-   return -ENXIO;
+   PTR_ERR(ci->usb_phy) == -ENXIO) {
+   ret = -ENXIO;
+   goto clear_of_node;
+   }
 
-   if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy))
-   return -EPROBE_DEFER;
+   if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy)) {
+   ret = -EPROBE_DEFER;
+   goto clear_of_node;
+   }
 
if (IS_ERR(ci->phy))
ci->phy = NULL;
@@ -952,7 +966,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ret = ci_usb_phy_init(ci);
if (ret) {
dev_err(dev, "unable to init phy: %d\n", ret);
-   return ret;
+   goto clear_of_node;
}
 
ci->hw_bank.phys = res->start;
@@ -1058,6 +1072,8 @@ stop:
ci_role_destroy(ci);
 deinit_phy:
ci_usb_phy_exit(ci);
+clear_of_node:
+   dev->of_node = NULL;
 
return ret;
 }
@@ -1076,6 +1092,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
ci_extcon_unregister(ci);
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
+   ci->dev->of_node = NULL;
ci_usb_phy_exit(ci);
 
return 0;
-- 
2.7.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 v8 6/8] ARM: dts: imx6qdl: Enable usb node children with

[Peter Chen]

From: Joshua Clayton 

Give usb nodes #address and #size attributes, so that a child node
representing a permanently connected device such as an onboard hub may
be addressed with a  attribute

Signed-off-by: Joshua Clayton 
Signed-off-by: Peter Chen 
---
 arch/arm/boot/dts/imx6qdl.dtsi | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b13b0b2..8fc66e1 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -935,6 +935,8 @@
 
usbh1: usb@02184200 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+   #address-cells = <1>;
+   #size-cells = <0>;
reg = <0x02184200 0x200>;
interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>;
clocks = < IMX6QDL_CLK_USBOH3>;
@@ -949,6 +951,8 @@
 
usbh2: usb@02184400 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+   #address-cells = <1>;
+   #size-cells = <0>;
reg = <0x02184400 0x200>;
interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
clocks = < IMX6QDL_CLK_USBOH3>;
@@ -962,6 +966,8 @@
 
usbh3: usb@02184600 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+   #address-cells = <1>;
+   #size-cells = <0>;
reg = <0x02184600 0x200>;
interrupts = <0 42 IRQ_TYPE_LEVEL_HIGH>;
clocks = < IMX6QDL_CLK_USBOH3>;
-- 
2.7.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 v8 3/8] binding-doc: usb: usb-device: add optional properties for power sequence

[Peter Chen]

Add optional properties for power sequence.

Signed-off-by: Peter Chen 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/usb/usb-device.txt | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt 
b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..3661dd2 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,6 +13,10 @@ Required properties:
 - reg: the port number which this device is connecting to, the range
   is 1-31.
 
+Optional properties:
+power sequence properties, see
+Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt for detail
+
 Example:
 
  {
@@ -21,8 +25,12 @@ Example:
#address-cells = <1>;
#size-cells = <0>;
 
-   hub: genesys@1 {
+   genesys: hub@1 {
compatible = "usb5e3,608";
reg = <1>;
+
+   clocks = < IMX6SX_CLK_CKO>;
+   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+   reset-duration-us = <10>;
};
 }
-- 
2.7.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 v8 1/8] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

[Peter Chen]

Add binding doc for generic power sequence library.

Signed-off-by: Peter Chen 
Acked-by: Philipp Zabel 
Acked-by: Rob Herring 
---
 .../bindings/power/pwrseq/pwrseq-generic.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
new file mode 100644
index 000..ebf0d47
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,48 @@
+The generic power sequence library
+
+Some hard-wired devices (eg USB/MMC) need to do power sequence before
+the device can be enumerated on the bus, the typical power sequence
+like: enable USB PHY clock, toggle reset pin, etc. But current
+Linux device driver lacks of such code to do it, it may cause some
+hard-wired devices works abnormal or can't be recognized by
+controller at all. The power sequence will be done before this device
+can be found at the bus.
+
+The power sequence properties is under the device node.
+
+Optional properties:
+- clocks: the input clocks for device.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
+
+Below is the example of USB power sequence properties on USB device
+nodes which have two level USB hubs.
+
+ {
+   vbus-supply = <_usb_otg1_vbus>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_usb_otg1_id>;
+   status = "okay";
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   genesys: hub@1 {
+   compatible = "usb5e3,608";
+   reg = <1>;
+
+   clocks = < IMX6SX_CLK_CKO>;
+   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+   reset-duration-us = <10>;
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   asix: ethernet@1 {
+   compatible = "usbb95,1708";
+   reg = <1>;
+
+   clocks = < IMX6SX_CLK_IPG>;
+   reset-gpios = < 6 GPIO_ACTIVE_LOW>; /* 
ethernet_rst */
+   reset-duration-us = <15>;
+   };
+   };
+};
-- 
2.7.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 v8 8/8] ARM: dts: imx6q-evi: Fix onboard hub reset line

[Peter Chen]

From: Joshua Clayton 

Previously the onboard hub was made to work by treating its
reset gpio as a regulator enable.
Get rid of that kludge now that pwseq has added reset gpio support
Move pin muxing the hub reset pin into the usbh1 group

Signed-off-by: Joshua Clayton 
Signed-off-by: Peter Chen 
---
 arch/arm/boot/dts/imx6q-evi.dts | 25 +++--
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 4fa5601..49c6f61 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -54,18 +54,6 @@
reg = <0x1000 0x4000>;
};
 
-   reg_usbh1_vbus: regulator-usbhubreset {
-   compatible = "regulator-fixed";
-   regulator-name = "usbh1_vbus";
-   regulator-min-microvolt = <500>;
-   regulator-max-microvolt = <500>;
-   enable-active-high;
-   startup-delay-us = <2>;
-   pinctrl-names = "default";
-   pinctrl-0 = <_usbh1_hubreset>;
-   gpio = < 12 GPIO_ACTIVE_HIGH>;
-   };
-
reg_usb_otg_vbus: regulator-usbotgvbus {
compatible = "regulator-fixed";
regulator-name = "usb_otg_vbus";
@@ -204,12 +192,18 @@
 };
 
  {
-   vbus-supply = <_usbh1_vbus>;
pinctrl-names = "default";
pinctrl-0 = <_usbh1>;
dr_mode = "host";
disable-over-current;
status = "okay";
+
+   usb2415host: hub@1 {
+   compatible = "usb424,2513";
+   reg = <1>;
+   reset-gpios = < 12 GPIO_ACTIVE_LOW>;
+   reset-duration-us = <3000>;
+   };
 };
 
  {
@@ -467,11 +461,6 @@
MX6QDL_PAD_GPIO_3__USB_H1_OC 0x1b0b0
/* usbh1_b OC */
MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0
-   >;
-   };
-
-   pinctrl_usbh1_hubreset: usbh1hubresetgrp {
-   fsl,pins = <
MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0
>;
};
-- 
2.7.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 v8 2/8] power: add power sequence library

[Peter Chen]

We have an well-known problem that the device needs to do some power
sequence before it can be recognized by related host, the typical
example like hard-wired mmc devices and usb devices.

This power sequence is hard to be described at device tree and handled by
related host driver, so we have created a common power sequence
library to cover this requirement. The core code has supplied
some common helpers for host driver, and individual power sequence
libraries handle kinds of power sequence for devices. The pwrseq
librares always need to allocate extra instance for compatible
string match.

pwrseq_generic is intended for general purpose of power sequence, which
handles gpios and clocks currently, and can cover other controls in
future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence is needed, else call of_pwrseq_on_list
/of_pwrseq_off_list instead (eg, USB hub driver).

For new power sequence library, it can add its compatible string
to pwrseq_of_match_table, then the pwrseq core will match it with
DT's, and choose this library at runtime.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
Reviewed-by: Matthias Kaehlcke 
Tested-by: Matthias Kaehlcke 
---
 MAINTAINERS   |   9 ++
 drivers/power/Kconfig |   1 +
 drivers/power/Makefile|   1 +
 drivers/power/pwrseq/Kconfig  |  19 
 drivers/power/pwrseq/Makefile |   2 +
 drivers/power/pwrseq/core.c   | 191 ++
 drivers/power/pwrseq/pwrseq_generic.c | 183 
 include/linux/power/pwrseq.h  |  72 +
 8 files changed, 478 insertions(+)
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 include/linux/power/pwrseq.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3bb6640..7e1fcb9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9384,6 +9384,15 @@ F:   include/linux/pm_*
 F: include/linux/powercap.h
 F: drivers/powercap/
 
+POWER SEQUENCE LIBRARY
+M: Peter Chen 
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
+L: linux...@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/power/pwrseq/
+F: drivers/power/pwrseq/
+F: include/linux/power/pwrseq.h/
+
 POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
 M: Sebastian Reichel 
 M: Dmitry Eremin-Solenikov 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..f6aa4fd 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -515,3 +515,4 @@ endif # POWER_SUPPLY
 
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
+source "drivers/power/pwrseq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d..4ed2e12 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65217)+= tps65217_charger.o
 obj-$(CONFIG_POWER_RESET)  += reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)   += axp288_charger.o
+obj-$(CONFIG_POWER_SEQUENCE)   += pwrseq/
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
new file mode 100644
index 000..3859a67
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,19 @@
+#
+# Power Sequence library
+#
+
+config POWER_SEQUENCE
+   bool
+
+menu "Power Sequence Support"
+
+config PWRSEQ_GENERIC
+   bool "Generic power sequence control"
+   depends on OF
+   select POWER_SEQUENCE
+   help
+  It is used for drivers which needs to do power sequence
+  (eg, turn on clock, toggle reset gpio) before the related
+  devices can be found by hardware. This generic one can be
+  used for common power sequence control.
+endmenu
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 000..ad82389
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWER_SEQUENCE) += core.o
+obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 000..9cb1223
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,191 @@
+/*
+ * core.c  power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 

[PATCH v8 7/8] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

[Peter Chen]

The current dts describes USB HUB's property at USB controller's
entry, it is improper. The USB HUB should be the child node
under USB controller, and power sequence properties are under
it. Besides, using gpio pinctrl setting for USB2415's reset pin.

Tested-by: Maciej S. Szmigiero 
Signed-off-by: Peter Chen 
Signed-off-by: Joshua Clayton 
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi 
b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..87fe31f 100644
--- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
@@ -9,6 +9,8 @@
  *
  */
 
+#include 
+
 / {
aliases {
backlight = 
@@ -58,17 +60,6 @@
#address-cells = <1>;
#size-cells = <0>;
 
-   reg_usb_h1_vbus: regulator@0 {
-   compatible = "regulator-fixed";
-   reg = <0>;
-   regulator-name = "usb_h1_vbus";
-   regulator-min-microvolt = <500>;
-   regulator-max-microvolt = <500>;
-   enable-active-high;
-   startup-delay-us = <2>; /* USB2415 requires a POR of 1 
us minimum */
-   gpio = < 12 0>;
-   };
-
reg_panel: regulator@1 {
compatible = "regulator-fixed";
reg = <1>;
@@ -188,7 +179,7 @@
 
pinctrl_usbh: usbhgrp {
fsl,pins = <
-   MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x8000
+   MX6QDL_PAD_GPIO_17__GPIO7_IO12  0x1b0b0
MX6QDL_PAD_NANDF_CS2__CCM_CLKO2 0x130b0
>;
};
@@ -259,9 +250,16 @@
  {
pinctrl-names = "default";
pinctrl-0 = <_usbh>;
-   vbus-supply = <_usb_h1_vbus>;
-   clocks = < IMX6QDL_CLK_CKO>;
status = "okay";
+
+   usb2415: hub@1 {
+   compatible = "usb424,2514";
+   reg = <1>;
+
+   clocks = < IMX6QDL_CLK_CKO>;
+   reset-gpios = < 12 GPIO_ACTIVE_LOW>;
+   reset-duration-us = <3000>;
+   };
 };
 
  {
-- 
2.7.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 v8 0/8] power: add power sequence library

[Peter Chen]

Hi all,

This is a follow-up for my last power sequence framework patch set [1].
According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
power sequence instances will be added at postcore_initcall, the match
criteria is compatible string first, if the compatible string is not
matched between dts and library, it will try to use generic power sequence.
 
The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence instance is needed, for more power sequences
are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub 
driver).

In future, if there are special power sequence requirements, the special
power sequence library can be created.

This patch set is tested on i.mx6 sabresx evk using a dts change, I use
two hot-plug devices to simulate this use case, the related binding
change is updated at patch [1/6], The udoo board changes were tested
using my last power sequence patch set.[3]

Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
need to power on itself before it can be found by ULPI bus.

[1] http://www.spinics.net/lists/linux-usb/msg142755.html
[2] http://www.spinics.net/lists/linux-usb/msg143106.html
[3] http://www.spinics.net/lists/linux-usb/msg142815.html

Changes for v8:
- Allocate one extra pwrseq instance if pwrseq_get has succeed, it can avoid
  preallocate instances problem which the number of instance is decided at
  compile time, thanks for Heiko Stuebner's suggestion [Patch 2/8]
- Delete pwrseq_compatible_sample.c which is the demo purpose to show compatible
  match method. [Patch 2/8]
- Add Maciej S. Szmigiero's tested-by. [Patch 7/8]

Changes for v7:
- Create kinds of power sequence instance at postcore_initcall, and match
  the instance with node using compatible string, the beneit of this is
  the host driver doesn't need to consider which pwrseq instance needs
  to be used, and pwrseq core will match it, however, it eats some memories
  if less power sequence instances are used. [Patch 2/8]
- Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 
2/8]
- Fix the comments Vaibhav Hiremath adds for error path for clock and do not
  use device_node for parameters at pwrseq_on. [Patch 2/8]
- Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
- Tested three pwrseq instances together using both specific compatible string 
and
  generic libraries.

Changes for v6:
- Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
- Change chipidea core of_node assignment for coming user. (patch [5/6])
- Applies Joshua Clayton's three dts changes for two boards,
  the USB device's reg has only #address-cells, but without #size-cells.

Changes for v5:
- Delete pwrseq_register/pwrseq_unregister, which is useless currently
- Fix the linker error when the pwrseq user is compiled as module

Changes for v4:
- Create the patch on next-20160722 
- Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
- Using more friendly wait method for reset gpio [Patch 2/6]
- Support multiple input clocks [Patch 2/6]
- Add Rob Herring's ack for DT changes
- Add Joshua Clayton's Tested-by

Changes for v3:
- Delete "power-sequence" property at binding-doc, and change related code
  at both library and user code.
- Change binding-doc example node name with Rob's comments
- of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
  add additional code request gpio with proper gpio flags
- Add Philipp Zabel's Ack and MAINTAINER's entry

Changes for v2:
- Delete "pwrseq" prefix and clock-names for properties at dt binding
- Should use structure not but its pointer for kzalloc
- Since chipidea core has no of_node, let core's of_node equals glue
  layer's at core's probe

Joshua Clayton (2):
  ARM: dts: imx6qdl: Enable usb node children with 
  ARM: dts: imx6q-evi: Fix onboard hub reset line

Peter Chen (6):
  binding-doc: power: pwrseq-generic: add binding doc for generic power
sequence library
  power: add power sequence library
  binding-doc: usb: usb-device: add optional properties for power
sequence
  usb: core: add power sequence handling for USB devices
  usb: chipidea: let chipidea core device of_node equal's glue layer
device of_node
  ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

 .../bindings/power/pwrseq/pwrseq-generic.txt   |  48 ++
 .../devicetree/bindings/usb/usb-device.txt |  10 +-
 MAINTAINERS|   9 +
 arch/arm/boot/dts/imx6q-evi.dts|  25 +--
 arch/arm/boot/dts/imx6qdl-udoo.dtsi|  26 ++-
 arch/arm/boot/dts/imx6qdl.dtsi |   6 +
 drivers/power/Kconfig  |   1 +
 drivers/power/Makefile |   1 +
 drivers/power/pwrseq/Kconfig   |  19 ++
 drivers/power/pwrseq/Makefile  |   2 +
 drivers/power/pwrseq/core.c   

Re: USB crash on BeagleBoard-xM

[Snaper]

Hi Oliver,

On 10/10/2016 08:19, Oliver Neukum wrote:

it is very hard for us to say something about that specific kernel.
We don't know the kernel tree. And the error seems to be in the OF
code. I think you are in the wrong list, or can you reproduce
teh problem with a mainline kernel?


I have reported this in 
https://bugzilla.kernel.org/show_bug.cgi?id=169291 and have followed the 
advice given by Greg there, to report to this ML.


I'm not using this kernel any longer (I created the micro-SD card with 
the OS for this machine with another image), so if this is of no use for 
the developers, it can be closed.


João M. S. Silva
--
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

[RFC][PATCH 0/2] dwc2 fixes for HiKey

[John Stultz]

I wanted to send out two patches we're using on the dwc2
driver in order to make HiKey function.

The first is seemingly just a bug fix we ran into when
changing modes while the bus was auto-suspended.

The second is a little more interesting as it works around
a limitation of the the device to negotiate the usb speed
automatically.

I wanted to send these out for initial review and consideration.

Feedback would be greatly appreciated!

thanks
-john


Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: John Youn 
Cc: Douglas Anderson 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: Chen Yu 

Chen Yu (2):
  usb: dwc2: Force port resume on switching to device mode
  usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

 arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  1 +
 drivers/usb/dwc2/core.h   |  7 +++
 drivers/usb/dwc2/hcd.c| 83 +++
 drivers/usb/dwc2/platform.c   |  3 ++
 4 files changed, 94 insertions(+)

-- 
2.7.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: [RFC][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

[John Stultz]

On Thu, Oct 13, 2016 at 4:29 PM, John Stultz  wrote:
> From: Chen Yu 
>
> The Hi6220's usb controller is limited in that it does not
> automatically autonegotiate the usb speed. Thus it requires a
> quirk so that we can manually negotiate the best usb speed for
> the attached device.
>
> Cc: Wei Xu 
> Cc: Guodong Xu 
> Cc: Amit Pundir 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: John Youn 
> Cc: Douglas Anderson 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Chen Yu 
> Signed-off-by: John Stultz 
> ---
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  1 +
>  drivers/usb/dwc2/core.h   |  7 +++
>  drivers/usb/dwc2/hcd.c| 75 
> +++
>  drivers/usb/dwc2/platform.c   |  3 ++
>  4 files changed, 86 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 17839db..2c8f435 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -752,6 +752,7 @@
> g-np-tx-fifo-size = <128>;
> g-tx-fifo-size = <128 128 128 128 128 128>;
> interrupts = <0 77 0x4>;
> +   hi6220,change_speed_quirk;


I also realize I'm missing the DT binding documentation for this, but
I wanted to get a rough idea if this approach was ok first.
On the next submission (if there aren't major objections to the
approach) I can split the binding documentation and dts change into
different patches.

thanks
-john
--
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] net: asix: Avoid looping when the device does not respond

[Guenter Roeck]

Check answers from USB stack and avoid re-sending the request
multiple times if the device does not respond.

This fixes the following problem, observed with a probably flaky adapter.

[62108.732707] usb 1-3: new high-speed USB device number 5 using xhci_hcd
[62108.914421] usb 1-3: New USB device found, idVendor=0b95, idProduct=7720
[62108.914463] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[62108.914476] usb 1-3: Product: AX88x72A
[62108.914486] usb 1-3: Manufacturer: ASIX Elec. Corp.
[62108.914495] usb 1-3: SerialNumber: 01
[62114.109109] asix 1-3:1.0 (unnamed net_device) (uninitialized):
Failed to write reg index 0x: -110
[62114.109139] asix 1-3:1.0 (unnamed net_device) (uninitialized):
Failed to send software reset: ff92
[62119.109048] asix 1-3:1.0 (unnamed net_device) (uninitialized):
Failed to write reg index 0x: -110
...

Since the USB timeout is 5 seconds, and the operation is retried 30 times,
this results in

[62278.180353] INFO: task mtpd:1725 blocked for more than 120 seconds.
[62278.180373]   Tainted: GW  3.18.0-13298-g94ace9e #1
[62278.180383] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
...
[62278.180957] kworker/2:0 D  0  5744  2 0x
[62278.180978] Workqueue: usb_hub_wq hub_event
[62278.181029]  880177f833b8 0046 88017fd0 
88017b126d80
[62278.181048]  880177f83fd8 880065a71b60 00013340 
880065a71b60
[62278.181065]  0286 000103b1c199 1388 
0002
[62278.181081] Call Trace:
[62278.181092]  [] ? console_conditional_schedule+0x2c/0x2c
[62278.181105]  [] schedule+0x69/0x6b
[62278.181117]  [] schedule_timeout+0xe3/0x11d
[62278.181133]  [] ? trace_timer_start+0x51/0x51
[62278.181146]  [] do_wait_for_common+0x12f/0x16c
[62278.181162]  [] ? wake_up_process+0x39/0x39
[62278.181174]  [] wait_for_common+0x52/0x6d
[62278.181187]  [] wait_for_completion_timeout+0x13/0x15
[62278.181201]  [] usb_start_wait_urb+0x93/0xf1
[62278.181214]  [] usb_control_msg+0xe1/0x11d
[62278.181230]  [] usbnet_write_cmd+0x9c/0xc6 [usbnet]
[62278.181286]  [] asix_write_cmd+0x4e/0x7e [asix]
[62278.181300]  [] asix_set_sw_mii+0x25/0x4e [asix]
[62278.181314]  [] asix_mdio_read+0x51/0x109 [asix]
...

Signed-off-by: Guenter Roeck 
---
 drivers/net/usb/asix_common.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index f79eb12c326a..125cff57c759 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -433,13 +433,13 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, 
int loc)
mutex_lock(>phy_mutex);
do {
ret = asix_set_sw_mii(dev, 0);
-   if (ret == -ENODEV)
+   if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, , 0);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
-   if (ret == -ENODEV) {
+   if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(>phy_mutex);
return ret;
}
@@ -497,13 +497,13 @@ int asix_mdio_read_nopm(struct net_device *netdev, int 
phy_id, int loc)
mutex_lock(>phy_mutex);
do {
ret = asix_set_sw_mii(dev, 1);
-   if (ret == -ENODEV)
+   if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, , 1);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
-   if (ret == -ENODEV) {
+   if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(>phy_mutex);
return ret;
}
-- 
2.5.0

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

[RFC][PATCH 1/2] usb: dwc2: Force port resume on switching to device mode

[John Stultz]

From: Chen Yu 

We've seen failures when switching between host and gadget mode,
which was diagnosed as being caused due to the bus being
auto-suspended when we switched.

So this patch forces a port resume when switching to device
mode if the bus is suspended.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: John Youn 
Cc: Douglas Anderson 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Signed-off-by: Chen Yu 
Signed-off-by: John Stultz 
---
 drivers/usb/dwc2/hcd.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index df5a065..b374e60 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -54,6 +54,9 @@
 #include "core.h"
 #include "hcd.h"
 
+
+static void dwc2_port_resume(struct dwc2_hsotg *hsotg);
+
 /*
  * =
  *  Host Core Layer Functions
@@ -3204,6 +3207,11 @@ static void dwc2_conn_id_status_change(struct 
work_struct *work)
if (gotgctl & GOTGCTL_CONID_B) {
/* Wait for switch to device mode */
dev_dbg(hsotg->dev, "connId B\n");
+   if (hsotg->bus_suspended) {
+   dev_info(hsotg->dev,
+   "Do port resume before switching to device 
mode\n");
+   dwc2_port_resume(hsotg);
+   }
while (!dwc2_is_device_mode(hsotg)) {
dev_info(hsotg->dev,
 "Waiting for Peripheral Mode, Mode=%s\n",
-- 
2.7.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

[RFC][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

[John Stultz]

From: Chen Yu 

The Hi6220's usb controller is limited in that it does not
automatically autonegotiate the usb speed. Thus it requires a
quirk so that we can manually negotiate the best usb speed for
the attached device.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: John Youn 
Cc: Douglas Anderson 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Signed-off-by: Chen Yu 
Signed-off-by: John Stultz 
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  1 +
 drivers/usb/dwc2/core.h   |  7 +++
 drivers/usb/dwc2/hcd.c| 75 +++
 drivers/usb/dwc2/platform.c   |  3 ++
 4 files changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 17839db..2c8f435 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -752,6 +752,7 @@
g-np-tx-fifo-size = <128>;
g-tx-fifo-size = <128 128 128 128 128 128>;
interrupts = <0 77 0x4>;
+   hi6220,change_speed_quirk;
};
 
mailbox: mailbox@f751 {
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2a21a04..24fea73 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -823,6 +823,10 @@ struct dwc2_hregs_backup {
  * @frame_list_sz:  Frame list size
  * @desc_gen_cache: Kmem cache for generic descriptors
  * @desc_hsisoc_cache:  Kmem cache for hs isochronous descriptors
+ * @change_speed_quirk: Change speed configuration to DWC2_SPEED_PARAM_FULL
+ *  while full speed device connect. And change speed
+ *  back to DWC2_SPEED_PARAM_HIGH while device is gone.
+ * @device_count:   Number of devices connect to root hub
  *
  * These are for peripheral mode:
  *
@@ -951,6 +955,9 @@ struct dwc2_hsotg {
struct kmem_cache *desc_gen_cache;
struct kmem_cache *desc_hsisoc_cache;
 
+   int change_speed_quirk;
+   unsigned int device_count;
+
 #ifdef DEBUG
u32 frrem_samples;
u64 frrem_accum;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index b374e60..663033e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4868,6 +4868,75 @@ static void _dwc2_hcd_clear_tt_buffer_complete(struct 
usb_hcd *hcd,
spin_unlock_irqrestore(>lock, flags);
 }
 
+/*
+ * HPRT0_SPD_HIGH_SPEED: high speed
+ * HPRT0_SPD_FULL_SPEED: full speed
+ */
+static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
+{
+   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+   if (hsotg->core_params->speed == speed)
+   return;
+
+   hsotg->core_params->speed = speed;
+   queue_work(hsotg->wq_otg, >wf_otg);
+}
+
+static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
+{
+   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+   if (!hsotg->change_speed_quirk)
+   return 1;
+
+   hsotg->device_count++;
+   dev_info(hsotg->dev, "Device count is %u after alloc dev\n",
+   hsotg->device_count);
+
+   return 1;
+}
+
+static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
+{
+   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+   if (!hsotg->change_speed_quirk)
+   return;
+
+   if (hsotg->device_count)
+   hsotg->device_count--;
+
+   dev_info(hsotg->dev, "Device count is %u after free dev\n",
+   hsotg->device_count);
+
+   if (hsotg->device_count == 1 && udev->parent &&
+   udev->parent->speed > USB_SPEED_UNKNOWN &&
+   udev->parent->speed < USB_SPEED_HIGH) {
+   dev_info(hsotg->dev, "Set speed to default high-speed\n");
+   dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
+   }
+}
+
+static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
+{
+   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+   if (!hsotg->change_speed_quirk)
+   return 0;
+
+   if (udev->speed == USB_SPEED_HIGH) {
+   dev_info(hsotg->dev, "Set speed to high-speed\n");
+   dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
+   } else if (udev->speed == USB_SPEED_FULL
+   || udev->speed == USB_SPEED_LOW) {
+   dev_info(hsotg->dev, "Set speed to full-speed\n");
+   dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
+   }
+
+   return 0;
+}
+
 static struct hc_driver dwc2_hc_driver = {
.description = "dwc2_hsotg",
   

Re: USB hot-plug not working (ASUS TP301UA-C4028T)

[Pierre de Villemereuil]

Hi!

I've branched the kernel package on the OpenSUSE Build Server, I'll 
try to apply the patch there (this ought to be cleanest method).

Starting from the root of the kernel tarball, the patch should be 
applied to drivers/pci/pci.c, am I right?

Cheers,
Pierre.


Le mercredi 12 octobre 2016, 14:23:35 NZDT Alan Stern a écrit :
> On Wed, 12 Oct 2016, Pierre de Villemereuil wrote:
> > Hi!
> > 
> > I'm sorry, I'm not savvy enough to know what to do with this (I 
know
> > basics on how to code and compile, but I'm no dev). Could 
someone
> > guide me through it?
> > 
> > I gather this patch needs to be applied to some kernel module 
code
> > which needs to be compiled and reloaded into my current kernel,
> > right?
> 
> More likely you'll have to rebuild an entire new kernel, not just 
a
> single module.
> 
> Search the support site for the distribution you are using.  It 
ought
> to contain reasonably thorough instructions on how to build and 
install
> your own version of their kernel.
> 
> Once you know how to do all that, I can tell you how the patch 
should
> be applied -- that's the easy part.
> 
> 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: USB hot-plug not working (ASUS TP301UA-C4028T)

[Alan Stern]

On Fri, 14 Oct 2016, Pierre de Villemereuil wrote:

> Hi!
> 
> I've branched the kernel package on the OpenSUSE Build Server, I'll 
> try to apply the patch there (this ought to be cleanest method).
> 
> Starting from the root of the kernel tarball, the patch should be 
> applied to drivers/pci/pci.c, am I right?

You just cd to the top directory of the kernel source, and do

patch -p1 http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: dummy_hcd: clear struct gadget on removal

[Alan Stern]

On Tue, 11 Oct 2016, Rob Herring wrote:

> The addition of CONFIG_DEBUG_TEST_DRIVER_REMOVE uncovered a bug in
> dummy_hcd driver. The problem is the gadget's struct device is allocated
> once in the initcall, so re-probing the device causes this warning:
> 
> [   13.516309] kobject (8f92f8b4): tried to init an initialized object, 
> something is seriously wrong.
> [   13.518358] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.8.0-rc4-3-gbea5b15 #1
> [   13.520014] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> Debian-1.8.2-1 04/01/2014
> [   13.522207]  8f92f8b4 8f92f8b4 80031d20 88bb56c9 80031d3c 88bb7dd3 
> 898316d4 8f92f8b4
> [   13.524132]  8f92f868 899b8c6c 8f92f8ac 80031d4c 88d83cce 8f92f868 
> 8f92f8ac 80031d58
> [   13.526087]  88d844bb 8f92f868 80031d84 88ef3c1d  89863eac 
>  8004f760
> [   13.527980] Call Trace:
> [   13.528540]  [<88bb56c9>] dump_stack+0x16/0x1d
> [   13.529544]  [<88bb7dd3>] kobject_init+0x73/0x80
> [   13.530602]  [<88d83cce>] device_initialize+0x1e/0xe0
> [   13.531889]  [<88d844bb>] device_register+0xb/0x20
> [   13.532997]  [<88ef3c1d>] usb_add_gadget_udc_release+0x8d/0x270
> [   13.534333]  [<88ef3e9a>] usb_add_gadget_udc+0xa/0x10
> [   13.535465]  [<88ef775e>] dummy_udc_probe+0x14e/0x1a0
> [   13.536623]  [<88d89781>] platform_drv_probe+0x31/0x90
> [   13.537830]  [<88d875aa>] ? driver_sysfs_add+0x6a/0x90
> [   13.539014]  [<88d87e3a>] driver_probe_device+0x12a/0x490
> [   13.540228]  [<88cbc39b>] ? acpi_driver_match_device+0x36/0x50
> [   13.541658]  [<88d88307>] __device_attach_driver+0x77/0x110
> [   13.542946]  [<8949712d>] ? klist_next+0x6d/0x10c
> [   13.544053]  [<88d88290>] ? __driver_attach+0xf0/0xf0
> [   13.545180]  [<88d864f7>] bus_for_each_drv+0x47/0x80
> [   13.549626]  [<88d87b85>] __device_attach+0xb5/0x130
> [   13.550493]  [<88d88290>] ? __driver_attach+0xf0/0xf0
> [   13.551517]  [<88d883cd>] device_initial_probe+0xd/0x10
> [   13.552485]  [<88d86787>] bus_probe_device+0x77/0x80
> [   13.553298]  [<88d8417e>] device_add+0x34e/0x5a0
> [   13.554025]  [<88bc4840>] ? kvasprintf_const+0x40/0x90
> [   13.554860]  [<88bb7d1b>] ? kobject_set_name_vargs+0x6b/0x90
> [   13.555770]  [<88d89e6c>] platform_device_add+0xfc/0x280
> [   13.556640]  [<89ad0b84>] init+0x20b/0x2ec
> [   13.557317]  [<89ad0979>] ? usb_udc_init+0x3f/0x3f
> [   13.558154]  [<89a96c1d>] do_one_initcall+0x7c/0xfb
> [   13.558947]  [<89a96d5e>] ? kernel_init_freeable+0xc2/0x15e
> [   13.559851]  [<89a96d81>] kernel_init_freeable+0xe5/0x15e
> [   13.560726]  [<894974fb>] kernel_init+0xb/0x100
> [   13.561705]  [<888c727c>] ? schedule_tail+0xc/0x50
> [   13.562786]  [<894a1942>] ret_from_kernel_thread+0xe/0x24
> [   13.563993]  [<894974f0>] ? rest_init+0x110/0x110
> 
> Fix this by clearing struct gadget containing the struct device on
> removal. This could also be fixed by disabling bind/unbind, but that is
> perhaps a useful feature to simulate device attach/detach.
> 
> Reported-by: kernel test robot 
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Rob Herring 
> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c 
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 77d07904f932..64f3d3cdcb91 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1060,6 +1060,7 @@ static int dummy_udc_remove(struct platform_device 
> *pdev)
>  
>   device_remove_file(>gadget.dev, _attr_function);
>   usb_del_gadget_udc(>gadget);
> + memset(>gadget, 0, sizeof(dum->gadget));
>   return 0;
>  }

Have you checked that no other parts of the driver are still using the
data structure while it is zeroed?  What about the host-side parts?  I 
see a few calls to udc_dev() in host code dev_dbg() lines, and 
references to fields in dum->gadget in more active parts of the code.

It might be a lot simpler in the end to forbid rebinding.  And if 
people want to support emulated attach/detach, it can be done via a 
writable sysfs attribute.

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 net] qmi_wwan: add support for Quectel EC21 and EC25

[David Miller]

From: Bjørn Mork 
Date: Mon, 10 Oct 2016 21:12:49 +0200

> The Quectel EC21 and EC25 need the same "set DTR" request as devices
> based on the MDM9230 chipset, but has no USB3 support. Our best guess
> is that the "set DTR" functionality depends on chipset and/or
> baseband firmware generation. But USB3 is still an optional feature.
> 
> Since we cannot enable this unconditionally for all older devices, and
> there doesn't appear to be anything we can use in the USB descriptors
> to identify these chips, we are forced to use a device specific quirk
> flag.
> 
> Reported-and-tested-by: Sebastian Sjoholm 
> Signed-off-by: Bjørn Mork 

Applied.
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Felipe Balbi]

Hi,

Baolin Wang  writes:
>> Baolin Wang  writes:
> Baolin Wang  writes:
> I'm thinking this is a bug in configfs interface of Gadget API, not
> dwc3. The only reason for this to happen would be if we get to
> ->udc_stop() with endpoints still enabled.
>
> Can you check if that's the case? i.e. can you check if any endpoints
> are still enabled when we get here?

 No, it is not any endpoints are still enabled. Like I said in commit
 message, we will start end transfer command when disable the endpoint,
 if the end transfer command complete event comes after we have freed
 the gadget irq, it will trigger the interrupt line all the time to
 crash the system.
>>>
>>> I see what the problem is. Databook tells us we *must* set CMDIOC when
>>> issuing EndTransfer command and we should always wait for Command
>>> Complete IRQ. However, we took a shortcut and just delayed for 100us
>>> after issuing End Transfer.
>>
>> Yes, but 100us delay is not enough in some scenarios, like changing
>> function with configfs frequently, we will met problems.
>
> heh, 100us *is* enough. However we still get an IRQ because we requested
> for it. If you want to fix this, then you need to find a way to get rid
> of that 100us udelay() and add a proper wait_for_completion() to delay
> execution until command complete IRQ fires up.

 I haven't tested this yet, but it shows the idea (I think we might still
 have a race with ep_queue if we're still waiting for End Transfer, but
>>>
>>> Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
>>> queuing one request.
>>
>> Yeah, I'll add that check later. I still need to make sure we would
>> still try to kick any transfers that might have been queued while
>> waiting for End Transfer Command Complete IRQ.
>
> OK. But I still worried if there are other races in some places which

There are no other places where this needs to be sorted out.

> is not easy to find out by testing. No introducing race condition
> would be one better solution, anyway I would like to test the patch
> you send out firstly.

Right, but your patch was working around another problem, rather then
fixing it.

Here's another version which should make sure everything remains working
as it should.

8<
>From 1f922a902a21d5cee32082be18bbfbf1d46137e4 Mon Sep 17 00:00:00 2001
From: Felipe Balbi 
Date: Thu, 13 Oct 2016 14:09:47 +0300
Subject: [PATCH] usb: dwc3: gadget: wait for End Transfer to complete

Instead of just delaying for 100us, we should
actually wait for End Transfer Command Complete
interrupt before moving on. Note that this should
only be done if we're dealing with one of the core
revisions that actually require the interrupt before
moving on.

Reported-by: Baolin Wang 
Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.h   | 10 +-
 drivers/usb/dwc3/gadget.c | 31 ---
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e878366ead00..cf495d932252 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -504,6 +505,7 @@ struct dwc3_event_buffer {
  * @endpoint: usb endpoint
  * @pending_list: list of pending requests for this endpoint
  * @started_list: list of started requests on this endpoint
+ * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
  * @lock: spinlock for endpoint request queue traversal
  * @regs: pointer to first endpoint register
  * @trb_pool: array of transaction buffers
@@ -529,6 +531,8 @@ struct dwc3_ep {
struct list_headpending_list;
struct list_headstarted_list;
 
+   wait_queue_head_t   wait_end_transfer;
+
spinlock_t  lock;
void __iomem*regs;
 
@@ -545,7 +549,8 @@ struct dwc3_ep {
 #define DWC3_EP_BUSY   (1 << 4)
 #define DWC3_EP_PENDING_REQUEST(1 << 5)
 #define DWC3_EP_MISSED_ISOC(1 << 6)
-
+#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
+#define DWC3_EP_KICK_POST_END_TRANSFER (1 << 8)
/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN(1 << 31)
 
@@ -1047,6 +1052,9 @@ struct dwc3_event_depevt {
 #define DEPEVT_TRANSFER_BUS_EXPIRY 2
 
u32 parameters:16;
+
+/* For Command Complete Events */
+#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
 } __packed;
 
 /**
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ba05b12e49a..a3f81b5ba901 100644
--- a/drivers/usb/dwc3/gadget.c
+++ 

Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Baolin Wang]

Hi Felipe,

On 13 October 2016 at 19:23, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
 Baolin Wang  writes:
 I'm thinking this is a bug in configfs interface of Gadget API, not
 dwc3. The only reason for this to happen would be if we get to
 ->udc_stop() with endpoints still enabled.

 Can you check if that's the case? i.e. can you check if any endpoints
 are still enabled when we get here?
>>>
>>> No, it is not any endpoints are still enabled. Like I said in commit
>>> message, we will start end transfer command when disable the endpoint,
>>> if the end transfer command complete event comes after we have freed
>>> the gadget irq, it will trigger the interrupt line all the time to
>>> crash the system.
>>
>> I see what the problem is. Databook tells us we *must* set CMDIOC when
>> issuing EndTransfer command and we should always wait for Command
>> Complete IRQ. However, we took a shortcut and just delayed for 100us
>> after issuing End Transfer.
>
> Yes, but 100us delay is not enough in some scenarios, like changing
> function with configfs frequently, we will met problems.

 heh, 100us *is* enough. However we still get an IRQ because we requested
 for it. If you want to fix this, then you need to find a way to get rid
 of that 100us udelay() and add a proper wait_for_completion() to delay
 execution until command complete IRQ fires up.
>>>
>>> I haven't tested this yet, but it shows the idea (I think we might still
>>> have a race with ep_queue if we're still waiting for End Transfer, but
>>
>> Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
>> queuing one request.
>
> Yeah, I'll add that check later. I still need to make sure we would
> still try to kick any transfers that might have been queued while
> waiting for End Transfer Command Complete IRQ.

OK. But I still worried if there are other races in some places which
is not easy to find out by testing. No introducing race condition
would be one better solution, anyway I would like to test the patch
you send out firstly.

>
>>> that's easy to sort out by checking for DWC3_EP_END_TRANSFER_PENDING
>>> before calling kick_transfer). Could you have a look and, perhaps, run a
>>> test?
>>
>> Sure. I will test it and send out the result tomorrow.
>
> Thank you
>
> --
> balbi



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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

On 13 October 2016 at 20:17, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep 
> *dep, unsigned cmd,
>   int susphy = false;
>   int ret = -EINVAL;
>
> + if (!dwc->pullups_connected)
> + return -ESHUTDOWN;
> +
>>
>> you skip trace_dwc3_gadget_ep_cmd()
>
> Yes, we did not need trace here since we did not send out the command.
>
 What in such case: enumeration will not work and this will be because
 this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
 will not know where the problem is.
 In my opinion this trace could be useful.
>>>
>>> We have returned the '-ESHUTDOWN' error number for user to know what
>>> happened.
>>
>> No, this is actually not good and Janusz has a very valid point
>> here. When we're debugging something in dwc3, we want to rely on
>> tracepoints to tell us what's going on. I don't want to have to add more
>> debugging messages to print out that ESHUTDOWN error just so I can
>> figure out what's going on. You should be patching this in a way that
>> the tracepoint is still printed out properly.
>
> Fine. Will fix this in next version.
>

 BTW, did you test this patch with device mode?
 Seems in my setup this fail - DWC3_DEPCMD_SETEPCONFIG for ep0out and
 gadget_start() failed (enumeration fail).
 Don't we need to queue ep0 cmds before pullup?
>>>
>>> Baolin, it's clear to me that you're not testing any of the patches
>>
>> I am sure I tested every patch I send to you. But this one is really
>> my mistake and I thought this is just one small change with just eye
>> review. I am really sorry for troubles.
>
> fair enough, luckily Janusz caught it before any harm could be
> done. Thanks for taking the time to explain.

Thanks for understanding and thanks for Janusz's testing.


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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Felipe Balbi]

Hi,

Baolin Wang  writes:
 @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep 
 *dep, unsigned cmd,
   int susphy = false;
   int ret = -EINVAL;

 + if (!dwc->pullups_connected)
 + return -ESHUTDOWN;
 +
>
> you skip trace_dwc3_gadget_ep_cmd()

 Yes, we did not need trace here since we did not send out the command.

>>> What in such case: enumeration will not work and this will be because
>>> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
>>> will not know where the problem is.
>>> In my opinion this trace could be useful.
>>
>> We have returned the '-ESHUTDOWN' error number for user to know what
>> happened.
>
> No, this is actually not good and Janusz has a very valid point
> here. When we're debugging something in dwc3, we want to rely on
> tracepoints to tell us what's going on. I don't want to have to add more
> debugging messages to print out that ESHUTDOWN error just so I can
> figure out what's going on. You should be patching this in a way that
> the tracepoint is still printed out properly.

 Fine. Will fix this in next version.

>>>
>>> BTW, did you test this patch with device mode?
>>> Seems in my setup this fail - DWC3_DEPCMD_SETEPCONFIG for ep0out and
>>> gadget_start() failed (enumeration fail).
>>> Don't we need to queue ep0 cmds before pullup?
>>
>> Baolin, it's clear to me that you're not testing any of the patches
>
> I am sure I tested every patch I send to you. But this one is really
> my mistake and I thought this is just one small change with just eye
> review. I am really sorry for troubles.

fair enough, luckily Janusz caught it before any harm could be
done. Thanks for taking the time to explain.

-- 
balbi


signature.asc
Description: PGP signature

Re: Interactive whiteboards

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Oct 13, 2016 at 01:57:54PM +0200, María Cano wrote:
> Okay, I will collect the following information:
> 
> 1) Plugging and unplugging USB diff with: cat /sys/kernel/debug/usb/devices

Yes.

> 2) Plugging and unplugging USB diff with: cat /proc/bus/input/devices

Yes.

> 3) What can I do to know for sure that the system sees the device as a
> serial device?

We should be able to determine that from step one above.

thanks,

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

Re: Interactive whiteboards

[María Cano]

Okay, I will collect the following information:

1) Plugging and unplugging USB diff with: cat /sys/kernel/debug/usb/devices

2) Plugging and unplugging USB diff with: cat /proc/bus/input/devices

3) What can I do to know for sure that the system sees the device as a
serial device?

Is there any other commands that might be useful?

2016-10-13 10:34 GMT+02:00 Greg KH :
> On Thu, Oct 13, 2016 at 10:24:38AM +0200, María Cano wrote:
>> Ok, I still have more IWB to investigate. I'll paste the new information 
>> soon.
>> We use a 3.10 kernel in our day-to-day (because is the kernel we have
>> fewer problems) but if you think it is advisable, I can use a newer
>> kernel (I recently tested with the kernel 4.6.1 and all these boards
>> were not working yet).
>>
>>
>> MULTICLASS BOARD (FIRST MODEL)
>> root@minino:/sys/kernel/debug/usb# cat devices
>> T:  Bus=03 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
>> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
>> P:  Vendor=093a ProdID=2510 Rev= 1.00
>> S:  Manufacturer=PIXART
>> S:  Product=USB OPTICAL MOUSE
>> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
>> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
>> E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=10ms
>>
>> T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
>> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
>> P:  Vendor=1c4f ProdID=0002 Rev= 1.10
>> S:  Manufacturer=SIGMACHIP
>> S:  Product=USB Keyboard
>> C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr= 98mA
>> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=01 Driver=usbhid
>> E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=10ms
>> I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
>> E:  Ad=82(I) Atr=03(Int.) MxPS=   3 Ivl=10ms
>>
>> T:  Bus=01 Lev=02 Prnt=02 Port=01 Cnt=01 Dev#=  6 Spd=12   MxCh= 0
>> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
>> P:  Vendor=0d8c ProdID=000c Rev= 1.00
>> S:  Product=C-Media USB Headphone Set
>> C:* #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=100mA
>> I:* If#= 0 Alt= 0 #EPs= 0 Cls=01(audio) Sub=01 Prot=00 Driver=snd-usb-audio
>> I:* If#= 1 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
>> I:  If#= 1 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
>> E:  Ad=01(O) Atr=09(Isoc) MxPS= 200 Ivl=1ms
>> I:* If#= 2 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
>> I:  If#= 2 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
>> E:  Ad=82(I) Atr=05(Isoc) MxPS= 100 Ivl=1ms
>> I:* If#= 3 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
>> E:  Ad=83(I) Atr=03(Int.) MxPS=   4 Ivl=32ms
>>
>> T:  Bus=01 Lev=02 Prnt=02 Port=02 Cnt=02 Dev#=  7 Spd=12   MxCh= 0
>> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
>> P:  Vendor=a3f0 ProdID=a002 Rev= 2.00
>> S:  Manufacturer=HS
>> S:  Product=Touch Board
>> C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
>> I:* If#= 0 Alt= 0 #EPs= 2 Cls=03(HID  ) Sub=01 Prot=00 Driver=usbhid
>> E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=5ms
>> E:  Ad=01(O) Atr=03(Int.) MxPS=  64 Ivl=5ms
>> I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
>> E:  Ad=82(I) Atr=03(Int.) MxPS=  64 Ivl=5ms
>
> So I've deleted the hub and storage devices from your system, and am
> left with the above devices that might be the touch board.
>
> The last one looks like the "real" one, and it is saying it is a HID
> device, and so the hid driver is bound to it.  So it really looks like
> the device is attached properly to the kernel.
>
> However, the HID protocol is a common one for companies to use to write
> userspace drivers for (it was the only way old versions of Windows would
> allow it), so they might be abusing the HID protocol to bind a userspace
> program to the device.
>
> What does /proc/bus/input/devices say about this input device?
>
> You might have more luck on the linux-input mailing list, as that's
> where the hid and input driver developers are.
>
>> HITACHI STARBOARD (FIRST MODEL)
>> root@minino:/sys/kernel/debug/usb# cat devices
>>
>> T:  Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=12   MxCh= 0
>> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
>> P:  Vendor=1477 ProdID=0008 Rev=13.13
>> S:  Manufacturer=eIT-Xiroku
>> S:  Product=Optical Touch Sensor HTM131
>> S:  SerialNumber=002301101075
>> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=500mA
>> I:  If#= 0 Alt= 0 #EPs= 0 Cls=ff(vend.) Sub=00 Prot=ff Driver=lsadrv
>> I:* If#= 0 Alt= 1 #EPs= 1 Cls=ff(vend.) Sub=00 Prot=ff Driver=lsadrv
>> E:  Ad=82(I) Atr=01(Isoc) MxPS= 900 Ivl=1ms
>>
>> T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=1.5  MxCh= 0
>> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
>> P:  Vendor=093a ProdID=2510 Rev= 1.00
>> S:  Manufacturer=PIXART
>> S:  Product=USB OPTICAL MOUSE
>> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
>> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
>> E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=10ms
>>
>>

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

Hi Felipe,

On 13 October 2016 at 19:22, Felipe Balbi  wrote:
>
> Hi,
>
> Janusz Dziedzic  writes:
 Baolin Wang  writes:
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 1783406..ca2ae5b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep 
>>> *dep, unsigned cmd,
>>>   int susphy = false;
>>>   int ret = -EINVAL;
>>>
>>> + if (!dwc->pullups_connected)
>>> + return -ESHUTDOWN;
>>> +

 you skip trace_dwc3_gadget_ep_cmd()
>>>
>>> Yes, we did not need trace here since we did not send out the command.
>>>
>> What in such case: enumeration will not work and this will be because
>> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
>> will not know where the problem is.
>> In my opinion this trace could be useful.
>
> We have returned the '-ESHUTDOWN' error number for user to know what
> happened.

 No, this is actually not good and Janusz has a very valid point
 here. When we're debugging something in dwc3, we want to rely on
 tracepoints to tell us what's going on. I don't want to have to add more
 debugging messages to print out that ESHUTDOWN error just so I can
 figure out what's going on. You should be patching this in a way that
 the tracepoint is still printed out properly.
>>>
>>> Fine. Will fix this in next version.
>>>
>>
>> BTW, did you test this patch with device mode?
>> Seems in my setup this fail - DWC3_DEPCMD_SETEPCONFIG for ep0out and
>> gadget_start() failed (enumeration fail).
>> Don't we need to queue ep0 cmds before pullup?
>
> Baolin, it's clear to me that you're not testing any of the patches

I am sure I tested every patch I send to you. But this one is really
my mistake and I thought this is just one small change with just eye
review. I am really sorry for troubles.

> you're sending me. I just reviewed this part of the code and we _do_
> indeed enable the control pipe before connecting pullups and that *must*
> be done this way, otherwise we won't be able to receive first Setup
> packet from host.
>
> How have you tested this? Against which tree?
>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Felipe Balbi]

Hi,

Baolin Wang  writes:
>>> Baolin Wang  writes:
>>> I'm thinking this is a bug in configfs interface of Gadget API, not
>>> dwc3. The only reason for this to happen would be if we get to
>>> ->udc_stop() with endpoints still enabled.
>>>
>>> Can you check if that's the case? i.e. can you check if any endpoints
>>> are still enabled when we get here?
>>
>> No, it is not any endpoints are still enabled. Like I said in commit
>> message, we will start end transfer command when disable the endpoint,
>> if the end transfer command complete event comes after we have freed
>> the gadget irq, it will trigger the interrupt line all the time to
>> crash the system.
>
> I see what the problem is. Databook tells us we *must* set CMDIOC when
> issuing EndTransfer command and we should always wait for Command
> Complete IRQ. However, we took a shortcut and just delayed for 100us
> after issuing End Transfer.

 Yes, but 100us delay is not enough in some scenarios, like changing
 function with configfs frequently, we will met problems.
>>>
>>> heh, 100us *is* enough. However we still get an IRQ because we requested
>>> for it. If you want to fix this, then you need to find a way to get rid
>>> of that 100us udelay() and add a proper wait_for_completion() to delay
>>> execution until command complete IRQ fires up.
>>
>> I haven't tested this yet, but it shows the idea (I think we might still
>> have a race with ep_queue if we're still waiting for End Transfer, but
>
> Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
> queuing one request.

Yeah, I'll add that check later. I still need to make sure we would
still try to kick any transfers that might have been queued while
waiting for End Transfer Command Complete IRQ.

>> that's easy to sort out by checking for DWC3_EP_END_TRANSFER_PENDING
>> before calling kick_transfer). Could you have a look and, perhaps, run a
>> test?
>
> Sure. I will test it and send out the result tomorrow.

Thank you

-- 
balbi


signature.asc
Description: PGP signature

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

Hi,

On 13 October 2016 at 19:22, Felipe Balbi  wrote:
>
> Hi,
>
> Janusz Dziedzic  writes:
 Baolin Wang  writes:
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 1783406..ca2ae5b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep 
>>> *dep, unsigned cmd,
>>>   int susphy = false;
>>>   int ret = -EINVAL;
>>>
>>> + if (!dwc->pullups_connected)
>>> + return -ESHUTDOWN;
>>> +

 you skip trace_dwc3_gadget_ep_cmd()
>>>
>>> Yes, we did not need trace here since we did not send out the command.
>>>
>> What in such case: enumeration will not work and this will be because
>> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
>> will not know where the problem is.
>> In my opinion this trace could be useful.
>
> We have returned the '-ESHUTDOWN' error number for user to know what
> happened.

 No, this is actually not good and Janusz has a very valid point
 here. When we're debugging something in dwc3, we want to rely on
 tracepoints to tell us what's going on. I don't want to have to add more
 debugging messages to print out that ESHUTDOWN error just so I can
 figure out what's going on. You should be patching this in a way that
 the tracepoint is still printed out properly.
>>>
>>> Fine. Will fix this in next version.
>>>
>>
>> BTW, did you test this patch with device mode?
>> Seems in my setup this fail - DWC3_DEPCMD_SETEPCONFIG for ep0out and
>> gadget_start() failed (enumeration fail).
>> Don't we need to queue ep0 cmds before pullup?
>
> Baolin, it's clear to me that you're not testing any of the patches
> you're sending me. I just reviewed this part of the code and we _do_
> indeed enable the control pipe before connecting pullups and that *must*
> be done this way, otherwise we won't be able to receive first Setup
> packet from host.

I am very sorry for this mistake. Since the original patch is adding
some 'pullups_connected' check in other places to avoid queuing
requests, but you suggested me this only affected the endpoint command
transfer, so I just follow your advice without one clear testing. I
really sorry for that. Please ignore this patch and I promise I will
test it enough before sending out any patches. Sorry again for
troubles.

>
> How have you tested this? Against which tree?
>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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/6] memstick: rtsx_usb_ms: Manage runtime PM when accessing the device

[Ulf Hansson]

Accesses to the rtsx usb device, which is the parent of the rtsx memstick
device, must not be done unless it's runtime resumed. This is currently not
the case and it could trigger various errors.

Fix this by properly deal with runtime PM in this regards. This means
making sure the device is runtime resumed, when serving requests via the
->request() callback or changing settings via the ->set_param() callbacks.

Cc: 
Cc: Ritesh Raj Sarraf 
Cc: Alan Stern 
Signed-off-by: Ulf Hansson 
---
 drivers/memstick/host/rtsx_usb_ms.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/memstick/host/rtsx_usb_ms.c 
b/drivers/memstick/host/rtsx_usb_ms.c
index 1b99489..2e3cf01 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -524,6 +524,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
int rc;
 
if (!host->req) {
+   pm_runtime_get_sync(ms_dev(host));
do {
rc = memstick_next_req(msh, >req);
dev_dbg(ms_dev(host), "next req %d\n", rc);
@@ -544,6 +545,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
host->req->error);
}
} while (!rc);
+   pm_runtime_put(ms_dev(host));
}
 
 }
@@ -570,6 +572,7 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
dev_dbg(ms_dev(host), "%s: param = %d, value = %d\n",
__func__, param, value);
 
+   pm_runtime_get_sync(ms_dev(host));
mutex_lock(>dev_mutex);
 
err = rtsx_usb_card_exclusive_check(ucr, RTSX_USB_MS_CARD);
@@ -635,6 +638,7 @@ static int rtsx_usb_ms_set_param(struct memstick_host *msh,
}
 out:
mutex_unlock(>dev_mutex);
+   pm_runtime_put(ms_dev(host));
 
/* power-on delay */
if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON)
-- 
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/6] mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when unused

[Ulf Hansson]

The rtsx_usb_sdmmc driver may bail out in its ->set_ios() callback when no
SD card is inserted. This is wrong, as it could cause the device to remain
runtime resumed when it's unused. Fix this behaviour.

Tested-by: Ritesh Raj Sarraf 
Cc: 
Cc: Alan Stern 
Signed-off-by: Ulf Hansson 
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c 
b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 4106295..e0b8590 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1138,11 +1138,6 @@ static void sdmmc_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
dev_dbg(sdmmc_dev(host), "%s\n", __func__);
mutex_lock(>dev_mutex);
 
-   if (rtsx_usb_card_exclusive_check(ucr, RTSX_USB_SD_CARD)) {
-   mutex_unlock(>dev_mutex);
-   return;
-   }
-
sd_set_power_mode(host, ios->power_mode);
sd_set_bus_width(host, ios->bus_width);
sd_set_timing(host, ios->timing, >ddr_mode);
-- 
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 6/6] mmc: core: Don't power off the card when starting the host

[Ulf Hansson]

The MMC_CAP2_NO_PRESCAN_POWERUP was invented to avoid running the power up
sequence, mmc_power_up(), during ->probe() of the mmc host driver, but
instead defer this to the mmc detect work. This is especially useful for
those hosts that suffers from a long initialization time, as this time
would otherwise add up to the total boot time.

However, due to the introduction of runtime PM of mmc host devices in the
mmc core, this behaviour changed a bit. More precisely, it caused the mmc
core to runtime resume the host device during ->probe() of the host driver.
In cases like the rtsx_usb_sdmmc, runtime resuming the device may be costly
and thus affecting the total boot time.

To improve this behaviour when using MMC_CAP2_NO_PRESCAN_POWERUP, let's
postpone also calling mmc_power_off() when starting the host. This change
allows the mmc core to avoid runtime resuming the device, as it don't need
to claim the host for that execution path.

Cc: Ritesh Raj Sarraf 
Cc: Alan Stern 
Signed-off-by: Ulf Hansson 
---
 drivers/mmc/core/core.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2553d90..2ad7291 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2824,12 +2824,11 @@ void mmc_start_host(struct mmc_host *host)
host->rescan_disable = 0;
host->ios.power_mode = MMC_POWER_UNDEFINED;
 
-   mmc_claim_host(host);
-   if (host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)
-   mmc_power_off(host);
-   else
+   if (!(host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)) {
+   mmc_claim_host(host);
mmc_power_up(host, host->ocr_avail);
-   mmc_release_host(host);
+   mmc_release_host(host);
+   }
 
mmc_gpiod_request_cd_irq(host);
_mmc_detect_change(host, 0, false);
-- 
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/6] mmc: rtsx_usb_sdmmc: Enable runtime PM autosuspend

[Ulf Hansson]

Enable runtime PM autosuspend for the rtsx_usb_sdmmc driver to avoid the
device being runtime suspended and runtime resumed between each request.
Let's use a default timeout of 50ms, to be consistent with other mmc hosts.

Cc: Ritesh Raj Sarraf 
Cc: Alan Stern 
Signed-off-by: Ulf Hansson 
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c 
b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 6e9c0f8..dc1abd1 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1374,6 +1374,8 @@ static int rtsx_usb_sdmmc_drv_probe(struct 
platform_device *pdev)
 
mutex_init(>host_mutex);
rtsx_usb_init_host(host);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
pm_runtime_enable(>dev);
 
 #ifdef RTSX_USB_USE_LEDS_CLASS
@@ -1428,6 +1430,7 @@ static int rtsx_usb_sdmmc_drv_remove(struct 
platform_device *pdev)
 
mmc_free_host(mmc);
pm_runtime_disable(>dev);
+   pm_runtime_dont_use_autosuspend(>dev);
platform_set_drvdata(pdev, NULL);
 
dev_dbg(&(pdev->dev),
-- 
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/6] memstick: rtsx_usb_ms: Runtime resume the device when polling for cards

[Ulf Hansson]

From: Alan Stern 

Accesses to the rtsx usb device, which is the parent of the rtsx memstick
device, must not be done unless it's runtime resumed.

Therefore when the rtsx_usb_ms driver polls for inserted memstick cards,
let's add pm_runtime_get|put*() to make sure accesses is done when the
rtsx usb device is runtime resumed.

Reported-by: Ritesh Raj Sarraf 
Tested-by: Ritesh Raj Sarraf 
Signed-off-by: Alan Stern 
Cc: 
Signed-off-by: Ulf Hansson 
---
 drivers/memstick/host/rtsx_usb_ms.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/memstick/host/rtsx_usb_ms.c 
b/drivers/memstick/host/rtsx_usb_ms.c
index d34bc35..1b99489 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -681,6 +681,7 @@ static int rtsx_usb_detect_ms_card(void *__host)
int err;
 
for (;;) {
+   pm_runtime_get_sync(ms_dev(host));
mutex_lock(>dev_mutex);
 
/* Check pending MS card changes */
@@ -703,6 +704,7 @@ static int rtsx_usb_detect_ms_card(void *__host)
}
 
 poll_again:
+   pm_runtime_put(ms_dev(host));
if (host->eject)
break;
 
-- 
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/6] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led

[Ulf Hansson]

Accesses of the rtsx sdmmc's parent device, which is the rtsx usb device,
must be done when it's runtime resumed. Currently this isn't case when
changing the led, so let's fix this by adding a pm_runtime_get_sync() and
a pm_runtime_put() around those operations.

Reported-by: Ritesh Raj Sarraf 
Tested-by: Ritesh Raj Sarraf 
Cc: 
Cc: Alan Stern 
Signed-off-by: Ulf Hansson 
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c 
b/drivers/mmc/host/rtsx_usb_sdmmc.c
index e0b8590..6e9c0f8 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1309,6 +1309,7 @@ static void rtsx_usb_update_led(struct work_struct *work)
container_of(work, struct rtsx_usb_sdmmc, led_work);
struct rtsx_ucr *ucr = host->ucr;
 
+   pm_runtime_get_sync(sdmmc_dev(host));
mutex_lock(>dev_mutex);
 
if (host->led.brightness == LED_OFF)
@@ -1317,6 +1318,7 @@ static void rtsx_usb_update_led(struct work_struct *work)
rtsx_usb_turn_on_led(ucr);
 
mutex_unlock(>dev_mutex);
+   pm_runtime_put(sdmmc_dev(host));
 }
 #endif
 
-- 
1.9.1

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

[PATCH 0/6] mmc/memstick: rtsx_usb: Fix runtime PM issues

[Ulf Hansson]

The rtsx_usb_sdmmc (mmc/sd) and rtsx_usb_ms (memstick) devices are interfacing
an rtsx_usb device (managed by an mfd driver) while communicating with the
cards. The mmc/sd and memstick devices are children of the usb device.

Issues has been reported [1] for rtsx, which have been investigated, discussed,
fixed, tested, etc, particularly when using mmc/sd cards. During the
investigation, some changes was proposed and tested successfully. In this
series I have picked up these changes and created some proper patches with
change-logs.

It turned out that most of the problems was related to the runtime PM
deployment in the memstick and the mmc/sd driver, which are fixed in this
series.

A couple of more issues were also discussed [2], which needs to be fixed as
well. Although they are out of scope for this series, so we will have to look
into those at a later point. Here's a brief summary of these leftovers:

*)
It seems reasonable to turn off autosuspend for usb devices and instead rely on
the usb device's children to deal with autosuspend. The reason is simply that
the children has better knowledge of when using autosuspend makes sense.

**)
Polling card detect mode is used both for mmc/sd and memstick. Because of the
lack of synchronization between the polling attempts for these subsystems, the
usb device may be powered on for unnecessary long intervals, thus we are
wasting power.

Besides the above changes, I folded in a change in the mmc core which improves
the behaviour for mmc hosts using MMC_CAP2_NO_PRESCAN_POWERUP, which is the
case for the rtsx_usb_sdmmc driver.  This change should improve the boot time
with ~100ms per rtsx_usb_sdmmc device.

Finally, as I couldn't find an active maintainer for the memstick
subsystem/driver and because the author of the drivers can be reached, I
volunteer to take this all through my mmc tree. Please tell me if that is
problem to any of you.

[1]
https://www.spinics.net/lists/linux-usb/msg146998.html
[2]
https://www.spinics.net/lists/linux-mmc/msg39235.html


Alan Stern (1):
  memstick: rtsx_usb_ms: Runtime resume the device when polling for
cards

Ulf Hansson (5):
  mmc: rtsx_usb_sdmmc: Avoid keeping the device runtime resumed when
unused
  mmc: rtsx_usb_sdmmc: Handle runtime PM while changing the led
  memstick: rtsx_usb_ms: Manage runtime PM when accessing the device
  mmc: rtsx_usb_sdmmc: Enable runtime PM autosuspend
  mmc: core: Don't power off the card when starting the host

 drivers/memstick/host/rtsx_usb_ms.c |  6 ++
 drivers/mmc/core/core.c |  9 -
 drivers/mmc/host/rtsx_usb_sdmmc.c   | 10 +-
 3 files changed, 15 insertions(+), 10 deletions(-)

-- 
1.9.1

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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Felipe Balbi]

Hi,

Janusz Dziedzic  writes:
>>> Baolin Wang  writes:
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1783406..ca2ae5b 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
>> unsigned cmd,
>>   int susphy = false;
>>   int ret = -EINVAL;
>>
>> + if (!dwc->pullups_connected)
>> + return -ESHUTDOWN;
>> +
>>>
>>> you skip trace_dwc3_gadget_ep_cmd()
>>
>> Yes, we did not need trace here since we did not send out the command.
>>
> What in such case: enumeration will not work and this will be because
> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
> will not know where the problem is.
> In my opinion this trace could be useful.

 We have returned the '-ESHUTDOWN' error number for user to know what
 happened.
>>>
>>> No, this is actually not good and Janusz has a very valid point
>>> here. When we're debugging something in dwc3, we want to rely on
>>> tracepoints to tell us what's going on. I don't want to have to add more
>>> debugging messages to print out that ESHUTDOWN error just so I can
>>> figure out what's going on. You should be patching this in a way that
>>> the tracepoint is still printed out properly.
>>
>> Fine. Will fix this in next version.
>>
>
> BTW, did you test this patch with device mode?
> Seems in my setup this fail - DWC3_DEPCMD_SETEPCONFIG for ep0out and
> gadget_start() failed (enumeration fail).
> Don't we need to queue ep0 cmds before pullup?

Baolin, it's clear to me that you're not testing any of the patches
you're sending me. I just reviewed this part of the code and we _do_
indeed enable the control pipe before connecting pullups and that *must*
be done this way, otherwise we won't be able to receive first Setup
packet from host.

How have you tested this? Against which tree?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Baolin Wang]

On 13 October 2016 at 18:56, Felipe Balbi  wrote:
>
> Hi,
>
> Felipe Balbi  writes:
>> Hi,
>>
>> Baolin Wang  writes:
>> I'm thinking this is a bug in configfs interface of Gadget API, not
>> dwc3. The only reason for this to happen would be if we get to
>> ->udc_stop() with endpoints still enabled.
>>
>> Can you check if that's the case? i.e. can you check if any endpoints
>> are still enabled when we get here?
>
> No, it is not any endpoints are still enabled. Like I said in commit
> message, we will start end transfer command when disable the endpoint,
> if the end transfer command complete event comes after we have freed
> the gadget irq, it will trigger the interrupt line all the time to
> crash the system.

 I see what the problem is. Databook tells us we *must* set CMDIOC when
 issuing EndTransfer command and we should always wait for Command
 Complete IRQ. However, we took a shortcut and just delayed for 100us
 after issuing End Transfer.
>>>
>>> Yes, but 100us delay is not enough in some scenarios, like changing
>>> function with configfs frequently, we will met problems.
>>
>> heh, 100us *is* enough. However we still get an IRQ because we requested
>> for it. If you want to fix this, then you need to find a way to get rid
>> of that 100us udelay() and add a proper wait_for_completion() to delay
>> execution until command complete IRQ fires up.
>
> I haven't tested this yet, but it shows the idea (I think we might still
> have a race with ep_queue if we're still waiting for End Transfer, but

Yes, maybe we need check DWC3_EP_END_TRANSFER_PENDING flag when
queuing one request.

> that's easy to sort out by checking for DWC3_EP_END_TRANSFER_PENDING
> before calling kick_transfer). Could you have a look and, perhaps, run a
> test?

Sure. I will test it and send out the result tomorrow.

>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index e878366ead00..24a77e9f9bba 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -504,6 +505,7 @@ struct dwc3_event_buffer {
>   * @endpoint: usb endpoint
>   * @pending_list: list of pending requests for this endpoint
>   * @started_list: list of started requests on this endpoint
> + * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
>   * @lock: spinlock for endpoint request queue traversal
>   * @regs: pointer to first endpoint register
>   * @trb_pool: array of transaction buffers
> @@ -529,6 +531,8 @@ struct dwc3_ep {
> struct list_headpending_list;
> struct list_headstarted_list;
>
> +   wait_queue_head_t   wait_end_transfer;
> +
> spinlock_t  lock;
> void __iomem*regs;
>
> @@ -545,7 +549,7 @@ struct dwc3_ep {
>  #define DWC3_EP_BUSY   (1 << 4)
>  #define DWC3_EP_PENDING_REQUEST(1 << 5)
>  #define DWC3_EP_MISSED_ISOC(1 << 6)
> -
> +#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
> /* This last one is specific to EP0 */
>  #define DWC3_EP0_DIR_IN(1 << 31)
>
> @@ -1047,6 +1051,9 @@ struct dwc3_event_depevt {
>  #define DEPEVT_TRANSFER_BUS_EXPIRY 2
>
> u32 parameters:16;
> +
> +/* For Command Complete Events */
> +#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
>  } __packed;
>
>  /**
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 3ba05b12e49a..8037bff43485 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> reg |= DWC3_DALEPENA_EP(dep->number);
> dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
>
> +   init_waitqueue_head(>wait_end_transfer);
> +
> if (usb_endpoint_xfer_control(desc))
> return 0;
>
> @@ -2156,6 +2158,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>  {
> struct dwc3_ep  *dep;
> u8  epnum = event->endpoint_number;
> +   u8  cmd;
>
> dep = dwc->eps[epnum];
>
> @@ -2200,8 +2203,13 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
> return;
> }
> break;
> -   case DWC3_DEPEVT_RXTXFIFOEVT:
> case DWC3_DEPEVT_EPCMDCMPLT:
> +   cmd = DEPEVT_PARAMETER_CMD(event->parameters);
> +
> +   if (cmd == DWC3_DEPCMD_ENDTRANSFER)
> +   dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
> +   break;
> +   case DWC3_DEPEVT_RXTXFIFOEVT:
> break;
> }
>  }
> @@ -2246,6 +2254,7 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
>  }
>
>  static void dwc3_stop_active_transfer(struct 

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Janusz Dziedzic]

On 13 October 2016 at 12:41, Baolin Wang  wrote:
> On 13 October 2016 at 17:49, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> Baolin Wang  writes:
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 1783406..ca2ae5b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
> unsigned cmd,
>   int susphy = false;
>   int ret = -EINVAL;
>
> + if (!dwc->pullups_connected)
> + return -ESHUTDOWN;
> +
>>
>> you skip trace_dwc3_gadget_ep_cmd()
>
> Yes, we did not need trace here since we did not send out the command.
>
 What in such case: enumeration will not work and this will be because
 this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
 will not know where the problem is.
 In my opinion this trace could be useful.
>>>
>>> We have returned the '-ESHUTDOWN' error number for user to know what
>>> happened.
>>
>> No, this is actually not good and Janusz has a very valid point
>> here. When we're debugging something in dwc3, we want to rely on
>> tracepoints to tell us what's going on. I don't want to have to add more
>> debugging messages to print out that ESHUTDOWN error just so I can
>> figure out what's going on. You should be patching this in a way that
>> the tracepoint is still printed out properly.
>
> Fine. Will fix this in next version.
>

BTW, did you test this patch with device mode?
Seems in my setup this fail - DWC3_DEPCMD_SETEPCONFIG for ep0out and
gadget_start() failed (enumeration fail).
Don't we need to queue ep0 cmds before pullup?

BR
Janusz
>>
>> Keep in mind that you should be setting ret to -ESHUTDOWN and make sure
>> the trace is printed. Same patch should, then, patch trace.h to handle
>> -ESHUTDOWN as an error case, i.e. following hunk should be added:
>>
>> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
>> index d93780e84f07..70b783f0507d 100644
>> --- a/drivers/usb/dwc3/debug.h
>> +++ b/drivers/usb/dwc3/debug.h
>> @@ -319,6 +319,8 @@ static inline const char *dwc3_ep_cmd_status_string(int 
>> status)
>> switch (status) {
>> case -ETIMEDOUT:
>> return "Timed Out";
>> +   case -ESHUTDOWN:
>> +   return "Shut Down";
>> case 0:
>> return "Successful";
>> case DEPEVT_TRANSFER_NO_RESOURCE:
>>
>> --
>> balbi
>
>
>
> --
> Baolin.wang
> Best Regards
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Felipe Balbi]

Hi,

Felipe Balbi  writes:
> Hi,
>
> Baolin Wang  writes:
> I'm thinking this is a bug in configfs interface of Gadget API, not
> dwc3. The only reason for this to happen would be if we get to
> ->udc_stop() with endpoints still enabled.
>
> Can you check if that's the case? i.e. can you check if any endpoints
> are still enabled when we get here?

 No, it is not any endpoints are still enabled. Like I said in commit
 message, we will start end transfer command when disable the endpoint,
 if the end transfer command complete event comes after we have freed
 the gadget irq, it will trigger the interrupt line all the time to
 crash the system.
>>>
>>> I see what the problem is. Databook tells us we *must* set CMDIOC when
>>> issuing EndTransfer command and we should always wait for Command
>>> Complete IRQ. However, we took a shortcut and just delayed for 100us
>>> after issuing End Transfer.
>>
>> Yes, but 100us delay is not enough in some scenarios, like changing
>> function with configfs frequently, we will met problems.
>
> heh, 100us *is* enough. However we still get an IRQ because we requested
> for it. If you want to fix this, then you need to find a way to get rid
> of that 100us udelay() and add a proper wait_for_completion() to delay
> execution until command complete IRQ fires up.

I haven't tested this yet, but it shows the idea (I think we might still
have a race with ep_queue if we're still waiting for End Transfer, but
that's easy to sort out by checking for DWC3_EP_END_TRANSFER_PENDING
before calling kick_transfer). Could you have a look and, perhaps, run a
test?

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e878366ead00..24a77e9f9bba 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -504,6 +505,7 @@ struct dwc3_event_buffer {
  * @endpoint: usb endpoint
  * @pending_list: list of pending requests for this endpoint
  * @started_list: list of started requests on this endpoint
+ * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete
  * @lock: spinlock for endpoint request queue traversal
  * @regs: pointer to first endpoint register
  * @trb_pool: array of transaction buffers
@@ -529,6 +531,8 @@ struct dwc3_ep {
struct list_headpending_list;
struct list_headstarted_list;
 
+   wait_queue_head_t   wait_end_transfer;
+
spinlock_t  lock;
void __iomem*regs;
 
@@ -545,7 +549,7 @@ struct dwc3_ep {
 #define DWC3_EP_BUSY   (1 << 4)
 #define DWC3_EP_PENDING_REQUEST(1 << 5)
 #define DWC3_EP_MISSED_ISOC(1 << 6)
-
+#define DWC3_EP_END_TRANSFER_PENDING (1 << 7)
/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN(1 << 31)
 
@@ -1047,6 +1051,9 @@ struct dwc3_event_depevt {
 #define DEPEVT_TRANSFER_BUS_EXPIRY 2
 
u32 parameters:16;
+
+/* For Command Complete Events */
+#define DEPEVT_PARAMETER_CMD(n) (((n) & (0xf << 24)) >> 24)
 } __packed;
 
 /**
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ba05b12e49a..8037bff43485 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -598,6 +598,8 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
reg |= DWC3_DALEPENA_EP(dep->number);
dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
 
+   init_waitqueue_head(>wait_end_transfer);
+
if (usb_endpoint_xfer_control(desc))
return 0;
 
@@ -2156,6 +2158,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
 {
struct dwc3_ep  *dep;
u8  epnum = event->endpoint_number;
+   u8  cmd;
 
dep = dwc->eps[epnum];
 
@@ -2200,8 +2203,13 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
return;
}
break;
-   case DWC3_DEPEVT_RXTXFIFOEVT:
case DWC3_DEPEVT_EPCMDCMPLT:
+   cmd = DEPEVT_PARAMETER_CMD(event->parameters);
+
+   if (cmd == DWC3_DEPCMD_ENDTRANSFER)
+   dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
+   break;
+   case DWC3_DEPEVT_RXTXFIFOEVT:
break;
}
 }
@@ -2246,6 +2254,7 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
 }
 
 static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum, bool force)
+__releases(>lock) __acquires(>lock)
 {
struct dwc3_ep *dep;
struct dwc3_gadget_ep_cmd_params params;
@@ -2295,6 +2304,12 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, 
u32 epnum, bool force)
memset(, 0, sizeof(params));
ret = dwc3_send_gadget_ep_cmd(dep, cmd, );
WARN_ON_ONCE(ret);
+
+   dep->flags |= 

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

On 13 October 2016 at 17:49, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 1783406..ca2ae5b 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
 unsigned cmd,
   int susphy = false;
   int ret = -EINVAL;

 + if (!dwc->pullups_connected)
 + return -ESHUTDOWN;
 +
>
> you skip trace_dwc3_gadget_ep_cmd()

 Yes, we did not need trace here since we did not send out the command.

>>> What in such case: enumeration will not work and this will be because
>>> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
>>> will not know where the problem is.
>>> In my opinion this trace could be useful.
>>
>> We have returned the '-ESHUTDOWN' error number for user to know what
>> happened.
>
> No, this is actually not good and Janusz has a very valid point
> here. When we're debugging something in dwc3, we want to rely on
> tracepoints to tell us what's going on. I don't want to have to add more
> debugging messages to print out that ESHUTDOWN error just so I can
> figure out what's going on. You should be patching this in a way that
> the tracepoint is still printed out properly.

Fine. Will fix this in next version.

>
> Keep in mind that you should be setting ret to -ESHUTDOWN and make sure
> the trace is printed. Same patch should, then, patch trace.h to handle
> -ESHUTDOWN as an error case, i.e. following hunk should be added:
>
> diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
> index d93780e84f07..70b783f0507d 100644
> --- a/drivers/usb/dwc3/debug.h
> +++ b/drivers/usb/dwc3/debug.h
> @@ -319,6 +319,8 @@ static inline const char *dwc3_ep_cmd_status_string(int 
> status)
> switch (status) {
> case -ETIMEDOUT:
> return "Timed Out";
> +   case -ESHUTDOWN:
> +   return "Shut Down";
> case 0:
> return "Successful";
> case DEPEVT_TRANSFER_NO_RESOURCE:
>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Felipe Balbi]

Hi,

Baolin Wang  writes:
 I'm thinking this is a bug in configfs interface of Gadget API, not
 dwc3. The only reason for this to happen would be if we get to
 ->udc_stop() with endpoints still enabled.

 Can you check if that's the case? i.e. can you check if any endpoints
 are still enabled when we get here?
>>>
>>> No, it is not any endpoints are still enabled. Like I said in commit
>>> message, we will start end transfer command when disable the endpoint,
>>> if the end transfer command complete event comes after we have freed
>>> the gadget irq, it will trigger the interrupt line all the time to
>>> crash the system.
>>
>> I see what the problem is. Databook tells us we *must* set CMDIOC when
>> issuing EndTransfer command and we should always wait for Command
>> Complete IRQ. However, we took a shortcut and just delayed for 100us
>> after issuing End Transfer.
>
> Yes, but 100us delay is not enough in some scenarios, like changing
> function with configfs frequently, we will met problems.

heh, 100us *is* enough. However we still get an IRQ because we requested
for it. If you want to fix this, then you need to find a way to get rid
of that 100us udelay() and add a proper wait_for_completion() to delay
execution until command complete IRQ fires up.

-- 
balbi


signature.asc
Description: PGP signature

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Felipe Balbi]

Hi,

Baolin Wang  writes:
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 1783406..ca2ae5b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
>>> unsigned cmd,
>>>   int susphy = false;
>>>   int ret = -EINVAL;
>>>
>>> + if (!dwc->pullups_connected)
>>> + return -ESHUTDOWN;
>>> +

 you skip trace_dwc3_gadget_ep_cmd()
>>>
>>> Yes, we did not need trace here since we did not send out the command.
>>>
>> What in such case: enumeration will not work and this will be because
>> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
>> will not know where the problem is.
>> In my opinion this trace could be useful.
>
> We have returned the '-ESHUTDOWN' error number for user to know what
> happened.

No, this is actually not good and Janusz has a very valid point
here. When we're debugging something in dwc3, we want to rely on
tracepoints to tell us what's going on. I don't want to have to add more
debugging messages to print out that ESHUTDOWN error just so I can
figure out what's going on. You should be patching this in a way that
the tracepoint is still printed out properly.

Keep in mind that you should be setting ret to -ESHUTDOWN and make sure
the trace is printed. Same patch should, then, patch trace.h to handle
-ESHUTDOWN as an error case, i.e. following hunk should be added:

diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index d93780e84f07..70b783f0507d 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -319,6 +319,8 @@ static inline const char *dwc3_ep_cmd_status_string(int 
status)
switch (status) {
case -ETIMEDOUT:
return "Timed Out";
+   case -ESHUTDOWN:
+   return "Shut Down";
case 0:
return "Successful";
case DEPEVT_TRANSFER_NO_RESOURCE:

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v7 2/8] power: add power sequence library

[Peter Chen]

On Thu, Oct 13, 2016 at 09:04:42AM +0200, Heiko Stuebner wrote:
> > > > +static int __init pwrseq_generic_register(void)
> > > > +{
> > > > +   struct pwrseq_generic *pwrseq_gen;
> > > > +   int i;
> > > > +
> > > > +   for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> > > > +   pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> > > > +   if (!pwrseq_gen)
> > > > +   return -ENOMEM;
> > > > +
> > > > +   pwrseq_gen->pwrseq.pwrseq_of_match_table = 
> > > > generic_id_table;
> > > > +   pwrseq_gen->pwrseq.get = pwrseq_generic_get;
> > > > +   pwrseq_gen->pwrseq.on = pwrseq_generic_on;
> > > > +   pwrseq_gen->pwrseq.off = pwrseq_generic_off;
> > > > +   pwrseq_gen->pwrseq.put = pwrseq_generic_put;
> > > > +   pwrseq_gen->pwrseq.free = pwrseq_generic_free;
> > > > +
> > > > +   pwrseq_register(_gen->pwrseq);
> > > > +   }
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +postcore_initcall(pwrseq_generic_register)
> > > 
> > > I see that you need to have it preallocated for the compatible matching,
> > > but wouldn't it also work to either just register the type and allocate
> > > dynamically or otherwise just allocate a new spare everytime
> > > pwrseq_generic_get() picks up the previous spare?
> > 
> > Before compatible matching, the host driver doesn't know which pwrseq type
> > for its child node, then doesn't know which pwrseq instance needs to be
> > allocated. From dts, we don't know which pwrseq type for the node.
> 
> yes, that is why I was suggesting allocating one (or two) here in 
> pwrseq_generic_register() and every time pwrseq_generic_get() grabs the last 
> free sequence instance, you allocate a new free spare one in that function.

Good idea.

-- 

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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

On 13 October 2016 at 16:28, Janusz Dziedzic  wrote:
> On 13 October 2016 at 10:21, Baolin Wang  wrote:
>> Hi,
>>
>> On 13 October 2016 at 16:16, Janusz Dziedzic  
>> wrote:
>>> On 13 October 2016 at 09:37, Baolin Wang  wrote:
 Hi,

 On 13 October 2016 at 15:06, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> When system has stpped the gadget, we should avoid queuing any requests
>
> queueing is *not* a problem. Starting is. In fact, that's what your
> patch is doing.

 OK.

>
>> which will cause tranfer failed. Thus adding some disconnect checking to
>^^^
>transfer

 Sorry for spelling mistake, will fix it.

>
>> avoid this situation.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> Changes since v2:
>>  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
>>  - Rename completion name and issue complete() at one place.
>>  - Move completion initialization into dwc3_gadget_init().
>>
>> Changes since v1:
>>  - Split into 2 separate ptaches.
>>  - Choose complete mechanism instead of polling.
>> ---
>>  drivers/usb/dwc3/gadget.c |3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1783406..ca2ae5b 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
>> unsigned cmd,
>>   int susphy = false;
>>   int ret = -EINVAL;
>>
>> + if (!dwc->pullups_connected)
>> + return -ESHUTDOWN;
>> +
>>>
>>> you skip trace_dwc3_gadget_ep_cmd()
>>
>> Yes, we did not need trace here since we did not send out the command.
>>
> What in such case: enumeration will not work and this will be because
> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
> will not know where the problem is.
> In my opinion this trace could be useful.

We have returned the '-ESHUTDOWN' error number for user to know what happened.

>
> BR
> Janusz
>
>>>
>>> BR
>>> Janusz
>>>
>>   /*
>>* Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that 
>> if
>>* we're issuing an endpoint command, we must check if
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> balbi



 --
 Baolin.wang
 Best Regards
 --
 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
>>
>>
>>
>> --
>> Baolin.wang
>> Best Regards



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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Janusz Dziedzic]

On 13 October 2016 at 10:21, Baolin Wang  wrote:
> Hi,
>
> On 13 October 2016 at 16:16, Janusz Dziedzic  
> wrote:
>> On 13 October 2016 at 09:37, Baolin Wang  wrote:
>>> Hi,
>>>
>>> On 13 October 2016 at 15:06, Felipe Balbi  wrote:

 Hi,

 Baolin Wang  writes:
> When system has stpped the gadget, we should avoid queuing any requests

 queueing is *not* a problem. Starting is. In fact, that's what your
 patch is doing.
>>>
>>> OK.
>>>

> which will cause tranfer failed. Thus adding some disconnect checking to
^^^
transfer
>>>
>>> Sorry for spelling mistake, will fix it.
>>>

> avoid this situation.
>
> Signed-off-by: Baolin Wang 
> ---
> Changes since v2:
>  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
>  - Rename completion name and issue complete() at one place.
>  - Move completion initialization into dwc3_gadget_init().
>
> Changes since v1:
>  - Split into 2 separate ptaches.
>  - Choose complete mechanism instead of polling.
> ---
>  drivers/usb/dwc3/gadget.c |3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 1783406..ca2ae5b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
> unsigned cmd,
>   int susphy = false;
>   int ret = -EINVAL;
>
> + if (!dwc->pullups_connected)
> + return -ESHUTDOWN;
> +
>>
>> you skip trace_dwc3_gadget_ep_cmd()
>
> Yes, we did not need trace here since we did not send out the command.
>
What in such case: enumeration will not work and this will be because
this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
will not know where the problem is.
In my opinion this trace could be useful.

BR
Janusz

>>
>> BR
>> Janusz
>>
>   /*
>* Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
>* we're issuing an endpoint command, we must check if
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

 --
 balbi
>>>
>>>
>>>
>>> --
>>> Baolin.wang
>>> Best Regards
>>> --
>>> 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
>
>
>
> --
> Baolin.wang
> Best Regards
--
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: Interactive whiteboards

[Greg KH]

On Thu, Oct 13, 2016 at 10:24:38AM +0200, María Cano wrote:
> Ok, I still have more IWB to investigate. I'll paste the new information soon.
> We use a 3.10 kernel in our day-to-day (because is the kernel we have
> fewer problems) but if you think it is advisable, I can use a newer
> kernel (I recently tested with the kernel 4.6.1 and all these boards
> were not working yet).
> 
> 
> MULTICLASS BOARD (FIRST MODEL)
> root@minino:/sys/kernel/debug/usb# cat devices
> T:  Bus=03 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
> P:  Vendor=093a ProdID=2510 Rev= 1.00
> S:  Manufacturer=PIXART
> S:  Product=USB OPTICAL MOUSE
> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
> E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=10ms
> 
> T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
> P:  Vendor=1c4f ProdID=0002 Rev= 1.10
> S:  Manufacturer=SIGMACHIP
> S:  Product=USB Keyboard
> C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr= 98mA
> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=01 Driver=usbhid
> E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=10ms
> I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
> E:  Ad=82(I) Atr=03(Int.) MxPS=   3 Ivl=10ms
> 
> T:  Bus=01 Lev=02 Prnt=02 Port=01 Cnt=01 Dev#=  6 Spd=12   MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=0d8c ProdID=000c Rev= 1.00
> S:  Product=C-Media USB Headphone Set
> C:* #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=100mA
> I:* If#= 0 Alt= 0 #EPs= 0 Cls=01(audio) Sub=01 Prot=00 Driver=snd-usb-audio
> I:* If#= 1 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
> I:  If#= 1 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
> E:  Ad=01(O) Atr=09(Isoc) MxPS= 200 Ivl=1ms
> I:* If#= 2 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
> I:  If#= 2 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
> E:  Ad=82(I) Atr=05(Isoc) MxPS= 100 Ivl=1ms
> I:* If#= 3 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
> E:  Ad=83(I) Atr=03(Int.) MxPS=   4 Ivl=32ms
> 
> T:  Bus=01 Lev=02 Prnt=02 Port=02 Cnt=02 Dev#=  7 Spd=12   MxCh= 0
> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=a3f0 ProdID=a002 Rev= 2.00
> S:  Manufacturer=HS
> S:  Product=Touch Board
> C:* #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
> I:* If#= 0 Alt= 0 #EPs= 2 Cls=03(HID  ) Sub=01 Prot=00 Driver=usbhid
> E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=5ms
> E:  Ad=01(O) Atr=03(Int.) MxPS=  64 Ivl=5ms
> I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
> E:  Ad=82(I) Atr=03(Int.) MxPS=  64 Ivl=5ms

So I've deleted the hub and storage devices from your system, and am
left with the above devices that might be the touch board.

The last one looks like the "real" one, and it is saying it is a HID
device, and so the hid driver is bound to it.  So it really looks like
the device is attached properly to the kernel.

However, the HID protocol is a common one for companies to use to write
userspace drivers for (it was the only way old versions of Windows would
allow it), so they might be abusing the HID protocol to bind a userspace
program to the device.

What does /proc/bus/input/devices say about this input device?

You might have more luck on the linux-input mailing list, as that's
where the hid and input driver developers are.

> HITACHI STARBOARD (FIRST MODEL)
> root@minino:/sys/kernel/debug/usb# cat devices
> 
> T:  Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=12   MxCh= 0
> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=1477 ProdID=0008 Rev=13.13
> S:  Manufacturer=eIT-Xiroku
> S:  Product=Optical Touch Sensor HTM131
> S:  SerialNumber=002301101075
> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 0 Cls=ff(vend.) Sub=00 Prot=ff Driver=lsadrv
> I:* If#= 0 Alt= 1 #EPs= 1 Cls=ff(vend.) Sub=00 Prot=ff Driver=lsadrv
> E:  Ad=82(I) Atr=01(Isoc) MxPS= 900 Ivl=1ms
> 
> T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=1.5  MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
> P:  Vendor=093a ProdID=2510 Rev= 1.00
> S:  Manufacturer=PIXART
> S:  Product=USB OPTICAL MOUSE
> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
> E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=10ms
> 
> 
> T:  Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
> P:  Vendor=046d ProdID=c31c Rev=64.00
> S:  Manufacturer=Logitech
> S:  Product=USB Keyboard
> C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr= 90mA
> I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=01 Driver=usbhid
> E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=10ms
> I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
> E:  Ad=82(I) Atr=03(Int.) MxPS=   4 Ivl=255ms

Re: Interactive whiteboards

[María Cano]

Ok, I still have more IWB to investigate. I'll paste the new information soon.
We use a 3.10 kernel in our day-to-day (because is the kernel we have
fewer problems) but if you think it is advisable, I can use a newer
kernel (I recently tested with the kernel 4.6.1 and all these boards
were not working yet).


MULTICLASS BOARD (FIRST MODEL)
root@minino:/sys/kernel/debug/usb# cat devices
T:  Bus=05 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#=  1 Spd=12   MxCh= 2
B:  Alloc=  0/900 us ( 0%), #Int=  0, #Iso=  0
D:  Ver= 1.10 Cls=09(hub  ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1d6b ProdID=0001 Rev= 3.10
S:  Manufacturer=Linux 3.10.1-picaros uhci_hcd
S:  Product=UHCI Host Controller
S:  SerialNumber=:00:1d.3
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=00 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   2 Ivl=255ms

T:  Bus=04 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#=  1 Spd=12   MxCh= 2
B:  Alloc=  0/900 us ( 0%), #Int=  0, #Iso=  0
D:  Ver= 1.10 Cls=09(hub  ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1d6b ProdID=0001 Rev= 3.10
S:  Manufacturer=Linux 3.10.1-picaros uhci_hcd
S:  Product=UHCI Host Controller
S:  SerialNumber=:00:1d.2
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=00 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   2 Ivl=255ms

T:  Bus=03 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#=  1 Spd=12   MxCh= 2
B:  Alloc= 11/900 us ( 1%), #Int=  1, #Iso=  0
D:  Ver= 1.10 Cls=09(hub  ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1d6b ProdID=0001 Rev= 3.10
S:  Manufacturer=Linux 3.10.1-picaros uhci_hcd
S:  Product=UHCI Host Controller
S:  SerialNumber=:00:1d.1
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=00 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   2 Ivl=255ms

T:  Bus=03 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=093a ProdID=2510 Rev= 1.00
S:  Manufacturer=PIXART
S:  Product=USB OPTICAL MOUSE
C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=usbhid
E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=10ms

T:  Bus=02 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#=  1 Spd=12   MxCh= 2
B:  Alloc= 25/900 us ( 3%), #Int=  2, #Iso=  0
D:  Ver= 1.10 Cls=09(hub  ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1d6b ProdID=0001 Rev= 3.10
S:  Manufacturer=Linux 3.10.1-picaros uhci_hcd
S:  Product=UHCI Host Controller
S:  SerialNumber=:00:1d.0
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=00 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   2 Ivl=255ms

T:  Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#=  2 Spd=1.5  MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=1c4f ProdID=0002 Rev= 1.10
S:  Manufacturer=SIGMACHIP
S:  Product=USB Keyboard
C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr= 98mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=01 Driver=usbhid
E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=10ms
I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=82(I) Atr=03(Int.) MxPS=   3 Ivl=10ms

T:  Bus=01 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#=  1 Spd=480  MxCh= 8
B:  Alloc=  2/800 us ( 0%), #Int=  4, #Iso=  0
D:  Ver= 2.00 Cls=09(hub  ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1d6b ProdID=0002 Rev= 3.10
S:  Manufacturer=Linux 3.10.1-picaros ehci_hcd
S:  Product=EHCI Host Controller
S:  SerialNumber=:00:1d.7
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=  0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=00 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   4 Ivl=256ms

T:  Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=480  MxCh= 7
D:  Ver= 2.00 Cls=09(hub  ) Sub=00 Prot=02 MxPS=64 #Cfgs=  1
P:  Vendor=1a40 ProdID=0201 Rev= 1.00
S:  Product=USB 2.0 Hub [MTT]
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=01 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   1 Ivl=256ms
I:* If#= 0 Alt= 1 #EPs= 1 Cls=09(hub  ) Sub=00 Prot=02 Driver=hub
E:  Ad=81(I) Atr=03(Int.) MxPS=   1 Ivl=256ms

T:  Bus=01 Lev=02 Prnt=02 Port=01 Cnt=01 Dev#=  6 Spd=12   MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=0d8c ProdID=000c Rev= 1.00
S:  Product=C-Media USB Headphone Set
C:* #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 0 Cls=01(audio) Sub=01 Prot=00 Driver=snd-usb-audio
I:* If#= 1 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
I:  If#= 1 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
E:  Ad=01(O) Atr=09(Isoc) MxPS= 200 Ivl=1ms
I:* If#= 2 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
I:  If#= 2 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=snd-usb-audio
E:  Ad=82(I) Atr=05(Isoc) MxPS= 100 Ivl=1ms
I:* If#= 3 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=83(I) Atr=03(Int.) MxPS=   4 Ivl=32ms

T:  Bus=01 Lev=02 Prnt=02 Port=02 Cnt=02 Dev#=  7 Spd=12   MxCh= 0
D:  Ver= 2.00 

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

Hi,

On 13 October 2016 at 16:16, Janusz Dziedzic  wrote:
> On 13 October 2016 at 09:37, Baolin Wang  wrote:
>> Hi,
>>
>> On 13 October 2016 at 15:06, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> Baolin Wang  writes:
 When system has stpped the gadget, we should avoid queuing any requests
>>>
>>> queueing is *not* a problem. Starting is. In fact, that's what your
>>> patch is doing.
>>
>> OK.
>>
>>>
 which will cause tranfer failed. Thus adding some disconnect checking to
>>>^^^
>>>transfer
>>
>> Sorry for spelling mistake, will fix it.
>>
>>>
 avoid this situation.

 Signed-off-by: Baolin Wang 
 ---
 Changes since v2:
  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
  - Rename completion name and issue complete() at one place.
  - Move completion initialization into dwc3_gadget_init().

 Changes since v1:
  - Split into 2 separate ptaches.
  - Choose complete mechanism instead of polling.
 ---
  drivers/usb/dwc3/gadget.c |3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 1783406..ca2ae5b 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
 unsigned cmd,
   int susphy = false;
   int ret = -EINVAL;

 + if (!dwc->pullups_connected)
 + return -ESHUTDOWN;
 +
>
> you skip trace_dwc3_gadget_ep_cmd()

Yes, we did not need trace here since we did not send out the command.

>
> BR
> Janusz
>
   /*
* Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
* we're issuing an endpoint command, we must check if
 --
 1.7.9.5

 --
 To unsubscribe from this list: send the line "unsubscribe linux-usb" in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>> --
>>> balbi
>>
>>
>>
>> --
>> Baolin.wang
>> Best Regards
>> --
>> 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



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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Janusz Dziedzic]

On 13 October 2016 at 09:37, Baolin Wang  wrote:
> Hi,
>
> On 13 October 2016 at 15:06, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> Baolin Wang  writes:
>>> When system has stpped the gadget, we should avoid queuing any requests
>>
>> queueing is *not* a problem. Starting is. In fact, that's what your
>> patch is doing.
>
> OK.
>
>>
>>> which will cause tranfer failed. Thus adding some disconnect checking to
>>^^^
>>transfer
>
> Sorry for spelling mistake, will fix it.
>
>>
>>> avoid this situation.
>>>
>>> Signed-off-by: Baolin Wang 
>>> ---
>>> Changes since v2:
>>>  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
>>>  - Rename completion name and issue complete() at one place.
>>>  - Move completion initialization into dwc3_gadget_init().
>>>
>>> Changes since v1:
>>>  - Split into 2 separate ptaches.
>>>  - Choose complete mechanism instead of polling.
>>> ---
>>>  drivers/usb/dwc3/gadget.c |3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 1783406..ca2ae5b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
>>> unsigned cmd,
>>>   int susphy = false;
>>>   int ret = -EINVAL;
>>>
>>> + if (!dwc->pullups_connected)
>>> + return -ESHUTDOWN;
>>> +

you skip trace_dwc3_gadget_ep_cmd()

BR
Janusz

>>>   /*
>>>* Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
>>>* we're issuing an endpoint command, we must check if
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> balbi
>
>
>
> --
> Baolin.wang
> Best Regards
> --
> 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: [RESEND PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget

[Baolin Wang]

On 13 October 2016 at 15:54, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> Hi,
>>
>> On 13 October 2016 at 15:08, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> Baolin Wang  writes:
 @@ -1487,10 +1496,22 @@ static int dwc3_gadget_pullup(struct usb_gadget 
 *g, int is_on)

   is_on = !!is_on;

 +try_again:
   spin_lock_irqsave(>lock, flags);
   ret = dwc3_gadget_run_stop(dwc, is_on, false);
   spin_unlock_irqrestore(>lock, flags);

 + if (ret == -EBUSY) {
 + ret = wait_for_completion_timeout(>ep0_in_setup,
 +   msecs_to_jiffies(500));
 + if (ret == 0) {
 + dev_err(dwc->dev, "timeout to stop gadget.\n");
 + ret = -ETIMEDOUT;
 + } else {
 + goto try_again;
>>>
>>> you are not really reading my comments. It's the third time I tell you
>>> there's no need for try_again. If you can't complete a control transfer
>>> in 500ms, you already have other issues. Take this thing out of here.
>>
>> I think you misunderstood the code. If there is 500ms timeout, we will
>> return '-ETIMEDOUT' error. If the control transfer is completed before
>> timeout, we can not just return and we need try again to stop the
>> gadget, right? Any other good suggestion? Thanks.
>
> Yeah, change the patch a bit so you wait for completion before calling
> dwc3_gadget_runt_stop()? I mean, move the !is_on && ep0_state check
> before calling dwc3_gadget_run_stop() and wait_for_completion_timeout()
> there.

OK. I will refactor the patch. Thanks.

>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Baolin Wang]

Hi,

On 13 October 2016 at 15:51, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> Hi,
>>
>> On 13 October 2016 at 15:02, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> Baolin Wang  writes:
 @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
   dwc->gadget_driver  = NULL;
   spin_unlock_irqrestore(>lock, flags);

 + /*
 +  * Since the xHCI will share the same interrupt with gadget, thus 
 when
 +  * free the gadget irq, it will not shutdown this gadget irq line. 
 Then
 +  * the gadget driver can not handle the end transfer command complete
 +  * event after free the gadget irq, which will hang the system to 
 crash.
 +  *
 +  * So we should wait for the end transfer command complete event 
 before
 +  * free it to avoid this situation.
 +  */
 + dwc3_wait_command_complete(dwc);
>>>
>>> this doesn't make sense. We have already masked all interrupts before
>>> getting here. We have also, already, disabled all endpoints.
>>
>> No, you just mask the interrupts described in DEVTEN register, and you
>> did not disable the endpoint command complete event.
>
> True that
>
>>> I'm thinking this is a bug in configfs interface of Gadget API, not
>>> dwc3. The only reason for this to happen would be if we get to
>>> ->udc_stop() with endpoints still enabled.
>>>
>>> Can you check if that's the case? i.e. can you check if any endpoints
>>> are still enabled when we get here?
>>
>> No, it is not any endpoints are still enabled. Like I said in commit
>> message, we will start end transfer command when disable the endpoint,
>> if the end transfer command complete event comes after we have freed
>> the gadget irq, it will trigger the interrupt line all the time to
>> crash the system.
>
> I see what the problem is. Databook tells us we *must* set CMDIOC when
> issuing EndTransfer command and we should always wait for Command
> Complete IRQ. However, we took a shortcut and just delayed for 100us
> after issuing End Transfer.

Yes, but 100us delay is not enough in some scenarios, like changing
function with configfs frequently, we will met problems.

>
> It seems as if *that* should be fixed. We should start actually waiting
> for Command Complete IRQ.
>
> --
> balbi



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

Re: [RESEND PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> Hi,
>
> On 13 October 2016 at 15:08, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> Baolin Wang  writes:
>>> @@ -1487,10 +1496,22 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
>>> int is_on)
>>>
>>>   is_on = !!is_on;
>>>
>>> +try_again:
>>>   spin_lock_irqsave(>lock, flags);
>>>   ret = dwc3_gadget_run_stop(dwc, is_on, false);
>>>   spin_unlock_irqrestore(>lock, flags);
>>>
>>> + if (ret == -EBUSY) {
>>> + ret = wait_for_completion_timeout(>ep0_in_setup,
>>> +   msecs_to_jiffies(500));
>>> + if (ret == 0) {
>>> + dev_err(dwc->dev, "timeout to stop gadget.\n");
>>> + ret = -ETIMEDOUT;
>>> + } else {
>>> + goto try_again;
>>
>> you are not really reading my comments. It's the third time I tell you
>> there's no need for try_again. If you can't complete a control transfer
>> in 500ms, you already have other issues. Take this thing out of here.
>
> I think you misunderstood the code. If there is 500ms timeout, we will
> return '-ETIMEDOUT' error. If the control transfer is completed before
> timeout, we can not just return and we need try again to stop the
> gadget, right? Any other good suggestion? Thanks.

Yeah, change the patch a bit so you wait for completion before calling
dwc3_gadget_runt_stop()? I mean, move the !is_on && ep0_state check
before calling dwc3_gadget_run_stop() and wait_for_completion_timeout()
there.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> Hi,
>
> On 13 October 2016 at 15:02, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> Baolin Wang  writes:
>>> @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>>>   dwc->gadget_driver  = NULL;
>>>   spin_unlock_irqrestore(>lock, flags);
>>>
>>> + /*
>>> +  * Since the xHCI will share the same interrupt with gadget, thus when
>>> +  * free the gadget irq, it will not shutdown this gadget irq line. 
>>> Then
>>> +  * the gadget driver can not handle the end transfer command complete
>>> +  * event after free the gadget irq, which will hang the system to 
>>> crash.
>>> +  *
>>> +  * So we should wait for the end transfer command complete event 
>>> before
>>> +  * free it to avoid this situation.
>>> +  */
>>> + dwc3_wait_command_complete(dwc);
>>
>> this doesn't make sense. We have already masked all interrupts before
>> getting here. We have also, already, disabled all endpoints.
>
> No, you just mask the interrupts described in DEVTEN register, and you
> did not disable the endpoint command complete event.

True that

>> I'm thinking this is a bug in configfs interface of Gadget API, not
>> dwc3. The only reason for this to happen would be if we get to
>> ->udc_stop() with endpoints still enabled.
>>
>> Can you check if that's the case? i.e. can you check if any endpoints
>> are still enabled when we get here?
>
> No, it is not any endpoints are still enabled. Like I said in commit
> message, we will start end transfer command when disable the endpoint,
> if the end transfer command complete event comes after we have freed
> the gadget irq, it will trigger the interrupt line all the time to
> crash the system.

I see what the problem is. Databook tells us we *must* set CMDIOC when
issuing EndTransfer command and we should always wait for Command
Complete IRQ. However, we took a shortcut and just delayed for 100us
after issuing End Transfer.

It seems as if *that* should be fixed. We should start actually waiting
for Command Complete IRQ.

-- 
balbi


signature.asc
Description: PGP signature

Re: [RESEND PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget

[Baolin Wang]

Hi,

On 13 October 2016 at 15:08, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> @@ -1487,10 +1496,22 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
>> int is_on)
>>
>>   is_on = !!is_on;
>>
>> +try_again:
>>   spin_lock_irqsave(>lock, flags);
>>   ret = dwc3_gadget_run_stop(dwc, is_on, false);
>>   spin_unlock_irqrestore(>lock, flags);
>>
>> + if (ret == -EBUSY) {
>> + ret = wait_for_completion_timeout(>ep0_in_setup,
>> +   msecs_to_jiffies(500));
>> + if (ret == 0) {
>> + dev_err(dwc->dev, "timeout to stop gadget.\n");
>> + ret = -ETIMEDOUT;
>> + } else {
>> + goto try_again;
>
> you are not really reading my comments. It's the third time I tell you
> there's no need for try_again. If you can't complete a control transfer
> in 500ms, you already have other issues. Take this thing out of here.

I think you misunderstood the code. If there is 500ms timeout, we will
return '-ETIMEDOUT' error. If the control transfer is completed before
timeout, we can not just return and we need try again to stop the
gadget, right? Any other good suggestion? Thanks.


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

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

Hi,

On 13 October 2016 at 15:06, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> When system has stpped the gadget, we should avoid queuing any requests
>
> queueing is *not* a problem. Starting is. In fact, that's what your
> patch is doing.

OK.

>
>> which will cause tranfer failed. Thus adding some disconnect checking to
>^^^
>transfer

Sorry for spelling mistake, will fix it.

>
>> avoid this situation.
>>
>> Signed-off-by: Baolin Wang 
>> ---
>> Changes since v2:
>>  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
>>  - Rename completion name and issue complete() at one place.
>>  - Move completion initialization into dwc3_gadget_init().
>>
>> Changes since v1:
>>  - Split into 2 separate ptaches.
>>  - Choose complete mechanism instead of polling.
>> ---
>>  drivers/usb/dwc3/gadget.c |3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1783406..ca2ae5b 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
>> unsigned cmd,
>>   int susphy = false;
>>   int ret = -EINVAL;
>>
>> + if (!dwc->pullups_connected)
>> + return -ESHUTDOWN;
>> +
>>   /*
>>* Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
>>* we're issuing an endpoint command, we must check if
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Baolin Wang]

Hi,

On 13 October 2016 at 15:02, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>>   dwc->gadget_driver  = NULL;
>>   spin_unlock_irqrestore(>lock, flags);
>>
>> + /*
>> +  * Since the xHCI will share the same interrupt with gadget, thus when
>> +  * free the gadget irq, it will not shutdown this gadget irq line. Then
>> +  * the gadget driver can not handle the end transfer command complete
>> +  * event after free the gadget irq, which will hang the system to 
>> crash.
>> +  *
>> +  * So we should wait for the end transfer command complete event before
>> +  * free it to avoid this situation.
>> +  */
>> + dwc3_wait_command_complete(dwc);
>
> this doesn't make sense. We have already masked all interrupts before
> getting here. We have also, already, disabled all endpoints.

No, you just mask the interrupts described in DEVTEN register, and you
did not disable the endpoint command complete event.

>
> I'm thinking this is a bug in configfs interface of Gadget API, not
> dwc3. The only reason for this to happen would be if we get to
> ->udc_stop() with endpoints still enabled.
>
> Can you check if that's the case? i.e. can you check if any endpoints
> are still enabled when we get here?

No, it is not any endpoints are still enabled. Like I said in commit
message, we will start end transfer command when disable the endpoint,
if the end transfer command complete event comes after we have freed
the gadget irq, it will trigger the interrupt line all the time to
crash the system.

-- 
Baolin.wang
Best Regards
--
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] usb: dwc3: gadget: Wait for end transfer complete before free irq

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>   dwc->gadget_driver  = NULL;
>   spin_unlock_irqrestore(>lock, flags);
>  
> + /*
> +  * Since the xHCI will share the same interrupt with gadget, thus when
> +  * free the gadget irq, it will not shutdown this gadget irq line. Then
> +  * the gadget driver can not handle the end transfer command complete
> +  * event after free the gadget irq, which will hang the system to crash.
> +  *
> +  * So we should wait for the end transfer command complete event before
> +  * free it to avoid this situation.
> +  */
> + dwc3_wait_command_complete(dwc);

this doesn't make sense. We have already masked all interrupts before
getting here. We have also, already, disabled all endpoints.

I'm thinking this is a bug in configfs interface of Gadget API, not
dwc3. The only reason for this to happen would be if we get to
->udc_stop() with endpoints still enabled.

Can you check if that's the case? i.e. can you check if any endpoints
are still enabled when we get here?

-- 
balbi


signature.asc
Description: PGP signature

Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> When system has stpped the gadget, we should avoid queuing any requests

queueing is *not* a problem. Starting is. In fact, that's what your
patch is doing.

> which will cause tranfer failed. Thus adding some disconnect checking to
   ^^^
   transfer

> avoid this situation.
>
> Signed-off-by: Baolin Wang 
> ---
> Changes since v2:
>  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
>  - Rename completion name and issue complete() at one place.
>  - Move completion initialization into dwc3_gadget_init().
>
> Changes since v1:
>  - Split into 2 separate ptaches.
>  - Choose complete mechanism instead of polling.
> ---
>  drivers/usb/dwc3/gadget.c |3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 1783406..ca2ae5b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned 
> cmd,
>   int susphy = false;
>   int ret = -EINVAL;
>  
> + if (!dwc->pullups_connected)
> + return -ESHUTDOWN;
> +
>   /*
>* Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
>* we're issuing an endpoint command, we must check if
> -- 
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi


signature.asc
Description: PGP signature

Re: [RESEND PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> @@ -1487,10 +1496,22 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, 
> int is_on)
>  
>   is_on = !!is_on;
>  
> +try_again:
>   spin_lock_irqsave(>lock, flags);
>   ret = dwc3_gadget_run_stop(dwc, is_on, false);
>   spin_unlock_irqrestore(>lock, flags);
>  
> + if (ret == -EBUSY) {
> + ret = wait_for_completion_timeout(>ep0_in_setup,
> +   msecs_to_jiffies(500));
> + if (ret == 0) {
> + dev_err(dwc->dev, "timeout to stop gadget.\n");
> + ret = -ETIMEDOUT;
> + } else {
> + goto try_again;

you are not really reading my comments. It's the third time I tell you
there's no need for try_again. If you can't complete a control transfer
in 500ms, you already have other issues. Take this thing out of here.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v7 2/8] power: add power sequence library

[Heiko Stuebner]

Am Donnerstag, 13. Oktober 2016, 09:22:16 CEST schrieb Peter Chen:
> On Wed, Oct 12, 2016 at 12:30:29PM +0200, Heiko Stuebner wrote:
> > Hi,
> > 
> > Am Dienstag, 20. September 2016, 11:36:41 CEST schrieb Peter Chen:
> > > We have an well-known problem that the device needs to do some power
> > > sequence before it can be recognized by related host, the typical
> > > example like hard-wired mmc devices and usb devices.
> > > 
> > > This power sequence is hard to be described at device tree and handled
> > > by
> > > related host driver, so we have created a common power sequence
> > > library to cover this requirement. The core code has supplied
> > > some common helpers for host driver, and individual power sequence
> > > libraries handle kinds of power sequence for devices.
> > > 
> > > pwrseq_generic is intended for general purpose of power sequence, which
> > > handles gpios and clocks currently, and can cover regulator and pinctrl
> > > in future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > > if only one power sequence is needed, else call of_pwrseq_on_list
> > > /of_pwrseq_off_list instead (eg, USB hub driver).
> > > 
> > > Signed-off-by: Peter Chen 
> > > Tested-by Joshua Clayton 
> > > Reviewed-by: Matthias Kaehlcke 
> > > Tested-by: Matthias Kaehlcke 
> > 
> > first of all, glad to see this move forward. I've only some qualms with
> > the
> > static number of allocated power sequences below.
> 
> Thanks for commenting it, the preallocate way is not a good way, but I
> can't find suitable way. See below comments.
> 
> > > +static int __init pwrseq_generic_register(void)
> > > +{
> > > + struct pwrseq_generic *pwrseq_gen;
> > > + int i;
> > > +
> > > + for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> > > + pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> > > + if (!pwrseq_gen)
> > > + return -ENOMEM;
> > > +
> > > + pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> > > + pwrseq_gen->pwrseq.get = pwrseq_generic_get;
> > > + pwrseq_gen->pwrseq.on = pwrseq_generic_on;
> > > + pwrseq_gen->pwrseq.off = pwrseq_generic_off;
> > > + pwrseq_gen->pwrseq.put = pwrseq_generic_put;
> > > + pwrseq_gen->pwrseq.free = pwrseq_generic_free;
> > > +
> > > + pwrseq_register(_gen->pwrseq);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +postcore_initcall(pwrseq_generic_register)
> > 
> > I see that you need to have it preallocated for the compatible matching,
> > but wouldn't it also work to either just register the type and allocate
> > dynamically or otherwise just allocate a new spare everytime
> > pwrseq_generic_get() picks up the previous spare?
> 
> Before compatible matching, the host driver doesn't know which pwrseq type
> for its child node, then doesn't know which pwrseq instance needs to be
> allocated. From dts, we don't know which pwrseq type for the node.

yes, that is why I was suggesting allocating one (or two) here in 
pwrseq_generic_register() and every time pwrseq_generic_get() grabs the last 
free sequence instance, you allocate a new free spare one in that function.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xhci_hcd crash on linux 4.7.0

[Felipe Balbi]

Hi,

(please avoid top-posting, see: http://daringfireball.net/2007/07/on_top)

Alex Damian  writes:
> Forgot to mention, I just reproduced it on the mainline 4.8.1 kernel.
>
> On Wed, Oct 12, 2016 at 5:13 PM, Alex Damian  wrote:
>> Hello,
>>
>> To follow up on the original bug report. I am still experiencing
>> memory corruption problems in the xhci stack.
>>
>> One thing I noticed is that the corruption always occur on a secondary
>> CPU (ie. the stack trace starts on cpu_startup_entry) and it is always
>> going on when trying to handle an intrerrupt.
>>
>> Seems to me that a mutex or something similar is not correctly locked,
>> but I don't have any experience with the code around this part, so I
>> have no idea where to look.
>>
>> Pointers, ideas, suggestions ?

How about we start with Mathias' suggestion to enable xhci debugging
messages?

Quoting it again here:

>>> Enabling xhci debug could reveal something.
>>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control


(keeping context below)

>> On Thu, Aug 25, 2016 at 2:22 PM, Mathias Nyman
>>  wrote:
>>> On 29.07.2016 17:41, Alex Damian wrote:

 On Fri, Jul 29, 2016 at 2:53 PM, Greg KH  wrote:
>
> On Fri, Jul 29, 2016 at 10:58:03AM +0100, Alex Damian wrote:
>>
>> Hi Greg,
>>
>> I managed to reproduce with a untainted kernel, see dmesg paste below.
>> The stack seemed corrupted as well ?
>>
>> I refered to it as a crash since after a couple of these issues, the
>> machine hard freezes - I set up a serial console via a USB cable, but
>> I don't get the kernel oops out of the machine. The network is also
>> dead before getting any data. I could not think of any other way to
>> get a console out of a Macbook - any ideas ?
>>
>> There is a progressive level of deterioration going on below, this is
>> why I'm adding multiple pastes. See the obviously invalid pointer
>> 0001 in 3rd paste below. Also, see the protection fault in
>> the last paste. To me, something is trampling all over memory, and it
>> is usb-related.
>
>
> Not good, thanks for reproducing it without the closed kernel drivers.
>
> If you disable the list debug kernel option, do you have any problems
> with the machine?  We aren't having any other reports of issues like
> this at the moment, which makes me worry that it's something unique to
> your situation/hardware.


 I strongly suspect it's related to the macbook 12,1 hardware. I
 haven't been able
 to reproduce this with other machines, including other macbook
 versions with the same peripherals.

 This machine has never been stable in this particular peripheral
 configuration.
 I had Apple run all HW diagnostics on the machine, I ran the memcheck
 to verify that
 the RAM is ok - all results are clean. The machine is very stable under
 Mac OSX.

> And you don't know that it's a USB problem, only that USB is the one
> that is showing the issue.  Anyone could be writing over memory.


 True. However it seems particularly related to the USB mouse - that's
 how I manage
 to reproduce the error.

>
> Also, any chance you can use 'git bisect' to track down an offending
> commit?  I'm assuming that this used to work properly and something
> recently caused the issue, correct?


 The earliest kernels I've tested are in the 3.3 range. All kernels
 before 4.7 just lock up.
 4.7 is the first kernel where I have meaningful dmesg errors before
 locking up. As such,
 there is very little that I can do to bisect :(.

>>>
>>> Going through xhci related issues that occurred during my vacation.
>>>
>>> There is one command list related issue fixed in 4.8-rc3, any chance you
>>> could try it?
>>> Alternatively just add the following patch added to 4.7:
>>> 33be126 xhci: always handle "Command Ring Stopped" events
>>>
>>> Enabling xhci debug could reveal something.
>>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>>>
>>> -Mathias
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi


signature.asc
Description: PGP signature

Re: Interactive whiteboards

[Greg KH]

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Oct 13, 2016 at 07:37:33AM +0200, María Cano wrote:
> Thanks for your interest in this matter.
> If it's ok, I will update in this document collected related
> information about interactive whiteboards:
> 
> https://docs.google.com/document/d/18028I8M8el_iJGb3hk3WDxQ8m642Li_yeLeuwD1lL1E/edit?usp=sharing

No, please post it here so that _everyone_ can review it and work on it.
Putting it at some random url isn't going to help us out.

thanks,

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