[PATCH] uas: replace WARN_ON_ONCE() with assert_spin_locked().
spin_is_locked() always return false in uniprocessor configuration and
therefore it
would be advise to repalce with assert_spin_locked().
Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
---
drivers/usb/storage/uas.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..8e5877d 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo,
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
uas_log_cmd_state(cmnd, caller);
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
WARN_ON_ONCE(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
@@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
struct uas_dev_info *devinfo = cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
cmdinfo-state |= IS_IN_WORK_LIST;
schedule_work(devinfo-work);
}
@@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT |
@@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct urb *urb;
int err;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
if (cmdinfo-state SUBMIT_STATUS_URB) {
urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo-stream);
if (!urb)
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: driver option create ttyUSB
On Thu, 2014-08-07 at 20:31 -0700, Greg KH wrote: On Fri, Aug 08, 2014 at 10:56:57AM +0800, w wrote: If use symlinks it can work well. But why program in userspace can prevent driver release the node? The program just open the node, nothing else. If it holds the node open, of course the kernel can't release it, the resource is still in use. That's the way it is supposed to be, don't you agree? Actually on second thought, why? While the device is open there's an obvious reason to not throw away an fd and its associated baggage. But why the device node? That would be necessary only if you were to use the number as an index. 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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration Really? On what processor type? I don't see that being the case on a few processors, but I didn't check them all, or I might be totally confused with all of the arch_spin_is_locked() definitions in the tree. and therefore it would be advise to repalce with assert_spin_locked(). This implies that all uses of this function is wrong and should be replaced and removed, right? 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: Re: driver option create ttyUSB
On Mon, 2014-08-11 at 16:22 +0800, Greg KH wrote: On Mon, Aug 11, 2014 at 09:10:35AM +0200, Oliver Neukum wrote: On Thu, 2014-08-07 at 20:31 -0700, Greg KH wrote: On Fri, Aug 08, 2014 at 10:56:57AM +0800, w wrote: If use symlinks it can work well. But why program in userspace can prevent driver release the node? The program just open the node, nothing else. If it holds the node open, of course the kernel can't release it, the resource is still in use. That's the way it is supposed to be, don't you agree? Actually on second thought, why? While the device is open there's an obvious reason to not throw away an fd and its associated baggage. But why the device node? Because it's easier that way for the kernel code :) I doubt it. It is more obvious, but it isn't easier. Generically removing the possibility of an attempt to open a disconnected device or do any operation on it should make device drivers simpler. That would be necessary only if you were to use the number as an index. It shouldn't be, so we can change this in the driver core if it's really an issue. So far, no one has objected to it. I seem to recall quite some complaints. 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: ar71xx: Problems with USB WIFI dongles
Hi all, On Sat, Aug 9, 2014 at 12:03 PM, Kristian Evensen kristian.even...@gmail.com wrote: 2) When disconnecting any WIFI dongle, I always get a kernel oops. This happens irrespective of if the dongle is active (for example connected to network) or if I have just connected it to the router. The kernel oops looks like the following (the crash happens at the same place independent of device): [ 429.73] usb 1-1.1.5.4: USB disconnect, device number 10 [ 429.76] CPU 0 Unable to handle kernel paging request at virtual address 00100104, epc == 86e93018, ra == 86e93010 [ 429.77] Oops[#1]: [ 429.77] CPU: 0 PID: 292 Comm: khubd Not tainted 3.14.12 #4 [ 429.77] task: 878ad770 ti: 87b0c000 task.ti: 87b0c000 [ 429.77] $ 0 : 00200200 00100100 [ 429.77] $ 4 : 87b0dce0 87b0dee0 8035 0008 [ 429.77] $ 8 : 00090040 86a6a080 0020 00090040 [ 429.77] $12 : 0013 000e 0007 0001 [ 429.77] $16 : 85cc91a0 87b0dcd8 00100100 85cc91a8 [ 429.77] $20 : 85cc91a0 00200200 86948000 87b41838 [ 429.77] $24 : 80327df0 8010fe04 [ 429.77] $28 : 87b0c000 87b0dcc8 86e93010 [ 429.77] Hi: 0350 [ 429.77] Lo: 0006 [ 429.77] epc : 86e93018 ieee80211_remove_interfaces+0x128/0x1b4 [mac80211] [ 429.77] Not tainted [ 429.77] ra: 86e93010 ieee80211_remove_interfaces+0x120/0x1b4 [mac80211] [ 429.77] Status: 1100dc03 KERNEL EXL IE [ 429.77] Cause : 008c [ 429.77] BadVA : 00100104 [ 429.77] PrId : 0001974c (MIPS 74Kc) [ 429.77] Modules linked in: ath9k rtl8192se rtl8192de rtl8192cu rtl8192ce ath9k_htc ath9k_common rtl_usb rtl_pci pppoe ppp_async option iptable_nat ath9k_hw ath usb_wwan smsc95xx rtlwifi rtl8187 rt73usb rt2x00usb rt2x00lib rndis_host qmi_wwan pppox ppp_generic nf_nat_ipv4 nf_conntrack_ipv4 mac80211 ipt_MASQUERADE huawei_cdc_ncm cfg80211 cdc_ncm cdc_ether ax88179_178a asix xt_time xt_tcpudp xt_tcpmss xt_string xt_statistic xt_state xt_recent xt_quota xt_pkttype xt_owner xt_nfacct xt_nat xt_multiport xt_mark xt_mac xt_limit xt_length xt_id xt_hl xt_helper xt_ecn xt_dscp xt_conntrack xt_connmark xt_connlimit xt_connbytes xt_comment xt_addrtype xt_TCPMSS xt_REDIRECT xt_NFQUEUE xt_LOG xt_HL xt_DSCP xt_CT xt_CLASSIFY usbserial usbnet ts_kmp ts_fsm ts_bm slhc rtl8192c_common nfnetlink_queue nfnetlink_acct nf_nat_irc nf_nat_ftp nf_nat nf_defrag_ipv4 nf_conntrack_irc nf_conntrack_ftp iptable_raw iptable_mangle iptable_filter ipt_REJECT ipt_ECN ipheth ip_tables crc16 crc_itu_t crc_ccitt compat cdc_wdm cdc_acm act_connmark act_skbedit act_mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route cls_fw sch_hfsc sch_ingress ledtrig_usbdev ip6t_REJECT ip6table_raw ip6table_mangle ip6table_filter ip6_tables x_tables nf_conntrack_ipv6 nf_defrag_ipv6 ifb ipv6 eeprom_93cx6 arc4 crypto_blkcipher ehci_platform ehci_hcd gpio_button_hotplug usbcore nls_base usb_common mii [ 429.77] Process khubd (pid: 292, threadinfo=87b0c000, task=878ad770, tls=) [ 429.77] Stack : 85cc93a0 86f69350 87b0dcd8 87b0dcd8 00100100 00200200 [ 429.77] 86948000 85cc8ae0 85cc9460 86b9fa20 86f69350 86e80fac [ 429.77] 0004 85cc8ae0 85cc9460 86b9fa20 85cc8ae0 86e14ddc 87b0dd1c 87b0bbd0 [ 429.77] 86b9fa20 86948000 86b9fa20 86948000 86b9fa00 87b2c580 879ff000 800def9c [ 429.77] 87b39180 80131108 86b9fa20 86f69350 87b39180 879ff000 86b9fa00 8008d734 [ 429.77] ... [ 429.77] Call Trace: [ 429.77] [86e93018] ieee80211_remove_interfaces+0x128/0x1b4 [mac80211] [ 429.77] [86e80fac] ieee80211_unregister_hw+0x3c/0xe4 [mac80211] [ 429.77] [86e14ddc] rtl_usb_disconnect+0x4c/0xf4 [rtl_usb] [ 429.77] [87b2c580] usb_deregister+0x228/0x2ec [usbcore] [ 429.77] [8008d734] __device_release_driver+0x6c/0xd0 [ 429.77] [80117f64] device_release_driver+0x28/0x40 [ 429.77] [800edffc] bus_remove_device+0xec/0x120 [ 429.77] [801176fc] device_del+0x110/0x170 [ 429.77] [87b2ad58] usb_disable_device+0xb0/0x1d8 [usbcore] [ 429.77] [87b22920] usb_disconnect+0xac/0x408 [usbcore] [ 429.77] [87b24a68] usb_reset_device+0xd28/0x15c8 [usbcore] [ 429.77] [80121c0c] do_exit+0x72c/0x744 [ 429.77] [ 429.77] [ 429.77] Code: 27a40018 8fa2001c 8fa30018 ac620004 ac43 8fb00010 3c030010 3c020020 24630100 [ 430.11] ---[ end trace 6ca2573cb8fc44d5 ]--- I did some more looking into and testing of this kernel panic and it is fixed by the following commit https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/net/mac80211/iface.c?id=87757a917b0b3c0787e0563c679762152be81312. This commit is included in 3.10-stable and the latest compat-wireless, but not in the current compat-wireless (22-5-2014) used by OpenWRT. So the problem is already solved, sorry
RE: [PATCH] usb: xhci: Fix Set TR Dequeue Pointer cycle state for quirky xHCs
It's starting to get a bit too complicated. xhci find_new_dequeue_state() now has four places where it can toggle the cycle bit in addition to the cycle toggle in find_trb_seg(). In the end we really just want to do it max once. I'll see if I can simplify the whole cycle bit code somehow. Have a look at the patches I submitted last year. I remember simplifying a lot of that code. David
RE: [RFTv2 PATCH] xhci: rework cycle bit checking for new dequeue pointers
(Also, I'm wondering if we should just drop the code that skips whole segments to make it simpler, since I can hardly think of any real-world examples where a single TD would cover a whole segment.) That can happen, raw access to USB disks can generate very long fragment lists. OTOH simplifying the code is probably a good idea. David
[PATCH] usb: gadget/uvc: remove unused DRIVER_VERSION
The define is unused and can be thrown into the garbage bag of other unused defines. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/gadget/uvc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index 49d9573..5ae5c5e 100644 --- a/drivers/usb/gadget/uvc.h +++ b/drivers/usb/gadget/uvc.h @@ -96,7 +96,6 @@ extern unsigned int uvc_gadget_trace_param; * Driver specific constants */ -#define DRIVER_VERSION 0.1.0 #define DRIVER_VERSION_NUMBER KERNEL_VERSION(0, 1, 0) #define UVC_NUM_REQUESTS 16 -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget/uvc: remove unused DRIVER_VERSION
The define is unused and can be thrown into the garbage bag of other unused defines. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/gadget/uvc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index 49d9573..5ae5c5e 100644 --- a/drivers/usb/gadget/uvc.h +++ b/drivers/usb/gadget/uvc.h @@ -96,7 +96,6 @@ extern unsigned int uvc_gadget_trace_param; * Driver specific constants */ -#define DRIVER_VERSION 0.1.0 #define DRIVER_VERSION_NUMBER KERNEL_VERSION(0, 1, 0) #define UVC_NUM_REQUESTS 16 -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget/uvc: remove DRIVER_VERSION{,_NUMBER}
As the driver is mainline we can remove the version numbers. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/gadget/uvc.h | 3 --- drivers/usb/gadget/uvc_v4l2.c | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index 49d9573..e593a10 100644 --- a/drivers/usb/gadget/uvc.h +++ b/drivers/usb/gadget/uvc.h @@ -96,9 +96,6 @@ extern unsigned int uvc_gadget_trace_param; * Driver specific constants */ -#define DRIVER_VERSION 0.1.0 -#define DRIVER_VERSION_NUMBER KERNEL_VERSION(0, 1, 0) - #define UVC_NUM_REQUESTS 16 #define UVC_MAX_REQUEST_SIZE 64 #define UVC_MAX_EVENTS 4 diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index 58633bf..65af2f8 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -194,7 +194,6 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg) strlcpy(cap-card, cdev-gadget-name, sizeof(cap-card)); strlcpy(cap-bus_info, dev_name(cdev-gadget-dev), sizeof cap-bus_info); - cap-version = DRIVER_VERSION_NUMBER; cap-capabilities = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING; break; } -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/6] usb: core: TPL should apply for both OTG and EH
/**
* usb_enumerate_device_otg - FIXME (usbcore-internal) @@ -2261,21
+2259,6 @@ static int usb_enumerate_device_otg(struct usb_device *udev)
}
}
}
-
- if (!is_targeted(udev)) {
-
- /* Maybe it can talk to us, though we can't talk to it.
-* (Includes HNP test device.)
-*/
- if (udev-bus-b_hnp_enable || udev-bus-is_b_host) {
- err = usb_port_suspend(udev, PMSG_SUSPEND);
- if (err 0)
- dev_dbg(udev-dev, HNP fail, %d\n, err);
- }
- err = -ENOTSUPP;
If I did not miss anything then I see an issue if above hunk is removed.
Suppose, there is an OTG B device which is not supported by host, then
the above hunk was suspending the bus so that B device can get a chance
to become host. By removing above hunk, how do we cater this scenario?
~Pratyush
PS: All other changes of the series seems fine to me.
Thanks, Pratyush, I will change the code like below to cover this case.
+ if (IS_ENABLED(CONFIG_USB_OTG_WHITELIST) hcd-tpl_support
+ !is_targeted(udev) IS_ENABLED(CONFIG_USB_OTG)) {
+ /* Maybe it can talk to us, though we can't talk to it.
+* (Includes HNP test device.)
+*/
+ if (udev-bus-b_hnp_enable || udev-bus-is_b_host) {
+ err = usb_port_suspend(udev, PMSG_AUTO_SUSPEND);
+ if (err 0)
+ dev_dbg(udev-dev, HNP fail, %d\n, err);
+ }
+ return -ENOTSUPP;
+ }
+
Peter
- goto fail;
- }
-fail:
#endif
return err;
}
@@ -2319,6 +2302,9 @@ static int usb_enumerate_device(struct usb_device
*udev)
if (err 0)
return err;
+ if (!is_targeted(udev))
+ return -ENOTSUPP;
+
usb_detect_interface_quirks(udev);
return 0;
diff --git a/drivers/usb/core/otg_whitelist.h
b/drivers/usb/core/otg_whitelist.h
index e8cdce5..4d6a134 100644
--- a/drivers/usb/core/otg_whitelist.h
+++ b/drivers/usb/core/otg_whitelist.h
@@ -10,8 +10,8 @@
*/
/*
- * This OTG Whitelist is the OTG Targeted Peripheral List. It
should
- * mostly use of USB_DEVICE() or USB_DEVICE_VER() entries..
+ * This OTG and Embedded Host Whitelist is Targeted Peripheral List.
+ * It should mostly use of USB_DEVICE() or USB_DEVICE_VER() entries..
*
* YOU _SHOULD_ CHANGE THIS LIST TO MATCH YOUR PRODUCT AND ITS TESTING!
*/
@@ -49,9 +49,9 @@ static struct usb_device_id whitelist_table [] = {
static int is_targeted(struct usb_device *dev) {
struct usb_device_id*id = whitelist_table;
+ struct usb_hcd *hcd = bus_to_hcd(dev-bus);
- /* possible in developer configs only! */
- if (!dev-bus-otg_port)
+ if (!IS_ENABLED(CONFIG_USB_OTG_WHITELIST) || !hcd-tpl_support)
return 1;
/* HNP test device is _never_ targeted (see OTG spec 6.6.6) */ @@
-99,14 +99,12 @@ static int is_targeted(struct usb_device *dev)
/* add other match criteria here ... */
- /* OTG MESSAGE: report errors here, customize to match your product
*/
- dev_err(dev-dev, device v%04x p%04x is not supported\n,
- le16_to_cpu(dev-descriptor.idVendor),
- le16_to_cpu(dev-descriptor.idProduct));
-#ifdef CONFIG_USB_OTG_WHITELIST
+ /*
+* Required error message for OTG EH compliance test,
+* customize it to match your product
+*/
+ dev_err(dev-dev, unsupported device\n);
+
return 0;
-#else
- return 1;
-#endif
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb
in the body of a message to majord...@vger.kernel.org More majordomo
info at http://vger.kernel.org/majordomo-info.html
--
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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
Hi Greg, Please find my comment in line. -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Monday, August 11, 2014 1:58 PM To: Sharma, Sanjeev Cc: hdego...@redhat.com; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; linux-s...@vger.kernel.org Subject: Re: [PATCH] uas: replace WARN_ON_ONCE() with assert_spin_locked(). On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration Really? On what processor type? I don't see that being the case on a few processors, but I didn't check them all, or I might be totally confused with all of the arch_spin_is_locked() definitions in the tree. specially in case of mips and powerpc but we can configure our kernel to uniprocessor configuration for specific processor and in that case we should be extra cautious. Please check spinlock_up.h and therefore it would be advise to repalce with assert_spin_locked(). This implies that all uses of this function is wrong and should be replaced and removed, right? Yes and there are only few places in driver for which I am submitting patches which need to be change and at other places it has been already taken care. Please have an look into the Old discussion for more details. http://linux-kernel.2935.n7.nabble.com/Is-spin-is-locked-safe-to-use-with-BUG-ON-WARN-ON-td654800.html#a921802 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: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
Hi Peter, On Mon, Aug 11, 2014 at 12:56:47AM +, Peter Chen wrote: I have seen this call is still returning EOPNOTSUPP which is also returned by usb_gadget_connect and usb_gadget_disconnect if the pullup function is not available. Should it not handle that case somehow special? Is it still a valid condition to bailout if vbus is not active? Hi Michael, I did this for possible pullup dp without connection (eg load gadget driver before vbus connect), it breaks USB spec, see 7.1.7.3 Connect and Disconnect Signaling at USB 2.0 spec. I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. I still work with the uvc gadget. The code in gadget/f_uvc.c calls usb_function_deactivate. The code of uvc_function_bind will bail out in the error code path, when the usb cable is not plugged (vbus not valid) in that situation. When the spec defines that calling pullup is only allowed on valid vbus, we should somehow tell something different then EOPNOTSUPP so the caller and that can bail out differently. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: gadget: serial: remove PREFIX macro
Hi,
this patch was lying around for some time...
are there any comments or objections on this?
regards,
richard
On Fri, 18 Jul 2014 11:39:46 +0200
Richard Leitner richard.leit...@skidata.com wrote:
Remove the ttyGS PREFIX macro from u_serial.c and replace all occurences with
the hardcoded ttyGS string.
This macro was mostly used in a few debug/warning messages and a lot of
hardcoded ttyGS existed beneath. It may have been used for renaming the
tty, but if done so most debug messages would have ignored this because
of the hardcoded strings.
Due to the fact the usage of this PREFIX instead of the hardcoded strings
in all debug calls would have resulted in a hard to read/grep code it
is removed completely.
Signed-off-by: Richard Leitner richard.leit...@skidata.com
---
note: previous discussion was at https://lkml.org/lkml/2014/6/27/193
---
drivers/usb/gadget/u_serial.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
index ad0aca8..7e7a6b0 100644
--- a/drivers/usb/gadget/u_serial.c
+++ b/drivers/usb/gadget/u_serial.c
@@ -55,11 +55,8 @@
* for a telephone or fax link. And ttyGS2 might be something that just
* needs a simple byte stream interface for some messaging protocol that
* is managed in userspace ... OBEX, PTP, and MTP have been mentioned.
- */
-
-#define PREFIX ttyGS
-
-/*
+ *
+ *
* gserial is the lifecycle interface, used by USB functions
* gs_port is the I/O nexus, used by the tty driver
* tty_struct links to the tty/filesystem framework
@@ -385,7 +382,7 @@ __acquires(port-port_lock)
list_del(req-list);
req-zero = (gs_buf_data_avail(port-port_write_buf) == 0);
- pr_vdebug(PREFIX %d: tx len=%d, 0x%02x 0x%02x 0x%02x ...\n,
+ pr_vdebug(ttyGS%d: tx len=%d, 0x%02x 0x%02x 0x%02x ...\n,
port-port_num, len, *((u8 *)req-buf),
*((u8 *)req-buf+1), *((u8 *)req-buf+2));
@@ -503,12 +500,12 @@ static void gs_rx_push(unsigned long _port)
switch (req-status) {
case -ESHUTDOWN:
disconnect = true;
- pr_vdebug(PREFIX %d: shutdown\n, port-port_num);
+ pr_vdebug(ttyGS%d: shutdown\n, port-port_num);
break;
default:
/* presumably a transient fault */
- pr_warning(PREFIX %d: unexpected RX status %d\n,
+ pr_warn(ttyGS%d: unexpected RX status %d\n,
port-port_num, req-status);
/* FALLTHROUGH */
case 0:
@@ -537,7 +534,7 @@ static void gs_rx_push(unsigned long _port)
if (count != size) {
/* stop pushing; TTY layer can't handle more */
port-n_read += count;
- pr_vdebug(PREFIX %d: rx block %d/%d\n,
+ pr_vdebug(ttyGS%d: rx block %d/%d\n,
port-port_num,
count, req-actual);
break;
@@ -569,7 +566,7 @@ static void gs_rx_push(unsigned long _port)
if (do_push)
tasklet_schedule(port-push);
else
- pr_warning(PREFIX %d: RX not scheduled?\n,
+ pr_warn(ttyGS%d: RX not scheduled?\n,
port-port_num);
}
}
@@ -985,7 +982,7 @@ static void gs_unthrottle(struct tty_struct *tty)
* read queue backs up enough we'll be NAKing OUT packets.
*/
tasklet_schedule(port-push);
- pr_vdebug(PREFIX %d: unthrottle\n, port-port_num);
+ pr_vdebug(ttyGS%d: unthrottle\n, port-port_num);
}
spin_unlock_irqrestore(port-port_lock, flags);
}
@@ -1295,7 +1292,7 @@ static int userial_init(void)
return -ENOMEM;
gs_tty_driver-driver_name = g_serial;
- gs_tty_driver-name = PREFIX;
+ gs_tty_driver-name = ttyGS;
/* uses dynamically assigned dev_t values */
gs_tty_driver-type = TTY_DRIVER_TYPE_SERIAL;
--
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: gadget/uvc: remove DRIVER_VERSION{,_NUMBER}
Hi, On Mon, Aug 11, 2014 at 01:15:06PM +0200, Michael Grzeschik wrote: As the driver is mainline we can remove the version numbers. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/gadget/uvc.h | 3 --- drivers/usb/gadget/uvc_v4l2.c | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index 49d9573..e593a10 100644 --- a/drivers/usb/gadget/uvc.h +++ b/drivers/usb/gadget/uvc.h @@ -96,9 +96,6 @@ extern unsigned int uvc_gadget_trace_param; * Driver specific constants */ -#define DRIVER_VERSION 0.1.0 -#define DRIVER_VERSION_NUMBERKERNEL_VERSION(0, 1, 0) - #define UVC_NUM_REQUESTS 16 #define UVC_MAX_REQUEST_SIZE 64 #define UVC_MAX_EVENTS 4 diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index 58633bf..65af2f8 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -194,7 +194,6 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg) strlcpy(cap-card, cdev-gadget-name, sizeof(cap-card)); strlcpy(cap-bus_info, dev_name(cdev-gadget-dev), sizeof cap-bus_info); - cap-version = DRIVER_VERSION_NUMBER; did you run v4l2-compliance ? IIRC it will fail if you don't provide cap-version. -- balbi signature.asc Description: Digital signature
RE: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, 11 Aug 2014, Peter Chen wrote: Hello Peter, I have seen this call is still returning EOPNOTSUPP which is also returned by usb_gadget_connect and usb_gadget_disconnect if the pullup function is not available. Should it not handle that case somehow special? Is it still a valid condition to bailout if vbus is not active? Hi Michael, I did this for possible pullup dp without connection (eg load gadget driver before vbus connect), it breaks USB spec, see 7.1.7.3 Connect and Disconnect Signaling at USB 2.0 spec. 7.1.7.3 doesn't say anything about enabling the D+ pullup before Vbus is active. The closest it comes is the definition of Delta-t2 (TSIGATT), but that is a _maximum_ time, not a _minimum_ time. What makes you think turning on the pullup when Vbus is off is bad? I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? 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 0/3] usbip: move usbip out of staging
On Fri, Aug 8, 2014 at 10:41 PM, Valentina Manea valentina.mane...@gmail.com wrote: On Thu, Aug 7, 2014 at 1:16 PM, Greg KH gre...@linuxfoundation.org wrote: The top of the MAINTAINERS should have this information, look in the S: Status section about what type of category you are in. Basically I'd like you to be the one to handle patches that are sent in for the code, just by reviewing them, you don't have to send them on to me if you don't want to (some usb subsystem maintainers do do this, like for usb-serial and xhci), it's up to you. Also, any bug reports or questions about the code would come to you first, along with the rest of the linux-usb@vger developers, you aren't alone in this at all. Ok, this sounds good. I suppose sending you the patches involves picking them up from the email, batching them and sending with send-email. Shuah, I would appreciate if you'd like to help with this. The more eyes for review, the better. Great. I am happy to help. Please add me to the MAINTAINERS entry for this driver. shuah...@samsung.com is the email address. thanks, -- Shuah -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, Aug 11, 2014 at 09:35:46AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Peter Chen wrote: Hello Peter, I have seen this call is still returning EOPNOTSUPP which is also returned by usb_gadget_connect and usb_gadget_disconnect if the pullup function is not available. Should it not handle that case somehow special? Is it still a valid condition to bailout if vbus is not active? Hi Michael, I did this for possible pullup dp without connection (eg load gadget driver before vbus connect), it breaks USB spec, see 7.1.7.3 Connect and Disconnect Signaling at USB 2.0 spec. 7.1.7.3 doesn't say anything about enabling the D+ pullup before Vbus is active. The closest it comes is the definition of Delta-t2 (TSIGATT), but that is a _maximum_ time, not a _minimum_ time. What makes you think turning on the pullup when Vbus is off is bad? I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. I'm not sure how this could be made to work in a composite gadget, though. What if there were two uvc functions? The pullup would have to remain disabled until both userspace components were ready. 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: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, Aug 11, 2014 at 09:44:05AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. that might be too late, although propbably not catastrophic. I'm not sure how this could be made to work in a composite gadget, though. What if there were two uvc functions? The pullup would have you can refcount the disables, right ? to remain disabled until both userspace components were ready. right -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget/uvc: remove DRIVER_VERSION{,_NUMBER}
Hi, On Mon, Aug 11, 2014 at 08:32:37AM -0500, Felipe Balbi wrote: Hi, On Mon, Aug 11, 2014 at 01:15:06PM +0200, Michael Grzeschik wrote: As the driver is mainline we can remove the version numbers. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/gadget/uvc.h | 3 --- drivers/usb/gadget/uvc_v4l2.c | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index 49d9573..e593a10 100644 --- a/drivers/usb/gadget/uvc.h +++ b/drivers/usb/gadget/uvc.h @@ -96,9 +96,6 @@ extern unsigned int uvc_gadget_trace_param; * Driver specific constants */ -#define DRIVER_VERSION 0.1.0 -#define DRIVER_VERSION_NUMBER KERNEL_VERSION(0, 1, 0) - #define UVC_NUM_REQUESTS 16 #define UVC_MAX_REQUEST_SIZE 64 #define UVC_MAX_EVENTS 4 diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index 58633bf..65af2f8 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -194,7 +194,6 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg) strlcpy(cap-card, cdev-gadget-name, sizeof(cap-card)); strlcpy(cap-bus_info, dev_name(cdev-gadget-dev), sizeof cap-bus_info); - cap-version = DRIVER_VERSION_NUMBER; did you run v4l2-compliance ? IIRC it will fail if you don't provide cap-version. True! Is it right to change it to LINUX_VERSION_CODE as v4l_querycap? I was unsure as checkpatch was barfing about using this. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: gadget: serial: remove PREFIX macro
On Mon, Aug 11, 2014 at 03:04:35PM +0200, Richard Leitner wrote: Hi, this patch was lying around for some time... are there any comments or objections on this? you realise we're still in the middle of the merge window, right ? -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, Aug 11, 2014 at 08:45:52AM -0500, Felipe Balbi wrote: On Mon, Aug 11, 2014 at 09:44:05AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. that might be too late, although propbably not catastrophic. The uvc gadget already does disable the udc on uvc_function_bind with usb_function_deactivate. I'm not sure how this could be made to work in a composite gadget, though. What if there were two uvc functions? The pullup would have you can refcount the disables, right ? The refcounting is handled by usb_function_deactivate. to remain disabled until both userspace components were ready. right It got enabled when the userspace opens the associated v4l2 video device. IMHO we have all parts to keep the gadget safe for working until the userspace is ready to work. Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: gadget/uvc: remove DRIVER_VERSION{,_NUMBER}
Hi, On Mon, Aug 11, 2014 at 03:50:08PM +0200, Michael Grzeschik wrote: As the driver is mainline we can remove the version numbers. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/gadget/uvc.h | 3 --- drivers/usb/gadget/uvc_v4l2.c | 1 - 2 files changed, 4 deletions(-) diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index 49d9573..e593a10 100644 --- a/drivers/usb/gadget/uvc.h +++ b/drivers/usb/gadget/uvc.h @@ -96,9 +96,6 @@ extern unsigned int uvc_gadget_trace_param; * Driver specific constants */ -#define DRIVER_VERSION 0.1.0 -#define DRIVER_VERSION_NUMBERKERNEL_VERSION(0, 1, 0) - #define UVC_NUM_REQUESTS 16 #define UVC_MAX_REQUEST_SIZE 64 #define UVC_MAX_EVENTS 4 diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index 58633bf..65af2f8 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -194,7 +194,6 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg) strlcpy(cap-card, cdev-gadget-name, sizeof(cap-card)); strlcpy(cap-bus_info, dev_name(cdev-gadget-dev), sizeof cap-bus_info); - cap-version = DRIVER_VERSION_NUMBER; did you run v4l2-compliance ? IIRC it will fail if you don't provide cap-version. True! Is it right to change it to LINUX_VERSION_CODE as v4l_querycap? I was unsure as checkpatch was barfing about using this. I'd ask Laurent :-) Laurent, any hints ? -- balbi signature.asc Description: Digital signature
Re: usb_gadget_state_work not triggering sysfs_notify
Ping!
On Mon, Jul 07, 2014 at 12:53:45PM +0200, Michael Grzeschik wrote:
I am currently working with an udc where the userspace should react on
plugging
and unplugging of the device. The layers used trigger the
usb_gadget_state_work
as expected, but the sysfs_notify never cross the userspace layer.
static void usb_gadget_state_work(struct work_struct *work)
{
struct usb_gadget *gadget = work_to_gadget(work);
sysfs_notify(gadget-dev.kobj, NULL, state);
}
When I create my own device attribute and call this function on it, everything
works fine and my test application reacts on the event.
I thing we mess with an regression where the attribute is not found in
sysfs_notify. I see that kernfs_find_and_get is returning NULL and therefor
never call kernfs_notify.
Does anybody know what could be the matter here?
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, 11 Aug 2014, Felipe Balbi wrote: On Mon, Aug 11, 2014 at 09:44:05AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. that might be too late, although propbably not catastrophic. It looks like the UDC core needs to be improved a little. Right now, udc_bind_to_driver() unconditionally calls usb_gadget_connect(), thereby overriding any calls the gadget driver may have made during its bind callback. What we should do instead is call usb_gadget_connect() only if the gadget driver didn't try to change the pullup setting. I'm not sure how this could be made to work in a composite gadget, though. What if there were two uvc functions? The pullup would have you can refcount the disables, right ? Not according to the kerneldoc in gadget.h. 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: XHCI_TRUST_TX_LENGTH quirk?
On 08/11/2014 02:15 PM, David Laight wrote: From: Behalf Of Evan Langlois Sorry if this is answered somewhere, but I couldn't find anything specific on Google as far as how to determine if this quirk is needed or if its configurable at boot/run-time. Its an HP laptop, and it looks like other HP models have the problem. I most often get this when turning on a USB TV Tuner, which is also HP branded but shows up as a Hauppage device. I get glitches on the TV output, less with tvtime than other viewers, and these glitches go away if I transcode a DVD while watching TV .. yeah, I know that makes no sense. Not sure if the glitches are related to the dmesg warnings. I'll attach the lspci to the message in case anyone needs it. Please advise on the next step to proceed. I'm running a Ubuntu kernel, but I imagine a stock kernel would react the same. uname -a ... Linux Taro 3.13.0-32-lowlatency #57-Ubuntu SMP PREEMPT Tue Jul 15 04:08:59 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux dmesg output (all the repeats deleted) ... [270905.416782] xhci_hcd :00:10.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? Given that the code always checks the receive transfer length (in order to output the message), maybe the code should be refactored to always use the receive transfer length and the quirk removed. I doubt there are any controllers that write a non-zero 'residual' byte count at the end of a full transfer. I think that sounds like an idea. I got a quirk patch for this controller waiting already, but in case we hit this again with some other controller vendor then this change could be made. -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: XHCI_TRUST_TX_LENGTH quirk?
I thought there was some reason why the code didn't already trust the length. I even made a comment to the fact that the system already tells me that its needed, so my patch to turn it on seems silly. I just assumed there was a reason it didn't trust it and printed a message asking if it should. On Mon, Aug 11, 2014 at 9:42 AM, Mathias Nyman mathias.ny...@linux.intel.com wrote: On 08/11/2014 02:15 PM, David Laight wrote: From: Behalf Of Evan Langlois Sorry if this is answered somewhere, but I couldn't find anything specific on Google as far as how to determine if this quirk is needed or if its configurable at boot/run-time. Its an HP laptop, and it looks like other HP models have the problem. I most often get this when turning on a USB TV Tuner, which is also HP branded but shows up as a Hauppage device. I get glitches on the TV output, less with tvtime than other viewers, and these glitches go away if I transcode a DVD while watching TV .. yeah, I know that makes no sense. Not sure if the glitches are related to the dmesg warnings. I'll attach the lspci to the message in case anyone needs it. Please advise on the next step to proceed. I'm running a Ubuntu kernel, but I imagine a stock kernel would react the same. uname -a ... Linux Taro 3.13.0-32-lowlatency #57-Ubuntu SMP PREEMPT Tue Jul 15 04:08:59 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux dmesg output (all the repeats deleted) ... [270905.416782] xhci_hcd :00:10.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? Given that the code always checks the receive transfer length (in order to output the message), maybe the code should be refactored to always use the receive transfer length and the quirk removed. I doubt there are any controllers that write a non-zero 'residual' byte count at the end of a full transfer. I think that sounds like an idea. I got a quirk patch for this controller waiting already, but in case we hit this again with some other controller vendor then this change could be made. -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: [RFC] usb: gadget: serial: remove PREFIX macro
Hi, On Mon, 11 Aug 2014 08:50:34 -0500 Felipe Balbi ba...@ti.com wrote: On Mon, Aug 11, 2014 at 03:04:35PM +0200, Richard Leitner wrote: Hi, this patch was lying around for some time... are there any comments or objections on this? you realise we're still in the middle of the merge window, right ? yeah I know, but I didn't knew that I shouldn't ask for the status of my patches during merge windows... Sorry for that, this was one of my first patches submitted, so I hope you'll be clement with beginners like me ;-) I'll come back to you when the merge window is closed. thanks regards, richard -- 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: detected XactErr
Alan Stern stern@... writes: On Wed, 29 Aug 2012, Gary E. Miller wrote: Yo Alan! On Wed, 29 Aug 2012 17:08:43 -0400 (EDT) Alan Stern stern@... wrote: Uh, not easy. This is a production machine. Acceptable downtime is very small. There should be a way to turn it off at runtime. No, there shouldn't. It's a debugging feature; while debugging we want to see all occurrences of these messages. Production machines should not run debugging kernels. We'll have to agree to disagree. My experience is the fun bugs only happen in production. A big NASA study years ago proved that after spending $1M per line of code that only half the bugs could be caught before live missions. Well, put it this way: Production machines should not run debugging kernels unless you're actually trying to debug something. For normal operation, a debugging kernel should not be used. Remember, the purpose of a debugging kernel is not to provoke bugs or notify you that the bugs exist. Normal kernels do these things just fine. The purpose is to help find the cause of bugs. Alan Stern I believe the focus of this thread has gone in wrong direction. The question should not be about how to disable the flood of detected XactErr messages, but (a) why does it occur in this particular case, and (b) why there are so many of them. My opinion here is: (a) The fact of appearance of detected XactErr message indicates some BRUTAL CONDITION with communication pipe on the particular USB port. This message is generated after the host hardware was unable to receive proper protocol response from a device after THREE BACK-To-BACK ATTEMPTS (assuming CERR in EHCI.c is set to 3). This means that this situation is not some accidental signal glitch but a fatal condition on the pipe. In other words, the USB device has likely lost its configuration, or went dead. Therefore, the host should not re-try this low-level transaction, and rather resort to some higher-level recovery procedure (port reset and re-enumeration). Thus, we are coming to (b): (b) Instead of switching to recovery, the Linux USB driver attempts 32 additional re-tries. As explained in (a), these retries serve no purpose, except they generate really alarming debug logs that would be impossible to miss. Sorry to reviving 2-years-old thread. My problem with Linux USB stack is why it is doing extra 32 attempts to a dead link. What is the rationale behind this 32-times recovery policy? Thanks, Alexei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: gadget: serial: remove PREFIX macro
Hi, On Mon, Aug 11, 2014 at 04:52:38PM +0200, Richard Leitner wrote: On Mon, 11 Aug 2014 08:50:34 -0500 Felipe Balbi ba...@ti.com wrote: On Mon, Aug 11, 2014 at 03:04:35PM +0200, Richard Leitner wrote: Hi, this patch was lying around for some time... are there any comments or objections on this? you realise we're still in the middle of the merge window, right ? yeah I know, but I didn't knew that I shouldn't ask for the status of my patches during merge windows... well, usually during merge windows most maintainers take a break, if you will. For many, it's the only time we have to sit back and enjoy the show. Sorry for that, this was one of my first patches submitted, so I hope you'll be clement with beginners like me ;-) hehe, no problem. I'll come back to you when the merge window is closed. sure thing. Once v3.17-rc1 is tagged, I'll start queueing patches. Once they are in my final branches (fixes or next) you'll get an automated email. Until then you can, if you wish, poll my testing branches (testing/fixes and testing/next) to check the status of your patches. cheers -- balbi signature.asc Description: Digital signature
Re: detected XactErr
On Mon, 11 Aug 2014, Alexei P wrote: I believe the focus of this thread has gone in wrong direction. The question should not be about how to disable the flood of detected XactErr messages, but (a) why does it occur in this particular case, and (b) why there are so many of them. My opinion here is: (a) The fact of appearance of detected XactErr message indicates some BRUTAL CONDITION with communication pipe on the particular USB port. Usually it means that the computer did not receive a reply packet in response to some packet it sent to the device. This could be because noise disrupted either the packet sent to the device or the reply packet, or because the device has crashed and isn't sending anything. This message is generated after the host hardware was unable to receive proper protocol response from a device after THREE BACK-To-BACK ATTEMPTS (assuming CERR in EHCI.c is set to 3). This means that this situation is not some accidental signal glitch but a fatal condition on the pipe. Not true. People have observed conditions where the transaction errors occurred multiple times in a row (more than three) and then went away. There are examples hidden away in the email archives of such occurrences. You may be able to find some if you read through the email exchanges leading up to the commit that introduced the multiple-retry mechanism. In other words, the USB device has likely lost its configuration, or went dead. Therefore, the host should not re-try this low-level transaction, and rather resort to some higher-level recovery procedure (port reset and re-enumeration). Thus, we are coming to (b): Since the initial assumption above is wrong, this conclusion is invalid. (b) Instead of switching to recovery, the Linux USB driver attempts 32 additional re-tries. As explained in (a), these retries serve no purpose, except they generate really alarming debug logs that would be impossible to miss. They do serve a purpose. Sometimes they are able to re-establish communication. Sorry to reviving 2-years-old thread. My problem with Linux USB stack is why it is doing extra 32 attempts to a dead link. What is the rationale behind this 32-times recovery policy? The number 32 was picked more or less arbitrarily. Experience has shown, however, that 3 is definitely too small. 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: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
Hi Alan, On Mon, Aug 11, 2014 at 10:28:12AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: On Mon, Aug 11, 2014 at 09:44:05AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. that might be too late, although propbably not catastrophic. It looks like the UDC core needs to be improved a little. Right now, udc_bind_to_driver() unconditionally calls usb_gadget_connect(), thereby overriding any calls the gadget driver may have made during its bind callback. What we should do instead is call usb_gadget_connect() only if the gadget driver didn't try to change the pullup setting. I remember last years discussion regarding the usage of usb_gadget_deactivate and usb_gadget_activate in composite.c: http://markmail.org/message/haqte4elxt5xnuf2 -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to repalce with assert_spin_locked(). Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com --- drivers/usb/storage/uas.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f42785..8e5877d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, caller); - WARN_ON_ONCE(!spin_is_locked(devinfo-lock)); + assert_spin_locked(devinfo-lock); Seems to me that replacing WARN_ON_ONCE (which may be annoying but only creates a traceback, and only once) with assert_spin_locked (which crashes the kernel) is a bit drastic. Guenter -- 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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
Hi, On 08/11/2014 08:19 PM, Guenter Roeck wrote: On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to repalce with assert_spin_locked(). Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com --- drivers/usb/storage/uas.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f42785..8e5877d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, caller); -WARN_ON_ONCE(!spin_is_locked(devinfo-lock)); +assert_spin_locked(devinfo-lock); Seems to me that replacing WARN_ON_ONCE (which may be annoying but only creates a traceback, and only once) with assert_spin_locked (which crashes the kernel) is a bit drastic. I can see your point, but so far these paranoia checks have never triggered, and having them trigger _always_ one some uni-processor (which is the reason for this patch) to me seems the worse problem of the 2. Ideally we would have a warn_spin_not_locked or such ... Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uas: replace WARN_ON_ONCE() with assert_spin_locked().
Hi,
On 08/11/2014 09:59 AM, Sanjeev Sharma wrote:
spin_is_locked() always return false in uniprocessor configuration and
therefore it
would be advise to repalce with assert_spin_locked().
Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
Looks sane / good to me:
Acked-by: Hans de Goede hdego...@redhat.com
Regards,
Hans
---
drivers/usb/storage/uas.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..8e5877d 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info
*devinfo,
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
uas_log_cmd_state(cmnd, caller);
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
WARN_ON_ONCE(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
@@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
struct uas_dev_info *devinfo = cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
cmdinfo-state |= IS_IN_WORK_LIST;
schedule_work(devinfo-work);
}
@@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT |
@@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct urb *urb;
int err;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ assert_spin_locked(devinfo-lock);
if (cmdinfo-state SUBMIT_STATUS_URB) {
urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo-stream);
if (!urb)
--
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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
On Mon, Aug 11, 2014 at 08:55:07PM +0200, Hans de Goede wrote: Hi, On 08/11/2014 08:19 PM, Guenter Roeck wrote: On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to repalce with assert_spin_locked(). Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com --- drivers/usb/storage/uas.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f42785..8e5877d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, caller); - WARN_ON_ONCE(!spin_is_locked(devinfo-lock)); + assert_spin_locked(devinfo-lock); Seems to me that replacing WARN_ON_ONCE (which may be annoying but only creates a traceback, and only once) with assert_spin_locked (which crashes the kernel) is a bit drastic. I can see your point, but so far these paranoia checks have never triggered, and having them trigger _always_ one some uni-processor (which is the reason for this patch) to me seems the worse problem of the 2. If those are just paranoia checks, it might make sense to use lockdep_assert_held() to reduce runtime overhead if lockdep debugging is disabled. Ideally we would have a warn_spin_not_locked or such ... There is WARN_ON_SMP, which might have been a better choice if the _ONCE isn't that important (which one should think if it is ok to crash the kernel if the problem is seen). Guenter -- 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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
Hi, On 08/11/2014 09:08 PM, Guenter Roeck wrote: On Mon, Aug 11, 2014 at 08:55:07PM +0200, Hans de Goede wrote: Hi, On 08/11/2014 08:19 PM, Guenter Roeck wrote: On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to repalce with assert_spin_locked(). Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com --- drivers/usb/storage/uas.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f42785..8e5877d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, caller); - WARN_ON_ONCE(!spin_is_locked(devinfo-lock)); + assert_spin_locked(devinfo-lock); Seems to me that replacing WARN_ON_ONCE (which may be annoying but only creates a traceback, and only once) with assert_spin_locked (which crashes the kernel) is a bit drastic. I can see your point, but so far these paranoia checks have never triggered, and having them trigger _always_ one some uni-processor (which is the reason for this patch) to me seems the worse problem of the 2. If those are just paranoia checks, it might make sense to use lockdep_assert_held() to reduce runtime overhead if lockdep debugging is disabled. Ah yes, that is a good idea. Sanjeev, can you please send a v2 using lockdep_assert_held() ? Thanks Regards, Hans (the uas driver maintainer) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, 11 Aug 2014, Michael Grzeschik wrote: Hi Alan, On Mon, Aug 11, 2014 at 10:28:12AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: On Mon, Aug 11, 2014 at 09:44:05AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. that might be too late, although propbably not catastrophic. It looks like the UDC core needs to be improved a little. Right now, udc_bind_to_driver() unconditionally calls usb_gadget_connect(), thereby overriding any calls the gadget driver may have made during its bind callback. What we should do instead is call usb_gadget_connect() only if the gadget driver didn't try to change the pullup setting. I remember last years discussion regarding the usage of usb_gadget_deactivate and usb_gadget_activate in composite.c: http://markmail.org/message/haqte4elxt5xnuf2 Okay. I don't really care what approach gets used, so long as the final version works properly. 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
To otg-fsm or not to otg-fsm? That is the question.
Hey Linux USBer's, I'm looking to add host support to the qualcomm OTG USB driver. The driver in mainline currently has gadget support only. I'm starting with an out-of-tree driver that has full gadget and host support. It has it's own OTG state machine implement in the driver, that I was about to forward-port to top-of-tree. However, I found the files /usb/common/usb-otg-fsm.c and I'm wondering what the status of that is. There appears to be only one driver currently using it. Should I modify the qualcomm driver (drivers/usb/phy/phy-msm-usb.c to use that state machine, or just try to mainline the state machine from the out-of-tree driver? I'm suspecting the former will be more work, but I want to do the right thing. Really I'm just checking that using the generic state machine is the preferred method of supporting USB OTG drivers going forward. Thanks! -- Tim Bird Senior Software Engineer, Sony Mobile Architecture Group Chair, CE Workgroup, Linux Foundation -- 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] uas: replace WARN_ON_ONCE() with assert_spin_locked().
On Mon, Aug 11, 2014 at 11:56:17AM +, Sharma, Sanjeev wrote: Hi Greg, Please find my comment in line. As it should, but where is it, your quoting is all messed up... -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Monday, August 11, 2014 1:58 PM To: Sharma, Sanjeev Cc: hdego...@redhat.com; kra...@redhat.com; mdharm-...@one-eyed-alien.net; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; linux-s...@vger.kernel.org Subject: Re: [PATCH] uas: replace WARN_ON_ONCE() with assert_spin_locked(). On Mon, Aug 11, 2014 at 01:29:26PM +0530, Sanjeev Sharma wrote: spin_is_locked() always return false in uniprocessor configuration Really? On what processor type? I don't see that being the case on a few processors, but I didn't check them all, or I might be totally confused with all of the arch_spin_is_locked() definitions in the tree. specially in case of mips and powerpc but we can configure our kernel to uniprocessor configuration for specific processor and in that case we should be extra cautious. Please check spinlock_up.h Shouldn't you just fix your .h file to not do this? x86 is not this way, nor is ARM, right? and therefore it would be advise to repalce with assert_spin_locked(). This implies that all uses of this function is wrong and should be replaced and removed, right? Yes and there are only few places in driver for which I am submitting patches which need to be change and at other places it has been already taken care. Please have an look into the Old discussion for more details. http://linux-kernel.2935.n7.nabble.com/Is-spin-is-locked-safe-to-use-with-BUG-ON-WARN-ON-td654800.html#a921802 Ok, just remove all the tests, it doesn't help anyone :) 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: To otg-fsm or not to otg-fsm? That is the question.
Hey Linux USBer's,
I'm looking to add host support to the qualcomm OTG USB driver. The
driver in mainline currently has gadget support only. I'm starting with
an out-of-tree driver that has full gadget and host support. It has it's
own OTG state machine implement in the driver, that I was about to
forward-port to top-of-tree.
However, I found the files /usb/common/usb-otg-fsm.c and I'm wondering
what the status of that is. There appears to be only one driver
currently using it. Should I modify the qualcomm driver
(drivers/usb/phy/phy-msm-usb.c to use that state machine, or just try to
mainline the state machine from the out-of-tree driver? I'm suspecting
the former will be more work, but I want to do the right thing.
Hi Tim,
The OTG FSM at /usb/common/usb-otg-fsm.c is a software OTG FSM implementation
which follows On-The-Go and Embedded Host Supplement to the USB Revision 2.0
Specification
(Revision 2.0 version 1.1a) Chapter 7 State Diagrams.
If the platform doesn't support hardware otg fsm, it should follow above fsm
implementation since
it follows the newest OTG EH spec. Besides, as far as I know, the qualcomm
uses
chipidea core for its usb2, then it should be easy for qualcomm using this
common
otg fsm implementation.
Peter
Really I'm just checking that using the generic state machine is the
preferred method of supporting USB OTG drivers going forward.
Thanks!
-- Tim Bird
Senior Software Engineer, Sony Mobile
Architecture Group Chair, CE Workgroup, Linux Foundation
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h��� �G���h�(�階�ݢj���m��z�ޖ���f���h���~�m�
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, Aug 11, 2014 at 02:34:33PM +0200, Michael Grzeschik wrote: Hi Peter, On Mon, Aug 11, 2014 at 12:56:47AM +, Peter Chen wrote: I have seen this call is still returning EOPNOTSUPP which is also returned by usb_gadget_connect and usb_gadget_disconnect if the pullup function is not available. Should it not handle that case somehow special? Is it still a valid condition to bailout if vbus is not active? Hi Michael, I did this for possible pullup dp without connection (eg load gadget driver before vbus connect), it breaks USB spec, see 7.1.7.3 Connect and Disconnect Signaling at USB 2.0 spec. I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. I still work with the uvc gadget. The code in gadget/f_uvc.c calls usb_function_deactivate. The code of uvc_function_bind will bail out in the error code path, when the usb cable is not plugged (vbus not valid) in that situation. I see the problem, doesn't there have problem with usb cable connected? I don't see any places can avoid pull up dp at udc-core during loading gadget driver. When the spec defines that calling pullup is only allowed on valid vbus, we should somehow tell something different then EOPNOTSUPP so the caller and that can bail out differently. I will have a look for this problem -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Revert usb: chipidea: udc: .pullup is valid only when vbus is there
On Mon, Aug 11, 2014 at 04:16:11PM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Michael Grzeschik wrote: Hi Alan, On Mon, Aug 11, 2014 at 10:28:12AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: On Mon, Aug 11, 2014 at 09:44:05AM -0400, Alan Stern wrote: On Mon, 11 Aug 2014, Felipe Balbi wrote: I don't know what exactly problem you met, but current pullup dp during loading gadget driver behavior is not suitable for webcam and android use case even vbus is there. Why not? What's wrong with it? g_uvc needs a userspace counterpart. If we connect to the host before userspace is ready, we might connect as a non-usable function. Okay, but can't that be fixed in g_uvc? Have it disable the pullup until the userspace component is ready, then enable the pullup. that might be too late, although propbably not catastrophic. It looks like the UDC core needs to be improved a little. Right now, udc_bind_to_driver() unconditionally calls usb_gadget_connect(), thereby overriding any calls the gadget driver may have made during its bind callback. What we should do instead is call usb_gadget_connect() only if the gadget driver didn't try to change the pullup setting. I remember last years discussion regarding the usage of usb_gadget_deactivate and usb_gadget_activate in composite.c: http://markmail.org/message/haqte4elxt5xnuf2 Okay. I don't really care what approach gets used, so long as the final version works properly. No, I don't see this problem has fixed, android kernel simply deletes usb_gadget_connect(udc-gadget) at udc_bind_to_driver https://android.googlesource.com/kernel/common/+/b94f8f691c550d1e07c21d6c672b916377c9d6d4%5E%21/#F0 -- 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
[PATCH v2] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
spin_is_locked() always return false in uniprocessor configuration and
therefore it
would be advise to replace with lockdep_assert_held().
Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
---
Changes in v2:
- replaced WARN_ON_ONCE() with lockdep_assert_held() to
avoid runtime overhead instead of assert_spin_locked()
drivers/usb/storage/uas.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..05b2d8e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo,
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
uas_log_cmd_state(cmnd, caller);
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
WARN_ON_ONCE(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
@@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
struct uas_dev_info *devinfo = cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
cmdinfo-state |= IS_IN_WORK_LIST;
schedule_work(devinfo-work);
}
@@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT |
@@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct urb *urb;
int err;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
if (cmdinfo-state SUBMIT_STATUS_URB) {
urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo-stream);
if (!urb)
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
Hi,
On 08/12/2014 08:08 AM, Sanjeev Sharma wrote:
spin_is_locked() always return false in uniprocessor configuration and
therefore it
would be advise to replace with lockdep_assert_held().
Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
---
Changes in v2:
- replaced WARN_ON_ONCE() with lockdep_assert_held() to
avoid runtime overhead instead of assert_spin_locked()
Thanks!
Acked-by: Hans de Goede hdego...@redhat.com
Regards,
Hans
drivers/usb/storage/uas.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..05b2d8e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info
*devinfo,
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
uas_log_cmd_state(cmnd, caller);
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
WARN_ON_ONCE(cmdinfo-state COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
@@ -181,7 +181,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
struct uas_dev_info *devinfo = cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
cmdinfo-state |= IS_IN_WORK_LIST;
schedule_work(devinfo-work);
}
@@ -283,7 +283,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
if (cmdinfo-state (COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT |
@@ -622,7 +622,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct urb *urb;
int err;
- WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+ lockdep_assert_held(devinfo-lock);
if (cmdinfo-state SUBMIT_STATUS_URB) {
urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo-stream);
if (!urb)
--
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
