Re: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: Hayes Wang hayesw...@realtek.com Date: Tue, 20 Jan 2015 03:24:52 + David Miller [mailto:da...@davemloft.net] Sent: Tuesday, January 20, 2015 10:52 AM [...] agg-list is not local, you have to use a spinlock to protect modifications to it, some other sites which modify agg-list do take the lock properly. You cannot modify a list like agg-list without proper locking. Excuse me. I don't understand. Before step1 tp_rx_done-listA-listB-listC-listD-... rx_queue- Because the other function would chage tp-rx_done, I need move the lists with spin lock. After step1 tp_rx_done- rx_queue-listA-listB-listC-listD-... Now I dequeue one of the lists from the list_head and deal with it. tp_rx_done- rx_queue-listA-listC-listD-... listB Then, if I want to put it back to rx_queue, I have to use spin lock. Why? No other function would chage rx_queue and the items in it. What keeps rtl_start_rx() from running in parallel with r8152_submit_rx(), or any other accessor of the RX agg-list? You also keep using different terminology from me when discussing what lists do or do not need protection, and that is going to make it difficult for anyone to follow our conversation at all. We're talking specifically about RX agg-list objects and whether access to them need synchronization or not. -- 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 8/8] phy: add driver for TI TUSB1210 ULPI PHY
Hi Heikki, On Fri, Jan 23, 2015 at 05:12:58PM +0200, Heikki Krogerus wrote: TUSB1210 ULPI PHY has vendor specific register for eye diagram tuning. On some platforms the system firmware has set optimized value to it. In order to not loose the optimized value, the driver stores it during probe and restores it every time the PHY is powered back on. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: Kishon Vijay Abraham I kis...@ti.com --- drivers/phy/Kconfig| 6 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-tusb1210.c | 133 + 3 files changed, 140 insertions(+) create mode 100644 drivers/phy/phy-tusb1210.c diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 26a7623..52ee720 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -284,4 +284,10 @@ config PHY_QCOM_UFS help Support for UFS PHY on QCOM chipsets. +config PHY_TUSB1210 + tristate TI TUSB1210 ULPI PHY module + depends on USB_ULPI_BUS + help + Support for TI TUSB1210 USB ULPI PHY. + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index cfbb720..03f3d85 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -37,3 +37,4 @@ obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o +obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o diff --git a/drivers/phy/phy-tusb1210.c b/drivers/phy/phy-tusb1210.c new file mode 100644 index 000..1551ae8 --- /dev/null +++ b/drivers/phy/phy-tusb1210.c @@ -0,0 +1,133 @@ +/** + * tusb1210.c - TUSB1210 USB ULPI PHY driver + * + * Copyright (C) 2015 Intel Corporation + * + * Author: Heikki Krogerus heikki.kroge...@linux.intel.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include linux/module.h +#include linux/ulpi/driver.h +#include linux/gpio/consumer.h + +#include ulpi_phy.h + +#define TUSB1210_VENDOR_SPECIFIC2 0x80 + +struct tusb1210 { + struct ulpi *ulpi; + struct phy *phy; + struct gpio_desc *gpio_reset; + struct gpio_desc *gpio_cs; + u8 eye_diagram_tuning; +}; + +static int tusb1210_power_on(struct phy *phy) +{ + struct tusb1210 *tusb = phy_get_drvdata(phy); + + gpiod_set_value_cansleep(tusb-gpio_reset, 1); + gpiod_set_value_cansleep(tusb-gpio_cs, 1); + + /* Restore eye diagram optimisation value */ + ulpi_write(tusb-ulpi, TUSB1210_VENDOR_SPECIFIC2, +tusb-eye_diagram_tuning); After you power on phy, ulpi bus may not be available right away. In intel case, phy power on happens during dwc3 power on. ULPI bus is not available until OTG controller and phy are in sync. In resume, you can't restore eye diagram from here. + + return 0; +} + +static int tusb1210_power_off(struct phy *phy) +{ + struct tusb1210 *tusb = phy_get_drvdata(phy); + + gpiod_set_value_cansleep(tusb-gpio_reset, 0); + gpiod_set_value_cansleep(tusb-gpio_cs, 0); + + return 0; +} + +static struct phy_ops phy_ops = { + .power_on = tusb1210_power_on, + .power_off = tusb1210_power_off, + .init = tusb1210_power_on, + .exit = tusb1210_power_off, + .owner = THIS_MODULE, +}; + +static int tusb1210_probe(struct ulpi *ulpi) +{ + struct gpio_desc *gpio; + struct tusb1210 *tusb; + int ret; + + tusb = devm_kzalloc(ulpi-dev, sizeof(*tusb), GFP_KERNEL); + if (!tusb) + return -ENOMEM; + + gpio = devm_gpiod_get(ulpi-dev, reset); + if (!IS_ERR(gpio)) { + ret = gpiod_direction_output(gpio, 0); + if (ret) + return ret; + tusb-gpio_reset = gpio; + } You cannot proceed with probe if gpio reset is not available. Different from CS, it's a mandatory pin to toggle in order to power on/off phy and get it in sync with OTG controller. + + gpio = devm_gpiod_get(ulpi-dev, cs); + if (!IS_ERR(gpio)) { + ret = gpiod_direction_output(gpio, 0); + if (ret) + return ret; + tusb-gpio_cs = gpio; + } + + /* Store initial eye diagram optimisation value */ + ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2); It's unclear if ulpi is accessible at this point. You can't read it at this point. Br, David + if (ret 0) + return ret; + + tusb-eye_diagram_tuning = ret; + + tusb-phy = ulpi_phy_create(ulpi, phy_ops); + if (IS_ERR(tusb-phy)) + return PTR_ERR(tusb-phy); + + tusb-ulpi = ulpi; + + phy_set_drvdata(tusb-phy, tusb); + ulpi_set_drvdata(ulpi,
Re: [PATCH] usb: Retry port status check on resume to work around RH bugs
On Fri, 23 Jan 2015, Julius Werner wrote: The EHCI controller on the RK3288 SoC is violating basic parts of the USB spec and thereby unable to properly resume a suspended port. It does not start SOF generation within 3ms of finishing resume signaling, so the attached device will drop off the bus again. This is a particular problem with runtime PM, where accessing the device will trigger a resume that immediately makes it unavailabe (and reenumerate with a new handle). Thankfully, the persist feature is generally able to work around stuff like that. Unfortunately, it doesn't quite work in this particular case because the controller will turn off the CurrentConnectStatus bit for an instant while the device is reconnecting, which causes the kernel to conclude that it permanently disappeared. This patch adds a tiny retry mechanism to the core port resume code which will catch this case and shouldn't have any notable impact on other controllers. Signed-off-by: Julius Werner jwer...@chromium.org --- drivers/usb/core/hub.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index aeb50bb..d1d0a66 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2896,10 +2896,12 @@ static int port_is_suspended(struct usb_hub *hub, unsigned portstatus) */ static int check_port_resume_type(struct usb_device *udev, struct usb_hub *hub, int port1, - int status, unsigned portchange, unsigned portstatus) + int status, u16 portchange, u16 portstatus) { struct usb_port *port_dev = hub-ports[port1 - 1]; + int retries = 3; +retry: /* Is a warm reset needed to recover the connection? */ if (status == 0 udev-reset_resume hub_port_warm_reset_required(hub, port1, portstatus)) { @@ -2909,8 +2911,15 @@ static int check_port_resume_type(struct usb_device *udev, else if (status || port_is_suspended(hub, portstatus) || !port_is_power_on(hub, portstatus) || !(portstatus USB_PORT_STAT_CONNECTION)) { - if (status = 0) + if (status = 0) { + if (retries--) { + udelay(200); You should use a sleeping function call, not a delay. + status = hub_port_status(hub, port1, + portstatus, portchange); + goto retry; + } status = -ENODEV; + } } /* Can't do a normal resume if the port isn't enabled, Otherwise this is okay. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc()
On Sat, 24 Jan 2015, Alexey Khoroshilov wrote: oxu_qh_alloc() acquires oxu-mem_lock spinlock, finds free qh slot and calls ehci_qtd_alloc() to initialize qh-dummy, but ehci_qtd_alloc() acquires oxu-mem_lock as well. That means an unavoidable deadlock in oxu_qh_alloc(). The patch fixes the issue by introduction of unlocked version of ehci_qtd_alloc(). This deadlock would affect anybody the moment they plugged a USB device into one of these controllers, right? Which means nobody can possibly be using them under Linux. So wouldn't it be better to remove the driver entirely? Alan Stern Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov khoroshi...@ispras.ru --- drivers/usb/host/oxu210hp-hcd.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c index 036924e640f5..15d4cf2c35cd 100644 --- a/drivers/usb/host/oxu210hp-hcd.c +++ b/drivers/usb/host/oxu210hp-hcd.c @@ -573,13 +573,11 @@ static inline void oxu_qtd_free(struct oxu_hcd *oxu, struct ehci_qtd *qtd) spin_unlock(oxu-mem_lock); } -static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu) +static struct ehci_qtd *ehci_qtd_alloc_unlocked(struct oxu_hcd *oxu) { int i; struct ehci_qtd *qtd = NULL; - spin_lock(oxu-mem_lock); - for (i = 0; i QTD_NUM; i++) if (!oxu-qtd_used[i]) break; @@ -598,6 +596,17 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu) oxu-qtd_used[i] = 1; } + return qtd; +} + +static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu) +{ + struct ehci_qtd *qtd; + + spin_lock(oxu-mem_lock); + + qtd = ehci_qtd_alloc_unlocked(oxu); + spin_unlock(oxu-mem_lock); return qtd; @@ -651,7 +660,7 @@ static struct ehci_qh *oxu_qh_alloc(struct oxu_hcd *oxu) INIT_LIST_HEAD(qh-qtd_list); /* dummy td enables safe urb queuing */ - qh-dummy = ehci_qtd_alloc(oxu); + qh-dummy = ehci_qtd_alloc_unlocked(oxu); if (qh-dummy == NULL) { oxu_dbg(oxu, no dummy td\n); oxu-qh_used[i] = 0; -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB autosuspend causing trouble with bluetooth
On Wed, Jan 21, 2015 at 12:47 AM, Oliver Neukum oneu...@suse.de wrote: On Tue, 2015-01-20 at 23:25 +0400, Kirill Elagin wrote: Hm, I'm pretty sure I never touched anything with `port` in its name, all the ports are set to `auto` (that's what laptop-mode-tools does). Here we go. Right now I think I see three possibly unrelated issues: Issue #1. BT trackpad not working properly when connected to the builtin bluetooth adapter. -- The adapter is attached to a USB1.1 hub: # lsusb -t ... /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M |__ Port 2: Dev 2, If 0, Class=Wireless, Driver=btusb, 12M |__ Port 2: Dev 2, If 1, Class=Wireless, Driver=btusb, 12M |__ Port 2: Dev 2, If 2, Class=Vendor Specific Class, Driver=, 12M |__ Port 2: Dev 2, If 3, Class=Application Specific Interface, Driver=, 12M ... # cat usb3/power/control auto # cat usb3/3-*/usb3-port*/power/control auto auto Try setting this to on. # cat usb3/3-2/power/control on Here auto should work. Please try. [..] Issue #2. No hotplug with USB1.1: -- To see this I pick one physical port. When I plug a USB1.1 device it appears on bus 4 port 2; a USB2.0 device appears on bus 1 port 4. # cat usb4/power/control auto # cat usb4/4-*/usb4-port*/power/control auto auto Please set this to on # cat usb1/power/control auto # cat usb1/1-*/usb1-port*/power/control Please set this to on I started with the defaults (`auto` everywhere) and did: for f in /sys/bus/usb/devices/usb*/*-*/*-port*/power/control; do echo on $f; done This had absolutely _no_ effect on hotplug. As before, when I insert the keyboard receiver nothing happens until I echo `on` to `usb6/power/control`. [..] Issue #3. No hot-plug-out for USB1.1. I think that the first two issues are fixed by keeping all the USB1.1 hubs and the builtin BT always `on`, but I just wanted to know whether those are hardware or software bugs. I suspect this is caused by outdated laptop-mode-tools. I have laptop-mode-tools-1.66 and actually I initially blamed the new runtime-pm module. But it turns out, `laptop_mode` doesn't touch the ports at all. Basically setting the port controls (as opposed to hub and device controls) to auto tells the kernel that it may disable hotplugging to save energy. Hotunplug for devices that need remote wakeup will still work. Likewise if you disable autosuspend of the attached devices. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Fix force effect modifications for the Microsoft Sidewinder Force Feedback Pro 2 joystick
Hi, Yes, confirmed. The description below still holds. Cheers, Jim On 23/01/2015 18:54, Benjamin Tissoires wrote: I have no clue what this code is doing (don't know much about the ff system). Quoting your initial 0001/0001, I thing the commit message should be the following. Jim, can you please validate it? The FF2 driver (usbhid/hid-pidff.c) does not set the effect ID when uploading an effect. The result is that the initial upload works but subsequent uploads to modify effect parameters are all directed at the last-created effect. The targeted effect ID must be passed back to the device when effect parameters are changed. This is done at the start of pidff_set_condition_report, pidff_set_periodic_report etc. based on the value of pidff-block_load[PID_EFFECT_ BLOCK_INDEX].value[0]. However, this value is only ever set during pidff_request_effect_upload. The result is stored in pidff-pid_id[effect-id] at the end of pid_upload_effect, for later use. However, if an effect is modified and re-sent then this identifier is not being copied back from pidff-pid_id[effect-id] before sending the command to the device. The fix is to do this at the start of pidff_upload_effect. --- As said, I have no clue* of what this is supposed to do, so I can not add my reviewed-by here. The patch seems good however so nothing should prevent us to take it if it fixes your problem :) Cheers, Benjamin * OK, I am pushing a little bit, I just don't want today to go deep in the code :-) On Fri, Jan 23, 2015 at 12:21 PM, Jim Keir jimk...@oracledbadirect.com wrote: --- Hi, Changes from previous patches: - All changes removed from higher-level files - Extra debug line removed Signed-off-by: Jim Keir jimk...@oracledbadirect.com drivers/hid/usbhid/hid-pidff.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c index 0b531c6..1b3fa70 100644 --- a/drivers/hid/usbhid/hid-pidff.c +++ b/drivers/hid/usbhid/hid-pidff.c @@ -568,6 +568,12 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect, int type_id; int error; + pidff-block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0; + if (old effect) { + pidff-block_load[PID_EFFECT_BLOCK_INDEX].value[0] = + pidff-pid_id[effect-id]; + } + switch (effect-type) { case FF_CONSTANT: if (!old) { -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-input 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
[PATCH 24/32] usb: use %*pb[l] to print bitmaps including cpumasks and nodemasks
printk and friends can now formap bitmaps using '%*pb[l]'. cpumask and nodemask also provide cpumask_pr_args() and nodemask_pr_args() respectively which can be used to generate the two printf arguments necessary to format the specified cpu/nodemask. * drivers/uwb/drp.c::uwb_drp_handle_alien_drp() was formatting mas.bm into a buffer but never used it. Removed. This patch is dependent on the following two patches. lib/vsprintf: implement bitmap printing through '%*pb[l]' cpumask, nodemask: implement cpumask/nodemask_pr_args() Please wait till the forementioned patches are merged to mainline before applying to subsystem trees. Signed-off-by: Tejun Heo t...@kernel.org Cc: Andrew Morton a...@linux-foundation.org Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: linux-usb@vger.kernel.org --- drivers/usb/host/whci/debug.c | 7 ++- drivers/usb/wusbcore/reservation.c | 5 ++--- drivers/usb/wusbcore/wa-rpipe.c| 5 ++--- drivers/usb/wusbcore/wusbhc.c | 7 ++- drivers/uwb/drp.c | 2 -- drivers/uwb/uwb-debug.c| 16 +--- 6 files changed, 13 insertions(+), 29 deletions(-) diff --git a/drivers/usb/host/whci/debug.c b/drivers/usb/host/whci/debug.c index ba61dae..774b89d 100644 --- a/drivers/usb/host/whci/debug.c +++ b/drivers/usb/host/whci/debug.c @@ -86,17 +86,14 @@ static void qset_print(struct seq_file *s, struct whc_qset *qset) static int di_print(struct seq_file *s, void *p) { struct whc *whc = s-private; - char buf[72]; int d; for (d = 0; d whc-n_devices; d++) { struct di_buf_entry *di = whc-di_buf[d]; - bitmap_scnprintf(buf, sizeof(buf), -(unsigned long *)di-availability_info, UWB_NUM_MAS); - seq_printf(s, DI[%d]\n, d); - seq_printf(s, availability: %s\n, buf); + seq_printf(s, availability: %*pb\n, + UWB_NUM_MAS, (unsigned long *)di-availability_info); seq_printf(s, %c%c key idx: %d dev addr: %d\n, (di-addr_sec_info WHC_DI_SECURE) ? 'S' : ' ', (di-addr_sec_info WHC_DI_DISABLE) ? 'D' : ' ', diff --git a/drivers/usb/wusbcore/reservation.c b/drivers/usb/wusbcore/reservation.c index d5efd0f..7b1b2e2 100644 --- a/drivers/usb/wusbcore/reservation.c +++ b/drivers/usb/wusbcore/reservation.c @@ -49,14 +49,13 @@ static void wusbhc_rsv_complete_cb(struct uwb_rsv *rsv) struct wusbhc *wusbhc = rsv-pal_priv; struct device *dev = wusbhc-dev; struct uwb_mas_bm mas; - char buf[72]; dev_dbg(dev, %s: state = %d\n, __func__, rsv-state); switch (rsv-state) { case UWB_RSV_STATE_O_ESTABLISHED: uwb_rsv_get_usable_mas(rsv, mas); - bitmap_scnprintf(buf, sizeof(buf), mas.bm, UWB_NUM_MAS); - dev_dbg(dev, established reservation: %s\n, buf); + dev_dbg(dev, established reservation: %*pb\n, + UWB_NUM_MAS, mas.bm); wusbhc_bwa_set(wusbhc, rsv-stream, mas); break; case UWB_RSV_STATE_NONE: diff --git a/drivers/usb/wusbcore/wa-rpipe.c b/drivers/usb/wusbcore/wa-rpipe.c index a80c5d2..c7ecdbe 100644 --- a/drivers/usb/wusbcore/wa-rpipe.c +++ b/drivers/usb/wusbcore/wa-rpipe.c @@ -496,10 +496,9 @@ void wa_rpipes_destroy(struct wahc *wa) struct device *dev = wa-usb_iface-dev; if (!bitmap_empty(wa-rpipe_bm, wa-rpipes)) { - char buf[256]; WARN_ON(1); - bitmap_scnprintf(buf, sizeof(buf), wa-rpipe_bm, wa-rpipes); - dev_err(dev, BUG: pipes not released on exit: %s\n, buf); + dev_err(dev, BUG: pipes not released on exit: %*pb\n, + wa-rpipes, wa-rpipe_bm); } kfree(wa-rpipe_bm); } diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c index 3e1ba51..94f401a 100644 --- a/drivers/usb/wusbcore/wusbhc.c +++ b/drivers/usb/wusbcore/wusbhc.c @@ -496,11 +496,8 @@ static void __exit wusbcore_exit(void) { clear_bit(0, wusb_cluster_id_table); if (!bitmap_empty(wusb_cluster_id_table, CLUSTER_IDS)) { - char buf[256]; - bitmap_scnprintf(buf, sizeof(buf), wusb_cluster_id_table, -CLUSTER_IDS); - printk(KERN_ERR BUG: WUSB Cluster IDs not released - on exit: %s\n, buf); + printk(KERN_ERR BUG: WUSB Cluster IDs not released on exit: %*pb\n, + CLUSTER_IDS, wusb_cluster_id_table); WARN_ON(1); } usb_unregister_notify(wusb_usb_notifier); diff --git a/drivers/uwb/drp.c b/drivers/uwb/drp.c index 05b7bd7..8fc1b78 100644 --- a/drivers/uwb/drp.c +++ b/drivers/uwb/drp.c @@ -619,11 +619,9 @@ static void uwb_drp_handle_alien_drp(struct uwb_rc *rc, struct uwb_ie_drp