Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist
Why do we even need that? If you take patch that makes ulpi_init a subsys_initcall you won't have this problem, and no additional weird hacks and errors will be needed On Sun, May 24, 2015 at 11:09 AM, Sudip Mukherjee sudipm.mukher...@gmail.com wrote: On Sun, May 24, 2015 at 12:19:48AM -0700, Greg KH wrote: On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: ULPI registers it's bus at module_init so if the bus fails to register, the module will fail to load and all will be well in the world. However, if the ULPI code is built-in rather than a module, the bus initialization may fail but we'd still try to register drivers later onto a non-existant bus, which will panic the kernel. Fix that by checking that the bus was indeed initialized before trying to register drivers on top of it. Signed-off-by: Sasha Levin sasha.le...@oracle.com --- drivers/usb/common/ulpi.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 0e6f968..0b0a5e7 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) if (!drv-probe) return -EINVAL; + /* Was the bus registered successfully? */ + if (!ulpi_bus.p) + return -ENODEV; Ick, no, don't go mucking around in the bus internals like this, that's not ok. You should either know the bus is registered, or something is really wrong with the design here. can't we use a variable which can be initialized to 1 in ulpi_init() if the bus registers successfully and later check that? regards sudip greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- 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 v2 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On Thu, May 21, 2015 at 08:11:27PM -0700, David Cohen wrote: On Thu, May 21, 2015 at 08:09:54PM -0700, David Cohen wrote: Hi, On Fri, May 22, 2015 at 10:07:05AM +0800, Lu Baolu wrote: Many drivers and modules depend on ULPI bus registeration to register ULPI interfaces and drivers. It's more appropriate to register ULPI bus in subsys_initcall instead of module_init. Kernel panic has been reported with some kind of kernel config. Even though I agree subsys_initcall is better to register ulpi bus, it's still no excuse to have kernel panic. What about ULPI bus being compiled as module? IMHO this is avoiding the proper kernel panic fix which should be failing gracefully (or defer probe) from tusb1210 driver. Maybe I need to express myself better :) IMHO we should not consider work done with this patch only. It's still incomplete. Then please fix it properly, this is not the correct solution. 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 1/1] usb: ulpi: don't register drivers if bus doesn't exist
On Thu, May 21, 2015 at 04:57:52PM +0800, Lu Baolu wrote: ULPI registers its bus at module_init, so if the bus fails to register, the module will fail to load and all will be well in the world. However, if the ULPI code is built-in rather than a module, the bus initialization may fail, but we'd still try to register drivers later onto a non-existent bus, which will panic the kernel. Fix that by checking that the bus was indeed initialized before trying to register drivers on top of it. Signed-off-by: Sasha Levin sasha.le...@oracle.com Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Signed-off-by: Lu Baolu baolu...@linux.intel.com Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com Reviewed-by: David Cohen david.a.co...@linux.intel.com --- drivers/usb/common/ulpi.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 0e6f968..af52b46 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) if (!drv-probe) return -EINVAL; + /* Was the bus registered successfully? */ + if (unlikely(WARN_ON(!ulpi_bus.p))) never use unlikely/likely unless you can actually measure the difference if the marking is not used. Hint, on driver probe time, you can't. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XHCI, brain-dead scanner, and microframe rounding
Aww, that's too bad. Let me know if you'd like me to test a modified version when you get the time. --Mike Mammarella On May 21, 2015, at 4:18 AM, Mathias Nyman wrote: Hi The fix went upstream, but caused regression for other users, and had to be reverted. The cause of the regression was found but the new version was never properly tested and got left behind as more urgent issues arrived. I still need to attend a few other issues before taking up this again -Mathias On 21.05.2015 13:38, Hans-Peter Jansen wrote: Dear Mathias, just a heads up: retesting with 4.0.4 revealed, that this issue isn't fixed for my scanner still. To recap: driving the scanner through a ehci port is fine, and fails miserably with xhci. OK: T: Bus=03 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 4 D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 P: Vendor=1d6b ProdID=0002 Rev=04.00 S: Manufacturer=Linux 4.0.4-2.g4f5e0d5-desktop ehci_hcd S: Product=EHCI Host Controller S: SerialNumber=:06:04.2 C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=0mA I: If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub T: Bus=03 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#= 3 Spd=480 MxCh= 0 D: Ver= 2.00 Cls=ff(vend.) Sub=ff Prot=ff MxPS=64 #Cfgs= 1 P: Vendor=04b8 ProdID=0119 Rev=01.00 S: Manufacturer=EPSON S: Product=EPSON Scanner C: #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr=2mA I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none) NOT OK: T: Bus=06 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh=14 D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=1d6b ProdID=0002 Rev=04.00 S: Manufacturer=Linux 4.0.4-2.g4f5e0d5-desktop xhci-hcd S: Product=xHCI Host Controller S: SerialNumber=:00:14.0 C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=0mA I: If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub T: Bus=06 Lev=01 Prnt=01 Port=10 Cnt=02 Dev#= 10 Spd=480 MxCh= 0 D: Ver= 2.00 Cls=ff(vend.) Sub=ff Prot=ff MxPS=64 #Cfgs= 1 P: Vendor=04b8 ProdID=0119 Rev=01.00 S: Manufacturer=EPSON S: Product=EPSON Scanner C: #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr=2mA I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=usbfs Additional notes: xsane scanner discovery takes ages (20-30 secs) to find the scanner in the failing case. After selecting the correct device, it takes another delay of 20-30 secs. for presenting the error dialog: error during device I/O. The same procedure with ehci takes about a second until the device selection is shown, and another 0.5 secs later it presents the fully functional scanning UI. This behavior persists since Linux 3.16.x (where I setup this box). Please let me know, if I can be of any help for you for resolving this issue. I find it a little sad, that at the dawn of USB 3.1, we still fight with such issues on the linux USB 3.0 front. Don't forget the many frustrated users observing this, that will not speak up. Cheers, Pete On Donnerstag, 29. Januar 2015 18:42:05 Mathias Nyman wrote: On 27.01.2015 14:12, Gunter Königsmann wrote: That's very good news indeed. Will re-run the tests on my scanner and looking forward to the fix entering mainline. In the meantime I can acknowledge that with the fix my computer accepts USB memory sticks that normally didn't work. Kind regards, Gunter. Did some cleaning of the patch, and noticed it still had a few bits wrong, but apparently it worked anyway. I added the fixes on top of the ep_reset_halt_test branch. Can any of you with a failing scanner test that it still works? git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git and the ep_reset_halt_test branch, Thanks -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 -- 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: renesas_usbhs: avoid uninitialized variable use
On Fri, May 22, 2015 at 11:33:57AM +, Yoshihiro Shimoda wrote:
Hi Arnd,
Sent: Friday, May 22, 2015 8:07 PM
After the renesas_usbhs driver is enabled in ARM multi_v7_defconfig,
we now get a new warning:
renesas_usbhs/mod.c: In function 'usbhs_interrupt':
renesas_usbhs/mod.c:246:7: warning: 'intenb1' may be used uninitialized in
this function [-Wmaybe-uninitialized]
gcc correctly points to a problem here, for the case that the
device is in host mode, we use the intenb1 variable without
having assigned it first. The state-intsts1 has a similar
problem, but gcc cannot know that.
This avoids the problem by initializing both sides of the
comparison to zero when we don't read them from the respective
registers.
Signed-off-by: Arnd Bergmann a...@arndb.de
Fixes: 88a25e02f3 (usb: renesas_usbhs: Add access control for INTSTS1 and
INTENB1 register)
Thank you very much for the patch!
Acked-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
(I'm not sure why a toolchain I used (Linaro GCC 2014.11) doesn't show this
warning...)
Best regards,
Yoshihiro Shimoda
Reviewed-by: Simon Horman horms+rene...@verge.net.au
diff --git a/drivers/usb/renesas_usbhs/mod.c
b/drivers/usb/renesas_usbhs/mod.c
index e5ce6e6d4f51..d4be5d594896 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -223,6 +223,8 @@ static int usbhs_status_get_each_irq(struct usbhs_priv
*priv,
if (usbhs_mod_is_host(priv)) {
state-intsts1 = usbhs_read(priv, INTSTS1);
intenb1 = usbhs_read(priv, INTENB1);
+ } else {
+ state-intsts1 = intenb1 = 0;
}
/* mask */
--
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: usb: gadget: f_fs: O_NONBLOCK waits MAX_SCHEDULE_TIMEOUT
FunctionFS is very specific, because read/write operations are directly translated into USB requests, which are asynchronous, so you cannot use O_NONBLOCK. If you need non-blocking API you can use Asynchronous I/O (AIO). You can find some examples in kernel sources (tools/usb/ffs-aio-example/). Br, Robert Baldyga Thank you, that sounds like the best approach. In this case I think perhaps the long wait without any data is an problem with the imx6 Chipidea USB controller. What's the possible problem? Sorry for the delay in replying, I have been getting some more details with a USB Analyser. The scenario is that the NCM device is enumerating so we see the messages to: SetAddress (1) GetDescriptor (Device) GetDescriptor (StringN) GetDescriptor (Configuration) SetConfiguration (1) GetDescriptor (String iInterface) GetDescriptor (String iInterface) At this point the NCM host sends Writes to the F_FS EP0 but for some reason the host device does not respond and only issues SOF packets for hours. This happens occasionally and is fixed by turning the device off and on again. We may find this 'some reason', is it device error or host error? Do you have below patch in your code: commit 953c66469735aed8d2ada639a72b150f01dae605 Author: Abbas Raza abbas_r...@mentor.com Date: Thu Jul 17 19:34:31 2014 +0800 usb: chipidea: udc: Disable auto ZLP generation on ep0 There are 2 methods for ZLP (zero-length packet) generation: 1) In software 2) Automatic generation by device controller 1) is implemented in UDC driver and it attaches ZLP to IN packet if descriptor-size wLength 2) can be enabled/disabled by setting ZLT bit in the QH Peter Unless I am mistaken from a NCM gadget point of view the attached device is working correctly and there is no way to know it has failed, is that correct? I guess it should suspend and drop the connections if there is no traffic for more than 10ms? If the Device side NAK host's IN/OUT token continually, the pipe will not be stopped, the host will send token continually until the application cancel this request. -- 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] drivers: usb :fsl: Add support for USB controller version-2.5
-Original Message-
From: Greg KH [mailto:g...@kroah.com]
Sent: Monday, September 29, 2014 11:56 AM
To: Badola Nikhil-B46172
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH] drivers: usb :fsl: Add support for USB controller
version-
2.5
On Mon, Sep 29, 2014 at 03:46:02AM +, nikhil.bad...@freescale.com
wrote:
-Original Message-
From: Greg KH [mailto:g...@kroah.com]
Sent: Wednesday, September 24, 2014 10:19 AM
To: Badola Nikhil-B46172
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH] drivers: usb :fsl: Add support for USB
controller version-2.5
On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote:
Add support for USB controller version-2.5 used in
T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A.
Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com
---
- Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124
drivers: usb: fsl: Check IP version 2.4 for mph USB controller
drivers/usb/host/fsl-mph-dr-of.c | 5 +
include/linux/fsl_devices.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/usb/host/fsl-mph-dr-of.c
b/drivers/usb/host/fsl-mph-dr-of.c
index e0315de..45b9e36 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -127,6 +127,7 @@ static int usb_get_ver_info(struct device_node
*np)
* returns 1 for usb controller version 1.6
* returns 2 for usb controller version 2.2
* returns 3 for usb controller version 2.4
+ * returns 4 for usb controller version 2.5
* returns 0 otherwise
*/
if (of_device_is_compatible(np, fsl-usb2-dr)) { @@ -136,6
+137,8 @@ static int usb_get_ver_info(struct device_node *np)
ver = FSL_USB_VER_2_2;
else if (of_device_is_compatible(np,
fsl-usb2-dr-v2.4))
ver = FSL_USB_VER_2_4;
+else if (of_device_is_compatible(np,
fsl-usb2-dr-v2.5))
+ver = FSL_USB_VER_2_5;
else /* for previous controller versions */
ver = FSL_USB_VER_OLD;
@@ -153,6 +156,8 @@ static int usb_get_ver_info(struct device_node
*np)
ver = FSL_USB_VER_2_2;
else if (of_device_is_compatible(np,
fsl-usb2-mph-v2.4))
ver = FSL_USB_VER_2_4;
+else if (of_device_is_compatible(np,
fsl-usb2-mph-v2.5))
+ver = FSL_USB_VER_2_5;
else /* for previous controller versions */
ver = FSL_USB_VER_OLD;
}
diff --git a/include/linux/fsl_devices.h
b/include/linux/fsl_devices.h index a82296a..2a2f56b 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -24,6 +24,7 @@
#define FSL_USB_VER_1_6 1
#define FSL_USB_VER_2_2 2
#define FSL_USB_VER_2_4 3
+#define FSL_USB_VER_2_5 4
Why not just keep going and add the rest of the numbers while you are at
it?
This is the last controller version of this family hence there would
not be any requirement to add further numbers.
People always bring products back, you never know this :)
Seriously, what are these two patches doing? You just set the value,
never do anything with it, what good is it?
We indeed use these macros for controller version specific code, for
example in following snippet from ehci-fsl.c if (pdata-have_sysif_regs
pdata-controller_ver FSL_USB_VER_1_6
(phy_mode == FSL_USB2_PHY_ULPI)) {
/* check PHY_CLK_VALID to get phy clk valid */ .
.
If we don't set the macros then by default FSL_USB_VER_OLD, which is
0, is assigned as controller version and in that case phy_clk_valid bit
would
not be checked for controller version 2.4 and 2.5.
You are relying on a define to be a specific value, which seems wrong as
it's
impossible to figure this type of thing out. Please use an enumerated type at
the very least if you want to test this type of thing.
Patch for replacing macros with enumerated type sent
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 00/11] usbip: features to USB over WebSocket
Hello, I see your point and what you have done in patches. I'm only showing you that you may achieve almost the same effect without any changes in kernel. I tested wstunnel. The performance comparison in my environment is as following. Round trip time of keyboard key down and up URBs at host. wstunnel: 4.4msec usbws(my patch): 2.7msec wstunnel passes TCP/IP stack twice so it's slower. I'd like to keep the usbws implementation because it's worth for performance. memory usage (VSZ/RSS) wstunnel usbws Server wstunnel 734K/20K usbipa25K/2K usbwsa 25K/2K Client wstunnel 75K/17K usbipa 113K/3K Memo about wstunnel 1) Implementation node.js application 2) installation # yum install npm # npm install -g wstunnel 3) Usage for USB/IP app# insmod usbip-core.ko app# isnmod vhci-hcd.ko app# usbipa -t 3240 app$ wstunnel -s 8080 -t localhost:3240 dev# insmod usbip-core.ko dev# isnmod usbip-host.ko dev$ wstunnel -t 3240 ws://app-side-addr:8080 dev# usbip -t 3240 connect -r localhost -b bus-id usbip -loop-back- wstunnel ---net--- wstunnel -loopback- usbipa 3240 8080 3240 Some issues are found in interoperability (to use usbws with wstunnel). a) wstunnel needs Sec-WebSocket-Protocol header with value 'tunnel-protocol'. It means that the other end must be wstunnel too. b) wstunnel sends multiple packets into one frame. usbws must implement buffering. c) wstunnel sends WebSocket with FIN bit=1. Poco library translates it to OP_CLOSE and succeeding readFrame() doesn't work. I may fix later regarding b) if it's needed by other implementations. Sorry for my late response. I concentrated to other project. Thank you for your help, n.iwata //
Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist
On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: ULPI registers it's bus at module_init so if the bus fails to register, the module will fail to load and all will be well in the world. However, if the ULPI code is built-in rather than a module, the bus initialization may fail but we'd still try to register drivers later onto a non-existant bus, which will panic the kernel. Fix that by checking that the bus was indeed initialized before trying to register drivers on top of it. Signed-off-by: Sasha Levin sasha.le...@oracle.com --- drivers/usb/common/ulpi.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 0e6f968..0b0a5e7 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) if (!drv-probe) return -EINVAL; + /* Was the bus registered successfully? */ + if (!ulpi_bus.p) + return -ENODEV; Ick, no, don't go mucking around in the bus internals like this, that's not ok. You should either know the bus is registered, or something is really wrong with the design here. 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] usb: ulpi: don't register drivers if bus doesn't exist
On Sun, May 24, 2015 at 12:19:48AM -0700, Greg KH wrote: On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: ULPI registers it's bus at module_init so if the bus fails to register, the module will fail to load and all will be well in the world. However, if the ULPI code is built-in rather than a module, the bus initialization may fail but we'd still try to register drivers later onto a non-existant bus, which will panic the kernel. Fix that by checking that the bus was indeed initialized before trying to register drivers on top of it. Signed-off-by: Sasha Levin sasha.le...@oracle.com --- drivers/usb/common/ulpi.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c index 0e6f968..0b0a5e7 100644 --- a/drivers/usb/common/ulpi.c +++ b/drivers/usb/common/ulpi.c @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) if (!drv-probe) return -EINVAL; + /* Was the bus registered successfully? */ + if (!ulpi_bus.p) + return -ENODEV; Ick, no, don't go mucking around in the bus internals like this, that's not ok. You should either know the bus is registered, or something is really wrong with the design here. can't we use a variable which can be initialized to 1 in ulpi_init() if the bus registers successfully and later check that? regards sudip greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- 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
