Re: [RFC] USB: cdc-wdm: Extend and improve subdriver interface
Oliver Neukum writes: > On Tue, 2013-08-20 at 12:35 +0200, Bjørn Mork wrote: > >> bjorn@nemi:~$ mbimcli -d /dev/cdc-wdm0 --query-packet-service-state > > How does that work internally? The userspace application/library creates a MBIM message and sends it to the firmware by writing the complete preformatted message to the character device. The firmware replies with its current state and the application reads this message, and decodes it and acts accordingly. The driver is not directly involved. It doesn't even know the protocol. It only provides the transport between the userspace application and the firmware, and all coding and decoding takes place there. The firmware is responsible for keeping track of the current modem state, including connection status, signal level, speed etc. Userspace will query the state when necessary. But it can also keep its own cached version, supported by unsolicited notifications from the firmware. The driver will forward these notifications to the character device, but it will not log or otherwise act on them. I assume this separate channel for device management was added to MBIM because it wouldn't scale to keep adding new CDC notifications and requests. The MBIM management protocol is easily extended by adding new commands and services, including vendor specific services, without requiring any change to the standard or driver. >> So I propose that we add something like this to cdc-wdm instead of >> extending the > > If you think the speed will also be handled at a non-generic > level, go ahead. Great. I'll cook something up then. > We can revisit that decision if new protocols show up. Yes. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] usb: ohci: add AMD remote wakeup quirk into ohci driver
The following patch is required to resolve remote wake issues with
certain devices.
Issue description:
If the remote wake is issued from the device in a specific timing
condition while the system is entering sleep state then it may cause
system to auto wake on subsequent sleep cycle.
Root cause:
Host controller rebroadcasts the Resume signal > 100 µseconds after
receiving the original resume event from the device. For proper
function, some devices may require the rebroadcast of resume event
within the USB spec of 100µS.
Workaroud:
1. Filter the special platforms, then judge of all the usb devices are
mouse or not. And get out the port id which attached a mouse with Pixart
controller.
2. Then reset the port which attached issue device during system resume
from S3.
[Q] Why the special devices are only mice? Would high speed devices
such as 3G modem or USB Bluetooth adapter trigger this issue?
- Current this sensitivity is only confined to devices that use Pixart
controllers. This controller is designed for use with LS mouse
devices only. We have not observed any other devices failing. There
may be a small risk for other devices also but this patch (reset
device in resume phase) will cover the cases if required.
[Q] Shouldn’t the resume signal be sent within 100 us for every
device?
- The Host controller may not send the resume signal within 100us,
this our host controller specification change. This is why we
require the patch to prevent side effects on certain known devices.
[Q] Why would clicking mouse INTENSELY to wake the system up trigger
this issue?
- This behavior is specific to the devices that use Pixart controller.
It is timing dependent on when the resume event is triggered during
the sleep state.
[Q] Is it a host controller issue or mouse?
- It is the host controller behavior during resume that triggers the
device incorrect behavior on the next resume.
This patch is for OHCI driver to add AMD remote wakeup fix.
Signed-off-by: Huang Rui
---
drivers/usb/host/ohci-hub.c | 45 +
drivers/usb/host/ohci-pci.c | 14 ++
drivers/usb/host/ohci.h | 23 ---
3 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
index 2347ab8..5a95c75 100644
--- a/drivers/usb/host/ohci-hub.c
+++ b/drivers/usb/host/ohci-hub.c
@@ -41,6 +41,7 @@
static void dl_done_list (struct ohci_hcd *);
static void finish_unlinks (struct ohci_hcd *, u16);
+static inline int root_port_reset (struct ohci_hcd *, unsigned);
#ifdef CONFIG_PM
static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
@@ -293,6 +294,47 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
return rc;
}
+/*
+ * Reset port which attached with an issue device.
+ *
+ * If the remote wake is issued from the device in a specific timing
+ * condition while the system is entering sleep state then it may
+ * cause system to auto wake on subsequent sleep cycle.
+ *
+ * Host controller rebroadcasts the Resume signal > 100 µseconds after
+ * receiving the original resume event from the device. For proper
+ * function, some devices may require the rebroadcast of resume event
+ * within the USB spec of 100µS.
+ *
+ * Without this quirk, some usb mouse controller would react
+ * differently to this unexpected event from some AMD host controller
+ * and will result in the mouse to assert a resume event on the
+ * subsequent S3 sleep even if the user does not initiate the wake
+ * event by clicking on the mouse. So it should reset the port which
+ * attached with issue mouse during S3 reusme phase.
+ */
+static int ohci_reset_port_by_amd_remote_wakeup(struct ohci_hcd *ohci)
+{
+ u32 temp;
+ struct usb_device *hdev, *child;
+ int port1, wIndex;
+
+ hdev = hcd_to_bus(ohci_to_hcd(ohci))->root_hub;
+
+ usb_hub_for_each_child(hdev, port1, child) {
+ wIndex = port1 - 1;
+ temp = roothub_portstatus(ohci, wIndex);
+ dbg_port(ohci, "Get port status", wIndex, temp);
+ if (is_issue_device_for_amd_quirk(child)) {
+ ohci_dbg(ohci, "Connencted issue device on port %d.\n",
+ wIndex);
+ root_port_reset(ohci, wIndex);
+ }
+ }
+
+ return 0;
+}
+
static int ohci_bus_resume (struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
@@ -309,6 +351,9 @@ static int ohci_bus_resume (struct usb_hcd *hcd)
rc = ohci_rh_resume (ohci);
spin_unlock_irq (&ohci->lock);
+ if (ohci->flags & OHCI_QUIRK_AMD_REMOTE_WAKEUP)
+ ohci_reset_port_by_amd_remote_wakeup(ohci);
+
/* poll until we know a device is connected or we autostop */
if (rc == 0)
usb_hcd_poll_rh_status(hcd);
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.
[PATCH v3 2/4] usb: pci-quirks: add remote wakeup quirk for AMD platforms
It add a remote wakeup pci quirk for some special AMD platforms. This
quirk filters southbridge revision which is 0x39 or 0x3a.
Signed-off-by: Huang Rui
---
drivers/usb/host/pci-quirks.c | 14 +-
drivers/usb/host/pci-quirks.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 2c76ef1..d440e73 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -140,9 +140,11 @@ int usb_amd_find_chipset_info(void)
rev = info.smbus_dev->revision;
if (rev >= 0x11 && rev <= 0x18)
info.sb_type = 2;
+ if (rev >= 0x39 && rev <= 0x3a)
+ info.sb_type = 4;
}
- if (info.sb_type == 0) {
+ if (info.sb_type == 0 || info.sb_type == 4) {
if (info.smbus_dev) {
pci_dev_put(info.smbus_dev);
info.smbus_dev = NULL;
@@ -197,6 +199,16 @@ commit:
}
EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info);
+int usb_amd_remote_wakeup_quirk(void)
+{
+ if (amd_chipset.sb_type != 4)
+ return 0;
+
+ printk(KERN_DEBUG "QUIRK: Enable AMD remote wakeup fix\n");
+ return 1;
+}
+EXPORT_SYMBOL_GPL(usb_amd_remote_wakeup_quirk);
+
/*
* The hardware normally enables the A-link power management feature, which
* lets the system lower the power consumption in idle states.
diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
index ed6700d..e28bbbe 100644
--- a/drivers/usb/host/pci-quirks.h
+++ b/drivers/usb/host/pci-quirks.h
@@ -5,6 +5,7 @@
void uhci_reset_hc(struct pci_dev *pdev, unsigned long base);
int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base);
int usb_amd_find_chipset_info(void);
+int usb_amd_remote_wakeup_quirk(void);
void usb_amd_dev_put(void);
void usb_amd_quirk_pll_disable(void);
void usb_amd_quirk_pll_enable(void);
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/4] usb: xhci: implement AMD remote wakeup quirk
The following patch is required to resolve remote wake issues with
certain devices.
Issue description:
If the remote wake is issued from the device in a specific timing
condition while the system is entering sleep state then it may cause
system to auto wake on subsequent sleep cycle.
Root cause:
Host controller rebroadcasts the Resume signal > 100 µseconds after
receiving the original resume event from the device. For proper
function, some devices may require the rebroadcast of resume event
within the USB spec of 100µS.
Workaroud:
1. Filter the special platforms, then judge of all the usb devices are
mouse or not. And get out the port id which attached a mouse with Pixart
controller.
2. Then reset the port which attached issue device during system resume
from S3.
[Q] Why the special devices are only mice? Would high speed devices
such as 3G modem or USB Bluetooth adapter trigger this issue?
- Current this sensitivity is only confined to devices that use Pixart
controllers. This controller is designed for use with LS mouse
devices only. We have not observed any other devices failing. There
may be a small risk for other devices also but this patch (reset
device in resume phase) will cover the cases if required.
[Q] Shouldn’t the resume signal be sent within 100 us for every
device?
- The Host controller may not send the resume signal within 100us,
this our host controller specification change. This is why we
require the patch to prevent side effects on certain known devices.
[Q] Why would clicking mouse INTENSELY to wake the system up trigger
this issue?
- This behavior is specific to the devices that use Pixart controller.
It is timing dependent on when the resume event is triggered during
the sleep state.
[Q] Is it a host controller issue or mouse?
- It is the host controller behavior during resume that triggers the
device incorrect behavior on the next resume.
This patch is only for xHCI driver and the quirk will be also added
into OHCI driver too.
Signed-off-by: Huang Rui
---
drivers/usb/host/xhci-pci.c | 4
drivers/usb/host/xhci.c | 57 +
drivers/usb/host/xhci.h | 35 ++--
3 files changed, 79 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c2d4950..cc2ff7e 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -90,6 +90,10 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
/* AMD PLL quirk */
if (pdev->vendor == PCI_VENDOR_ID_AMD && usb_amd_find_chipset_info())
xhci->quirks |= XHCI_AMD_PLL_FIX;
+
+ if (pdev->vendor == PCI_VENDOR_ID_AMD && usb_amd_remote_wakeup_quirk())
+ xhci->quirks |= XHCI_AMD_REMOTE_WAKEUP_QUIRK;
+
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
xhci->quirks |= XHCI_LPM_SUPPORT;
xhci->quirks |= XHCI_INTEL_HOST;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 49b6edb..bf749b0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -831,6 +831,60 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci)
}
/*
+ * Reset port which attached with an issue device.
+ *
+ * If the remote wake is issued from the device in a specific timing
+ * condition while the system is entering sleep state then it may
+ * cause system to auto wake on subsequent sleep cycle.
+ *
+ * Host controller rebroadcasts the Resume signal > 100 µseconds after
+ * receiving the original resume event from the device. For proper
+ * function, some devices may require the rebroadcast of resume event
+ * within the USB spec of 100µS.
+ *
+ * Without this quirk, some usb mouse controller would react
+ * differently to this unexpected event from some AMD host controller
+ * and will result in the mouse to assert a resume event on the
+ * subsequent S3 sleep even if the user does not initiate the wake
+ * event by clicking on the mouse. So it should reset the port which
+ * attached with issue mouse during S3 reusme phase.
+ */
+static int xhci_reset_port_by_amd_remote_wakeup(struct xhci_hcd *xhci)
+{
+ __le32 __iomem *addr, *base_addr;
+ u32 temp;
+ struct usb_device *hdev, *child;
+ unsigned long flags;
+ int port1, raw_port;
+
+ xhci_dbg(xhci, "%s, reset port attached with issue device\n",
+ __func__);
+
+ base_addr = &xhci->op_regs->port_status_base;
+
+ hdev = hcd_to_bus(xhci->main_hcd)->root_hub;
+
+ usb_hub_for_each_child(hdev, port1, child) {
+ raw_port = xhci_find_raw_port_number(xhci->main_hcd,
+ port1);
+ addr = (raw_port - 1) * NUM_PORT_REGS + base_addr;
+
+ temp = xhci_readl(xhci, addr);
+
+ if (is_issue_device_for_amd_quirk(child)) {
+ xhci_dbg(xhci, "Attached issue device on port%d.\n",
+
[PATCH v3 1/4] usb: core: quirks: add remote wakeup quirk for Pixart mice
It adds all issue mice on AMD special platforms for remote wakeup quirk.
Some mice with on Pixart controller would trigger remote wakeup quirk on
some AMD special platforms, this patch added all issue mice which tested.
But some manufactures might overwrite vendor id and product id in the
firmware when they get their USB hardware from other suppliers. So it might
not cover all the issue devices, and if find another issue device in
future, will add to mark as USB_QUIRK_AMD_REMOTE_WAKEUP.
Signed-off-by: Huang Rui
---
drivers/usb/core/hub.c | 14 ++
drivers/usb/core/quirks.c | 11 +++
include/linux/usb.h| 1 +
include/linux/usb/quirks.h | 4
4 files changed, 30 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index dde4c83..d8fb0ad 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5461,3 +5461,17 @@ acpi_handle usb_get_hub_port_acpi_handle(struct
usb_device *hdev,
return DEVICE_ACPI_HANDLE(&hub->ports[port1 - 1]->dev);
}
#endif
+
+bool is_issue_device_for_amd_quirk(struct usb_device *udev)
+{
+ if (!udev) {
+ dev_warn(&udev->dev, "Warn: no device attached!\n");
+ return false;
+ }
+
+ if (udev->quirks & USB_QUIRK_AMD_REMOTE_WAKEUP)
+ return true;
+ else
+ return false;
+}
+EXPORT_SYMBOL_GPL(is_issue_device_for_amd_quirk);
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 5b44cd4..243d277 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -144,6 +144,17 @@ static const struct usb_device_id usb_quirk_list[] = {
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* Lenovo Mouse with PixArt controller */
+ { USB_DEVICE(0x17ef, 0x602e), .driver_info =
USB_QUIRK_AMD_REMOTE_WAKEUP },
+
+ /* Pixart Mouse */
+ { USB_DEVICE(0x093a, 0x2500), .driver_info =
USB_QUIRK_AMD_REMOTE_WAKEUP },
+ { USB_DEVICE(0x093a, 0x2510), .driver_info =
USB_QUIRK_AMD_REMOTE_WAKEUP },
+ { USB_DEVICE(0x093a, 0x2521), .driver_info =
USB_QUIRK_AMD_REMOTE_WAKEUP },
+
+ /* Logitech Optical Mouse M90/M100 */
+ { USB_DEVICE(0x046d, 0xc05a), .driver_info =
USB_QUIRK_AMD_REMOTE_WAKEUP },
+
{ } /* terminating entry must be last */
};
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 001629c..09e0bd8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -591,6 +591,7 @@ extern struct usb_device *usb_get_dev(struct usb_device
*dev);
extern void usb_put_dev(struct usb_device *dev);
extern struct usb_device *usb_hub_find_child(struct usb_device *hdev,
int port1);
+extern bool is_issue_device_for_amd_quirk(struct usb_device *udev);
/**
* usb_hub_for_each_child - iterate over all child devices on the hub
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index 52f944d..741f2a9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -30,4 +30,8 @@
descriptor */
#define USB_QUIRK_DELAY_INIT 0x0040
+/* device needs reset during resume phase because of remote wakeup issue on
+ * some special AMD platforms */
+#define USB_QUIRK_AMD_REMOTE_WAKEUP0x0080
+
#endif /* __LINUX_USB_QUIRKS_H */
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] usb: implement AMD remote wakeup quirk
Hi Greg, Sarah, Alan, The following patches are required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root cause: Host controller rebroadcasts the Resume signal > 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. Reproduce steps: 1. Enable remote wakeup for usb mouse. 2. Execute S3. 3. Click mouse _intensely_ (more than 10 times) to wake the system up. 4. Then execute S3 again. 5. Observe that the system auto wake up. [Q] Why the special devices are only mice? Would high speed devices such as 3G modem or USB Bluetooth adapter trigger this issue? - Current this sensitivity is only confined to devices that use Pixart controllers. This controller is designed for use with LS mouse devices only. We have not observed any other devices failing. There may be a small risk for other devices also but this patch (reset device in resume phase) will cover the cases if required. [Q] Shouldn’t the resume signal be sent within 100 us for every device? - The Host controller may not send the resume signal within 100us, this our host controller specification change. This is why we require the patch to prevent side effects on certain known devices. [Q] Why would clicking mouse INTENSELY to wake the system up trigger this issue? - This behavior is specific to the devices that use Pixart controller. It is timing dependent on when the resume event is triggered during the sleep state. [Q] Is it a host controller issue or mouse? - It is the host controller behavior during resume that triggers the device incorrect behavior on the next resume. - Patch 1 is adding is_usb_mouse and quirks to avoid mixing with the xHCI changes, which is suggested by Alan. - Patch 2 is the implementation for xHCI driver. - Patch 3 is the implementation for OHCI driver. They are generated on gregkh/usb usb-next. Changes from v1 -> v2: - Add reproduce steps. - Add a patch to put the changes of core/hub.c and pci-quirks into one patch. - Remove all the split strings. - Refactor usb_amd_remote_wakeup_quirk to save a variable and less codes. - Remove 10ms delay in spinlock during xhci reset port by mouse. - Add an extra tab to algin to all quirks macros. - Remove backport request. - Add descriptions of the fixes and how they work. - Summarized questions from Sarah and Alan. Changes from v2 -> v3: - split first patch into two, the one add all the issue USB mice with vendor id and product id in core/quirk.c, the other one is for filtering the special AMD platforms. - Remove is_usb_mouse function, as Greg required that don't use device type for filtering. Thanks, Rui Huang Rui (4): usb: core: quirks: add remote wakeup quirk for Pixart mice usb: pci-quirks: add remote wakeup quirk for AMD platforms usb: xhci: implement AMD remote wakeup quirk usb: ohci: add AMD remote wakeup quirk into ohci driver drivers/usb/core/hub.c| 14 +++ drivers/usb/core/quirks.c | 11 + drivers/usb/host/ohci-hub.c | 45 ++ drivers/usb/host/ohci-pci.c | 14 +++ drivers/usb/host/ohci.h | 23 - drivers/usb/host/pci-quirks.c | 14 ++- drivers/usb/host/pci-quirks.h | 1 + drivers/usb/host/xhci-pci.c | 4 +++ drivers/usb/host/xhci.c | 57 +++ drivers/usb/host/xhci.h | 35 +- include/linux/usb.h | 1 + include/linux/usb/quirks.h| 4 +++ 12 files changed, 194 insertions(+), 29 deletions(-) -- 1.7.11.7 -- 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
Announcing libusbx-1.0.17 final
Hi All, I'm very happy to announce libusbx-1.0.17. 2013-09-06: v1.0.17 * Hotplug callbacks now always get passed a libusb_context, even if it is the default context. Previously NULL would be passed for the default context, but since the first context created is the default context, and most apps use only 1 context, this meant that apps explicitly creating a context would still get passed NULL * Android: Add .mk files to build with the Android NDK * Darwin: Add Xcode project * Darwin: Fix crash on unplug (#121) * Linux: Fix hang (deadlock) on libusb_exit * Linux: Fix libusbx build failure with --disable-udev (#124) * Linux: Fix libusb_get_device_list() hang with --disable-udev (#130) * OpenBSD: Update OpenBSD backend with support for control transfers to non-ugen(4) devices and make get_configuration() no longer generate I/O. Note that using this libusbx version on OpenBSD requires using OpenBSD 5.3-current or later. Users of older OpenBSD versions are advised to stay with the libusb shipped with OpenBSD (mpi) * Windows: fix libusb_dll_2010.vcxproj link errors (#129) * Various other bug fixes and improvements The (#xx) numbers are libusbx issue numbers, see ie: https://github.com/libusbx/libusbx/issues/121 You can download the 1.0.17 sources tarbal here: http://downloads.sourceforge.net/libusbx/libusbx-1.0.17.tar.bz2 Windows binaries should become available on sourceforge in a couple of days. Regards, Hans -- 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] USB: cdc-wdm: Extend and improve subdriver interface
On Fri, 2013-09-06 at 12:19 +0200, Bjørn Mork wrote: > I assume this separate channel for device management was added to MBIM > because it wouldn't scale to keep adding new CDC notifications and > requests. The MBIM management protocol is easily extended by adding > new > commands and services, including vendor specific services, without > requiring any change to the standard or driver. It suffers from the problem of requiring polling. The solution with notifications over CDC is more elegant in principle. But it doesn't matter. We implement for the protocols that exist. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Low performance of g_mass_storage virtual usb device
On Fri, 6 Sep 2013, Victor Leschuk wrote: > Hello, thanks for the answer. Maybe I sound stupid but how can I tell > g_mass_storage which controller to use? If I unload dummy_hcd and then > try to modprobe g_mass_storage I get an error "no such device". There is no way to tell g_mass_storage which device controller to use. It will bind to the first one that is available. Note that unless your computer has special hardware, without dummy-hcd there will be _no_ controllers for g_mass_storage to bind to. > # lsusb > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub > Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub > Bus 002 Device 003: ID 046d:c050 Logitech, Inc. RX 250 Optical Mouse > Bus 002 Device 004: ID 046d:c31d Logitech, Inc. > Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub This listing is irrelevant. lsusb is a host-side program, but g_mass_storage is a device-side driver. 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 04/11] usb: usb: dsps: update device tree bindings
On Tue, Aug 20, 2013 at 05:35:46PM +0100, Sebastian Andrzej Siewior wrote: > The support for both am335x-USB instances required changes to the device > tree bindings. This patch reflects these changes in the bindings > document. > > v3…v4: > - remove the child node for USB. This is driver specific on won't be > reflected in the device tree > - use the "mentor" prefix instead of "mg". > - use "dr_mode" istead of "mg,port-mode" for the port mode. The former > is used by a few other drivers. > > v2…v3: > - use proper usb-phy nodes in evm, bone and evmsk device tree. > > v1…v2: > - use mg prefix for the Metor Graphics specific attributes > - use power in mA not in mA/2 as specifed in the USB2.0 specification > - use usbX-phy instead of usbX_phy > - use dma-controller instead of dma > > Cc: Rob Herring > Cc: Pawel Moll > Cc: Mark Rutland > Cc: Stephen Warren > Cc: Ian Campbell > Cc: devicet...@vger.kernel.org > Signed-off-by: Sebastian Andrzej Siewior > --- > .../devicetree/bindings/usb/am33xx-usb.txt | 222 > ++--- > 1 file changed, 192 insertions(+), 30 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/am33xx-usb.txt > b/Documentation/devicetree/bindings/usb/am33xx-usb.txt > index dc9dc8c..20c2ff2 100644 > --- a/Documentation/devicetree/bindings/usb/am33xx-usb.txt > +++ b/Documentation/devicetree/bindings/usb/am33xx-usb.txt > @@ -1,35 +1,197 @@ > -AM33XX MUSB GLUE > - - compatible : Should be "ti,musb-am33xx" > - - reg : offset and length of register sets, first usbss, then for musb > instances > - - interrupts : usbss, musb instance interrupts in order > - - ti,hwmods : must be "usb_otg_hs" > - - multipoint : Should be "1" indicating the musb controller supports > - multipoint. This is a MUSB configuration-specific setting. > - - num-eps : Specifies the number of endpoints. This is also a > - MUSB configuration-specific setting. Should be set to "16" > - - ram-bits : Specifies the ram address size. Should be set to "12" > - - port0-mode : Should be "3" to represent OTG. "1" signifies HOST and "2" > - represents PERIPHERAL. > - - port1-mode : Should be "1" to represent HOST. "3" signifies OTG and "2" > - represents PERIPHERAL. > - - power : Should be "250". This signifies the controller can supply up to > - 500mA when operating in host mode. > + AM33xx MUSB > +~~~ > +- compatible: ti,am33xx-usb > +- reg: offset and length of the usbss register sets > +- ti,hwmods : must be "usb_otg_hs" > + > +The glue layer contains multiple child nodes. It is required the have > +at least a control module node, USB node and a PHY node. The second USB > +node and its PHY node is optional. The DMA node is also optional. > + > +Reset module > + > +- compatible: ti,am335x-usb-ctrl-module > +- reg: offset and length of the "USB control registers" in the "Control > + Module" block. A second offset and length for the USB wake up control > + in the same memory block. > +- reg-names: "phy_ctrl" for the "USB control registers" and "wakeup" for > + the USB wake up control register. > + > +USB PHY > +~~~ > +compatible: ti,am335x-usb-phy > +reg: offset and length of the "USB PHY" register space > +ti,ctrl_mod: reference to the "reset module" node > +reg-names: phy For consistency, these should be bullet bpoints (as with the Reset module properties above). Also, preferably: - reg-names: should contain "phy". > +The PHY should have a "phy" alias numbered properly in the alias > +node. Why? > + > +USB > +~~~ > +- compatible: ti,musb-am33xx > +- reg: offset and length of "USB Controller Registers", and offset and > + length of "USB Core" register space. > +- reg-names: control for the ""USB Controller Registers" and "mc" for > + "USB Core" register space Consistent quoting please. > +- interrupts: USB interrupt number - interrupts: should contain an interrupt-specifier for the USB interrupt. Is this the only interrupt? Is there a particular name for it in any data sheet? > +- interrupt-names: mc String property values quoted please: - interrupt-names: should contain "mc" for the USB interrupt. Is "mc" the name of the interrupt on the data sheet? > +- dr_mode: Should be one of "host", "peripheral" or "otg". > +- mentor,multipoint: Should be "1" indicating the musb controller supports > + multipoint. This is a MUSB configuration-specific setting. Is this a boolean value? Why is this not an empty property? > +- mentor,num-eps: Specifies the number of endpoints. This is also a > + MUSB configuration-specific setting. Should be set to "16" > +- mentor,ram-bits: Specifies the ram address size. Should be set to "12" Is this to define the address space the controller can see? What if it's offset from the CPU's view? This sounds like something better described via another means (like dma-ranges). > +- mentor,power: Should be "500". This signifies the controller can supply up > to > + 500mA when operating in ho
Re: [PATCH v3 1/4] usb: core: quirks: add remote wakeup quirk for Pixart mice
On Fri, Sep 06, 2013 at 06:24:41PM +0800, Huang Rui wrote:
> It adds all issue mice on AMD special platforms for remote wakeup quirk.
>
> Some mice with on Pixart controller would trigger remote wakeup quirk on
> some AMD special platforms, this patch added all issue mice which tested.
>
> But some manufactures might overwrite vendor id and product id in the
> firmware when they get their USB hardware from other suppliers. So it might
> not cover all the issue devices, and if find another issue device in
> future, will add to mark as USB_QUIRK_AMD_REMOTE_WAKEUP.
Who should we email about these problems, so that they can be tested,
you?
> Signed-off-by: Huang Rui
> ---
> drivers/usb/core/hub.c | 14 ++
> drivers/usb/core/quirks.c | 11 +++
> include/linux/usb.h| 1 +
> include/linux/usb/quirks.h | 4
> 4 files changed, 30 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index dde4c83..d8fb0ad 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5461,3 +5461,17 @@ acpi_handle usb_get_hub_port_acpi_handle(struct
> usb_device *hdev,
> return DEVICE_ACPI_HANDLE(&hub->ports[port1 - 1]->dev);
> }
> #endif
> +
> +bool is_issue_device_for_amd_quirk(struct usb_device *udev)
> +{
> + if (!udev) {
> + dev_warn(&udev->dev, "Warn: no device attached!\n");
> + return false;
> + }
> +
> + if (udev->quirks & USB_QUIRK_AMD_REMOTE_WAKEUP)
> + return true;
> + else
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(is_issue_device_for_amd_quirk);
That's a really ackward name for a global symbol. Please follow the
naming scheme of the usb global symbols please (hint, put "usb_" at the
front, and don't use a whole sentance for a function name.)
Also, as no one is calling this function in this patch, why do you have
it here?
This patch really doesn't even do anything, does it?
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 5b44cd4..243d277 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -144,6 +144,17 @@ static const struct usb_device_id usb_quirk_list[] = {
> /* INTEL VALUE SSD */
> { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
>
> + /* Lenovo Mouse with PixArt controller */
> + { USB_DEVICE(0x17ef, 0x602e), .driver_info =
> USB_QUIRK_AMD_REMOTE_WAKEUP },
> +
> + /* Pixart Mouse */
> + { USB_DEVICE(0x093a, 0x2500), .driver_info =
> USB_QUIRK_AMD_REMOTE_WAKEUP },
> + { USB_DEVICE(0x093a, 0x2510), .driver_info =
> USB_QUIRK_AMD_REMOTE_WAKEUP },
> + { USB_DEVICE(0x093a, 0x2521), .driver_info =
> USB_QUIRK_AMD_REMOTE_WAKEUP },
> +
> + /* Logitech Optical Mouse M90/M100 */
> + { USB_DEVICE(0x046d, 0xc05a), .driver_info =
> USB_QUIRK_AMD_REMOTE_WAKEUP },
> +
> { } /* terminating entry must be last */
> };
>
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 001629c..09e0bd8 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -591,6 +591,7 @@ extern struct usb_device *usb_get_dev(struct usb_device
> *dev);
> extern void usb_put_dev(struct usb_device *dev);
> extern struct usb_device *usb_hub_find_child(struct usb_device *hdev,
> int port1);
> +extern bool is_issue_device_for_amd_quirk(struct usb_device *udev);
>
> /**
> * usb_hub_for_each_child - iterate over all child devices on the hub
> diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
> index 52f944d..741f2a9 100644
> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -30,4 +30,8 @@
> descriptor */
> #define USB_QUIRK_DELAY_INIT 0x0040
>
> +/* device needs reset during resume phase because of remote wakeup issue on
> + * some special AMD platforms */
Can we get a listing of which AMD platforms have the problem here?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] usb: pci-quirks: add remote wakeup quirk for AMD platforms
On Fri, Sep 06, 2013 at 06:24:42PM +0800, Huang Rui wrote:
> It add a remote wakeup pci quirk for some special AMD platforms. This
> quirk filters southbridge revision which is 0x39 or 0x3a.
>
> Signed-off-by: Huang Rui
> ---
> drivers/usb/host/pci-quirks.c | 14 +-
> drivers/usb/host/pci-quirks.h | 1 +
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 2c76ef1..d440e73 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -140,9 +140,11 @@ int usb_amd_find_chipset_info(void)
> rev = info.smbus_dev->revision;
> if (rev >= 0x11 && rev <= 0x18)
> info.sb_type = 2;
> + if (rev >= 0x39 && rev <= 0x3a)
> + info.sb_type = 4;
You are adding another "type" here, yet we have no information on what
these "types" mean. Can you please use an enumerated type, or a #define
at the least, for all of these so we can start to tell them apart?
Also, what does 39 and 3a mean? Please use a #define with a name we can
understand so that others can know what this means.
> }
>
> - if (info.sb_type == 0) {
> + if (info.sb_type == 0 || info.sb_type == 4) {
Why type 4 here?
> if (info.smbus_dev) {
> pci_dev_put(info.smbus_dev);
> info.smbus_dev = NULL;
> @@ -197,6 +199,16 @@ commit:
> }
> EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info);
>
> +int usb_amd_remote_wakeup_quirk(void)
> +{
> + if (amd_chipset.sb_type != 4)
> + return 0;
> +
> + printk(KERN_DEBUG "QUIRK: Enable AMD remote wakeup fix\n");
dev_dbg() please?
> + return 1;
> +}
> +EXPORT_SYMBOL_GPL(usb_amd_remote_wakeup_quirk);
Nothing calls this function, so why have you added it here?
The way you have broken up the patches is a bit odd. You are creating
functions that aren't called yet, but do not describe this in the
changelog information, so people reviewing the patch would just assume
they are not needed at all (hint, like I just did...)
Please provide a better description, and justification, if you create a
new function, otherwise we will assume it is not needed at all.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/11] usb: usb: dsps: update code according to the binding document
On Tue, Aug 20, 2013 at 05:35:47PM +0100, Sebastian Andrzej Siewior wrote:
> This relfects the code and dts requires changes due to recent .dts
> binding updates:
> - use mg prefix for the Metor Graphics specific attributes
> - use power in mA not in mA/2 as specifed in the USB2.0 specification
> - remove the child node for USB. This is driver specific on won't be
> reflected in the device tree
> - use the "mentor" prefix instead of "mg".
> - use "dr_mode" istead of "mg,port-mode" for the port mode. The former
> is used by a few other drivers.
>
> Cc: Rob Herring
> Cc: Pawel Moll
> Cc: Mark Rutland
> Cc: Stephen Warren
> Cc: Ian Campbell
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> arch/arm/boot/dts/am335x-bone.dts | 2 +-
> arch/arm/boot/dts/am335x-evm.dts | 6 ++--
> arch/arm/boot/dts/am335x-evmsk.dts | 2 +-
> arch/arm/boot/dts/am33xx.dtsi | 67
> +++---
> drivers/usb/musb/musb_dsps.c | 54 --
> 5 files changed, 67 insertions(+), 64 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts
> b/arch/arm/boot/dts/am335x-bone.dts
> index a8907b5..e8447a7 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -127,7 +127,7 @@
> status = "okay";
> };
>
> - phy@47401300 {
> + usb-phy@47401300 {
> status = "okay";
> };
>
> diff --git a/arch/arm/boot/dts/am335x-evm.dts
> b/arch/arm/boot/dts/am335x-evm.dts
> index c26c16c..648a67e 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -178,11 +178,11 @@
> status = "okay";
> };
>
> - phy@47401300 {
> + usb-phy@47401300 {
> status = "okay";
> };
>
> - phy@47401b00 {
> + usb-phy@47401b00 {
> status = "okay";
> };
>
> @@ -194,7 +194,7 @@
> status = "okay";
> };
>
> - dma@07402000 {
> + dma-controller@07402000 {
> status = "okay";
> };
> };
> diff --git a/arch/arm/boot/dts/am335x-evmsk.dts
> b/arch/arm/boot/dts/am335x-evmsk.dts
> index e92446c..a6c5033 100644
> --- a/arch/arm/boot/dts/am335x-evmsk.dts
> +++ b/arch/arm/boot/dts/am335x-evmsk.dts
> @@ -214,7 +214,7 @@
> status = "okay";
> };
>
> - phy@47401300 {
> + usb-phy@47401300 {
> status = "okay";
> };
>
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index a38f8d3..f9c5da9 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -354,7 +354,7 @@
> status = "disabled";
> };
>
> - usb0_phy: phy@47401300 {
> + usb0_phy: usb-phy@47401300 {
> compatible = "ti,am335x-usb-phy";
> reg = <0x47401300 0x100>;
> reg-names = "phy";
> @@ -364,25 +364,19 @@
>
> usb0: usb@47401000 {
> compatible = "ti,musb-am33xx";
> - ranges;
> - #address-cells = <1>;
> - #size-cells = <1>;
> - reg = <0x47401000 0x200>;
> - reg-names = "control";
> status = "disabled";
> -
> - musb0: usb@47401400 {
> - compatible = "mg,musbmhdrc";
> - reg = <0x47401400 0x400>;
> - reg-names = "mc";
> - interrupts = <18>;
> - interrupt-names = "mc";
> - multipoint = <1>;
> - num-eps = <16>;
> - ram-bits = <12>;
> - port-mode = <3>;
> - power = <250>;
> - phys = <&usb0_phy>;
> + reg = <0x47401400 0x400
> + 0x47401000 0x200>;
> + reg-names = "mc", "control";
> +
> + interrupts = <18>;
> + interrupt-names = "mc";
> +
Re: [PATCH v3 4/4] usb: ohci: add AMD remote wakeup quirk into ohci driver
On Fri, Sep 06, 2013 at 06:24:44PM +0800, Huang Rui wrote:
> static void dl_done_list (struct ohci_hcd *);
> static void finish_unlinks (struct ohci_hcd *, u16);
> +static inline int root_port_reset (struct ohci_hcd *, unsigned);
How can a definition of a function be inline? :)
Can't you just place your calling function below this one so that you
don't need this?
>
> #ifdef CONFIG_PM
> static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
> @@ -293,6 +294,47 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
> return rc;
> }
>
> +/*
> + * Reset port which attached with an issue device.
> + *
> + * If the remote wake is issued from the device in a specific timing
> + * condition while the system is entering sleep state then it may
> + * cause system to auto wake on subsequent sleep cycle.
> + *
> + * Host controller rebroadcasts the Resume signal > 100 µseconds after
> + * receiving the original resume event from the device. For proper
> + * function, some devices may require the rebroadcast of resume event
> + * within the USB spec of 100µS.
> + *
> + * Without this quirk, some usb mouse controller would react
> + * differently to this unexpected event from some AMD host controller
Which AMD host controllers have this issue?
> + * and will result in the mouse to assert a resume event on the
> + * subsequent S3 sleep even if the user does not initiate the wake
> + * event by clicking on the mouse. So it should reset the port which
> + * attached with issue mouse during S3 reusme phase.
> + */
> +static int ohci_reset_port_by_amd_remote_wakeup(struct ohci_hcd *ohci)
> +{
> + u32 temp;
> + struct usb_device *hdev, *child;
> + int port1, wIndex;
> +
> + hdev = hcd_to_bus(ohci_to_hcd(ohci))->root_hub;
> +
> + usb_hub_for_each_child(hdev, port1, child) {
> + wIndex = port1 - 1;
> + temp = roothub_portstatus(ohci, wIndex);
> + dbg_port(ohci, "Get port status", wIndex, temp);
> + if (is_issue_device_for_amd_quirk(child)) {
> + ohci_dbg(ohci, "Connencted issue device on port %d.\n",
> + wIndex);
Please always run your patches through the scripts/checkpatch.pl tool,
so that I don't have to point out the obvious issues that it would
(hint, there's a problem in this function...)
> + root_port_reset(ohci, wIndex);
> + }
> + }
> +
> + return 0;
If the function can never "fail", and you never check the return value
when you call it, why return any value at all?
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
[PATCH] usb: musb: name ux500 platforms more broadly
The Kconfig help text is talking about the U5500 which is no longer supported by the kernel. Name the help text after the config symbol which is more correct. Signed-off-by: Linus Walleij --- drivers/usb/musb/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index c64ee09a7..a77ac0c 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -91,7 +91,7 @@ config USB_MUSB_BLACKFIN depends on (BF54x && !BF544) || (BF52x && ! BF522 && !BF523) config USB_MUSB_UX500 - tristate "U8500 and U5500" + tristate "Ux500 platforms" endchoice @@ -113,7 +113,7 @@ choice allow using DMA on multiplatform kernels. config USB_UX500_DMA - bool 'ST Ericsson U8500 and U5500' + bool 'ST Ericsson Ux500' depends on USB_MUSB_UX500 help Enable DMA transfers on UX500 platforms. -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] usb: ohci: add AMD remote wakeup quirk into ohci driver
On Fri, 6 Sep 2013, Huang Rui wrote:
> The following patch is required to resolve remote wake issues with
> certain devices.
>
> Issue description:
> If the remote wake is issued from the device in a specific timing
> condition while the system is entering sleep state then it may cause
> system to auto wake on subsequent sleep cycle.
>
> Root cause:
> Host controller rebroadcasts the Resume signal > 100 µseconds after
> receiving the original resume event from the device. For proper
> function, some devices may require the rebroadcast of resume event
> within the USB spec of 100µS.
>
> Workaroud:
> 1. Filter the special platforms, then judge of all the usb devices are
> mouse or not. And get out the port id which attached a mouse with Pixart
> controller.
> 2. Then reset the port which attached issue device during system resume
> from S3.
>
> [Q] Why the special devices are only mice? Would high speed devices
> such as 3G modem or USB Bluetooth adapter trigger this issue?
> - Current this sensitivity is only confined to devices that use Pixart
> controllers. This controller is designed for use with LS mouse
> devices only. We have not observed any other devices failing. There
> may be a small risk for other devices also but this patch (reset
> device in resume phase) will cover the cases if required.
>
> [Q] Shouldn’t the resume signal be sent within 100 us for every
> device?
> - The Host controller may not send the resume signal within 100us,
> this our host controller specification change. This is why we
> require the patch to prevent side effects on certain known devices.
>
> [Q] Why would clicking mouse INTENSELY to wake the system up trigger
> this issue?
> - This behavior is specific to the devices that use Pixart controller.
> It is timing dependent on when the resume event is triggered during
> the sleep state.
>
> [Q] Is it a host controller issue or mouse?
> - It is the host controller behavior during resume that triggers the
> device incorrect behavior on the next resume.
>
> This patch is for OHCI driver to add AMD remote wakeup fix.
>
> Signed-off-by: Huang Rui
> ---
> drivers/usb/host/ohci-hub.c | 45
> +
> drivers/usb/host/ohci-pci.c | 14 ++
> drivers/usb/host/ohci.h | 23 ---
> 3 files changed, 71 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
> index 2347ab8..5a95c75 100644
> --- a/drivers/usb/host/ohci-hub.c
> +++ b/drivers/usb/host/ohci-hub.c
> @@ -41,6 +41,7 @@
>
> static void dl_done_list (struct ohci_hcd *);
> static void finish_unlinks (struct ohci_hcd *, u16);
> +static inline int root_port_reset (struct ohci_hcd *, unsigned);
>
> #ifdef CONFIG_PM
> static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
> @@ -293,6 +294,47 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
> return rc;
> }
>
> +/*
> + * Reset port which attached with an issue device.
> + *
> + * If the remote wake is issued from the device in a specific timing
> + * condition while the system is entering sleep state then it may
> + * cause system to auto wake on subsequent sleep cycle.
> + *
> + * Host controller rebroadcasts the Resume signal > 100 µseconds after
> + * receiving the original resume event from the device. For proper
> + * function, some devices may require the rebroadcast of resume event
> + * within the USB spec of 100µS.
> + *
> + * Without this quirk, some usb mouse controller would react
> + * differently to this unexpected event from some AMD host controller
> + * and will result in the mouse to assert a resume event on the
> + * subsequent S3 sleep even if the user does not initiate the wake
> + * event by clicking on the mouse. So it should reset the port which
> + * attached with issue mouse during S3 reusme phase.
> + */
> +static int ohci_reset_port_by_amd_remote_wakeup(struct ohci_hcd *ohci)
> +{
> + u32 temp;
> + struct usb_device *hdev, *child;
> + int port1, wIndex;
> +
> + hdev = hcd_to_bus(ohci_to_hcd(ohci))->root_hub;
> +
> + usb_hub_for_each_child(hdev, port1, child) {
> + wIndex = port1 - 1;
> + temp = roothub_portstatus(ohci, wIndex);
> + dbg_port(ohci, "Get port status", wIndex, temp);
> + if (is_issue_device_for_amd_quirk(child)) {
> + ohci_dbg(ohci, "Connencted issue device on port %d.\n",
> + wIndex);
> + root_port_reset(ohci, wIndex);
> + }
> + }
> +
> + return 0;
> +}
This really is not the right way to do what you want.
What you ought to do is set the USB_QUIRK_RESET_RESUME flag for these
Pixart-based mice when they are attached to an AMD controller with the
resume bug. Therefore you should create a new flag inside the usb_hcd
structure:
unsignedamd_resume_bug:1;
You can set this flag dur
chipidea maintainership delays
Hi Alexander, I've been noticing that you haven't been keeping up with the chipidea patches being posted. Is there anything I can do here to help out with that? There seems to be lots of development activity in this area, and I don't want to see things slow down for different developers if there's a bottleneck in the development process somewhere. 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: FUSB200 xhci issue
On Fri, 6 Sep 2013, Oleksij Rempel wrote: > Hi Alan, > > i need more help. > > We have more test results now. Initial patch was probably fixing > fallowing issue - bad performance on EP4/EP3 if Intr transfer is used. > It is reproducible only on EHCI, but not reproducible on xHCI controller. Is this a high-speed device? What version of the kernel are you using? > For example, channel scan function intensively use this endpoints. On > xHCI, channel scan is done in 6 seconds. On EHCI it needs 25s. By > forcing Bulk transfer on host side, it is possible get same performance > on EHCI too. But it isn't working on all controllers because of limited > packed size (64B). > > Dumps are attached. One of them may include some other device, but only > ath9k_htc device has EP3 and EP4. The capture logs show that with EHCI, each endpoint transfers a packet every 2.5 ms or so, whereas xHCI transfers 2-3 packets per ms. > Is it protocol limitation or HC issue? It's not a protocol limitation. If it were, the xHCI communication would be just as slow as EHCI. One thing that might help: For each endpoint, keep multiple URBs queued (try 10) at all times instead of just a single URB. It's entirely possible that these results are caused by limitations of your EHCI hardware. Still, using multiple URBs could yield a significant improvement. 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 v3 3/4] usb: xhci: implement AMD remote wakeup quirk
On Fri, Sep 06, 2013 at 02:02:04PM -0700, Sarah Sharp wrote: > On Fri, Sep 06, 2013 at 06:24:43PM +0800, Huang Rui wrote: > > The following patch is required to resolve remote wake issues with > > certain devices. > > > > Issue description: > > If the remote wake is issued from the device in a specific timing > > condition while the system is entering sleep state then it may cause > > system to auto wake on subsequent sleep cycle. > > > > Root cause: > > Host controller rebroadcasts the Resume signal > 100 µseconds after > > receiving the original resume event from the device. For proper > > function, some devices may require the rebroadcast of resume event > > within the USB spec of 100µS. > > > > Workaroud: > > 1. Filter the special platforms, then judge of all the usb devices are > > mouse or not. And get out the port id which attached a mouse with Pixart > > controller. > > 2. Then reset the port which attached issue device during system resume > > from S3. > > Why are you doing this in the xHCI driver? You're resetting a device > from the host controller driver without letting the USB core know about > it? That seems like a bad idea. Ok, I see Alan has already suggested an alternative way to do this in the USB core. Please follow his suggestion. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usbcore: check usb device's state before sending a Set SEL control transfer
On 09/04/2013 09:25 PM, Martin MOKREJŠ wrote: Hi Xenia, thank you. I tested this patch on 3.11 kernel and the messages don't appear anymore upon LPM-capable device disconnect (tested with ASMedia AS2105 devices). Not much to show here, there is just no error/warning related to LPM while handling these devices. Probably better test is with Prolific-based device for which I get during connect: [ 999.906164] usb 4-2.1: new SuperSpeed USB device number 20 using xhci_hcd [ 999.928475] usb 4-2.1: New USB device found, idVendor=067b, idProduct=2773 [ 999.928488] usb 4-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 999.928495] usb 4-2.1: Product: USB-SATA Bridge [ 999.928501] usb 4-2.1: Manufacturer: Prolific Technology Inc. [ 999.928506] usb 4-2.1: SerialNumber: PROLIFICMP7 [ 999.929633] usb 4-2.1: Set SEL for device-initiated U1 failed. [ 999.930008] usb 4-2.1: Set SEL for device-initiated U2 failed. [ 999.930114] usb-storage 4-2.1:1.0: USB Mass Storage device detected [ 999.930178] scsi22 : usb-storage 4-2.1:1.0 [ 999.930804] usb 4-2.1: Set SEL for device-initiated U1 failed. [ 999.931681] usb 4-2.1: Set SEL for device-initiated U2 failed. and during disconnect kernel is also quiet and doesn't complain anymore about the LPM of already disconnected device: [ 1582.039612] usb 4-2.1: USB disconnect, device number 20 I think your patch works correctly and has shutdown the bogus/annoying message. ;-) Thank you. Martin Hi Martin, that 's great! if you notice anything else pls let me know :) ksenia -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] usb: xhci: implement AMD remote wakeup quirk
On Fri, Sep 06, 2013 at 06:24:43PM +0800, Huang Rui wrote:
> The following patch is required to resolve remote wake issues with
> certain devices.
>
> Issue description:
> If the remote wake is issued from the device in a specific timing
> condition while the system is entering sleep state then it may cause
> system to auto wake on subsequent sleep cycle.
>
> Root cause:
> Host controller rebroadcasts the Resume signal > 100 µseconds after
> receiving the original resume event from the device. For proper
> function, some devices may require the rebroadcast of resume event
> within the USB spec of 100µS.
>
> Workaroud:
> 1. Filter the special platforms, then judge of all the usb devices are
> mouse or not. And get out the port id which attached a mouse with Pixart
> controller.
> 2. Then reset the port which attached issue device during system resume
> from S3.
Why are you doing this in the xHCI driver? You're resetting a device
from the host controller driver without letting the USB core know about
it? That seems like a bad idea.
You don't even time the reset, or clear the reset change bit when the
reset completes. You're expecting the USB core to come along and clear
the reset change bit later, which I'm not sure if it will if it didn't
issue the reset.
Why aren't we making the USB core reset the device through the normal
code paths? Why don't you add new API to the USB host controller driver
to see if a host has this new quirk, and have the USB core do the reset?
A simpler solution would be to just set the XHCI_RESET_ON_RESUME quirk,
so that the host controller gets reset on resume from S3 and the devices
are reset and re-enumerated. I like that better than resetting a device
behind the USB core's back.
> [Q] Why the special devices are only mice? Would high speed devices
> such as 3G modem or USB Bluetooth adapter trigger this issue?
> - Current this sensitivity is only confined to devices that use Pixart
> controllers. This controller is designed for use with LS mouse
> devices only. We have not observed any other devices failing. There
> may be a small risk for other devices also but this patch (reset
> device in resume phase) will cover the cases if required.
>
> [Q] Shouldn’t the resume signal be sent within 100 us for every
> device?
> - The Host controller may not send the resume signal within 100us,
> this our host controller specification change. This is why we
> require the patch to prevent side effects on certain known devices.
>
> [Q] Why would clicking mouse INTENSELY to wake the system up trigger
> this issue?
> - This behavior is specific to the devices that use Pixart controller.
> It is timing dependent on when the resume event is triggered during
> the sleep state.
>
> [Q] Is it a host controller issue or mouse?
> - It is the host controller behavior during resume that triggers the
> device incorrect behavior on the next resume.
>
> This patch is only for xHCI driver and the quirk will be also added
> into OHCI driver too.
>
> Signed-off-by: Huang Rui
> ---
> drivers/usb/host/xhci-pci.c | 4
> drivers/usb/host/xhci.c | 57
> +
> drivers/usb/host/xhci.h | 35 ++--
> 3 files changed, 79 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index c2d4950..cc2ff7e 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -90,6 +90,10 @@ static void xhci_pci_quirks(struct device *dev, struct
> xhci_hcd *xhci)
> /* AMD PLL quirk */
> if (pdev->vendor == PCI_VENDOR_ID_AMD && usb_amd_find_chipset_info())
> xhci->quirks |= XHCI_AMD_PLL_FIX;
> +
> + if (pdev->vendor == PCI_VENDOR_ID_AMD && usb_amd_remote_wakeup_quirk())
> + xhci->quirks |= XHCI_AMD_REMOTE_WAKEUP_QUIRK;
> +
> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
> xhci->quirks |= XHCI_LPM_SUPPORT;
> xhci->quirks |= XHCI_INTEL_HOST;
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 49b6edb..bf749b0 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -831,6 +831,60 @@ static void xhci_clear_command_ring(struct xhci_hcd
> *xhci)
> }
>
> /*
> + * Reset port which attached with an issue device.
> + *
> + * If the remote wake is issued from the device in a specific timing
> + * condition while the system is entering sleep state then it may
> + * cause system to auto wake on subsequent sleep cycle.
> + *
> + * Host controller rebroadcasts the Resume signal > 100 µseconds after
> + * receiving the original resume event from the device. For proper
> + * function, some devices may require the rebroadcast of resume event
> + * within the USB spec of 100µS.
> + *
> + * Without this quirk, some usb mouse controller would react
> + * differently to this unexpected event from some AMD host controller
> + * and will result in the mouse to as
Re: FUSB200 xhci issue
Am 06.09.2013 23:09, schrieb Alan Stern: On Fri, 6 Sep 2013, Oleksij Rempel wrote: Hi Alan, i need more help. We have more test results now. Initial patch was probably fixing fallowing issue - bad performance on EP4/EP3 if Intr transfer is used. It is reproducible only on EHCI, but not reproducible on xHCI controller. Is this a high-speed device? yes What version of the kernel are you using? currently it is 3.11.0-rc7-wl-4-g52e0063, but i can reproduce this issue with older kernels too. For example, channel scan function intensively use this endpoints. On xHCI, channel scan is done in 6 seconds. On EHCI it needs 25s. By forcing Bulk transfer on host side, it is possible get same performance on EHCI too. But it isn't working on all controllers because of limited packed size (64B). Dumps are attached. One of them may include some other device, but only ath9k_htc device has EP3 and EP4. The capture logs show that with EHCI, each endpoint transfers a packet every 2.5 ms or so, whereas xHCI transfers 2-3 packets per ms. Is it protocol limitation or HC issue? It's not a protocol limitation. If it were, the xHCI communication would be just as slow as EHCI. One thing that might help: For each endpoint, keep multiple URBs queued (try 10) at all times instead of just a single URB. It's entirely possible that these results are caused by limitations of your EHCI hardware. Still, using multiple URBs could yield a significant improvement. Same issue on two different intel controllers. I'll try some thing different for comparison. -- Regards, Oleksij -- 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: FUSB200 xhci issue
On Fri, 6 Sep 2013, Oleksij Rempel wrote: > Am 06.09.2013 23:09, schrieb Alan Stern: > > On Fri, 6 Sep 2013, Oleksij Rempel wrote: > > > >> Hi Alan, > >> > >> i need more help. > >> > >> We have more test results now. Initial patch was probably fixing > >> fallowing issue - bad performance on EP4/EP3 if Intr transfer is used. > >> It is reproducible only on EHCI, but not reproducible on xHCI controller. > > > > Is this a high-speed device? > > yes > > > What version of the kernel are you using? > > currently it is 3.11.0-rc7-wl-4-g52e0063, but i can reproduce this > issue with older kernels too. > > >> For example, channel scan function intensively use this endpoints. On > >> xHCI, channel scan is done in 6 seconds. On EHCI it needs 25s. By > >> forcing Bulk transfer on host side, it is possible get same performance > >> on EHCI too. But it isn't working on all controllers because of limited > >> packed size (64B). > >> > >> Dumps are attached. One of them may include some other device, but only > >> ath9k_htc device has EP3 and EP4. > > > > The capture logs show that with EHCI, each endpoint transfers a packet > > every 2.5 ms or so, whereas xHCI transfers 2-3 packets per ms. > > > >> Is it protocol limitation or HC issue? > > > > It's not a protocol limitation. If it were, the xHCI communication > > would be just as slow as EHCI. > > > > One thing that might help: For each endpoint, keep multiple URBs queued > > (try 10) at all times instead of just a single URB. > > > > It's entirely possible that these results are caused by limitations of > > your EHCI hardware. Still, using multiple URBs could yield a > > significant improvement. > > Same issue on two different intel controllers. I'll try some thing > different for comparison. Don't try different hardware. Try submitting multiple URBs. 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
