RE: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
> -Original Message-
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Friday, March 18, 2016 7:51 PM
> To: Rajesh Bhagat
> Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
> gre...@linuxfoundation.org; mathias.ny...@intel.com; Sriram Dash
>
> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
> commmand timeout
>
> On Fri, 18 Mar 2016, Rajesh Bhagat wrote:
>
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -2897,10 +2897,14 @@ done:
> > /* The xHC may think the device is already reset,
> > * so ignore the status.
> > */
> > - if (hcd->driver->reset_device)
> > - hcd->driver->reset_device(hcd, udev);
> > -
> > - usb_set_device_state(udev, USB_STATE_DEFAULT);
> > + if (hcd->driver->reset_device) {
> > + status = hcd->driver->reset_device(hcd, udev);
> > + if (status == 0)
> > + usb_set_device_state(udev,
> > USB_STATE_DEFAULT);
> > + else
> > + usb_set_device_state(udev,
> USB_STATE_NOTATTACHED);
> > + } else
> > + usb_set_device_state(udev, USB_STATE_DEFAULT);
>
> This is a really bad patch:
>
> You left in the comment about ignoring the status, but then you changed the
> code so
> that it doesn't ignore the status!
>
My Apologies, I completely missed the above comment which was added before.
> You also called usb_set_device_state() more times than necessary. You could
> have
> done it like this:
>
> if (hcd->driver->reset_device)
> status = hcd->driver->reset_device(hcd, udev);
> if (status == 0)
> usb_set_device_state(udev, USB_STATE_DEFAULT);
> else
> usb_set_device_state(udev,
> USB_STATE_NOTATTACHED);
>
> (Even that could be simplified further, by combining it with the code that
> follows.)
>
> Finally, you violated the 80-column limit.
>
I agree to your point. Actually the intent was to check the return status of
reset_device which
is currently being ignored. I encountered the reset_device failure case in
resume operation (STR)
which is increasing the time of resume and causing unexpected crashes if return
value is not checked.
Do you agree it should be checked here? If yes, I can rework this patch.
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
> -Original Message-
> From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com]
> Sent: Friday, March 18, 2016 4:51 PM
> To: Rajesh Bhagat ; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Cc: gre...@linuxfoundation.org; mathias.ny...@intel.com; Sriram Dash
>
> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
> commmand timeout
>
> On 18.03.2016 09:01, Rajesh Bhagat wrote:
> > We are facing issue while performing the system resume operation from
> > STR where XHCI is going to indefinite hang/sleep state due to
> > wait_for_completion API called in function xhci_alloc_dev for command
> > TRB_ENABLE_SLOT which never completes.
> >
> > Now, xhci_handle_command_timeout function is called and prints
> > "Command timeout" message but never calls complete API for above
> > TRB_ENABLE_SLOT command as xhci_abort_cmd_ring is successful.
> >
> > Solution to above problem is:
> > 1. calling xhci_cleanup_command_queue API even if xhci_abort_cmd_ring
> > is successful or not.
> > 2. checking the status of reset_device in usb core code.
>
>
> 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.
> We need to look in more detail why we fail to call completion for that one
> aborted
> command.
>
I checked the below code, Please correct me if I am wrong
code waiting on wait_for_completion:
int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
{
...
ret = xhci_queue_slot_control(xhci, command, TRB_ENABLE_SLOT, 0);
...
wait_for_completion(command->completion); <=== waiting for command to
complete
code calling completion handler:
1. handle_cmd_completion -> xhci_complete_del_and_free_cmd
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.
> The bigger question is why the timeout happens in the first place?
>
We are doing suspend resume operation, It might be controller issue :(, IMO
software should not
hang/stop if hardware is not behaving correct.
> What kernel version, and what xhci vendor was this triggered on?
>
We are using 4.1.8 kernel
> It's possible that the timeout is related either to the locking issue found
> by Chris
> Bainbridge:
> http://marc.info/?l=linux-usb&m=145493945408601&w=2
>
> or the resume issues in this thread, (see full thread)
> http://marc.info/?l=linux-usb&m=145477850706552&w=2
>
> Does any of those proposed solutions fix the command timeout for you?
>
I will check the above patches and share status.
> -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 03/20/2016 03:43 AM, Geert Uytterhoeven wrote:
If CONFIG_PM=n:
drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no member
named ‘runtime_auto’
If PM is disabled, the runtime_auto flag is not available, but auto
suspend is not enabled anyway. Hence protect the check for runtime_auto
by #ifdef CONFIG_PM to fix this.
Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
Reported-by: Guenter Roeck
Signed-off-by: Geert Uytterhoeven
---
Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper to
include/linux/pm.h, which always return false if CONFIG_PM is disabled.
The only other user in non-core code (drivers/usb/core/sysfs.c) has a
big #ifdef CONFIG_PM check around all PM-related code.
Thoughts?
Not that it matters anymore since David reverted the original patch,
but my reason for not sending a similar patch was that I wasn't sure
if .runtime_auto should be accessed from drivers in the first place,
or if there is some logical problem with the code.
Guenter
---
drivers/net/usb/lan78xx.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index d36d5ebf37f355f2..7b9ac47b2ecf9905 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3271,7 +3271,9 @@ struct rtnl_link_stats64 *lan78xx_get_stats64(struct
net_device *netdev,
* periodic reading from HW will prevent from entering USB auto suspend.
* if autosuspend is disabled, read from HW.
*/
+#ifdef CONFIG_PM
if (!dev->udev->dev.power.runtime_auto)
+#endif
lan78xx_update_stats(dev);
mutex_lock(&dev->stats.access_lock);
--
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
From: Eric Dumazet
Date: Sun, 20 Mar 2016 09:35:52 -0700
> On Sun, 2016-03-20 at 11:43 +0100, Geert Uytterhoeven wrote:
>> If CONFIG_PM=n:
>>
>> drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
>> drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no
>> member named ‘runtime_auto’
>>
>> If PM is disabled, the runtime_auto flag is not available, but auto
>> suspend is not enabled anyway. Hence protect the check for runtime_auto
>> by #ifdef CONFIG_PM to fix this.
>>
>> Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
>> Reported-by: Guenter Roeck
>> Signed-off-by: Geert Uytterhoeven
>> ---
>> Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper to
>> include/linux/pm.h, which always return false if CONFIG_PM is disabled.
>>
>> The only other user in non-core code (drivers/usb/core/sysfs.c) has a
>> big #ifdef CONFIG_PM check around all PM-related code.
>>
>> Thoughts?
>> ---
>> drivers/net/usb/lan78xx.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>> index d36d5ebf37f355f2..7b9ac47b2ecf9905 100644
>> --- a/drivers/net/usb/lan78xx.c
>> +++ b/drivers/net/usb/lan78xx.c
>> @@ -3271,7 +3271,9 @@ struct rtnl_link_stats64 *lan78xx_get_stats64(struct
>> net_device *netdev,
>> * periodic reading from HW will prevent from entering USB auto suspend.
>> * if autosuspend is disabled, read from HW.
>> */
>> +#ifdef CONFIG_PM
>> if (!dev->udev->dev.power.runtime_auto)
>> +#endif
>> lan78xx_update_stats(dev);
>>
>> mutex_lock(&dev->stats.access_lock);
>
> Note that a ndo_get_stat64() handler is not allowed to sleep,
> so the mutex_lock() is not wise...
>
> Historically /proc/net/dev handler but also bonding ndo_get_stats() used
> RCU or a rwlock or a spinlock.
>
> So a complete fix would need to get rid of this mutex as well.
This function is also buggy for another reason. The driver needs to
capture the statistics when the runtime suspend happens.
Since it's much easier to find things wrong rather than right with
this new code, I've decided to completely revert the commit that added
lan78xx_get_stats64().
Once a correct version is implemented we can add it back.
Thanks.
N§²ζμrΈyϊθΨb²X¬ΆΗ§vΨ^)ήΊ{.nΗ+·₯{±ΊΖβΨ^nr‘φ¦zΛλh¨θΪ&’ψ �G«ιh�(ιέ’j"ϊΆm§�οκδzΉήΰώf£’·h§~m
Yet another Seagate quirk for unusual_uas.h
I recently purchased a "Seagate 2TB Expansion Desktop Drive" from a UK store. The unix command 'lsusb -v' shows: idVendor 0x0bc2 Seagate RSS LLC idProduct 0x331a When plugged in this failed to install correctly. The first few lines of the error messages (journalctl -f ) contained: kernel: usb 2-7: new SuperSpeed USB device number 6 using xhci_hcd kernel: usb 2-7: New USB device found, idVendor=0bc2, idProduct=331a kernel: usb 2-7: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 2-7: Product: Expansion Desk kernel: usb 2-7: Manufacturer: Seagate kernel: usb 2-7: SerialNumber: NA8EFXP3 mtp-probe[25968]: checking bus 2, device 6: "/sys/devices/pci:00/:00:14.0/usb2/2-7" mtp-probe[25968]: bus: 2, device: 6 was not an MTP device Following advice on the web I created file '/etc/modprobe.d/ignore_uas.conf' containing the line "options usb-storage quirks=0bc2:331a:u". The disk could then be mounted and used without any problems. Having checked the 'unusual_uas.h' code, both for my distribution (openSUSE 42.1 using linux 4.1.15-8) and the latest version of linux on github, I tried replacing the 'u' flag in the ignore file with a 't' flag and found that the device still worked correctly. As the 't' flag corresponds to US_FL_NO_ATA_1X and assuming that using uas, with a shorter message length, is better than bypassing it, I suggest that the following lines be inserted in the file 'unusual_uas.h': /* Reported-by: David Webb */ UNUSUAL_DEV(0x0bc2, 0x331a, 0x, 0x, "Seagate", "Expansion Desk", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), I presume that the "0x, 0x" and "USB_SC_DEVICE, USB_PR_DEVICE, NULL," bits are correct (I copied them from the other Seagate entries) but someone more knowledgeable should check. If you need any further information or listing, just let me know. Regards, David Webb. -- 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: Wait before re-enabling a port that has been disabled due to EMI
Hello Greg ! On dom, 2016-03-20 at 10:34 -0700, Greg KH wrote: > On Sun, Mar 20, 2016 at 06:09:57PM +0100, Guido Trentalancia wrote: > > > > [ 1295.575679] usb 6-2: FTDI USB Serial Device converter now > > attached > > to ttyUSB1 > > [ 1302.204285] usb usb6-port2: disabled by hub (EMI?), re- > > enabling... > > > > *** NOTE: EMI is probably still present here *** > > > > [ 1303.205202] usb 6-2: USB disconnect, device number 6 > > [ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter > > now > > disconnected from ttyUSB1 > > [ 1303.205950] ftdi_sio 6-2:1.0: device disconnected > > [ 1303.414089] usb 6-2: new full-speed USB device number 7 using > > uhci_hcd > > [ 1303.526226] usb 6-2: device descriptor read/64, error -71 > > [ 1303.894228] usb 6-2: new full-speed USB device number 8 using > > uhci_hcd > > [ 1304.006185] usb 6-2: device descriptor read/64, error -71 > > [ 1304.219089] usb 6-2: device descriptor read/64, error -71 > > [ 1304.422107] usb 6-2: new full-speed USB device number 9 using > > uhci_hcd > > [ 1304.640020] usb 6-2: device not accepting address 9, error -71 > > [ 1304.752024] usb 6-2: new full-speed USB device number 10 using > > uhci_hcd > > [ 1305.160020] usb 6-2: device not accepting address 10, error -71 > > [ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port > > 2 > > > > *** NOTE: Device is permanently disabled at this point *** > > > > I don't know whether my analysis is correct (and therefore the > > proposed > > solution appropriate), as reproducing the problem is very > > difficult... > > > Module parameters are "icky", isn't there some way we can just > dynamically determine this (i.e. fall back to longer and longer wait > times?) No, we cannot in general predict how long EMI lasts. But, for example, I have a known source of EMI close to the computer (RF transmitter) and I know it lasts a couple of seconds, therefore I am able to set up the parameter correctly. Also, I believe once it fails the first time, the device is permanently disabled by the USB driver (I am no USB driver expert, do you confirm ?). Therefore, by the way the driver is actually designed, it is not possible to try again with a longer wait interval. And it's much more complicate, plus you still have to hard-code a maximum value I suppose... > Have you been able to test this out and see if it works? So far, so good. But before several days of testing, I won't be able to tell for sure whether waiting those 2 seconds completely eliminates the issue. This is because I cannot reliably reproduce the condition, it depends on the (random) EMI. Finally, I have been conservative in the patch defaulting to zero seconds, but perhaps, it's worth defaulting to 1 second or something similar. > thanks, > > greg k-h You're welcome. Guido -- 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: Wait before re-enabling a port that has been disabled due to EMI
On Sun, Mar 20, 2016 at 06:09:57PM +0100, Guido Trentalancia wrote: > Hello. > > Considering that EM interference can last for a while (generally up to > seconds), I am currently testing the following patch for module usbcore > so that it is possible to specify an amount of time to wait before > trying to re-enable a port that has been previously disabled by the hub > (supposedly because of EM interference). > > Hopefully, setting the right positive value (for example, 2000 > milliseconds) would help overcome situations such as the following: > > [ 1295.575679] usb 6-2: FTDI USB Serial Device converter now attached > to ttyUSB1 > [ 1302.204285] usb usb6-port2: disabled by hub (EMI?), re-enabling... > > *** NOTE: EMI is probably still present here *** > > [ 1303.205202] usb 6-2: USB disconnect, device number 6 > [ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter now > disconnected from ttyUSB1 > [ 1303.205950] ftdi_sio 6-2:1.0: device disconnected > [ 1303.414089] usb 6-2: new full-speed USB device number 7 using > uhci_hcd > [ 1303.526226] usb 6-2: device descriptor read/64, error -71 > [ 1303.894228] usb 6-2: new full-speed USB device number 8 using > uhci_hcd > [ 1304.006185] usb 6-2: device descriptor read/64, error -71 > [ 1304.219089] usb 6-2: device descriptor read/64, error -71 > [ 1304.422107] usb 6-2: new full-speed USB device number 9 using > uhci_hcd > [ 1304.640020] usb 6-2: device not accepting address 9, error -71 > [ 1304.752024] usb 6-2: new full-speed USB device number 10 using > uhci_hcd > [ 1305.160020] usb 6-2: device not accepting address 10, error -71 > [ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port 2 > > *** NOTE: Device is permanently disabled at this point *** > > I don't know whether my analysis is correct (and therefore the proposed > solution appropriate), as reproducing the problem is very difficult... > > Regards, > > Guido > > Add an option to the usbcore module to wait a specified amount of time (in > milliseconds) before trying to re-enable a USB port that has been previously > disabled by the hub (possibly due to EMI). > > Signed-off-by: Guido Trentalancia > --- > drivers/usb/core/hub.c |8 > 1 file changed, 8 insertions(+) > > --- linux-4.4.6-orig/drivers/usb/core/hub.c 2016-03-20 16:47:09.358674922 > +0100 > +++ linux-4.4.6/drivers/usb/core/hub.c2016-03-20 16:47:51.960195139 > +0100 > @@ -89,6 +89,13 @@ MODULE_PARM_DESC(use_both_schemes, > "try the other device initialization scheme if the " > "first one fails"); > > +static int emi_recover_timer = 0; > +module_param(emi_recover_timer, int, S_IRUGO | S_IWUSR); > +MODULE_PARM_DESC(emi_recover_timer, > + "wait before trying to re-enable a port " > + "that has been disabled by the hub due to EMI " > + "(default 0 milliseconds)"); Module parameters are "icky", isn't there some way we can just dynamically determine this (i.e. fall back to longer and longer wait times?) Have you been able to test this out and see if it works? 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: PROBLEM: Osprey 2 Mini sometimes resets every 200ms forever
On Thu, 17 Mar 2016, ais523 wrote: > > There's no question about it -- your problem is caused by a buggy > > device. > > This actually doesn't surprise me at all; I became suspicious when I > saw that the serial number was 0123456789ABCDEF rather than something > that looked more like a serial number. > > As far as I can tell, the device pretends to be a mass storage device > for the purpose of installing drivers on Windows and OS X, in addition > to being a networking device. The mass storage part of the device > doesn't work under Linux because the device violates the protocol so > badly; but nor is it particularly useful or the reason that you'd use > the device in the first place. Meanwhile, its networking properties do > work under Linux, and are useful, but aren't usable until/unless the > device stops resetting. > > I've also verified that the device itself has the latest firmware; so > the problem hasn't been fixed by the manufacturer yet. > > Do you feel that it's worth implementing some sort of workaround in the > kernel? If not, I can either continue using my manual workaround of > repeatedly connecting and disconnecting the device until the reset > loops stop naturally, or perhaps trying to put pressure on the > manufacturer to fix the problem with their device. No in-kernel workaround is needed. You can simply specify a module parameter for the usb-storage driver that will prevent it from binding to the device. The parameter would be "1bbb:f000:i" (that's the vendor ID, the product ID, and 'i' for Ignore). Of course, putting pressure on the manufacturer to fix the bugs would also be a good idea. :-) 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
[PATCH] usb: Wait before re-enabling a port that has been disabled due to EMI
Hello.
Considering that EM interference can last for a while (generally up to
seconds), I am currently testing the following patch for module usbcore
so that it is possible to specify an amount of time to wait before
trying to re-enable a port that has been previously disabled by the hub
(supposedly because of EM interference).
Hopefully, setting the right positive value (for example, 2000
milliseconds) would help overcome situations such as the following:
[ 1295.575679] usb 6-2: FTDI USB Serial Device converter now attached
to ttyUSB1
[ 1302.204285] usb usb6-port2: disabled by hub (EMI?), re-enabling...
*** NOTE: EMI is probably still present here ***
[ 1303.205202] usb 6-2: USB disconnect, device number 6
[ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter now
disconnected from ttyUSB1
[ 1303.205950] ftdi_sio 6-2:1.0: device disconnected
[ 1303.414089] usb 6-2: new full-speed USB device number 7 using
uhci_hcd
[ 1303.526226] usb 6-2: device descriptor read/64, error -71
[ 1303.894228] usb 6-2: new full-speed USB device number 8 using
uhci_hcd
[ 1304.006185] usb 6-2: device descriptor read/64, error -71
[ 1304.219089] usb 6-2: device descriptor read/64, error -71
[ 1304.422107] usb 6-2: new full-speed USB device number 9 using
uhci_hcd
[ 1304.640020] usb 6-2: device not accepting address 9, error -71
[ 1304.752024] usb 6-2: new full-speed USB device number 10 using
uhci_hcd
[ 1305.160020] usb 6-2: device not accepting address 10, error -71
[ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port 2
*** NOTE: Device is permanently disabled at this point ***
I don't know whether my analysis is correct (and therefore the proposed
solution appropriate), as reproducing the problem is very difficult...
Regards,
Guido
Add an option to the usbcore module to wait a specified amount of time (in
milliseconds) before trying to re-enable a USB port that has been previously
disabled by the hub (possibly due to EMI).
Signed-off-by: Guido Trentalancia
---
drivers/usb/core/hub.c |8
1 file changed, 8 insertions(+)
--- linux-4.4.6-orig/drivers/usb/core/hub.c 2016-03-20 16:47:09.358674922
+0100
+++ linux-4.4.6/drivers/usb/core/hub.c 2016-03-20 16:47:51.960195139 +0100
@@ -89,6 +89,13 @@ MODULE_PARM_DESC(use_both_schemes,
"try the other device initialization scheme if the "
"first one fails");
+static int emi_recover_timer = 0;
+module_param(emi_recover_timer, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(emi_recover_timer,
+ "wait before trying to re-enable a port "
+ "that has been disabled by the hub due to EMI "
+ "(default 0 milliseconds)");
+
/* Mutual exclusion for EHCI CF initialization. This interferes with
* port reset on some companion controllers.
*/
@@ -4960,6 +4967,7 @@ static void port_event(struct usb_hub *h
if (!(portstatus & USB_PORT_STAT_ENABLE)
&& !connect_change && udev) {
dev_err(&port_dev->dev, "disabled by hub (EMI?),
re-enabling...\n");
+ msleep(emi_recover_timer);
connect_change = 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Sun, 2016-03-20 at 11:43 +0100, Geert Uytterhoeven wrote:
> If CONFIG_PM=n:
>
> drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
> drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no member
> named ‘runtime_auto’
>
> If PM is disabled, the runtime_auto flag is not available, but auto
> suspend is not enabled anyway. Hence protect the check for runtime_auto
> by #ifdef CONFIG_PM to fix this.
>
> Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
> Reported-by: Guenter Roeck
> Signed-off-by: Geert Uytterhoeven
> ---
> Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper to
> include/linux/pm.h, which always return false if CONFIG_PM is disabled.
>
> The only other user in non-core code (drivers/usb/core/sysfs.c) has a
> big #ifdef CONFIG_PM check around all PM-related code.
>
> Thoughts?
> ---
> drivers/net/usb/lan78xx.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index d36d5ebf37f355f2..7b9ac47b2ecf9905 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -3271,7 +3271,9 @@ struct rtnl_link_stats64 *lan78xx_get_stats64(struct
> net_device *netdev,
>* periodic reading from HW will prevent from entering USB auto suspend.
>* if autosuspend is disabled, read from HW.
>*/
> +#ifdef CONFIG_PM
> if (!dev->udev->dev.power.runtime_auto)
> +#endif
> lan78xx_update_stats(dev);
>
> mutex_lock(&dev->stats.access_lock);
Note that a ndo_get_stat64() handler is not allowed to sleep,
so the mutex_lock() is not wise...
Historically /proc/net/dev handler but also bonding ndo_get_stats() used
RCU or a rwlock or a spinlock.
So a complete fix would need to get rid of this mutex as well.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb: dwc2: Kernel 4.5 and socfpga problem
On 03/17/2016 09:35 PM, Phil Reid wrote: > G'day, > > Has anyone successful run the usb dwc2 from kenerl 4.5 on an socfpga. > Yes. I just tested on the devkit and USB is working fine: [0.655761] ffb4.usb supply vusb_d not found, using dummy regulator [0.662384] ffb4.usb supply vusb_a not found, using dummy regulator [0.943627] dwc2 ffb4.usb: EPs: 16, dedicated fifos, 8064 entries in SPRAM [0.951145] dwc2 ffb4.usb: DWC OTG Controller [0.955872] dwc2 ffb4.usb: new USB bus registered, assigned bus number 1 [0.962908] dwc2 ffb4.usb: irq 37, io mem 0x [0.968877] hub 1-0:1.0: USB hub found [0.972641] hub 1-0:1.0: 1 port detected [...] root@socfpga_cyclone5:~# uname -a Linux socfpga_cyclone5 4.5.0 #1 SMP Fri Mar 18 09:36:26 CDT 2016 armv7l GNU/Linux root@socfpga_cyclone5:~# [ 21.793687] usb 1-1: USB disconnect, device number 2 [ 24.173612] usb 1-1: new high-speed USB device number 3 using dwc2 > Inital I've had to remove the phys & phy-names property from the DT for > it to even probe. > Otherwise it was returning PROBE_DEFERED. > The 4.4 driver seems to be getting the same error but it continued > loading regardless. > What hardware are you using? Sockit, Atlas board, socrates, etc? > After fixing that and getting it to load its going into overcurrent fault. > Applying Dinh's patch from the Altera 4.4 tree > FogBugz #198256: Fix unnecessary USB overcurrent condition > hasn't help. The driver immediately goes into an overcurrent condition > at boot. I'm still trying to find a way to upstream this patch. Dinh -- 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: Limit qdepth at the scsi-host level
Hi,
On 19-03-16 18:06, James Bottomley wrote:
On Sat, 2016-03-19 at 09:59 +0100, Hans de Goede wrote:
Commit 64d513ac31bd ("scsi: use host wide tags by default") causes
the scsi-core to queue more cmnds then we can handle on devices with
multiple LUNs, limit the qdepth at the scsi-host level instead of
per slave to fix this.
Help me understand this bug a bit more. Are you saying that the commit
you identify is causing the block layer to queue more commands than
you've set the per-lun limit to? In which case we have a serious
problem for more than just UAS. Or are you saying that UAS always had
a global command limit, but it just didn't get set correctly; however,
it mostly worked until the above commit exposed the problem?
The latter. UAS has always had a global command limit, which so far
was enforced via a shared tag map rather then setting can_queue
correctly. The identified commit removed the usage of a shared
tag map which causes the core to queue more commands *in total*
then the global limit. I've no reason to believe that the core was
exceeding the per-lun limit on a single lun, but for uas exceeding
the limit when counting all queued commands per lun combined is just
as bad, since it really always was a global limit.
This theory is supported by the only bug report for 4.4 being a
(rare) multi-lun uas disk enclosure.
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
[PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
We are facing issue while performing the system resume operation from STR
where XHCI is going to indefinite hang/sleep state due to
wait_for_completion API called in function xhci_alloc_dev for command
TRB_ENABLE_SLOT which never completes.
Now, xhci_handle_command_timeout function is called and prints
"Command timeout" message but never calls complete API for above
TRB_ENABLE_SLOT command as xhci_abort_cmd_ring is successful.
Solution to above problem is:
1. calling xhci_cleanup_command_queue API even if xhci_abort_cmd_ring
is successful or not.
2. checking the status of reset_device in usb core code.
Before Fix:
root@phoenix:~# echo mem > /sys/power/state
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.001 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
PM: suspend of devices complete after 103.144 msecs
PM: late suspend of devices complete after 1.503 msecs
PM: noirq suspend of devices complete after 1.220 msecs
Disabling non-boot CPUs ...
CPU1: shutdown
Retrying again to check for CPU kill
CPU1 killed.
Enabling non-boot CPUs ...
CPU1 is up
PM: noirq resume of devices complete after 1.996 msecs
PM: early resume of devices complete after 1.152 msecs
usb usb1: root hub lost power or was reset
usb usb2: root hub lost power or was reset
- <> --
After Fix:
root@phoenix:~# echo mem > /sys/power/state
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.001 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
PM: suspend of devices complete after 103.086 msecs
PM: late suspend of devices complete after 1.517 msecs
PM: noirq suspend of devices complete after 1.217 msecs
Disabling non-boot CPUs ...
CPU1: shutdown
Retrying again to check for CPU kill
CPU1 killed.
Enabling non-boot CPUs ...
CPU1 is up
PM: noirq resume of devices complete after 1.991 msecs
PM: early resume of devices complete after 1.239 msecs
usb usb1: root hub lost power or was reset
usb usb2: root hub lost power or was reset
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
xhci-hcd xhci-hcd.0.auto: Error while assigning device slot ID
xhci-hcd xhci-hcd.0.auto: Max number of devices this xHCI host supports is 127.
PM: resume of devices complete after 75567.769 msecs
Restarting tasks ...
usb 1-1: USB disconnect, device number 2
usb 2-1: USB disconnect, device number 2
usb 2-1.1: USB disconnect, device number 3
done.
root@phoenix:~#
Signed-off-by: Sriram Dash
Signed-off-by: Rajesh Bhagat
---
drivers/usb/core/hub.c | 12
drivers/usb/host/xhci-ring.c |2 +-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 38cc4ba..c906018 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2897,10 +2897,14 @@ done:
/* The xHC may think the device is already reset,
* so ignore the status.
*/
- if (hcd->driver->reset_device)
- hcd->driver->reset_device(hcd, udev);
-
- usb_set_device_state(udev, USB_STATE_DEFAULT);
+ if (hcd->driver->reset_device) {
+ status = hcd->driver->reset_device(hcd, udev);
+ if (status == 0)
+ usb_set_device_state(udev,
USB_STATE_DEFAULT);
+ else
+ usb_set_device_state(udev,
USB_STATE_NOTATTACHED);
+ } else
+ usb_set_device_state(udev, USB_STATE_DEFAULT);
}
} else {
if (udev)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7cf6621..be8fd61 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1272,9 +1272,9 @@ void xhci_handle_command_timeout(unsigned long data)
Re: [GIT PULL] USB driver patches for 4.6-rc1
On Fri, Mar 18, 2016 at 04:01:51PM -0700, Linus Torvalds wrote: > On Fri, Mar 18, 2016 at 3:51 PM, Linus Torvalds > wrote: > > > > The commit that ends up being marked bad is odd, but there it is: > > 69bec7259853 "USB: core: let USB device know device node". > > Confirmed. Not only did it bisect to that, reverting it on top of the > current kernel fixes my machine. > > So that commit is somehow buggy. I don't see what it does that would > break even with OF disabled, but something does. > > I'll just revert it. The way it is done seems bogus anyway. It looks > at of_node when OF is disabled, but generally that isn't even > initialized as far as I can tell, and we have things like > dev_of_node() helpers to make sure you don't do that. > I am sorry to make things break, Nicolai Stange's found the root cause for this problem, and his patch fixed it. USB device structure (both struct usb_hcd and struct usb_device) is initialized by kzalloc, so the struct device in it is initialized by zero, and will not cause non-initialized for USB device, but you are right, a good practice is using dev_of_node for all devices in case the struct device is not zero-initialized. -- 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: [PATCH] usb: chipidea: Configure DMA properties and ops from DT
> -Original Message- > From: Peter Chen [mailto:hzpeterc...@gmail.com] > Sent: Friday, March 18, 2016 7:24 AM > To: Arnd Bergmann > Cc: Li Yang ; Bjorn Andersson > ; > Peter Chen ; Greg Kroah-Hartman > ; Rajesh Bhagat ; linux- > u...@vger.kernel.org; lkml ; Srinivas Kandagatla > ; linux-arm-msm m...@vger.kernel.org>; linux-arm-ker...@lists.infradead.org > Subject: Re: [PATCH] usb: chipidea: Configure DMA properties and ops from DT > > On Thu, Mar 17, 2016 at 04:52:55PM +0100, Arnd Bergmann wrote: > > On Monday 14 March 2016 18:51:08 Peter Chen wrote: > > > On Wed, Mar 09, 2016 at 05:16:50PM -0600, Li Yang wrote: > > > > On Tue, Mar 8, 2016 at 9:40 PM, Bjorn Andersson > > > > wrote: > > > > > On Tue, Mar 8, 2016 at 11:52 AM, Li Yang wrote: > > > > >> On Wed, Mar 2, 2016 at 4:59 PM, Li Yang wrote: > > > > >>> On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson > > > > >>> wrote: > > > > On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote: > > > > > > > > > > I had the chance to go through this with Arnd and the verdict is > > > > > that devices not described in DT should not do DMA (or allocate > > > > > buffers for doing DMA). > > > > > > > > > > So I believe the solution is to fall back on Peter's > > > > > description; the chipidea driver is the core driver and the > > > > > Qualcomm code should just be a platform layer. > > > > > > > > > > My suggestion is that we turn the chipidea core into a set of > > > > > APIs that can called by the platform specific pieces. That way > > > > > we will have the chipidea core be the device described in the DT. > > > > > > > > But like I said, this problem is not just existing for chipidea > > > > driver. We already found that the dwc3 driver is also suffering > > > > from the same issue. I don't know how many other drivers are > > > > impacted by this change, but I suspect there will be some. A grep > > > > of > > > > platform_device_add() in driver/ directory returns many possible > > > > drivers to be impacted. As far as I know, the > > > > drivers/net/ethernet/freescale/fman/mac.c is registering child > > > > ethernet devices that definitely will do dma. If you want to do this > > > > kind of rework to all these drivers, it will be a really big effort. > > > > > > > > > > +1 > > > > > > Yes, I think this DMA things should be covered by driver core too. > > > > > > > I don't think it's a very widespread problem, there are only very few > > developers that intentionally use this method, and some use the > > platform_device_register_full() call to create a device with a known > > mask, which is generally ok for the limited case where the driver is > > only ever going to run on a single platform, but not in the more > > general case that of_dma_configure is designed to handle. > > Even only for qualcomm platforms, it may be possible have different DMA masks > at > ARM64 platforms, so we may can't use a fixed value at glue layer driver. So, > using > of_dma_configure is suitable choice for DT platforms for this case, right? > > > > > I think we should fix the drivers to consistently use the device that > > was created by the platform (DT or ACPI or board file) to pass that > > into the DMA API, anything else will just cause more subtle bugs. > > > > Although I don't know what kinds of bugs it may have, it may be met before, > otherwise, why most of platform drivers need to call dma_set_coherent_mask or > dma_coerce_mask_and_coherent explicitly > > -- > > Best Regards, > Peter Chen Though chipidea platform drivers are calling functions mentioned by you i.e. dma_set_coherent_mask or dma_coerce_mask_and_coherent explicity e.g. in file drivers/usb/chipidea/ci_hdrc_imx.c. Still the mentioned error is coming while calling ci_hdrc_add_device which lies in chipidea/core.c. similar is the case with DWC3 driver. Best Regards, Rajesh Bhagat -- 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] cypress_m8: add sanity checking
An attack using missing endpoints exists. CVE-2016-3137 Signed-off-by: Oliver Neukum CC: sta...@vger.kernel.org --- drivers/usb/serial/cypress_m8.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c index 01bf533..1c6cbf5 100644 --- a/drivers/usb/serial/cypress_m8.c +++ b/drivers/usb/serial/cypress_m8.c @@ -447,6 +447,9 @@ static int cypress_generic_port_probe(struct usb_serial_port *port) struct usb_serial *serial = port->serial; struct cypress_private *priv; + if (!port->interrupt_out_urb || !port->interrupt_in_urb) + return -ENODEV; + priv = kzalloc(sizeof(struct cypress_private), GFP_KERNEL); if (!priv) return -ENOMEM; -- 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 v2 01/11] ARM: davinci: defined missing CFGCHIP2_REFFREQ_* macros for MUSB PHY
From: Petr Kulhavy Only few MUSB PHY reference clock frequencies were defined. This patch defines macros for the missing frequencies: 19.2MHz, 38.4MHz, 13MHz, 26MHz, 20MHz, 40MHz Signed-off-by: Petr Kulhavy . Acked-by: Sergei Shtylyov Signed-off-by: Bin Liu --- v2 changes: None. include/linux/platform_data/usb-davinci.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h index e0bc4ab..961d3dc 100644 --- a/include/linux/platform_data/usb-davinci.h +++ b/include/linux/platform_data/usb-davinci.h @@ -33,6 +33,12 @@ #define CFGCHIP2_REFFREQ_12MHZ (1 << 0) #define CFGCHIP2_REFFREQ_24MHZ (2 << 0) #define CFGCHIP2_REFFREQ_48MHZ (3 << 0) +#define CFGCHIP2_REFFREQ_19_2MHZ (4 << 0) +#define CFGCHIP2_REFFREQ_38_4MHZ (5 << 0) +#define CFGCHIP2_REFFREQ_13MHZ (6 << 0) +#define CFGCHIP2_REFFREQ_26MHZ (7 << 0) +#define CFGCHIP2_REFFREQ_20MHZ (8 << 0) +#define CFGCHIP2_REFFREQ_40MHZ (9 << 0) struct da8xx_ohci_root_hub; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb/core: usb_alloc_dev(): fix setting of ->portnum
On Fri, Mar 18, 2016 at 10:13:34AM -0400, Alan Stern wrote:
> On Thu, 17 Mar 2016, Nicolai Stange wrote:
>
> > With commit 69bec7259853 ("USB: core: let USB device know device node"),
> > the port1 argument of usb_alloc_dev() gets overwritten as follows:
> >
> > ... usb_alloc_dev(..., unsigned port1)
> > {
> > ...
> > if (!parent->parent) {
> > port1 = usb_hcd_find_raw_port_number(..., port1);
> > }
> > ...
> > }
> >
> > Later on, this now overwritten port1 gets assigned to ->portnum:
> >
> > dev->portnum = port1;
> >
> > However, since xhci_find_raw_port_number() isn't idempotent, the
> > aforementioned commit causes a number of KASAN splats like the following:
>
> ...
>
> > Fix this by not overwriting the port1 argument in usb_alloc_dev(), but
> > storing the raw port number as required by OF in an additional variable,
> > raw_port.
> >
> > Fixes: 69bec7259853 ("USB: core: let USB device know device node")
> > Signed-off-by: Nicolai Stange
> > ---
> > Applicable to linux-next-20160317
> >
> > Changes to v1:
> > - Initialize raw_port with port1
>
> Acked-by: Alan Stern
>
Thanks for fixing it in time, you are right. My patch would let the
device logical port number equals to device raw port number, but it
is wrong for xhci.
--
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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
If CONFIG_PM=n:
drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no member
named ‘runtime_auto’
If PM is disabled, the runtime_auto flag is not available, but auto
suspend is not enabled anyway. Hence protect the check for runtime_auto
by #ifdef CONFIG_PM to fix this.
Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
Reported-by: Guenter Roeck
Signed-off-by: Geert Uytterhoeven
---
Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper to
include/linux/pm.h, which always return false if CONFIG_PM is disabled.
The only other user in non-core code (drivers/usb/core/sysfs.c) has a
big #ifdef CONFIG_PM check around all PM-related code.
Thoughts?
---
drivers/net/usb/lan78xx.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index d36d5ebf37f355f2..7b9ac47b2ecf9905 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3271,7 +3271,9 @@ struct rtnl_link_stats64 *lan78xx_get_stats64(struct
net_device *netdev,
* periodic reading from HW will prevent from entering USB auto suspend.
* if autosuspend is disabled, read from HW.
*/
+#ifdef CONFIG_PM
if (!dev->udev->dev.power.runtime_auto)
+#endif
lan78xx_update_stats(dev);
mutex_lock(&dev->stats.access_lock);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
On Thu, Mar 17, 2016 at 09:01:25PM +0100, poma wrote:
> On 17.03.2016 19:02, Jes Sorensen wrote:
> > Jes Sorensen writes:
> >> Xose Vazquez Perez writes:
> >>> Hi,
> >>>
> >>> If I do:
> >>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
> >>
> >> Hi Xose,
> >>
> >> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
> >> field in struct use_device_id to carry information for the different
> >> types of devices. If you hot add a device like above, the driver will
> >> fail because that field now contains a NULL pointer.
> >>
> >> I should probably add a check for it in the probe function, but it will
> >> simply be there to spit out a warning that it doesn't work to hot add a
> >> device like this.
> >>
> >> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
> >> included in the device list.
> >>
> >> Cheers,
> >> Jes
> >
> > Hi Xose,
> >
> > I added the following patch to my tree to avoid this.
> >
> > Cheers,
> > Jes
> >
> > commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
> > Author: Jes Sorensen
> > Date: Thu Mar 17 13:53:48 2016 -0400
> >
> > rtl8xxxu: Fix OOPS if user tries to add device via /sys
> >
> > This driver relies on driver_info in struct usb_device_id, hence
> > adding a device via /sys/bus/usb/drivers/rtl8xxxu/new_id would result
> > in a NULL pointer dereference.
> >
> > Instead print a message and return -ENODEV
> >
> > Reported-by: Xose Vazquez Perez
> > Signed-off-by: Jes Sorensen
> >
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > index 8d893f4..55fc00e 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > @@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct usb_interface
> > *interface,
> >
> > udev = usb_get_dev(interface_to_usbdev(interface));
> >
> > + if (!id->driver_info) {
> > + dev_warn(&udev->dev,
> > +"rtl8xxxu relies on driver_info in struct
> > usb_device_id.\n");
> > + dev_warn(&udev->dev,
> > +"Adding a device via
> > /sys/bus/usb/drivers/rtl8xxxu/new_id is not supported!\n");
We do have a flag in the USB driver structure to prevent this from
happening, "no_dynamic_id", please just set that and then this should
not happen.
thanks,
gre k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/7] usb: mux: add driver for Intel gpio controlled port mux
In some Intel platforms, a single usb port is shared between USB host
and device controller. The shared port is under control of GPIO pins.
This patch adds the support for USB GPIO controlled port mux.
Signed-off-by: David Cohen
Signed-off-by: Lu Baolu
Reviewed-by: Heikki Krogerus
Reviewed-by: Felipe Balbi
Signed-off-by: Fengguang Wu
---
MAINTAINERS | 1 +
drivers/usb/mux/Kconfig | 9 +++
drivers/usb/mux/Makefile | 1 +
drivers/usb/mux/intel-mux-gpio.c | 120 +++
4 files changed, 131 insertions(+)
create mode 100644 drivers/usb/mux/intel-mux-gpio.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 0dbee11..99bc198 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11405,6 +11405,7 @@ L: linux-usb@vger.kernel.org
S: Supported
F: include/linux/usb/intel-mux.h
F: drivers/usb/mux/intel-mux.c
+F: drivers/usb/mux/intel-mux-gpio.c
USB PRINTER DRIVER (usblp)
M: Pete Zaitcev
diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
index 62e2cc3..2b197c1 100644
--- a/drivers/usb/mux/Kconfig
+++ b/drivers/usb/mux/Kconfig
@@ -9,4 +9,13 @@ config INTEL_USB_MUX
Common code for all Intel dual role port mux drivers. All Intel
usb port mux drivers should select it.
+config INTEL_MUX_GPIO
+ tristate "Intel dual role port mux controlled by GPIOs"
+ depends on GPIOLIB
+ depends on X86 && ACPI
+ select INTEL_USB_MUX
+ help
+ Say Y here to enable support for Intel dual role port mux
+ controlled by GPIOs.
+
endmenu
diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
index 84f0ae8..66f765c 100644
--- a/drivers/usb/mux/Makefile
+++ b/drivers/usb/mux/Makefile
@@ -2,3 +2,4 @@
# Makefile for USB port mux drivers
#
obj-$(CONFIG_INTEL_USB_MUX)+= intel-mux.o
+obj-$(CONFIG_INTEL_MUX_GPIO) += intel-mux-gpio.o
diff --git a/drivers/usb/mux/intel-mux-gpio.c b/drivers/usb/mux/intel-mux-gpio.c
new file mode 100644
index 000..df71283
--- /dev/null
+++ b/drivers/usb/mux/intel-mux-gpio.c
@@ -0,0 +1,120 @@
+/*
+ * USB Dual Role Port Mux driver controlled by gpios
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ * Author: David Cohen
+ * Author: Lu Baolu
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+
+struct vuport {
+ struct device *dev;
+ struct gpio_desc *gpio_vbus_en;
+ struct gpio_desc *gpio_usb_mux;
+};
+
+static struct vuport *vup;
+
+/*
+ * id == 0, HOST connected, USB port should be set to peripheral
+ * id == 1, HOST disconnected, USB port should be set to host
+ *
+ * Peripheral: set USB mux to peripheral and disable VBUS
+ * Host: set USB mux to host and enable VBUS
+ */
+static inline int vuport_set_port(struct device *dev, int id)
+{
+ dev_dbg(dev, "USB PORT ID: %s\n", id ? "HOST" : "PERIPHERAL");
+
+ gpiod_set_value_cansleep(vup->gpio_usb_mux, !id);
+ gpiod_set_value_cansleep(vup->gpio_vbus_en, id);
+
+ return 0;
+}
+
+static int vuport_cable_set(struct device *dev)
+{
+ return vuport_set_port(dev, 1);
+}
+
+static int vuport_cable_unset(struct device *dev)
+{
+ return vuport_set_port(dev, 0);
+}
+
+static int vuport_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ vup = devm_kzalloc(dev, sizeof(*vup), GFP_KERNEL);
+ if (!vup)
+ return -ENOMEM;
+
+ /* retrieve vbus and mux gpios */
+ vup->gpio_vbus_en = devm_gpiod_get_optional(dev,
+ "vbus_en", GPIOD_ASIS);
+ if (IS_ERR(vup->gpio_vbus_en))
+ return PTR_ERR(vup->gpio_vbus_en);
+
+ vup->gpio_usb_mux = devm_gpiod_get_optional(dev,
+ "usb_mux", GPIOD_ASIS);
+ if (IS_ERR(vup->gpio_usb_mux))
+ return PTR_ERR(vup->gpio_usb_mux);
+
+ vup->dev = dev;
+
+ return intel_usb_mux_bind_cable(dev, NULL, "USB-HOST",
+ vuport_cable_set,
+ vuport_cable_unset);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+ return intel_usb_mux_unbind_cable(&pdev->dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * In case a micro A cable was plugged in while device was sleeping,
+ * we missed the interrupt. We need to poll usb id gpio when waking the
+ * driver to detect the missed event.
+ * We use 'complete' callback to give time to all extcon listeners to
+ * resume before we send new events.
+ */
+static const struct dev_pm_ops vuport_pm_ops = {
+ .complete = intel_usb_mux_complete,
+};
+#endif
+
+static const struct platform_device_id vuport_platform_ids[] = {
+ { .name = "intel-mux-gpio", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(pl
Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support
On 03/04/2016 09:19 AM, Thierry Reding wrote:
From: Thierry Reding
Extend the binding to cover the set of feature found in Tegra210.
Acked-by: Stephen Warren
diff --git
a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
+ padctl@0,7009f000 {
...
+ pads {
...
+ };
+
+ ports {
...
+ };
As a comment not affecting my ack in any way: At the top-level, we place
all the child nodes into "array container" nodes named "pads" and
"ports". This is nice since it separates different types of child nodes
and allows easily adding more types of child nodes in the future without
interference, and in a way that allows us to explicitly know what each
node is without having to interpret its name or compatible value to do
so. However, we haven't done this with the per-lane child nodes inside
each pad. If we were to rev the design, I'd be tempted to suggest:
padctl@0,7009f000 {
pads {
usb2 {
lanes { // This level is new
usb2-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 v2 02/11] ARM: davinci: add set_parent callback for mux clocks
Introduce a set_parent callback that will be used for mux clocks, such as
the USB PHY muxes and the async3 clock domain mux.
Signed-off-by: David Lechner
---
v2 changes: This is a new patch in v2.
arch/arm/mach-davinci/clock.c | 17 -
arch/arm/mach-davinci/clock.h | 1 +
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
index 3424eac6..3143e8b 100644
--- a/arch/arm/mach-davinci/clock.c
+++ b/arch/arm/mach-davinci/clock.c
@@ -195,6 +195,13 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
return -EINVAL;
mutex_lock(&clocks_mutex);
+ if (clk->set_parent) {
+ int ret = clk->set_parent(clk, parent);
+ if (ret) {
+ mutex_unlock(&clocks_mutex);
+ return ret;
+ }
+ }
clk->parent = parent;
list_del_init(&clk->childnode);
list_add(&clk->childnode, &clk->parent->children);
@@ -224,8 +231,16 @@ int clk_register(struct clk *clk)
mutex_lock(&clocks_mutex);
list_add_tail(&clk->node, &clocks);
- if (clk->parent)
+ if (clk->parent) {
+ if (clk->set_parent) {
+ int ret = clk->set_parent(clk, clk->parent);
+ if (ret) {
+ mutex_unlock(&clocks_mutex);
+ return ret;
+ }
+ }
list_add_tail(&clk->childnode, &clk->parent->children);
+ }
mutex_unlock(&clocks_mutex);
/* If rate is already set, use it */
diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
index 1e4e836..e2a5437 100644
--- a/arch/arm/mach-davinci/clock.h
+++ b/arch/arm/mach-davinci/clock.h
@@ -106,6 +106,7 @@ struct clk {
int (*reset) (struct clk *clk, bool reset);
void (*clk_enable) (struct clk *clk);
void (*clk_disable) (struct clk *clk);
+ int (*set_parent) (struct clk *clk, struct clk *parent);
};
/* Clock flags: SoC-specific flags start at BIT(16) */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 3/5] usb: gadget: pch_udc: enable MSI if hardware supports
Try to enable MSI in case hardware supports it. At least Intel Quark is known
SoC which indeed does.
Signed-off-by: Andy Shevchenko
---
drivers/usb/gadget/udc/pch_udc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index 49fd97f..7736799 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -3132,6 +3132,8 @@ static int pch_udc_probe(struct pci_dev *pdev,
if (pch_udc_pcd_init(dev))
return -ENODEV;
+ pci_enable_msi(pdev);
+
retval = devm_request_irq(&pdev->dev, pdev->irq, pch_udc_isr,
IRQF_SHARED, KBUILD_MODNAME, dev);
if (retval) {
--
2.7.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 v7 1/4] gadget: Introduce the usb charger framework
This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.
The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.
The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state. It also supplies the
notification mechanism to userspace When the usb charger state is changed.
Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.
This patch doesn't yet integrate with the gadget code, so some functions
which rely on the 'gadget' are not completed, that will be implemented
in the following patches.
Signed-off-by: Baolin Wang
---
drivers/usb/gadget/Kconfig |7 +
drivers/usb/gadget/Makefile |1 +
drivers/usb/gadget/charger.c| 669 +++
include/linux/usb/usb_charger.h | 164 ++
4 files changed, 841 insertions(+)
create mode 100644 drivers/usb/gadget/charger.c
create mode 100644 include/linux/usb/usb_charger.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index af5d922..82a5b3c 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -133,6 +133,13 @@ config U_SERIAL_CONSOLE
help
It supports the serial gadget can be used as a console.
+config USB_CHARGER
+ bool "USB charger support"
+ help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
source "drivers/usb/gadget/udc/Kconfig"
#
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 598a67d..1e421c1 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -10,3 +10,4 @@ libcomposite-y:= usbstring.o config.o
epautoconf.o
libcomposite-y += composite.o functions.o configfs.o u_f.o
obj-$(CONFIG_USB_GADGET) += udc/ function/ legacy/
+obj-$(CONFIG_USB_CHARGER) += charger.o
diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
new file mode 100644
index 000..82a9973
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,669 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_CUR_PROTECT(50)
+#define DEFAULT_SDP_CUR_LIMIT (500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_DCP_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_CDP_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_ACA_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define UCHGER_STATE_LENGTH(50)
+
+static DEFINE_IDA(usb_charger_ida);
+static struct bus_type usb_charger_subsys = {
+ .name = "usb-charger",
+ .dev_name = "usb-charger",
+};
+
+static struct usb_charger *dev_to_uchger(struct device *udev)
+{
+ return container_of(udev, struct usb_charger, dev);
+}
+
+static ssize_t sdp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
+}
+
+static ssize_t sdp_limit_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ unsigned int sdp_limit;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &sdp_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = usb_charger_set_cur_limit_by_type(uchger, SDP_TYPE, sdp_limit);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_RW(sdp_limit);
+
+static ssize_t dcp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.dcp_cur_limit);
+}
+
+static ssize_t dcp_limit_store(struct device *dev,
+
Re: problems with cp210x
On Fri, Mar 18, 2016 at 10:25:29AM +0100, walter harms wrote: > > > Am 17.03.2016 18:04, schrieb Greg KH: > > On Thu, Mar 17, 2016 at 05:41:13PM +0100, walter harms wrote: > >> > >> > >> Am 29.02.2016 17:10, schrieb Greg KH: > >>> On Mon, Feb 29, 2016 at 02:47:45PM +0100, walter harms wrote: > Hello List, > > is someone aware of a problem with DTR in that driver ? > > I use: Linux version 3.4.53-MMI-SW.401_0.1-43 > >>> > >>> That's a very old and obsolete kernel version. Please ask for support > >>> from the vendor that is forcing you to live with that kernel version, > >>> that is what you are paying for. > >>> > >>> We can help you here if you try the latest kernel release, can you try > >>> 4.4? > >>> > >> > >> Hi, > >> i would like to give same feedback. > >> > >> Using a newer version of linux caused unexpected problems unrelated to the > >> interface > >> but we found a possible trouble spot. > > > > What problems did you have? > > > >> the chip cp2105 has a One Time Programmable register to define DTR etc. > >> we were not aware of that. > >> > >> I would suggest to add an ioctl to get the mode. (I did not see one). > >> so someone has a change to find the problem next time. > > > > The mode of what? The tty line? Something else? > > > > Please be more specific. > > > > > mea culpa, as usual i forgot that not everyone is working on the same problem > > I will write that in the slight hope that the next one will not search for not > existing hardware bugs ... > > Desc: the cp2105 is a USB->serial converter with GPIO option (!) Yes, this keeps coming up on the list, see the archives for some patches that add gpio support to the driver through the gpio layer in the kernel. I think they are still being worked on and not merged yet. > symptom: Devices attached to the serial port that require all Signals do not > work > but hardware (wiring etc) is ok. > Tests showed that toggling DTR signal did not work for no obvious > reason. >(RTS did work). > > reason: (tested on cp2105 prototype board) > the chip cp2105 has a One Time Programmable register to define > modem/gpio mode. > The configuration is normally done by the producer of the board. > If that register is not set (in our case) the DTR is not working. > > Note the documentation says: "Up to 1024 Bytes of EEPROM or OTP ROM" > but most times > the documentation refers to "EEPROM". > > suggestion: > there should be an ioctl() related to the configuration in the eeprom. If there's an EEPROM, then we should use the kernel's eeprom layer to properly access it. I don't know if anyone has started doing that, again, check the archives. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On 16 March 2016 at 19:20, Mark Brown wrote: > On Wed, Mar 16, 2016 at 01:05:27PM +0200, Felipe Balbi wrote: >> Mark Brown writes: >> > On Mon, Feb 29, 2016 at 11:22:12PM +0900, Mark Brown wrote: >> >> On Mon, Jan 04, 2016 at 11:04:26AM +0800, Baolin Wang wrote: > >> > I see Felipe is no longer at TI so his e-mail was bouncing - let's >> > resend this with his kernel.org address: > >> I don't have the patches on my inbox. Neither on kernel.org nor on my > > Right, they were last posted in January before you updated MAINTAINERS > so they'll have gone to your TI address. > >> linux.intel.com account. Care to resend ? > > Baolin, can you do that please? OK. I'd like to do it again. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
