RE: [PATCH v2 0/2] usb: renesas_usbhs: fix to avoid unexpected condition
Yoshihiro-san, Yoshihiro Shimoda writes: >> From: Yoshihiro Shimoda >> Sent: Thursday, March 10, 2016 11:30 AM >> >> This patch set is based on the latest Felipe's usb.git / testing/next branch. >> (commit id = ac5706631325ae3695bfa1527101ab2b2f64859f) > > Would you review the patch set? > > I confirmed that the patch set could be applied on the latest > testing/next and testing/fixes branches. I'm waiting for v4.6-rc1 to be tagged to carry on applying patches. Nothing really happens during the merge window ;-) -- balbi signature.asc Description: PGP signature
RE: [PATCH v2 0/2] usb: renesas_usbhs: fix to avoid unexpected condition
Hi Felipe-san, > From: Felipe Balbi [mailto:ba...@kernel.org] > Sent: Tuesday, March 22, 2016 4:16 PM > > Yoshihiro-san, > > Yoshihiro Shimoda writes: > >> From: Yoshihiro Shimoda > >> Sent: Thursday, March 10, 2016 11:30 AM > >> > >> This patch set is based on the latest Felipe's usb.git / testing/next > >> branch. > >> (commit id = ac5706631325ae3695bfa1527101ab2b2f64859f) > > > > Would you review the patch set? > > > > I confirmed that the patch set could be applied on the latest > > testing/next and testing/fixes branches. > > I'm waiting for v4.6-rc1 to be tagged to carry on applying > patches. Nothing really happens during the merge window ;-) Thank you for the reply. I got it :) Best regards, Yoshihiro Shimoda > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 1/2] usbip: modifications to event handler
Modification to usbip_event.c.
BEFORE) kernel threads are created in usbip_start_eh().
AFTER) one workqueue is created in new usbip_init_eh().
Event handler which was main loop of kernel thread is modified to
workqueue handler.
Events themselves are stored in struct usbip_device - same as before.
usbip_devices which have event are listed in event_list.
The handler picks an element from the list and wakeup usbip_device. The
wakeup method is same as before.
usbip_in_eh() substitutes statement which checks whether functions are
called from eh_ops or not. In this function, the worker context is used
for the checking. The context will be set in a variable in the
beginning of first event handling. usbip_in_eh() is used in event
handler so it works well.
Signed-off-by: Nobuo Iwata
---
drivers/usb/usbip/usbip_common.h | 4 +-
drivers/usb/usbip/usbip_event.c | 168 +++
2 files changed, 129 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/usbip/usbip_common.h b/drivers/usb/usbip/usbip_common.h
index 86b0847..2fbbc64 100644
--- a/drivers/usb/usbip/usbip_common.h
+++ b/drivers/usb/usbip/usbip_common.h
@@ -267,7 +267,6 @@ struct usbip_device {
struct task_struct *tcp_tx;
unsigned long event;
- struct task_struct *eh;
wait_queue_head_t eh_waitq;
struct eh_ops {
@@ -313,10 +312,13 @@ void usbip_pad_iso(struct usbip_device *ud, struct urb
*urb);
int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb);
/* usbip_event.c */
+int usbip_init_eh(void);
+void usbip_finish_eh(void);
int usbip_start_eh(struct usbip_device *ud);
void usbip_stop_eh(struct usbip_device *ud);
void usbip_event_add(struct usbip_device *ud, unsigned long event);
int usbip_event_happened(struct usbip_device *ud);
+int usbip_in_eh(struct task_struct *task);
static inline int interface_to_busnum(struct usb_interface *interface)
{
diff --git a/drivers/usb/usbip/usbip_event.c b/drivers/usb/usbip/usbip_event.c
index 2580a32..f163566 100644
--- a/drivers/usb/usbip/usbip_event.c
+++ b/drivers/usb/usbip/usbip_event.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2003-2008 Takahiro Hirofuchi
+ * Copyright (C) 2015 Nobuo Iwata
*
* This is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -19,17 +20,68 @@
#include
#include
+#include
+#include
#include "usbip_common.h"
-static int event_handler(struct usbip_device *ud)
+struct usbip_event {
+ struct list_head node;
+ struct usbip_device *ud;
+};
+
+static DEFINE_SPINLOCK(event_lock);
+static LIST_HEAD(event_list);
+
+static void set_event(struct usbip_device *ud, unsigned long event)
{
- usbip_dbg_eh("enter\n");
+ unsigned long flags;
- /*
-* Events are handled by only this thread.
-*/
- while (usbip_event_happened(ud)) {
+ spin_lock_irqsave(&ud->lock, flags);
+ ud->event |= event;
+ spin_unlock_irqrestore(&ud->lock, flags);
+}
+
+static void unset_event(struct usbip_device *ud, unsigned long event)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ud->lock, flags);
+ ud->event &= ~event;
+ spin_unlock_irqrestore(&ud->lock, flags);
+}
+
+static struct usbip_device *get_event(void)
+{
+ struct usbip_event *ue = NULL;
+ struct usbip_device *ud = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+ if (!list_empty(&event_list)) {
+ ue = list_first_entry(&event_list, struct usbip_event, node);
+ list_del(&ue->node);
+ }
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ if (ue) {
+ ud = ue->ud;
+ kfree(ue);
+ }
+ return ud;
+}
+
+static struct task_struct *worker_context;
+
+static void event_handler(struct work_struct *work)
+{
+ struct usbip_device *ud;
+
+ if (worker_context == NULL) {
+ worker_context = current;
+ }
+
+ while ((ud = get_event()) != NULL) {
usbip_dbg_eh("pending event %lx\n", ud->event);
/*
@@ -38,79 +90,102 @@ static int event_handler(struct usbip_device *ud)
*/
if (ud->event & USBIP_EH_SHUTDOWN) {
ud->eh_ops.shutdown(ud);
- ud->event &= ~USBIP_EH_SHUTDOWN;
+ unset_event(ud, USBIP_EH_SHUTDOWN);
}
/* Reset the device. */
if (ud->event & USBIP_EH_RESET) {
ud->eh_ops.reset(ud);
- ud->event &= ~USBIP_EH_RESET;
+ unset_event(ud, USBIP_EH_RESET);
}
/* Mark the device as unusable. */
if (ud->event & USBIP_EH_UNUSABLE) {
ud->eh_ops.unusable(ud);
- ud->event &= ~USBIP_EH_UNUSABLE;
+
[PATCH v1 1/1] usbip: safe completion against unbind operation
This patch adds a code fragment to ignore completing URBs in closing
connection.
Regarding this patch, 2 execution contexts are related.
1) stub_tx.c: stub_complete() which is called from USB core
1-1) add to unlink list and free URB or
1-2) move to tx list
2) stub_dev.c: stub_shutdown_connection() which is invoked by unbind
operation through sysfs.
2-1) stop TX/RX threads
2-2) close TCP connection and set ud.tcp_socket to NULL
2-3) cleanup pending URBs by stub_device_cleanup_urbs(sdev)
2-4) free unlink list (no lock)
In race condition URBs will be cleared in 2-3) may handled in 1).
In case 1-1), it will not be transferred bcause tx threads are stooped
in 2-1).
In case 1-2), may be freed in 2-4).
With this patch, after 2-2), completing URBs in 1) will not be handled
and cleared in 2-3).
The kernel log with patch is as below.
kernel: usbip-host 1-3: free sdev f680
kernel: usbip-host 1-3: free urb efcb8080
kernel: usbip-host 1-3: Enter
kernel: usbip-host 1-3: ignore urb for closed connection efc80c00 (*)
kernel: usbip-host 1-3: stopped by a call to usb_kill_urb() because of
cleaning up a virtual connection
kernel: usbip-host 1-3: free urb efc80c00 (**)
kernel: usbip-host 1-3: device reset
kernel: usbip-host 1-3: lock for reset
kernel: usbip_host: store_match_busid:178: del busid 1-3
kernel: uvcvideo: Found UVC 1.00 device Venus USB2.0 Camera (056e:700a)
kernel: input: Venus USB2.0 Camera as
/devices/pci:00/:00:1a.7/usb1/1-3/1-3:1.0/input/input18
(*) skipped with this patch in completion
(**) released in 2-3
Signed-off-by: Nobuo Iwata
---
drivers/usb/usbip/stub_tx.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c
index dbcabc9..a4efc5a 100644
--- a/drivers/usb/usbip/stub_tx.c
+++ b/drivers/usb/usbip/stub_tx.c
@@ -97,7 +97,11 @@ void stub_complete(struct urb *urb)
/* link a urb to the queue of tx. */
spin_lock_irqsave(&sdev->priv_lock, flags);
- if (priv->unlinking) {
+ if (sdev->ud.tcp_socket == NULL) {
+ dev_info(&urb->dev->dev,
+"ignore urb for closed connection %p", urb);
+ /* It will be freed in stub_device_cleanup_urbs(). */
+ } else if (priv->unlinking) {
stub_enqueue_ret_unlink(sdev, priv->seqnum, urb->status);
stub_free_priv_and_urb(priv);
} else {
--
2.1.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
[PATCH v1 1/1] usbip: adding names db to port operation
Adding names database to port command.
BEFORE) 'unknown' for vendor and product string.
Imported USB devices
Port 00: at Low Speed(1.5Mbps)
unknown vendor : unknown product (03f0:0224)
3-1 -> usbip://10.0.2.15:3240/5-1
-> remote bus/dev 005/002
AFTER) Most vendor string will be converted.
Imported USB devices
Port 00: at Low Speed(1.5Mbps)
Hewlett-Packard : unknown product (03f0:0224)
3-1 -> usbip://10.0.2.15:3240/5-1
-> remote bus/dev 005/002
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/src/usbip_port.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/usb/usbip/src/usbip_port.c b/tools/usb/usbip/src/usbip_port.c
index a2e884fd..7bd74fb 100644
--- a/tools/usb/usbip/src/usbip_port.c
+++ b/tools/usb/usbip/src/usbip_port.c
@@ -22,10 +22,13 @@ static int list_imported_devices(void)
struct usbip_imported_device *idev;
int ret;
+ if (usbip_names_init(USBIDS_FILE))
+ err("failed to open %s", USBIDS_FILE);
+
ret = usbip_vhci_driver_open();
if (ret < 0) {
err("open vhci_driver");
- return -1;
+ goto err_names_free;
}
printf("Imported USB devices\n");
@@ -35,13 +38,19 @@ static int list_imported_devices(void)
idev = &vhci_driver->idev[i];
if (usbip_vhci_imported_device_dump(idev) < 0)
- ret = -1;
+ goto err_driver_close;
}
usbip_vhci_driver_close();
+ usbip_names_free();
return ret;
+err_driver_close:
+ usbip_vhci_driver_close();
+err_names_free:
+ usbip_names_free();
+ return -1;
}
int usbip_port_show(__attribute__((unused)) int argc,
--
2.1.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
[PATCH v1 2/2] usbip: modifications to drivers using event handler
Modifications to code using usbip_event.c
Initialization and termination of workqueue are added to init and exit
routine of usbip_core respectively.
Signed-off-by: Nobuo Iwata
---
drivers/usb/usbip/stub_dev.c | 3 +--
drivers/usb/usbip/usbip_common.c | 7 +++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index a3ec49b..e286346 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -388,7 +388,6 @@ err_files:
err_port:
dev_set_drvdata(&udev->dev, NULL);
usb_put_dev(udev);
- kthread_stop_put(sdev->ud.eh);
busid_priv->sdev = NULL;
stub_device_free(sdev);
@@ -449,7 +448,7 @@ static void stub_disconnect(struct usb_device *udev)
}
/* If usb reset is called from event handler */
- if (busid_priv->sdev->ud.eh == current)
+ if (usbip_in_eh(current))
return;
/* shutdown the current connection */
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index facaaf0..aec2d65c 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -758,12 +758,19 @@ EXPORT_SYMBOL_GPL(usbip_recv_xbuff);
static int __init usbip_core_init(void)
{
+ int ret;
+
pr_info(DRIVER_DESC " v" USBIP_VERSION "\n");
+ ret = usbip_init_eh();
+ if (ret)
+ return ret;
+
return 0;
}
static void __exit usbip_core_exit(void)
{
+ usbip_finish_eh();
return;
}
--
2.1.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
[PATCH v1 0/2] usbip: event handler as one thread
Dear all, In existing implementation, event kernel threads are created for each port. The functions of the threads are closing connection and error handling so they don't have not so many events to handle. There' no need to have thread for each port. BEFORE) vhci side - VHCI_NPORTS(8) threads are created. $ ps aux | grep usbip root 10059 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10060 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10061 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10062 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10063 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10064 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10065 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] root 10066 0.0 0.0 0 0 ?S17:06 0:00 [usbip_eh] BEFORE) stub side - threads will be created every bind operation. $ ps aux | grep usbip root 8368 0.0 0.0 0 0 ?S17:56 0:00 [usbip_eh] root 8399 0.0 0.0 0 0 ?S17:56 0:00 [usbip_eh] This series of patches put event threads of stub and vhci driver as one workqueue. AFTER) only one event threads in each vhci and stub side. $ ps aux | grep usbip root 10457 0.0 0.0 0 0 ?S< 17:47 0:00 [usbip_event] *** BLURB HERE *** Nobuo Iwata (2): usbip: modifications to event handler usbip: modifications to drivers using event handler drivers/usb/usbip/stub_dev.c | 3 +- drivers/usb/usbip/usbip_common.c | 7 ++ drivers/usb/usbip/usbip_common.h | 4 +- drivers/usb/usbip/usbip_event.c | 168 +++ 4 files changed, 137 insertions(+), 45 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/2] usbip: modifications to event handler
Hi Nobuo, [auto build test ERROR on v4.5-rc7] [also build test ERROR on next-20160321] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Nobuo-Iwata/usbip-event-handler-as-one-thread/20160322-161050 config: xtensa-allmodconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa Note: the linux-review/Nobuo-Iwata/usbip-event-handler-as-one-thread/20160322-161050 HEAD 0fefef78d1ef7de850d42e55d026b8c2a4091838 builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): In file included from drivers/usb/usbip/stub_dev.c:25:0: drivers/usb/usbip/stub_dev.c: In function 'stub_probe': >> drivers/usb/usbip/stub_dev.c:391:27: error: 'struct usbip_device' has no >> member named 'eh' kthread_stop_put(sdev->ud.eh); ^ drivers/usb/usbip/usbip_common.h:292:16: note: in definition of macro 'kthread_stop_put' kthread_stop(k); \ ^ >> drivers/usb/usbip/stub_dev.c:391:27: error: 'struct usbip_device' has no >> member named 'eh' kthread_stop_put(sdev->ud.eh); ^ drivers/usb/usbip/usbip_common.h:293:19: note: in definition of macro 'kthread_stop_put' put_task_struct(k); \ ^ drivers/usb/usbip/stub_dev.c: In function 'stub_disconnect': drivers/usb/usbip/stub_dev.c:452:26: error: 'struct usbip_device' has no member named 'eh' if (busid_priv->sdev->ud.eh == current) ^ vim +391 drivers/usb/usbip/stub_dev.c 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 385 err_files: 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 386 usb_hub_release_port(udev->parent, udev->portnum, 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 387 (struct usb_dev_state *) udev); 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 388 err_port: b7945b77 drivers/staging/usbip/stub_dev.c Valentina Manea2014-01-23 389 dev_set_drvdata(&udev->dev, NULL); 695bcb1c drivers/staging/usbip/stub_dev.c Harvey Yang2012-11-06 390 usb_put_dev(udev); 695bcb1c drivers/staging/usbip/stub_dev.c Harvey Yang2012-11-06 @391 kthread_stop_put(sdev->ud.eh); 2d8f4595 drivers/staging/usbip/stub_dev.c Max Vozeler2011-01-12 392 aa5873e9 drivers/staging/usbip/stub_dev.c Endre Kollar 2010-07-27 393 busid_priv->sdev = NULL; 47c7157f drivers/staging/usbip/stub_dev.c Kulikov Vasiliy2010-07-12 394 stub_device_free(sdev); :: The code at line 391 was first introduced by commit :: 695bcb1c0a50e8fe04f0ab868cac849130788bc0 staging: usbip: put usb_device and kill event handler thread in error cleanups. :: TO: Harvey Yang :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH v8 1/5] leds: core: add generic support for RGB LED's
Hi Heiner, On 03/21/2016 06:24 PM, Heiner Kallweit wrote: Add generic support for RGB LED's. Basic idea is to use enum led_brightness also for the hue and saturation color components.This allows to implement the color extension w/o changes to struct led_classdev. Select LEDS_CLASS_RGB to enable building drivers using the RGB extension. Flag LED_SET_HUE_SAT allows to specify that hue / saturation should be overridden even if the provided values are zero. Some examples for writing values to /sys/class/leds//brightness: (now also hex notation can be used) 255 -> set full brightness and keep existing color if set 0 -> switch LED off but keep existing color so that it can be restored if the LED is switched on again later 0x100 -> switch LED off and set also hue and saturation to 0 0x00 -> set full brightness, full saturation and set hue to 0 (red) Signed-off-by: Heiner Kallweit --- v2: - move extension-specific code into a separate source file and introduce config symbol LEDS_HSV for it - create separate patch for the extension to sysfs - use variable name led_cdev as in the rest if the core - rename to_hsv to led_validate_brightness - rename LED_BRIGHTNESS_SET_COLOR to LED_SET_HSV - introduce helper is_off for checking whether V part of a HSV value is zero v3: - change Kconfig to use depend instead of select, add help message, and change config symbol to LEDS_COLOR - change LED core object file name in Makefile - rename flag LED_SET_HSV to LED_SET_COLOR - rename is_off to led_is_off - rename led_validate-brightness to led_confine_brightness - rename variable in led_confine_brightness - add dummy enum led_brightness value to enforce 32bit enum - rename led-hsv-core.c to led-color-core.c - move check of provided brightness value to led_confine_brightness v4: - change config symbol name to LEDS_RGB - change name of new file to led-rgb-core.c - factor out part of led_confine_brightness - change led_is_off to __is_set_brightness - in led_set_software_blink pass led_cdev->max_brightness instead of LED_FULL - rename LED_SET_COLOR to LED_SET_HUE_SAT v5: - change "RGB Color LED" to "RGB LED" in Kconfig - move definition of LED_HUE_SAT_MASK to drivers/leds/leds.h - rename LED_DEV_CAP_HSV to LED_DEV_CAP_RGB v6: - no change v7: - removed "Color" from RGB Color LED in commit message and title - don't include linux/kernel.h in led-rgb-core.c - keep definition of LED_DEV_CAP_[] flags together v8: - rename LEDS_RGB to LEDS_CLASS_RGB --- drivers/leds/Kconfig| 12 drivers/leds/Makefile | 4 +++- drivers/leds/led-class.c| 2 +- drivers/leds/led-core.c | 16 +--- drivers/leds/led-rgb-core.c | 39 +++ drivers/leds/leds.h | 18 ++ include/linux/leds.h| 11 ++- 7 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 drivers/leds/led-rgb-core.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 7f940c2..20697a2 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -19,6 +19,14 @@ config LEDS_CLASS This option enables the led sysfs class in /sys/class/leds. You'll need this to do anything useful with LEDs. If unsure, say N. +config LEDS_CLASS_RGB + bool "RGB LED Class Support" + depends on LEDS_CLASS + help + This option enables support for RGB LED devices. + Sysfs attribute brightness is interpreted as a HSV color value. + For details see Documentation/leds/leds-class.txt. + config LEDS_CLASS_FLASH tristate "LED Flash Class Support" depends on LEDS_CLASS @@ -29,6 +37,10 @@ config LEDS_CLASS_FLASH for the flash related features of a LED device. It can be built as a module. +if LEDS_CLASS_RGB +comment "RGB LED drivers" +endif # LEDS_CLASS_RGB + comment "LED drivers" config LEDS_88PM860X diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index e9d5309..7f1b2be 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -1,6 +1,8 @@ # LED Core -obj-$(CONFIG_NEW_LEDS) += led-core.o +obj-$(CONFIG_NEW_LEDS) += led-core-objs.o +led-core-objs-y:= led-core.o +led-core-objs-$(CONFIG_LEDS_CLASS_RGB) += led-rgb-core.o obj-$(CONFIG_LEDS_CLASS) += led-class.o obj-$(CONFIG_LEDS_CLASS_FLASH)+= led-class-flash.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index aa84e5b..007a5b3 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -53,7 +53,7 @@ static ssize_t brightness_store(struct device *dev, if (ret) goto unlock; - if (state == LED_OFF) + if (!__is_brightness_set(state)) led_trigger_remove(led_cdev); led_set_brightness(led_cdev, state); diff --git a/drivers/leds/led-cor
Re: 答复: 【xhci】suspend and resume core dump problem
On Mon, 2016-03-21 at 23:41 -0400, gre...@linuxfoundation.org wrote: > On Tue, Mar 22, 2016 at 03:17:08AM +, Lipengcheng wrote: > > Hi, > > Thanks for your reply. > > When the suspend and resume process , the operation of press ctrl + c > > will produce an interruput and cause to allocate memory failed. It causes > > the usb3 module can not be used and core dump after resume process. > > what are you pressing ctrl+c on? Why would that do anything on a USB > keyboard to interrupt the host? It causes a signal to be delivered. Obviously signals are ignored if memory allocation doesn't sleep. Your fix is wrong, but you found the cause. Somebody is using interruptible sleep at a place where uninterruptible sleep must be used. You need a stack trace to find out where the bug is. Can you replicate this on the current upstream? If not, please contact your vendor. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 2016-03-21 at 15:30 -0400, Alan Stern wrote: > On Mon, 21 Mar 2016, Oliver Neukum wrote: > > > We have an autosuspend timeout because we think that IO, if it will > > come at all, is likeliest to come soon. If, however, the IO is > > periodic that heuristics is false. > > To save most power the driver must either decide that the interval > > is too short or suspend immediately. So if we are lucky enough > > to have the frequency in the kernel, we should use that information. > > The autosuspend timeout is set by userspace. The kernel may assign a Thus it should apply to all IO originating in user space. But only to that IO. > default value, but the user can always override it. Given this, I > don't see how the kernel can use frequency information (and I'm not > sure where that information would come from in the first place). It can ignore internal IO for the purpose of the timeout. If such IO is performed while the device is active, don't alter the timer. Otherwise resume the device and look at the provided hint and suspend again immediately if the period is long enough. If IO is generated periodically in the kernel, the kernel must know that period. 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
[PATCH] sur40:fix DMA on stack
During the initialisation that driver uses a buffer on the stack
for DMA. That violates the cache coherency rules. The fix is to
allocate the buffer with kmalloc().
Signed-off-by: Oliver Neukum
---
drivers/input/touchscreen/sur40.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/sur40.c
b/drivers/input/touchscreen/sur40.c
index d214f22..5581954 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -196,29 +196,32 @@ static int sur40_command(struct sur40_state *dev,
/* Initialization routine, called from sur40_open */
static int sur40_init(struct sur40_state *dev)
{
- int result;
- u8 buffer[24];
+ int result = -ENOMEM;
+ u8 *buffer;
+ buffer = kmalloc(24, GFP_KERNEL);
+ if (!buffer)
+ goto error;
/* stupidly replay the original MS driver init sequence */
result = sur40_command(dev, SUR40_GET_VERSION, 0x00, buffer, 12);
if (result < 0)
- return result;
+ goto error;
result = sur40_command(dev, SUR40_GET_VERSION, 0x01, buffer, 12);
if (result < 0)
- return result;
+ goto error;
result = sur40_command(dev, SUR40_GET_VERSION, 0x02, buffer, 12);
if (result < 0)
- return result;
+ goto error;
result = sur40_command(dev, SUR40_UNKNOWN2,0x00, buffer, 24);
if (result < 0)
- return result;
+ goto error;
result = sur40_command(dev, SUR40_UNKNOWN1,0x00, buffer, 5);
if (result < 0)
- return result;
+ goto error;
result = sur40_command(dev, SUR40_GET_VERSION, 0x03, buffer, 12);
@@ -226,7 +229,8 @@ static int sur40_init(struct sur40_state *dev)
* Discard the result buffer - no known data inside except
* some version strings, maybe extract these sometime...
*/
-
+error:
+ kfree(buffer);
return result;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 答复: 答复: 【xhci】suspend and resume core dump problem
On 22.03.2016 08:05, Lipengcheng wrote: Hi, Why is that function failing? That seems suspicious. According to the analysis, in list_for_each_entry_safe(tt, n, &xhci->rh_bw[i].tts, tt_list) function, (tt->tt_list).prev=0x0. When list_del(&tt->tt_list) causing to core dump. Looks like you end up calling xhci_mem_cleanup() twice, once after a failed resume (or resume from hibernate), and a second time when allocating dma memory in xhci_mem_init() fails. This change should help for the xhci part of thee oops: diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index fbf75e5..406c872 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1856,6 +1856,7 @@ no_bw: kfree(xhci->usb3_ports); kfree(xhci->port_array); kfree(xhci->rh_bw); + xhci->rh_bw = NULL; xhci->page_size = 0; xhci->page_shift = 0; 3.10 also had some other similar issues when xhci_mem_init() called xhci_mem_cleanup() failed mid initialization. make sure you have the following patches included: commit 5dc2808c4729bf080487e61b80ee04e0fdb12a37 xhci: delete endpoints from bandwidth list before freeing whole device commit c207e7c50f31113c24a9f536fcab1e8a256985d7 xhci: Fix null pointer dereference if xhci initialization fails commit 0eda06c7c17ae48d7db69beef57f6e2b20bc3c72 usb: xhci: Fix OOPS in xhci error handling code As Oliver said, the other thing that needs to be fixed is the interruptible sleep somewhere in dma_alloc_coherent(). -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
Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad controller binding
On Fri, Mar 4, 2016 at 5:19 PM, Thierry Reding wrote: > From: Thierry Reding > > The NVIDIA Tegra XUSB pad controller provides a set of pads, each with a > set of lanes that are used for PCIe, SATA and USB. > > Signed-off-by: Thierry Reding > --- > Changes in v10: > - clarify that the hardware documentation means something different when > referring to a "port" (intra-SoC connectivity) Thierry I'm a bit out of sync, so can you resend these patches with collected ACKs after -rc1? Please send me the patches I can just merge into the pinctrl tree separately if possible, I encourage any DTS changes to go in orthogonally through ARM SoC. The DTS business I regard as kind of its own tree. Yours, Linus Walleij -- 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: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
On 22.03.2016 07:19, Rajesh Bhagat wrote:
-Original Message-
From: Mathias Nyman [mailto:mathias.ny...@intel.com]
Sent: Monday, March 21, 2016 2:46 PM
To: Rajesh Bhagat ; Mathias Nyman
; linux-usb@vger.kernel.org; linux-
ker...@vger.kernel.org
Cc: gre...@linuxfoundation.org; Sriram Dash
Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
commmand timeout
On 21.03.2016 06:18, Rajesh Bhagat wrote:
Hi
I think clearing the whole command ring is a bit too much in this case.
It may cause issues for all attached devices when one command times out.
Hi Mathias,
I understand your point, But I want to understand how would completion
handler be called if a command is timed out and xhci_abort_cmd_ring is
successful. In this case all the code would be waiting on completion handler
forever.
2. xhci_handle_command_timeout -> xhci_abort_cmd_ring(failure) ->
xhci_cleanup_command_queue -> xhci_complete_del_and_free_cmd
In our case command is timed out, Hence we hit the case #2 but
xhci_abort_cmd_ring is success which does not calls complete.
xhci_abort_cmd_ring() will write CA bit (CMD_RING_ABORT) to CRCR register.
This will generate a command completion event with status "command aborted" for
the pending command.
This event is then followed by a "command ring stopped" command completion
event.
See xHCI specs 5.4.5 and 4.6.1.2
handle_cmd_completion() will check if cmd_comp_code == COMP_CMD_ABORT, goto
event_handled, and call xhci_complete_del_and_free_cmd(cmd, cmd_comp_code) for
the aborted command.
If xHCI already processed the aborted command, we might only get a command ring
stopped event, in this case handle_cmd_completion() will call
xhci_handle_stopped_cmd_ring(xhci, cmd), which will turn the commands that were
tagged for "abort" that still remain on the command ring to NO-OP commands.
The completion callback will be called for these NO-OP command later when we
get a
command completion event for them.
Thanks Mathias for detailed explanation. Now I understand how completion
handler is
supposed to be called in this scenario.
But in our case, somehow we are not getting any event and handle_cmd_completion
function
is not getting called even after successful xhci_abort_cmd_ring when command
timed out.
Now, my point here is code prior to this patch xhci: rework command timeout and
cancellation,
Code would have returned in case command timed out in xhci_alloc_dev itself.
- /* XXX: how much time for xHC slot assignment? */
- timeleft = wait_for_completion_interruptible_timeout(
- command->completion,
- XHCI_CMD_DEFAULT_TIMEOUT);
- if (timeleft <= 0) {
- xhci_warn(xhci, "%s while waiting for a slot\n",
- timeleft == 0 ? "Timeout" : "Signal");
- /* cancel the enable slot request */
- ret = xhci_cancel_cmd(xhci, NULL, command->command_trb);
- return ret;
- }
+ wait_for_completion(command->completion);
But after this patch, we are waiting for hardware event, which is somehow not
generated
and causing a hang scenario.
IMO, The assumption that "xhci_abort_cmd_ring would always generate an event
and handle_cmd_completion would be called" will not be always be true if HW is
in bad state.
Please share your opinion.
writing the CA (command abort) bit in CRCR (command ring control register)
will stop the command ring,
and CRR (command ring running) will be set to 0 by xHC. xhci_abort_cmd_ring()
polls this bit up to 5 seconds.
If it's not 0 then the driver considers the command abort as failed.
The scenario you're thinking of is that xHC would still react to CA bit set, it
would stop the command
ring and set CRR 0, but not send a command completion event.
Have you tried adding some debug to handle_cmd_completion() and see if you
receive any event after command abortion?
The patches I mentioned earlier are also worth trying out, they might fix the
cause why command aborts in the first place.
-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
Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Tue, 22 Mar 2016, Oliver Neukum wrote: > On Mon, 2016-03-21 at 15:30 -0400, Alan Stern wrote: > > On Mon, 21 Mar 2016, Oliver Neukum wrote: > > > > > > We have an autosuspend timeout because we think that IO, if it will > > > come at all, is likeliest to come soon. If, however, the IO is > > > periodic that heuristics is false. > > > To save most power the driver must either decide that the interval > > > is too short or suspend immediately. So if we are lucky enough > > > to have the frequency in the kernel, we should use that information. > > > > The autosuspend timeout is set by userspace. The kernel may assign a > > Thus it should apply to all IO originating in user space. > But only to that IO. Fair enough. > > default value, but the user can always override it. Given this, I > > don't see how the kernel can use frequency information (and I'm not > > sure where that information would come from in the first place). > > It can ignore internal IO for the purpose of the timeout. > If such IO is performed while the device is active, don't > alter the timer. Come to think of it, we don't. If pm_runtime_get_sync() and then pm_runtime_put() are called while the device is already at full power, the PM core doesn't update the last_busy time. So if the driver doesn't update it either, the statistics collection won't interfere with autosuspend (except when it races with the autosuspend timer expiration). > Otherwise resume the device and look at > the provided hint and suspend again immediately if the period is long > enough. I don't see any point in resuming the device just in order to collect operating statistics. If it was already suspended then it wasn't operating, so there will be no statistics to collect. > If IO is generated periodically in the kernel, the kernel must know that > period. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Tue, 2016-03-22 at 10:21 -0400, Alan Stern wrote: > I don't see any point in resuming the device just in order to collect > operating statistics. If it was already suspended then it wasn't > operating, so there will be no statistics to collect. Indeed. In that case the point is moot. But it is correct to ask the core whether the device is autosuspended at that point rather than keep a private flag if you can. All that is relevant only if the upper layers ask for information that the driver cannot provide without resuming the device. Those are fundamentally different issues. 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
[PATCH 2/4] aiptek: stop saving struct usb_device
The device can now easily be derived from the interface.
Stop leaving a private copy.
Signed-off-by: Oliver Neukum
---
drivers/input/tablet/aiptek.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index 78ca448..ee80c11 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -307,7 +307,6 @@ struct aiptek_settings {
struct aiptek {
struct input_dev *inputdev; /* input device struct
*/
- struct usb_device *usbdev; /* usb device struct
*/
struct usb_interface *intf; /* usb interface struct
*/
struct urb *urb;/* urb for incoming reports
*/
dma_addr_t data_dma;/* our dma stuffage
*/
@@ -847,7 +846,7 @@ static int aiptek_open(struct input_dev *inputdev)
{
struct aiptek *aiptek = input_get_drvdata(inputdev);
- aiptek->urb->dev = aiptek->usbdev;
+ aiptek->urb->dev = interface_to_usbdev(aiptek->intf);;
if (usb_submit_urb(aiptek->urb, GFP_KERNEL) != 0)
return -EIO;
@@ -873,8 +872,9 @@ aiptek_set_report(struct aiptek *aiptek,
unsigned char report_type,
unsigned char report_id, void *buffer, int size)
{
- return usb_control_msg(aiptek->usbdev,
- usb_sndctrlpipe(aiptek->usbdev, 0),
+ struct usb_device *udev = interface_to_usbdev(aiptek->intf);
+ return usb_control_msg(udev,
+ usb_sndctrlpipe(udev, 0),
USB_REQ_SET_REPORT,
USB_TYPE_CLASS | USB_RECIP_INTERFACE |
USB_DIR_OUT, (report_type << 8) + report_id,
@@ -886,8 +886,9 @@ aiptek_get_report(struct aiptek *aiptek,
unsigned char report_type,
unsigned char report_id, void *buffer, int size)
{
- return usb_control_msg(aiptek->usbdev,
- usb_rcvctrlpipe(aiptek->usbdev, 0),
+ struct usb_device *udev = interface_to_usbdev(aiptek->intf);
+ return usb_control_msg(udev,
+ usb_rcvctrlpipe(udev, 0),
USB_REQ_GET_REPORT,
USB_TYPE_CLASS | USB_RECIP_INTERFACE |
USB_DIR_IN, (report_type << 8) + report_id,
@@ -1729,7 +1730,6 @@ aiptek_probe(struct usb_interface *intf, const struct
usb_device_id *id)
}
aiptek->inputdev = inputdev;
- aiptek->usbdev = usbdev;
aiptek->intf = intf;
aiptek->ifnum = intf->altsetting[0].desc.bInterfaceNumber;
aiptek->inDelay = 0;
@@ -1833,8 +1833,8 @@ aiptek_probe(struct usb_interface *intf, const struct
usb_device_id *id)
* input.
*/
usb_fill_int_urb(aiptek->urb,
-aiptek->usbdev,
-usb_rcvintpipe(aiptek->usbdev,
+usbdev,
+usb_rcvintpipe(usbdev,
endpoint->bEndpointAddress),
aiptek->data, 8, aiptek_irq, aiptek,
endpoint->bInterval);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] gtco: stop saving struct usb_device
The device can now easily be derived from the interface.
Stop leaving a private copy.
Signed-off-by: Oliver Neukum
---
drivers/input/tablet/gtco.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index 3a7f3a4..362ae3b 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -104,7 +104,6 @@ MODULE_DEVICE_TABLE (usb, gtco_usbid_table);
struct gtco {
struct input_dev *inputdevice; /* input device struct pointer */
- struct usb_device *usbdev; /* the usb device for this device */
struct usb_interface *intf; /* the usb interface for this device */
struct urb*urbinfo; /* urb for incoming reports */
dma_addr_tbuf_dma; /* dma addr of the data buffer*/
@@ -540,7 +539,7 @@ static int gtco_input_open(struct input_dev *inputdev)
{
struct gtco *device = input_get_drvdata(inputdev);
- device->urbinfo->dev = device->usbdev;
+ device->urbinfo->dev = interface_to_usbdev(device->intf);
if (usb_submit_urb(device->urbinfo, GFP_KERNEL))
return -EIO;
@@ -824,6 +823,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
int result = 0, retry;
int error;
struct usb_endpoint_descriptor *endpoint;
+ struct usb_device *udev = interface_to_usbdev(usbinterface);
/* Allocate memory for device structure */
gtco = kzalloc(sizeof(struct gtco), GFP_KERNEL);
@@ -838,11 +838,10 @@ static int gtco_probe(struct usb_interface *usbinterface,
gtco->inputdevice = input_dev;
/* Save interface information */
- gtco->usbdev = interface_to_usbdev(usbinterface);
gtco->intf = usbinterface;
/* Allocate some data for incoming reports */
- gtco->buffer = usb_alloc_coherent(gtco->usbdev, REPORT_MAX_SIZE,
+ gtco->buffer = usb_alloc_coherent(udev, REPORT_MAX_SIZE,
GFP_KERNEL, >co->buf_dma);
if (!gtco->buffer) {
dev_err(&usbinterface->dev, "No more memory for us buffers\n");
@@ -899,8 +898,8 @@ static int gtco_probe(struct usb_interface *usbinterface,
/* Couple of tries to get reply */
for (retry = 0; retry < 3; retry++) {
- result = usb_control_msg(gtco->usbdev,
-usb_rcvctrlpipe(gtco->usbdev, 0),
+ result = usb_control_msg(udev,
+usb_rcvctrlpipe(udev, 0),
USB_REQ_GET_DESCRIPTOR,
USB_RECIP_INTERFACE | USB_DIR_IN,
REPORT_DEVICE_TYPE << 8,
@@ -928,7 +927,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
}
/* Create a device file node */
- usb_make_path(gtco->usbdev, gtco->usbpath, sizeof(gtco->usbpath));
+ usb_make_path(udev, gtco->usbpath, sizeof(gtco->usbpath));
strlcat(gtco->usbpath, "/input0", sizeof(gtco->usbpath));
/* Set Input device functions */
@@ -945,15 +944,15 @@ static int gtco_probe(struct usb_interface *usbinterface,
gtco_setup_caps(input_dev);
/* Set input device required ID information */
- usb_to_input_id(gtco->usbdev, &input_dev->id);
+ usb_to_input_id(udev, &input_dev->id);
input_dev->dev.parent = &usbinterface->dev;
/* Setup the URB, it will be posted later on open of input device */
endpoint = &usbinterface->altsetting[0].endpoint[0].desc;
usb_fill_int_urb(gtco->urbinfo,
-gtco->usbdev,
-usb_rcvintpipe(gtco->usbdev,
+udev,
+usb_rcvintpipe(udev,
endpoint->bEndpointAddress),
gtco->buffer,
REPORT_MAX_SIZE,
@@ -977,7 +976,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
err_free_urb:
usb_free_urb(gtco->urbinfo);
err_free_buf:
- usb_free_coherent(gtco->usbdev, REPORT_MAX_SIZE,
+ usb_free_coherent(udev, REPORT_MAX_SIZE,
gtco->buffer, gtco->buf_dma);
err_free_devs:
input_free_device(input_dev);
@@ -994,13 +993,14 @@ static void gtco_disconnect(struct usb_interface
*interface)
{
/* Grab private device ptr */
struct gtco *gtco = usb_get_intfdata(interface);
+ struct usb_device *udev = interface_to_usbdev(interface);
/* Now reverse all the registration stuff */
if (gtco) {
input_unregister_device(gtco->inputdevice);
usb_kill_urb(gtco->urbinfo);
usb_free_urb(gtco->urbinfo);
- usb_free_coherent(gtco->usbdev, REPORT_MAX_SIZE,
+ usb_free_coherent(ude
[PATCH 0/4] tablet: remove private copy to USB device
We now have a macro to easily get to the USB device from the interface. So we are cleaning up all drivers to not store a private pointer. Oliver Neukum (4): acecad: stop saving struct usb_device aiptek: stop saving struct usb_device gtco: stop saving struct usb_device kbtab: stop saving struct usb_device drivers/input/tablet/acecad.c | 12 ++-- drivers/input/tablet/aiptek.c | 18 +- drivers/input/tablet/gtco.c | 24 drivers/input/tablet/kbtab.c | 8 4 files changed, 31 insertions(+), 31 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] acecad: stop saving struct usb_device
The device can now easily be derived from the interface.
Stop leaving a private copy.
Signed-off-by: Oliver Neukum
---
drivers/input/tablet/acecad.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/input/tablet/acecad.c b/drivers/input/tablet/acecad.c
index 889f6b7..e86e377 100644
--- a/drivers/input/tablet/acecad.c
+++ b/drivers/input/tablet/acecad.c
@@ -49,7 +49,6 @@ MODULE_LICENSE(DRIVER_LICENSE);
struct usb_acecad {
char name[128];
char phys[64];
- struct usb_device *usbdev;
struct usb_interface *intf;
struct input_dev *input;
struct urb *irq;
@@ -64,6 +63,7 @@ static void usb_acecad_irq(struct urb *urb)
unsigned char *data = acecad->data;
struct input_dev *dev = acecad->input;
struct usb_interface *intf = acecad->intf;
+ struct usb_device *udev = interface_to_usbdev(intf);
int prox, status;
switch (urb->status) {
@@ -110,15 +110,15 @@ resubmit:
if (status)
dev_err(&intf->dev,
"can't resubmit intr, %s-%s/input0, status %d\n",
- acecad->usbdev->bus->bus_name,
- acecad->usbdev->devpath, status);
+ udev->bus->bus_name,
+ udev->devpath, status);
}
static int usb_acecad_open(struct input_dev *dev)
{
struct usb_acecad *acecad = input_get_drvdata(dev);
- acecad->irq->dev = acecad->usbdev;
+ acecad->irq->dev = interface_to_usbdev(acecad->intf);
if (usb_submit_urb(acecad->irq, GFP_KERNEL))
return -EIO;
@@ -172,7 +172,6 @@ static int usb_acecad_probe(struct usb_interface *intf,
const struct usb_device_
goto fail2;
}
- acecad->usbdev = dev;
acecad->intf = intf;
acecad->input = input_dev;
@@ -251,12 +250,13 @@ static int usb_acecad_probe(struct usb_interface *intf,
const struct usb_device_
static void usb_acecad_disconnect(struct usb_interface *intf)
{
struct usb_acecad *acecad = usb_get_intfdata(intf);
+ struct usb_device *udev = interface_to_usbdev(intf);
usb_set_intfdata(intf, NULL);
input_unregister_device(acecad->input);
usb_free_urb(acecad->irq);
- usb_free_coherent(acecad->usbdev, 8, acecad->data, acecad->data_dma);
+ usb_free_coherent(udev, 8, acecad->data, acecad->data_dma);
kfree(acecad);
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] kbtab: stop saving struct usb_device
The device can now easily be derived from the interface.
Stop leaving a private copy.
Signed-off-by: Oliver Neukum
---
drivers/input/tablet/kbtab.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/input/tablet/kbtab.c b/drivers/input/tablet/kbtab.c
index d2ac7c2..e850d7e 100644
--- a/drivers/input/tablet/kbtab.c
+++ b/drivers/input/tablet/kbtab.c
@@ -31,7 +31,6 @@ struct kbtab {
unsigned char *data;
dma_addr_t data_dma;
struct input_dev *dev;
- struct usb_device *usbdev;
struct usb_interface *intf;
struct urb *irq;
char phys[32];
@@ -99,8 +98,9 @@ MODULE_DEVICE_TABLE(usb, kbtab_ids);
static int kbtab_open(struct input_dev *dev)
{
struct kbtab *kbtab = input_get_drvdata(dev);
+ struct usb_device *udev = interface_to_usbdev(kbtab->intf);
- kbtab->irq->dev = kbtab->usbdev;
+ kbtab->irq->dev = udev;
if (usb_submit_urb(kbtab->irq, GFP_KERNEL))
return -EIO;
@@ -135,7 +135,6 @@ static int kbtab_probe(struct usb_interface *intf, const
struct usb_device_id *i
if (!kbtab->irq)
goto fail2;
- kbtab->usbdev = dev;
kbtab->intf = intf;
kbtab->dev = input_dev;
@@ -188,12 +187,13 @@ static int kbtab_probe(struct usb_interface *intf, const
struct usb_device_id *i
static void kbtab_disconnect(struct usb_interface *intf)
{
struct kbtab *kbtab = usb_get_intfdata(intf);
+ struct usb_device *udev = interface_to_usbdev(intf);
usb_set_intfdata(intf, NULL);
input_unregister_device(kbtab->dev);
usb_free_urb(kbtab->irq);
- usb_free_coherent(kbtab->usbdev, 8, kbtab->data, kbtab->data_dma);
+ usb_free_coherent(udev, 8, kbtab->data, kbtab->data_dma);
kfree(kbtab);
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Tue, 22 Mar 2016, Oliver Neukum wrote: > On Tue, 2016-03-22 at 10:21 -0400, Alan Stern wrote: > > I don't see any point in resuming the device just in order to collect > > operating statistics. If it was already suspended then it wasn't > > operating, so there will be no statistics to collect. > > Indeed. In that case the point is moot. But it is correct to ask > the core whether the device is autosuspended at that point rather > than keep a private flag if you can. That's why we have pm_runtime_status_suspended(). > All that is relevant only if the upper layers ask for information > that the driver cannot provide without resuming the device. > Those are fundamentally different issues. Of course. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Tue, 2016-03-22 at 11:13 -0400, Alan Stern wrote: > > Indeed. In that case the point is moot. But it is correct to ask > > the core whether the device is autosuspended at that point rather > > than keep a private flag if you can. > > That's why we have pm_runtime_status_suspended(). I guess we are in violent agreement though we were unaware of being in that state. 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
[PATCH] Add DCD line support to CP210x driver
This patch adds DCD line support to CP210x USB serial driver.
First it enables CP210x events embedding to incoming URB's by calling:
cp210x_set_config_single(port, CP210X_EMBED_EVENTS, CP210X_ESCCHAR);
Then it parses incoming URB's via custom routine:
cp210x_process_read_urb(...)
searches for event sequences and handles all of DCD changes calling
usb_serial_handle_dcd_change(...)
Signed-off-by: Valentin Yakovenkov
---
drivers/usb/serial/cp210x.c | 115 +++-
1 file changed, 114 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 7a76fe4..d9cba4e 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -46,6 +46,7 @@ static void cp210x_break_ctl(struct tty_struct *, int);
static int cp210x_startup(struct usb_serial *);
static void cp210x_release(struct usb_serial *);
static void cp210x_dtr_rts(struct usb_serial_port *p, int on);
+static void cp210x_process_read_urb(struct urb *urb);
static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x045B, 0x0053) }, /* Renesas RX610 RX-Stick */
@@ -201,8 +202,17 @@ static const struct usb_device_id id_table[] = {
MODULE_DEVICE_TABLE(usb, id_table);
+#define CP210X_ESCCHAR 0x1e
+#define CP210X_STATE_IDLE 0
+#define CP210X_STATE_ESC 1
+#define CP210X_STATE_LS0 2
+#define CP210X_STATE_LS1 3
+#define CP210X_STATE_LS4
+#define CP210X_STATE_MS5
+
struct cp210x_serial_private {
__u8bInterfaceNumber;
+ int cp210x_tstate;
};
static struct usb_serial_driver cp210x_device = {
@@ -222,7 +232,8 @@ static struct usb_serial_driver cp210x_device = {
.tiocmset = cp210x_tiocmset,
.attach = cp210x_startup,
.release= cp210x_release,
- .dtr_rts= cp210x_dtr_rts
+ .dtr_rts= cp210x_dtr_rts,
+ .process_read_urb = cp210x_process_read_urb,
};
static struct usb_serial_driver * const serial_drivers[] = {
@@ -460,6 +471,11 @@ static int cp210x_open(struct tty_struct *tty, struct
usb_serial_port *port)
{
int result;
+ struct usb_serial *serial = port->serial;
+ struct cp210x_serial_private *spriv = usb_get_serial_data(serial);
+
+ spriv->cp210x_tstate = CP210X_STATE_IDLE;
+
result = cp210x_set_config_single(port, CP210X_IFC_ENABLE,
UART_ENABLE);
if (result) {
@@ -474,6 +490,15 @@ static int cp210x_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (tty)
cp210x_change_speed(tty, port, NULL);
+ /* Enable events embedding to data stream */
+ result = cp210x_set_config_single(port, CP210X_EMBED_EVENTS,
+ CP210X_ESCCHAR);
+ if (result) {
+ dev_err(&port->dev, "%s - Unable to enable event embedding on
UART\n",
+ __func__);
+ return result;
+ }
+
return usb_serial_generic_open(tty, port);
}
@@ -483,6 +508,94 @@ static void cp210x_close(struct usb_serial_port *port)
cp210x_set_config_single(port, CP210X_IFC_ENABLE, UART_DISABLE);
}
+static void cp210x_process_read_urb(struct urb *urb)
+{
+ struct usb_serial_port *port = urb->context;
+ char *ch = (char *)urb->transfer_buffer;
+ char *tbuf = (char *)urb->transfer_buffer;
+ int i;
+ int tcnt = 0;
+ struct usb_serial *serial = port->serial;
+ struct cp210x_serial_private *spriv = usb_get_serial_data(serial);
+
+ if (!urb->actual_length)
+ return;
+
+ /* Process escape chars */
+ for (i = 0; i < urb->actual_length; i++) {
+ char c = ch[i];
+
+ switch (spriv->cp210x_tstate) {
+ case CP210X_STATE_IDLE:
+ if (c == CP210X_ESCCHAR)
+ spriv->cp210x_tstate = CP210X_STATE_ESC;
+ else
+ tbuf[tcnt++] = c;
+ break;
+
+ case CP210X_STATE_ESC:
+ if (c == 0x01)
+ spriv->cp210x_tstate = CP210X_STATE_LS0;
+ else if (c == 0x02)
+ spriv->cp210x_tstate = CP210X_STATE_LS;
+ else if (c == 0x03)
+ spriv->cp210x_tstate = CP210X_STATE_MS;
+ else {
+ tbuf[tcnt++] = (c == 0x00) ? CP210X_ESCCHAR : c;
+ spriv->cp210x_tstate = CP210X_STATE_IDLE;
+ }
+ break;
+
+ case CP210X_STATE_LS0:
+ spriv->cp210x_tstate = CP210X_STATE_LS1;
[RFC] xhci: one more quirk for PANTHERPOINT
A T430 is reported to also need XHCI_SLOW_SUSPEND, lest a power
off turn into a reboot.
Signed-off-by: Oliver Neukum
---
drivers/usb/host/xhci-pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index f0640b7..6044508 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -142,9 +142,11 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
* switch the ports from xHCI to EHCI on shutdown. We can't use
* DMI information to find those particular boards (since each
* vendor will change the board name), so we have to key off all
-* PPT chipsets.
+* PPT chipsets. Yet a T430 is reported to not work
+* but also need XHCI_SLOW_SUSPEND
*/
xhci->quirks |= XHCI_SPURIOUS_REBOOT;
+ xhci->quirks |= XHCI_SLOW_SUSPEND;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] PANTHERPOINT needing yet another quirk
I have a report of a T430 laptop needing this unless you want shut downs turn into reboot. Is there any hope of investigating this further at Intel, or do we go for the cautious approach? Oliver Neukum (1): xhci: one more quirk for PANTHERPOINT drivers/usb/host/xhci-pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] aiptek: stop saving struct usb_device
Hi Oliver, [auto build test WARNING on input/next] [also build test WARNING on v4.5 next-20160322] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Oliver-Neukum/tablet-remove-private-copy-to-USB-device/20160322-231333 base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next coccinelle warnings: (new ones prefixed by >>) >> drivers/input/tablet/aiptek.c:849:54-55: Unneeded semicolon Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- 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] aiptek: fix semicolon.cocci warnings
drivers/input/tablet/aiptek.c:849:54-55: Unneeded semicolon
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
CC: Oliver Neukum
Signed-off-by: Fengguang Wu
---
aiptek.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -846,7 +846,7 @@ static int aiptek_open(struct input_dev
{
struct aiptek *aiptek = input_get_drvdata(inputdev);
- aiptek->urb->dev = interface_to_usbdev(aiptek->intf);;
+ aiptek->urb->dev = interface_to_usbdev(aiptek->intf);
if (usb_submit_urb(aiptek->urb, GFP_KERNEL) != 0)
return -EIO;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"
On 3/18/2016 10:21 PM, Doug Anderson wrote:
> Hi,
>
> On Wed, Mar 16, 2016 at 11:28 AM, John Youn wrote:
>> On 3/10/2016 11:14 AM, John Youn wrote:
>>> On 3/9/2016 11:06 AM, Doug Anderson wrote:
Stefan,
On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren
wrote:
>
>> Doug Anderson hat am 7. März 2016 um 22:30
>> geschrieben:
>>
>>
>> Stefan,
>>
>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren
>> wrote:
>>> Hi Doug,
>>>
Douglas Anderson hat am 4. März 2016 um 19:23
geschrieben:
This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on
bcm2835") now that we've found the root cause. See the change
titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()").
>>>
>>> adding a delay of 10 ms after a core reset might be a idea, but applying
>>> both
>>> patches breaks USB support on RPi :-(
>>>
>>> I'm getting the wrong register values ...
>>
>> Ugh. :(
>>
>> Just out of curiosity, if you loop and time long it takes for the
>> registers to get to the right state after reset, what do you get?
>> AKA, pick:
>>
>> https://chromium-review.googlesource.com/331260
>>
>> ...and let me know what it prints out.
>
> On my Raspberry Pi B i get the following:
>
> [2.084411] dwc2 2098.usb: mapped PA 2098 to VA cc88
> [2.084461] dwc2 2098.usb: cannot get otg clock
> [2.084549] dwc2 2098.usb: registering common handler for irq33
> [2.084713] dwc2 2098.usb: Configuration mismatch. dr_mode forced
> to host
> [2.153965] dwc2 2098.usb: Waited 49996 us, 0x00201000 =>
> 0x01001000,
> 0x => 0x02002000
> [2.174930] dwc2 2098.usb: Forcing mode to host
>
> So i changed the delay in patch #1 to msleep(50) and then both patches
> work like
> a charm.
Great news! :-)
John: it's pretty clear that there's something taking almost exactly
10ms on my system and almost exactly 50ms on Stefan's system. Is
there some register we could poll to see when this process is done?
...or can we look at the dwc2 revision number / feature register and
detect how long to delay?
>>>
>>> Hi Doug,
>>>
>>> I'll have to ask around to see if anyone knows about this. And I'll
>>> run some tests on the platforms I have available to me as well.
>>>
>>
>> There's still nothing definitive on our end as to why this is
>> happening. Also I don't think there is any other way to poll the
>> reset.
>
> One thing I noticed is that the delay was only needed on my OTG port
> (not my host-only port). ...I also noticed that while waiting the
> HPTXFSIZ was temporarily 0x while I was delaying. That seems
> to match Stephan.
>
> I wonder if perhaps the delay has to do with how long it took to
> detect that it needed to go into host mode?
>
> Ah, interesting. It looks as if "GOTGCTL" changes during this time
> too. To summarize:
>
> HOST-only (ff54.usb): needs no delay:
> GNPTXFSIZ: 0x04000400
> HPTXFSIZ: 0x02000800
> GOTGCTL: 0x002d
>
> OTG (ff58): needs 10ms delay. Before delay:
> GNPTXFSIZ: 0x00100400
> HPTXFSIZ: 0x
> GOTGCTL: 0x0001
> After delay:
> GNPTXFSIZ: 0x04000400
> HPTXFSIZ: 0x02000800
> GOTGCTL: 0x002c
>
> Could we loop until either BSesVld or ASesVld is set? Don't know much
> about OTG, but seems like that might be something sane?
>
>> Our hardware engineers asked for some more information to look
>> into it further. Doug, Stefan, Caesar, and anyone else with a related
>> platform, do you know the answers to the following:
>>
>> 1. What is the AHB Clock frequency? Is the AHB Clock gated during
>> Reset?
>
> According to commit 20bde643434d ("usb: dwc2: reduce dwc2 driver probe
> time"), our AHB clock is 150MHz. I'm nearly certain it is not gated.
>
>
>> 2. Also is the PHY clock stopped during the reset or is the PHY PLL
>> lock times high in the order of ms?
>
> I don't think so.
>
>
>> 3. In these cases, is the PHY actually an FS Transceiver and not a
>> UTMI/ULPI PHY?
>
> I don't think so. Should be answered by debug info spew below...
>
>
>> 4. Which version of the controller is being used in these cases?
>
> There are two controllers in my case and they behave differently. OTG
> takes 10ms and the host-only 0ms.
>
> Here is debug info:
>
> [1.752005] dwc2 ff54.usb: Core Release: 3.10a (snpsid=4f54310a)
> [1.758350] dwc2 ff54.usb: hwcfg1=
> [1.762807] dwc2 ff54.usb: hwcfg2=228fc856
> [1.767245] dwc2 ff54.usb: hwcfg3=03c0c068
> [1.771693] dwc2 ff54.usb: hwcfg4=c8004030
> [1.776149] dwc2 ff54.usb: grxfsiz=0400
> [1.780687] dwc2 ff54.usb: gnptxfsiz=04000400
> [1.785402] dwc2 ff54.usb: hptxfsiz=02000800
> [1.790024] dwc2 ff54.usb: De
Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"
John, On Tue, Mar 22, 2016 at 12:26 PM, John Youn wrote: > Thanks for the debug logs and everyones help. > > After reviewing with our hardware engineers, it seems this is likely > to do with the IDDIG debounce filtering when switching between > modes. You can see if this is enabled in your core by checking > GHWCFG4.IddgFltr. > > From the databook: > > OTG_EN_IDDIG_FILTER > > Specifies whether to add a filter on the "iddig" input from the > PHY. If your PHY already has a filter on iddig for de-bounce, then > it is not necessary to enable the one in the DWC_otg. The filter > is implemented in the DWC_otg_wpc module as a 20-bit counter that > works on the PHY clock. In the case of the UTMI+ PHY, this pin is > from the PHY. In the case of the ULPI PHY, this signal is > generated by the ULPI Wrapper inside the core. > > > This only affects OTG cores and the delay is 10ms for 30MHz PHY clock > and 50ms with a 6MHz PHY clock. Wow, nice to have it so perfectly explained! :) > The reason we see this after a reset is that by default the IDDIG > indicates device mode. But if the id pin is set to host the core will > immediately detect it after the reset and trigger the filtering delay > before changing to host mode. > > This delay is also triggered by force mode. This is the origin of the > 25 ms delay specified in the databook. The databook did not take into > account that there might be a 6MHz clock so this delay could actually > be up to 50 ms. So we aren't delaying enough time for those cases. > > I think this explains all the problems and platform differences we are > seeing related to this. > > I think we can just add an IDDIG delay after a force mode, a clear > force mode, and any time after reset where we don't do either a force > or clear force mode immediately afterwards. Maybe set the delay time > via a core parameter or measure it once on probe. Or we can probably > just poll for the mode change in all of the above conditions. The driver seems to be able to figure out the PHY clock in most cases in dwc2_calc_frame_interval(). It doesn't seem to handle 6MHz there, though. I wonder if that's another bug? Polling seems fine too, though. > Are you able to continue looking into this? If not, I can take it up. I'm pretty much out of time at this point and it sounds like you've though through all of the corner cases. If you can take it up that would be wonderful! :) I'm happy to give the patches a test, though! :) -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: sunxi-glue: Fix sunxi-musb driver not auto-loading
Hi Hans,
Am 21.03.2016 um 23:13 schrieb Hans de Goede:
> Add a missing MODULE_DEVICE_TABLE() line which was causing the
> sunxi-musb glue to not auto-load when build as a module.
"built"
>
Should this get a Fixes: header for backporting to stable?
> Signed-off-by: Hans de Goede
> ---
> drivers/usb/musb/sunxi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 4b472b8..9af9431 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -756,6 +756,7 @@ static const struct of_device_id sunxi_musb_match[] = {
> { .compatible = "allwinner,sun8i-a33-musb", },
> {}
> };
> +MODULE_DEVICE_TABLE(of, sunxi_musb_match);
>
> static struct platform_driver sunxi_musb_driver = {
> .probe = sunxi_musb_probe,
Has anyone looked into my earlier report about the module filename not
having musb-? I did not see any response and haven't rechecked v4.5.0.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"
On 3/22/2016 12:44 PM, Doug Anderson wrote: > John, > > On Tue, Mar 22, 2016 at 12:26 PM, John Youn wrote: >> Thanks for the debug logs and everyones help. >> >> After reviewing with our hardware engineers, it seems this is likely >> to do with the IDDIG debounce filtering when switching between >> modes. You can see if this is enabled in your core by checking >> GHWCFG4.IddgFltr. >> >> From the databook: >> >> OTG_EN_IDDIG_FILTER >> >> Specifies whether to add a filter on the "iddig" input from the >> PHY. If your PHY already has a filter on iddig for de-bounce, then >> it is not necessary to enable the one in the DWC_otg. The filter >> is implemented in the DWC_otg_wpc module as a 20-bit counter that >> works on the PHY clock. In the case of the UTMI+ PHY, this pin is >> from the PHY. In the case of the ULPI PHY, this signal is >> generated by the ULPI Wrapper inside the core. >> >> >> This only affects OTG cores and the delay is 10ms for 30MHz PHY clock >> and 50ms with a 6MHz PHY clock. > > Wow, nice to have it so perfectly explained! :) > > >> The reason we see this after a reset is that by default the IDDIG >> indicates device mode. But if the id pin is set to host the core will >> immediately detect it after the reset and trigger the filtering delay >> before changing to host mode. >> >> This delay is also triggered by force mode. This is the origin of the >> 25 ms delay specified in the databook. The databook did not take into >> account that there might be a 6MHz clock so this delay could actually >> be up to 50 ms. So we aren't delaying enough time for those cases. >> >> I think this explains all the problems and platform differences we are >> seeing related to this. >> >> I think we can just add an IDDIG delay after a force mode, a clear >> force mode, and any time after reset where we don't do either a force >> or clear force mode immediately afterwards. Maybe set the delay time >> via a core parameter or measure it once on probe. Or we can probably >> just poll for the mode change in all of the above conditions. > > The driver seems to be able to figure out the PHY clock in most cases > in dwc2_calc_frame_interval(). It doesn't seem to handle 6MHz there, > though. I wonder if that's another bug? > > Polling seems fine too, though. > > >> Are you able to continue looking into this? If not, I can take it up. > > I'm pretty much out of time at this point and it sounds like you've > though through all of the corner cases. If you can take it up that > would be wonderful! :) I'm happy to give the patches a test, though! > :) > Ok sure. I'll try to get something testable within a few days. Regards, 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
Re: [linux-sunxi] Re: [PATCH] musb: sunxi-glue: Fix sunxi-musb driver not auto-loading
Hi,
On 22-03-16 21:12, Andreas Färber wrote:
Hi Hans,
Am 21.03.2016 um 23:13 schrieb Hans de Goede:
Add a missing MODULE_DEVICE_TABLE() line which was causing the
sunxi-musb glue to not auto-load when build as a module.
"built"
Thanks.
Should this get a Fixes: header for backporting to stable?
Actually while looking for the commit id, it looks like someone
else has already beat me to it, and this is already fixes
in Linus' tree, so self NACK for this one.
Regards,
Hans
Signed-off-by: Hans de Goede
---
drivers/usb/musb/sunxi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 4b472b8..9af9431 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -756,6 +756,7 @@ static const struct of_device_id sunxi_musb_match[] = {
{ .compatible = "allwinner,sun8i-a33-musb", },
{}
};
+MODULE_DEVICE_TABLE(of, sunxi_musb_match);
static struct platform_driver sunxi_musb_driver = {
.probe = sunxi_musb_probe,
Has anyone looked into my earlier report about the module filename not
having musb-? I did not see any response and haven't rechecked v4.5.0.
Regards,
Andreas
--
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 v1 1/2] usbip: modifications to event handler
Hi, The errors are cleared by patch 2/2. Should I complete in patch 1/2? // > -Original Message- > From: kbuild test robot [mailto:l...@intel.com] > Sent: Tuesday, March 22, 2016 5:21 PM > To: fx IWATA NOBUO > Cc: kbuild-...@01.org; valentina.mane...@gmail.com; > shuah...@samsung.com; gre...@linuxfoundation.org; > linux-usb@vger.kernel.org; fx IWATA NOBUO; fx MICHIMURA TADAO > Subject: Re: [PATCH v1 1/2] usbip: modifications to event handler > > Hi Nobuo, > > [auto build test ERROR on v4.5-rc7] > [also build test ERROR on next-20160321] [if your patch is applied to the > wrong git tree, please drop us a note to help improving the system] > > url: > https://github.com/0day-ci/linux/commits/Nobuo-Iwata/usbip-event-handl > er-as-one-thread/20160322-161050 > config: xtensa-allmodconfig (attached as .config) > reproduce: > wget > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/s > bin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=xtensa > > Note: the > linux-review/Nobuo-Iwata/usbip-event-handler-as-one-thread/20160322-16 > 1050 HEAD 0fefef78d1ef7de850d42e55d026b8c2a4091838 builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > >In file included from drivers/usb/usbip/stub_dev.c:25:0: >drivers/usb/usbip/stub_dev.c: In function 'stub_probe': > >> drivers/usb/usbip/stub_dev.c:391:27: error: 'struct usbip_device' has > no member named 'eh' > kthread_stop_put(sdev->ud.eh); > ^ >drivers/usb/usbip/usbip_common.h:292:16: note: in definition of macro > 'kthread_stop_put' > kthread_stop(k); \ >^ > >> drivers/usb/usbip/stub_dev.c:391:27: error: 'struct usbip_device' has > no member named 'eh' > kthread_stop_put(sdev->ud.eh); > ^ >drivers/usb/usbip/usbip_common.h:293:19: note: in definition of macro > 'kthread_stop_put' > put_task_struct(k); \ > ^ >drivers/usb/usbip/stub_dev.c: In function 'stub_disconnect': >drivers/usb/usbip/stub_dev.c:452:26: error: 'struct usbip_device' has > no member named 'eh' > if (busid_priv->sdev->ud.eh == current) > ^ > > vim +391 drivers/usb/usbip/stub_dev.c > > 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 > 385 err_files: > 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 > 386 usb_hub_release_port(udev->parent, udev->portnum, > 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 > 387(struct usb_dev_state *) udev); > 3ff67445 drivers/usb/usbip/stub_dev.c Alexey Khoroshilov 2014-11-29 > 388 err_port: > b7945b77 drivers/staging/usbip/stub_dev.c Valentina Manea2014-01-23 > 389 dev_set_drvdata(&udev->dev, NULL); > 695bcb1c drivers/staging/usbip/stub_dev.c Harvey Yang2012-11-06 > 390 usb_put_dev(udev); > 695bcb1c drivers/staging/usbip/stub_dev.c Harvey Yang2012-11-06 > @391 kthread_stop_put(sdev->ud.eh); > 2d8f4595 drivers/staging/usbip/stub_dev.c Max Vozeler2011-01-12 > 392 > aa5873e9 drivers/staging/usbip/stub_dev.c Endre Kollar 2010-07-27 > 393 busid_priv->sdev = NULL; > 47c7157f drivers/staging/usbip/stub_dev.c Kulikov Vasiliy2010-07-12 > 394 stub_device_free(sdev); > > :: The code at line 391 was first introduced by commit > :: 695bcb1c0a50e8fe04f0ab868cac849130788bc0 staging: usbip: put > usb_device and kill event handler thread in error cleanups. > > :: TO: Harvey Yang > :: CC: Greg Kroah-Hartman > > --- > 0-DAY kernel test infrastructureOpen Source Technology > Center > https://lists.01.org/pipermail/kbuild-all Intel > Corporation -- 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 v1 1/2] usbip: modifications to event handler
On Tue, Mar 22, 2016 at 11:45:16PM +, fx IWATA NOBUO wrote: > Hi, > > The errors are cleared by patch 2/2. That's not ok, you can not have one patch that adds errors and the second fixes it. Each has to have no problems on their own. > Should I complete in patch 1/2? You need to fix patch 1/2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
On Tue, Mar 22, 2016 at 12:32 AM, Jaret Cantu wrote:
> The TX settings can be calibrated for particular hardware. The
> phy is reset by Linux, so this cannot be handled by the bootloader.
>
> The TRM mentions that the maximum resistance should be used for the
> DN/DP calibration in order to pass USB certification.
>
> The values for the TX registers are poorly described in the TRM.
> The meanings of the register values were taken from another
> Freescale-provided document:
> https://community.freescale.com/message/566147#comment-566912
>
I am checking with IC team about this value, and if this value can be
adapted for all SoCs
which use this PHY. Felipe, please hold on queue this patch, thanks.
Peter
> Signed-off-by: Jaret Cantu
> ---
> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>
> v2. Copying devicetree list
> Removed prettifying extra whitespace
> Removed unnecessary register rewrite on resume
> Use min and max constants for clarity
>
> .../devicetree/bindings/phy/mxs-usb-phy.txt| 10
> drivers/usb/phy/phy-mxs-usb.c | 58
>
> 2 files changed, 68 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> index 379b84a..1d25b04 100644
> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> @@ -12,6 +12,16 @@ Required properties:
> - interrupts: Should contain phy interrupt
> - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>
> +Optional properties:
> +- fsl,tx-cal-45-dn-ohms: Integer [30-55]. Resistance (in ohms) of switchable
> + high-speed trimming resistor connected in parallel with the 45 ohm resistor
> + that terminates the DN output signal. Default: 45
> +- fsl,tx-cal-45-dp-ohms: Integer [30-55]. Resistance (in ohms) of switchable
> + high-speed trimming resistor connected in parallel with the 45 ohm resistor
> + that terminates the DP output signal. Default: 45
> +- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
> + the 17.78mA TX reference current. Default: 100
> +
> Example:
> usbphy1: usbphy@020c9000 {
> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index 00bfea0..6c6b12c 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -27,6 +27,7 @@
> #define DRIVER_NAME "mxs_phy"
>
> #define HW_USBPHY_PWD 0x00
> +#define HW_USBPHY_TX 0x10
> #define HW_USBPHY_CTRL 0x30
> #define HW_USBPHY_CTRL_SET 0x34
> #define HW_USBPHY_CTRL_CLR 0x38
> @@ -38,6 +39,10 @@
> #define HW_USBPHY_IP_SET 0x94
> #define HW_USBPHY_IP_CLR 0x98
>
> +#define GM_USBPHY_TX_TXCAL45DP(x)(((x) & 0xf) << 16)
> +#define GM_USBPHY_TX_TXCAL45DN(x)(((x) & 0xf) << 8)
> +#define GM_USBPHY_TX_D_CAL(x)(((x) & 0xf) << 0)
> +
> #define BM_USBPHY_CTRL_SFTRST BIT(31)
> #define BM_USBPHY_CTRL_CLKGATE BIT(30)
> #define BM_USBPHY_CTRL_OTG_ID_VALUEBIT(27)
> @@ -115,6 +120,12 @@
> */
> #define MXS_PHY_NEED_IP_FIXBIT(3)
>
> +/* Minimum and maximum values for device tree entries */
> +#define MXS_PHY_TX_CAL45_MIN 30
> +#define MXS_PHY_TX_CAL45_MAX 55
> +#define MXS_PHY_TX_D_CAL_MIN 79
> +#define MXS_PHY_TX_D_CAL_MAX 119
> +
> struct mxs_phy_data {
> unsigned int flags;
> };
> @@ -164,6 +175,8 @@ struct mxs_phy {
> const struct mxs_phy_data *data;
> struct regmap *regmap_anatop;
> int port_id;
> + u32 tx_reg_set;
> + u32 tx_reg_mask;
> };
>
> static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> @@ -185,6 +198,20 @@ static void mxs_phy_clock_switch_delay(void)
> usleep_range(300, 400);
> }
>
> +static void mxs_phy_tx_init(struct mxs_phy *mxs_phy)
> +{
> + void __iomem *base = mxs_phy->phy.io_priv;
> + u32 phytx;
> +
> + /* Update TX register if there is anything to write */
> + if (mxs_phy->tx_reg_mask) {
> + phytx = readl(base + HW_USBPHY_TX);
> + phytx &= ~mxs_phy->tx_reg_mask;
> + phytx |= mxs_phy->tx_reg_set;
> + writel(phytx, base + HW_USBPHY_TX);
> + }
> +}
> +
> static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> {
> int ret;
> @@ -214,6 +241,8 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
> writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
>
> + mxs_phy_tx_init(mxs_phy);
> +
> return 0;
> }
>
> @@ -459,6 +488,7 @@ static int mxs_p
RE: [PATCH v1 1/2] usbip: modifications to event handler
Hello Greg, > adds errors and the second fixes it. I divided patches as 1/2 event handler itself 2/2 programs use event handler. So, programs use event handler which will be modified by 2/2 has errors at 1/2. > You need to fix patch 1/2 Could I send as one patch? Thank you, Nobuo // > -Original Message- > From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org] > Sent: Wednesday, March 23, 2016 10:40 AM > To: fx IWATA NOBUO > Cc: kbuild test robot; kbuild-...@01.org; valentina.mane...@gmail.com; > shuah...@samsung.com; linux-usb@vger.kernel.org; fx MICHIMURA TADAO > Subject: Re: [PATCH v1 1/2] usbip: modifications to event handler > > On Tue, Mar 22, 2016 at 11:45:16PM +, fx IWATA NOBUO wrote: > > Hi, > > > > The errors are cleared by patch 2/2. > > That's not ok, you can not have one patch that adds errors and the second > fixes it. Each has to have no problems on their own. > > > Should I complete in patch 1/2? > > You need to fix patch 1/2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/2] usbip: modifications to event handler
On Wed, Mar 23, 2016 at 01:54:21AM +, fx IWATA NOBUO wrote: > Hello Greg, > > > adds errors and the second fixes it. > > I divided patches as > 1/2 event handler itself > 2/2 programs use event handler. > > So, programs use event handler which will be modified by 2/2 has errors at > 1/2. That's a problem, we can not accept patches like this. > > You need to fix patch 1/2 > > Could I send as one patch? Maybe, it's your decision on how to do this best. 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: 答复: 答复: 答复: 【xhci】suspend and resume core dump problem
On 03/22/2016 08:09 PM, Lipengcheng wrote: >> >diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >> >index fbf75e5..406c872 100644 >> >--- a/drivers/usb/host/xhci-mem.c >> >+++ b/drivers/usb/host/xhci-mem.c >> >@@ -1856,6 +1856,7 @@ no_bw: >> > kfree(xhci->usb3_ports); >> > kfree(xhci->port_array); >> > kfree(xhci->rh_bw); >> >+ xhci->rh_bw = NULL; >> > >> > xhci->page_size = 0; >> > xhci->page_shift = 0; > We used to try to do it before, but it still has the same problem. > Hi Mathias, It looks like a wild pointer exists after xhci_mem_cleanup() is called. Let me try to reproduce it and seek for a fix. Best regards, Baolu -- 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 v1 1/2] usbip: modifications to event handler
> > So, programs use event handler which will be modified by 2/2 has errors > at 1/2. > > That's a problem, we can not accept patches like this. > > Could I send as one patch? > > Maybe, it's your decision on how to do this best. OK. Thank you for your help, n.iwata // > -Original Message- > From: linux-usb-ow...@vger.kernel.org > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of > gre...@linuxfoundation.org > Sent: Wednesday, March 23, 2016 11:15 AM > To: fx IWATA NOBUO > Cc: kbuild test robot; kbuild-...@01.org; valentina.mane...@gmail.com; > shuah...@samsung.com; linux-usb@vger.kernel.org; fx MICHIMURA TADAO > Subject: Re: [PATCH v1 1/2] usbip: modifications to event handler > > On Wed, Mar 23, 2016 at 01:54:21AM +, fx IWATA NOBUO wrote: > > Hello Greg, > > > > > adds errors and the second fixes it. > > > > I divided patches as > > 1/2 event handler itself > > 2/2 programs use event handler. > > > > So, programs use event handler which will be modified by 2/2 has errors > at 1/2. > > That's a problem, we can not accept patches like this. > > > > You need to fix patch 1/2 > > > > Could I send as one patch? > > Maybe, it's your decision on how to do this best. > > 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 -- 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: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
> -Original Message-
> From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com]
> Sent: Tuesday, March 22, 2016 5:36 PM
> To: Rajesh Bhagat
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; Sriram Dash
> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
> commmand timeout
>
> On 22.03.2016 07:19, Rajesh Bhagat wrote:
> >
> >
> >> -Original Message-
> >> From: Mathias Nyman [mailto:mathias.ny...@intel.com]
> >> Sent: Monday, March 21, 2016 2:46 PM
> >> To: Rajesh Bhagat ; Mathias Nyman
> >> ; linux-usb@vger.kernel.org; linux-
> >> ker...@vger.kernel.org
> >> Cc: gre...@linuxfoundation.org; Sriram Dash
> >> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation
> >> due to XHCI commmand timeout
> >>
> >> On 21.03.2016 06:18, Rajesh Bhagat wrote:
> >>>
> >>>
>
> Hi
>
> I think clearing the whole command ring is a bit too much in this case.
> It may cause issues for all attached devices when one command times out.
>
> >>>
> >>> Hi Mathias,
> >>>
> >>> I understand your point, But I want to understand how would
> >>> completion handler be called if a command is timed out and
> >>> xhci_abort_cmd_ring is successful. In this case all the code would be
> >>> waiting on
> completion handler forever.
> >>>
> >>>
> >>> 2. xhci_handle_command_timeout -> xhci_abort_cmd_ring(failure) ->
> >>> xhci_cleanup_command_queue -> xhci_complete_del_and_free_cmd
> >>>
> >>> In our case command is timed out, Hence we hit the case #2 but
> >>> xhci_abort_cmd_ring is success which does not calls complete.
> >>
> >> xhci_abort_cmd_ring() will write CA bit (CMD_RING_ABORT) to CRCR register.
> >> This will generate a command completion event with status "command
> >> aborted" for the pending command.
> >> This event is then followed by a "command ring stopped" command completion
> event.
> >>
> >> See xHCI specs 5.4.5 and 4.6.1.2
> >>
> >> handle_cmd_completion() will check if cmd_comp_code ==
> >> COMP_CMD_ABORT, goto event_handled, and call
> >> xhci_complete_del_and_free_cmd(cmd, cmd_comp_code) for the aborted
> command.
> >>
> >> If xHCI already processed the aborted command, we might only get a
> >> command ring stopped event, in this case handle_cmd_completion() will
> >> call xhci_handle_stopped_cmd_ring(xhci, cmd), which will turn the
> >> commands that were tagged for "abort" that still remain on the command
> >> ring to
> NO-OP commands.
> >>
> >> The completion callback will be called for these NO-OP command later
> >> when we get a command completion event for them.
> >>
> >
> > Thanks Mathias for detailed explanation. Now I understand how
> > completion handler is supposed to be called in this scenario.
> >
> > But in our case, somehow we are not getting any event and
> > handle_cmd_completion function is not getting called even after successful
> xhci_abort_cmd_ring when command timed out.
> >
> > Now, my point here is code prior to this patch xhci: rework command
> > timeout and cancellation, Code would have returned in case command timed
> > out in
> xhci_alloc_dev itself.
> >
> > - /* XXX: how much time for xHC slot assignment? */
> > - timeleft = wait_for_completion_interruptible_timeout(
> > - command->completion,
> > - XHCI_CMD_DEFAULT_TIMEOUT);
> > - if (timeleft <= 0) {
> > - xhci_warn(xhci, "%s while waiting for a slot\n",
> > - timeleft == 0 ? "Timeout" : "Signal");
> > - /* cancel the enable slot request */
> > - ret = xhci_cancel_cmd(xhci, NULL, command->command_trb);
> > - return ret;
> > - }
> > + wait_for_completion(command->completion);
> >
> > But after this patch, we are waiting for hardware event, which is
> > somehow not generated and causing a hang scenario.
> >
> > IMO, The assumption that "xhci_abort_cmd_ring would always generate an
> > event and handle_cmd_completion would be called" will not be always be true
> > if HW
> is in bad state.
> >
> > Please share your opinion.
> >
>
> writing the CA (command abort) bit in CRCR (command ring control register)
> will stop
> the command ring, and CRR (command ring running) will be set to 0 by xHC.
> xhci_abort_cmd_ring() polls this bit up to 5 seconds.
> If it's not 0 then the driver considers the command abort as failed.
>
> The scenario you're thinking of is that xHC would still react to CA bit set,
> it would stop
> the command ring and set CRR 0, but not send a command completion event.
>
> Have you tried adding some debug to handle_cmd_completion() and see if you
> receive
> any event after command abortion?
>
Yes. We have added debug prints at first line of handle_cmd_completion, and we
are not getting
those prints. The last print messages that we get are as below from
xhci_alloc_dev while resume
operation:
xhci-hcd xhci-hcd.0.auto: Com
Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
> The TX settings can be calibrated for particular hardware. The
> phy is reset by Linux, so this cannot be handled by the bootloader.
>
> The TRM mentions that the maximum resistance should be used for the
> DN/DP calibration in order to pass USB certification.
>
> The values for the TX registers are poorly described in the TRM.
> The meanings of the register values were taken from another
> Freescale-provided document:
> https://community.freescale.com/message/566147#comment-566912
>
> Signed-off-by: Jaret Cantu
> ---
> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>
> v2. Copying devicetree list
> Removed prettifying extra whitespace
> Removed unnecessary register rewrite on resume
> Use min and max constants for clarity
>
> .../devicetree/bindings/phy/mxs-usb-phy.txt| 10
> drivers/usb/phy/phy-mxs-usb.c | 58
>
> 2 files changed, 68 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> index 379b84a..1d25b04 100644
> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
> @@ -12,6 +12,16 @@ Required properties:
> - interrupts: Should contain phy interrupt
> - fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
>
> +
> + if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
> + val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
> + /* scale to 4-bit value */
> + val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
> + / (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
> + mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
> + mxs_phy->tx_reg_set |= GM_USBPHY_TX_D_CAL(val);
> + }
> +
I have tested "tx-d-cal", but it seems incorrect according to the xls you
have provided, would you please check it again or am I wrong?
dts changes:
+&usbphy1 {
+ fsl,tx-d-cal = <109>;
+};
+
+&usbphy2 {
+ fsl,tx-d-cal = <106>;
+};
+
The phy1's tx-d-cal is 0x3, and phy2's tx-d-cal is 0x4 after PHY
initialization,
but according to xls, it should be 0x4 and 0x5.
--
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
