Re: [PATCH] staging: dwc2: fix thinko in dwc2_hc_set_even_odd_frame()
On 07/17/2013 02:07 PM, Paul Zimmerman wrote: ... With the two additional patches I sent out yesterday and today, the driver is working really well for me on the Pi with your mainline- based kernel. I was able to run an overnight test copying data to/from a thumb drive (not to the root fs due to the mmcqd issue I mentioned above) while doing some light Ethernet traffic, and didn't see any issues, where before that would cause the USB to hang within a few minutes. With those patches applied, I get good results on most things, for example: Download a kernel .tar.xz file and write to built-in SD (at 2.5M/s!), a few times, using the built-in USB-based wired Ethernet. dd an entire USB-hosted SD card to /dev/null. apt-get install a couple small packages while doing that dd. Run concordance on a Logitech Harmony remote; identify it, and dump the firmware and config to a file on SD card. All that seemed to work fine. I still have the issue where SD card plug/unplug from a USB SD reader isn't recognized. Perhaps that's a kernel config issue, or perhaps there are still some USB issues? I was also able to successfully connect a WiFi dongle, where before it would not enumerate. I was not able to use the WiFi (maybe there is some network support missing from the kernel .config?) but it's still progress. I then wanted to try WiFi, so I plugged in a USB mouse/keyboard, and started X, trying to use GUI tools. Then I saw some issues. With just the USB mouse/keyboard attached (via a powered hub), and no WiFi device yet, they would work for a while, but pretty soon I kept seeing all USB devices just disappear; only the Linux Foundation 2.0 root hub would be left. Unplugging and replugging didn't fix this; I had to power-cycle. I wonder if there are issues with just USB interrupt transfers, which I assume both HID devices and the USB SD card plug/unplug notifications use?? I tried plugging in the WiFi device and no mouse/keyboard, and retrying a download of a large file using the built-in USB wired Ethernet, and that still worked fine. Unfortunately, I need to read up a bit more on how to configure WiFi without a GUI tool, or switch to Network Manager which I know how to configure manually, before I can really test WiFi. iwlist wlan0 scanning did seem to work fine though. So, definitely moving in the right direction:-) But, a few issues left yet. -- 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 01/15] drivers: phy: add generic PHY framework
On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:13 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id); Your naming is odd, no phy anywhere in it? You rely on the sender to never send a duplicate name.id pair? Why not create your own ids based on the number of phys in the system, like almost all other classes and subsystems do? hmm.. some PHY drivers use the id they provide to perform some of their internal operation as in [1] (This is used only if a single PHY provider implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO to give the PHY drivers an option to use auto id. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, who cares about the id? No one outside of the phy core ever should, because you pass back the only pointer that they really do care about, if they need to do anything with the device. Use that, and then you can hmm.. ok. rip out all of the search for a phy by a string logic, as that's not Actually this is needed for non-dt boot case. In the case of dt boot, we use a phandle by which the controller can get a reference to the phy. But in the case of non-dt boot, the controller can get a reference to the phy only by label. I don't understand. They registered the phy, and got back a pointer to it. Why can't they save it in their local structure to use it again later if needed? They should never have to ask for the device, as the One is a *PHY provider* driver which is a driver for some PHY device. This will use phy_create to create the phy. The other is a *PHY consumer* driver which might be any controller driver (can be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the phy (by *phandle* in the case of dt boot and *label* in the case of non-dt boot). device id might be unknown if there are multiple devices in the system. I agree with you on the device id part. That need not be known to the PHY driver. How does a consumer know which label to use in a non-dt system if there are multiple PHYs in the system? Do you have any drivers that are non-dt using this yet? 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 01/15] drivers: phy: add generic PHY framework
Hi, On Friday 19 July 2013 11:59 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:13 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id); Your naming is odd, no phy anywhere in it? You rely on the sender to never send a duplicate name.id pair? Why not create your own ids based on the number of phys in the system, like almost all other classes and subsystems do? hmm.. some PHY drivers use the id they provide to perform some of their internal operation as in [1] (This is used only if a single PHY provider implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO to give the PHY drivers an option to use auto id. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, who cares about the id? No one outside of the phy core ever should, because you pass back the only pointer that they really do care about, if they need to do anything with the device. Use that, and then you can hmm.. ok. rip out all of the search for a phy by a string logic, as that's not Actually this is needed for non-dt boot case. In the case of dt boot, we use a phandle by which the controller can get a reference to the phy. But in the case of non-dt boot, the controller can get a reference to the phy only by label. I don't understand. They registered the phy, and got back a pointer to it. Why can't they save it in their local structure to use it again later if needed? They should never have to ask for the device, as the One is a *PHY provider* driver which is a driver for some PHY device. This will use phy_create to create the phy. The other is a *PHY consumer* driver which might be any controller driver (can be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the phy (by *phandle* in the case of dt boot and *label* in the case of non-dt boot). device id might be unknown if there are multiple devices in the system. I agree with you on the device id part. That need not be known to the PHY driver. How does a consumer know which label to use in a non-dt system if there are multiple PHYs in the system? That should be passed using platform data. Do you have any drivers that are non-dt using this yet? yes. tw4030 (used in OMAP3) supports non-dt. [PATCH 04/15] ARM: OMAP: USB: Add phy binding information [PATCH 06/15] usb: musb: omap2430: use the new generic PHY framework of this patch series shows how it's used. Thanks Kishon -- 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: How should we handle isochronous underruns?
Ming Lei wrote: On Fri, Jul 19, 2013 at 5:20 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 18 Jul 2013, Clemens Ladisch wrote: In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. I prefer a function call over the flag. The function call can easily be issued just once, but the completion routine would have to clear the flag every time the URB gets used. IMO, one good candidate is to do it in usb_set_interface() This will result in a control request to be sent to the device, which might confuse its firmware. also it is reasonable, see blow: From 5.6.3 Isochronous Transfer Packet Size Constraints of USB spec 2.0: All device default interface settings must not include any isochronous endpoints with non-zero data payload sizes. That's what the spec says. In reality, there are devices that have non- zero data payload sizes in the default interface setting, and have no other alternate setting. that said all drivers which are using isoc endpoints have to call usb_set_interface(altsetting) explicitly before starting isoc schedule. But a set_interface request does not necessarily affect a single stream; there are devices that have multiple isochronous and bulk endpoints in a single interface, and where restarting one stream must not affect the others. Maybe we can use usb_reset_endpoint() for this purpose after all. It is a perfect fit, because we want to tell the HCD to reset the isochronous endpoint back to the start of stream state. I suggest not to introduce extra starting stream function to usb_reset_endpoint(), and if we have to do this, I suggest to add a new API for the purpose cleanly. I agree. Regards, Clemens -- 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] usbserial: append Petatel NP10T device to GSM modems list
This patch was tested on 3.10.1 kernel.
Same models of Petatel NP10T modems have different device IDs.
Unfortunately they have no additional revision information on a board
which may treat them as different devices. Currently I've seen only
two NP10T devices with various IDs. Possibly Petatel NP10T list will
be appended upon devices with new IDs will appear.
Signed-off-by: Daniil Bolsun dan.bol...@gmail.com
--- linux-3.10.1/drivers/usb/serial/option.c2013-07-01 01:13:29.0
+0300
+++ linux-3.10.1.np10t/drivers/usb/serial/option.c 2013-07-18
16:15:36.902600324 +0300
@@ -446,7 +446,8 @@ static void option_instat_callback(struc
/* Hyundai Petatel Inc. products */
#define PETATEL_VENDOR_ID 0x1ff4
-#define PETATEL_PRODUCT_NP10T 0x600e
+#define PETATEL_PRODUCT_NP10T_600A 0x600a
+#define PETATEL_PRODUCT_NP10T_600E 0x600e
/* TP-LINK Incorporated products */
#define TPLINK_VENDOR_ID 0x2357
@@ -1333,7 +1334,8 @@ static const struct usb_device_id option
{ USB_DEVICE_AND_INTERFACE_INFO(MEDIATEK_VENDOR_ID,
MEDIATEK_PRODUCT_DC_4COM2, 0xff, 0x02, 0x01) },
{ USB_DEVICE_AND_INTERFACE_INFO(MEDIATEK_VENDOR_ID,
MEDIATEK_PRODUCT_DC_4COM2, 0xff, 0x00, 0x00) },
{ USB_DEVICE(CELLIENT_VENDOR_ID, CELLIENT_PRODUCT_MEN200) },
- { USB_DEVICE(PETATEL_VENDOR_ID, PETATEL_PRODUCT_NP10T) },
+ { USB_DEVICE(PETATEL_VENDOR_ID, PETATEL_PRODUCT_NP10T_600A) },
+ { USB_DEVICE(PETATEL_VENDOR_ID, PETATEL_PRODUCT_NP10T_600E) },
{ USB_DEVICE(TPLINK_VENDOR_ID, TPLINK_PRODUCT_MA180),
.driver_info = (kernel_ulong_t)net_intf4_blacklist },
{ USB_DEVICE(CHANGHONG_VENDOR_ID, CHANGHONG_PRODUCT_CH690) },
--
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
Investment
ATTENTION. Let me use the liberty of this medium to inform you that my principal is interested in investing his bond as a silent business partner in your company. He would like to invest in private sector projects with an established company in any project(s) which are already in the market and have market value or new company requiring the injection of huge funds, provided there are lots of opportunities available, taking into proper consideration the Return on Investment (ROI) based on a ten (10) year strategic plan. Kindly indicate your interest in my client’s proposition by furnishing me with your Bio data, business/personal contact details and any other information/detail that may help in the actualization of the impending investment portfolio. Thanks, Milos -- 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] ux500 dma short transfers on MUSB
On Thu, Jul 11, 2013 at 10:44 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/11/2013 06:58 PM, Greg KH wrote: following scenario: you attach an UART-TO-USB adapter to your musb port running ux500-dma code. The USB UARt driver queues 1x RX URB with the size of 256 bytes (example) and the max packet size is 64. The other side sends only one byte because it is mean. That's actually quite common, why is it mean? Hehe. Because it is the unexpected one. common depends on the use case. Mass storage doesn't do this. Not sure if network notices the difference, maybe ncm. Now, the way I understand it is, you tell musb that the complete transfer of 256 bytes has ended instead one byte that really happened. Is my assumption wrong? What do you mean by tell musb? Of course the transfer has completed, that's all the device sent to the host controller, so it has to complete the transfer and send that on up to the driver that requested the urb. I don't understand the question/problem you are asking here, care to be more descriptive? Okay. musb offloads the actual transfer to the DMA engine it is using. Once it does so, it relies on whatever comes back from dma engine regarding transfer complete, transferred size etc. AFAIK ux500 musb dma code handles data which is multiple of max packet size in DMA. 1 byte should be in PIO mode. Which version of kernel you are using ? In case of ux500-dma (as far as I can tell) musb forwards the RX request to the DMA engine, which will receive one byte instead of the requested 256bytes. Since the DMA engine did not inform musb about the correct transfer size, musb will complete that URB with 256 bytes. If you take a look on ux500_dma_callback() you will see the line: ux500_channel-channel.actual_len = ux500_channel-cur_len; -actual_len is what musb thinks got transferred. -cur_len is what musb asked to transfer. I don't see where the case of a shorter transfer is considered. Again I have no HW I was just browsing. thanks, greg k-h Sebastian -- 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: [RFC] ux500 dma short transfers on MUSB
On 07/19/2013 09:59 AM, Rajaram R wrote: Okay. musb offloads the actual transfer to the DMA engine it is using. Once it does so, it relies on whatever comes back from dma engine regarding transfer complete, transferred size etc. AFAIK ux500 musb dma code handles data which is multiple of max packet size in DMA. 1 byte should be in PIO mode. Which version of kernel you are using ? I am looking at v3.11-rc1 right now and I don't have the HW. As I said: The URB scheduled for receive is 256 bytes in size, max packet size is 64. So DMA should be chosen, right? The UART on other side sends just one byte so the DMA receives just one byte regardless what has been requested. My question is how musb gets notified of this one byte. It might happen someplace but I don't see it. Sebastian -- 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: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
Alan Stern wrote:
On Thu, 18 Jul 2013, Ming Lei wrote:
On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu
wrote:
On Thu, 4 Jul 2013, Alan Stern wrote:
On Thu, 4 Jul 2013, Ming Lei wrote:
I had some more ideas about this. Instead of requiring drivers to set
URB_ISO_ASAP on just the first URB of an isochronous stream, we could
ask drivers to call usb_reset_endpoint() between streams. This would
tell the HCD that the next URB marks the start of a new stream, with no
need for a special flag.
This idea still requires changes from old drivers.
Also it might be not appropriate to call usb_reset_endpoint() in this case
because other host controller's implementation may do other unnecessary
thing for this situation.
Perhaps. I doubt that HCDs need to do anything when they reset an
isochronous endpoint, but you're right. It's safer to avoid the issue.
Another possibility, which would be even simpler, is for HCDs to assume
that if the endpoint's queue has been empty for more than 100 ms then a
new stream is starting. Then drivers wouldn't have to do anything
special at all. (Unless they are stopping and restarting streams very
rapidly,
... which happens when a stream is restarted after an error, or when two
sound files are played back-to-back. The audio driver will always
explicitly restart the stream (because checking whether the timeout has
elapsed would be too much of a bother), so the timeout is not useful
in practice.
In this case, we may use changing altsetting to decide start of streaming.
Yes indeed. But that could still require changes to old drivers.
To be even more safe, unlinking an URB should mark the end of a stream.
That's what snd-usb-audio does when it closes a stream; it kills all
the outstanding URBs.
But it might be possible that the queue is empty at that point.
In any case, there must be _some_ mechanism to explicitly restart
a stream. I do not really care if this is some URB flag or some
function call.
Thought about the problem further, looks there is one simple
approach for detecting underrun in case of empty queue:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Any comments on this way?
Thanks,
--
Ming Lei
--
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] ux500 dma short transfers on MUSB
On Fri, Jul 19, 2013 at 1:36 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/19/2013 09:59 AM, Rajaram R wrote: Okay. musb offloads the actual transfer to the DMA engine it is using. Once it does so, it relies on whatever comes back from dma engine regarding transfer complete, transferred size etc. AFAIK ux500 musb dma code handles data which is multiple of max packet size in DMA. 1 byte should be in PIO mode. Which version of kernel you are using ? I am looking at v3.11-rc1 right now and I don't have the HW. As I said: The URB scheduled for receive is 256 bytes in size, max packet size is 64. So DMA should be chosen, right? The UART on other side sends just one byte so the DMA receives just one byte regardless what has been requested. My question is how musb gets notified of this one byte. It might happen someplace but I don't see it. We program the DMA only when we receive RX interrupt and when the length of data is known. When you submit URB for RX the flow would be something like musb_start_urb==musb_ep_program, Note here we do not program the DMA. DMA is programmed musb_host_rx using channel_program callback. Note here we pass the length of data received. Now in ux500_dma file 'ux500_dma_is_compatible' fails the dma operation and the driver switches to FIFO mode in the same function (musb_host_rx). Cheers Sebastian -- 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/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
Hi Peter, thanks for forwarding this to the appropriate people mailing list. Hi Sarah, thanks for starting investigating this :) On Fri, Jul 19, 2013 at 1:37 AM, Peter Hurley pe...@hurleysoftware.com wrote: Before we revert to using the workaround, I'd like to suggest that this new hidden problem may be an interaction with the xhci_hcd host controller driver only. Looking at the related bug, the OP indicates the machine only has USB3 ports. Additionally, comments #7, #100, and #104 of the original bug report [1] add additional information that would seem to confirm this suspicion. Definitively, this is a USB3 problem. However, it is not generic (I can not reproduce it with my USB3 boards.) Question: does this USB device need a control transfer to reset its endpoints when the endpoints are not actually halted? If so, yes, that is a known xHCI driver bug that needs to be fixed. The xHCI host will not accept a Reset Endpoint command when the endpoints are not actually halted, but the USB core will send the control transfer to reset the endpoint. That means the device and host toggles will be out of sync, and all messages will start to fail with -EPIPE. Can the OP capture a usbmon trace when the device starts failing? That will reveal whether this actually is the issue. dmesg output with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on would also be helpful. Here is another linux-input thread were you have the usbmon traces: http://www.spinics.net/lists/linux-input/msg26542.html Wujun Zhou already did one test of a kernel patch for me (which did not solve the problem, because I was not at the USB level), so I bet he will be able to do some testings for you. In the logs he posted (logitech_work.pcapng.gz), the interesting part is starting from the capture #45: #45: SET_REPORT request to switch the receiver to the DJ mode (the receiver stops sending regular HID events, but goes into its proprietary protocol) #47: SET_REPORT response - all good #48: SET_REPORT request to ask the receiver to enumerate all of his devices (it is called right after we received the previous response) #49: SET_REPORT response - -EPIPE #50: URB_INTERRUPT_IN (~3 seconds later) - the device is working normally The weird thing is that only the first enumeration message failed with -EPIPE: the device answers later control transfer correctly (#54 / #55). Sarah, I forwarded your usbmon capture request to the OP in the bug report (I don't have an email address for the reporter). Here are some other helpful information: the first fix we have done is dcd9006b1b053c7b1c. It is linked to several bugs: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1072082 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1039143 https://bugzilla.redhat.com/show_bug.cgi?id=840391 https://bugzilla.kernel.org/show_bug.cgi?id=49781 Most of them are people complaining, but in one of the comments, adding a 500ms wait between the two control transfer (switch to DJ + enumerate) fixed the -EPIPE problem. I interpreted it as a scheduled problem (using direct call to usb_control_msg() vs use the scheduled one usbhid_submit_message()) but it was just delaying the problem out of the probe. Unfortunately, I missed that as I did not asked for the usbmon traces at that time. One last thing, I understood that Linus is also experiencing this problem... Adding him in CC to let him know of the progress. Cheers, Benjamin -- 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 0/4] usb: musb: fix USB enumeration issue in OMAP3 platform
Felipe, On Tuesday 16 July 2013 06:12 PM, Felipe Balbi wrote: Hi, On Tue, Jul 16, 2013 at 11:22:38AM +0530, Kishon Vijay Abraham I wrote: Tony, On Monday 08 July 2013 04:44 PM, Kishon Vijay Abraham I wrote: This series fixes the USB enumeration issue caused because of the controller not able to get a reference to the PHY because of incorrect binding in the board file. In the case of non-dt boot, the platform specific initialization file (board file) will do usb_bind_phy that binds the usb controller with the PHY using device names. After the device names are created using PLATFORM_DEVID_AUTO, our original method of binding by device names doesn't work reliably (since the device name changes). Hence the usb controller is made to use labels for getting the PHY. Here the PHY name is added in the plat data of USB controller device which should be used by the controller driver to get the PHY. Two new APIs usb_get_phy_by_name and devm_usb_get_phy_by_name are added to be used by the controller to get the PHY by name. You haven't picked this patch series? yeah and now it's too much to send during the -rc series. Here's what we can do, we can to minimum fix for -rc series (patching the names again) There are a lot of different boards available using OMAP3 and the device names (PHY and MUSB) vary across each board (apparently). It will be difficult to find the device names (for PHY and MUSB) for all the different boards available (partly because of non-availability of all the boards) and test it. Thanks Kishon -- 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] ux500 dma short transfers on MUSB
On 07/19/2013 10:26 AM, Rajaram R wrote: We program the DMA only when we receive RX interrupt and when the length of data is known. When you submit URB for RX the flow would be something like musb_start_urb==musb_ep_program, Note here we do not program the DMA. DMA is programmed musb_host_rx using channel_program callback. Note here we pass the length of data received. Now in ux500_dma file 'ux500_dma_is_compatible' fails the dma operation and the driver switches to FIFO mode in the same function (musb_host_rx). Okay. This is completely different from what I expected. That means while the URB is submitting you only program the musb controller for receive and nothing else. The next interrupt will notify that the musb's endpoint fifo is filled with X bytes and you program the dma engine to transfer X bytes. And then you wait for another interrupt which says DMA transfer completed. So in that case you don't have that problem I though you do. Thank you for clearing that up. ux500_dma_is_compatible() is testing length 512 and length 3. This looks like the fifo can be filled with more than 512 bytes. Do you know on top of your head how much that can be? Cheers Sebastian -- 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: How should we handle isochronous underruns?
Ming Lei wrote:
On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
In any case, there must be _some_ mechanism to explicitly restart
a stream. I do not really care if this is some URB flag or some
function call.
Thought about the problem further, looks there is one simple
approach for detecting underrun in case of empty queue:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
or delay URB resubmission to a workqueue?
Regards,
Clemens
--
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] ux500 dma short transfers on MUSB
On Fri, Jul 19, 2013 at 2:23 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/19/2013 10:26 AM, Rajaram R wrote: We program the DMA only when we receive RX interrupt and when the length of data is known. When you submit URB for RX the flow would be something like musb_start_urb==musb_ep_program, Note here we do not program the DMA. DMA is programmed musb_host_rx using channel_program callback. Note here we pass the length of data received. Now in ux500_dma file 'ux500_dma_is_compatible' fails the dma operation and the driver switches to FIFO mode in the same function (musb_host_rx). Okay. This is completely different from what I expected. That means while the URB is submitting you only program the musb controller for receive and nothing else. The next interrupt will notify that the musb's endpoint fifo is filled with X bytes and you program the dma engine to transfer X bytes. And then you wait for another interrupt which says DMA transfer completed. So in that case you don't have that problem I though you do. Thank you for clearing that up. ux500_dma_is_compatible() is testing length 512 and length 3. This looks like the fifo can be filled with more than 512 bytes. Do you know on top of your head how much that can be? That was work in progress. You may wish to look at this patch http://permalink.gmane.org/gmane.linux.usb.general/79858 If a device sends 513 bytes to a musb device. 512 bytes will be received in DMA mode and 1 byte in FIFO mode. FIFO will always handle data 512. Cheers Sebastian -- 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] ux500 dma short transfers on MUSB
On 07/19/2013 11:16 AM, Rajaram R wrote: That was work in progress. You may wish to look at this patch http://permalink.gmane.org/gmane.linux.usb.general/79858 If a device sends 513 bytes to a musb device. 512 bytes will be received in DMA mode and 1 byte in FIFO mode. FIFO will always handle data 512. Okay. Let me rephrase this: Lets say there is a read request for 4096 bytes and your maxpacket is 512 and total of 4096 bytes will be sent by the device. How many interrupts / read requests will there be? 8 like every maxpacket size or is it possible to stuff more data and transfer 4096 in one go? Cheers Sebastian -- 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] ux500 dma short transfers on MUSB
On Fri, Jul 19, 2013 at 2:51 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/19/2013 11:16 AM, Rajaram R wrote: That was work in progress. You may wish to look at this patch http://permalink.gmane.org/gmane.linux.usb.general/79858 If a device sends 513 bytes to a musb device. 512 bytes will be received in DMA mode and 1 byte in FIFO mode. FIFO will always handle data 512. Okay. Let me rephrase this: Lets say there is a read request for 4096 bytes and your maxpacket is 512 and total of 4096 bytes will be sent by the device. How many interrupts / read requests will there be? 8 like every maxpacket size or is it possible to stuff more data and transfer 4096 in one go? Not 100% sure, but as i remember it will be 8 times in the present driver (atleast host mode). But yes we can do 4096 at a time. Cheers Sebastian -- 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: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
In any case, there must be _some_ mechanism to explicitly restart
a stream. I do not really care if this is some URB flag or some
function call.
Thought about the problem further, looks there is one simple
approach for detecting underrun in case of empty queue:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()? Anyway we need to consider current drivers'
implementation.
or delay URB resubmission to a workqueue?
Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
but it isn't a big deal:
- they have worked well for long time in the underrun situation before
switching to tasklet, so we needn't worry about the change introduced
by switching URB-complete() to tasklet.
or
- change the resubmission into complete() since there are very few
such usage(I only found two drivers which resubmit isoc URBs in tasklet
when I was doing urb complete() cleanup work)
Thanks,
--
Ming Lei
--
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 09/21] usb/gadget: f_mass_storage: use fsg_common_setup in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 19 +++
1 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index f1b0da8..0f40d35 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3019,16 +3019,9 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
return ERR_PTR(-EINVAL);
}
- /* Allocate? */
- if (!common) {
- common = kzalloc(sizeof *common, GFP_KERNEL);
- if (!common)
- return ERR_PTR(-ENOMEM);
- common-free_storage_on_release = 1;
- } else {
- memset(common, 0, sizeof *common);
- common-free_storage_on_release = 0;
- }
+ common = fsg_common_setup(common, !!common);
+ if (IS_ERR(common))
+ return common;
common-sysfs = true;
common-state = FSG_STATE_IDLE;
@@ -3068,8 +3061,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
}
common-luns = curlun_it;
- init_rwsem(common-filesem);
-
for (i = 0, lcfg = cfg-luns; i nluns; ++i, ++curlun_it, ++lcfg) {
struct fsg_lun *curlun;
@@ -3169,8 +3160,6 @@ buffhds_first_it:
common-can_stall = cfg-can_stall
!(gadget_is_at91(common-gadget));
- spin_lock_init(common-lock);
- kref_init(common-ref);
/* Tell the thread to start working */
common-thread_task =
@@ -3179,8 +3168,6 @@ buffhds_first_it:
rc = PTR_ERR(common-thread_task);
goto error_release;
}
- init_completion(common-thread_notifier);
- init_waitqueue_head(common-fsg_wait);
/* Information */
INFO(common, FSG_DRIVER_DESC , version: FSG_DRIVER_VERSION \n);
--
1.7.0.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 17/21] usb/gadget: f_mass_storage: convert to new function interface with backward compatibility
Converting mass storage to the new function interface requires converting
the USB mass storage's function code and its users.
This patch converts the f_mass_storage.c to the new function interface.
The file is now compiled into a separate usb_f_mass_storage.ko module.
The old function interface is provided by means of a preprocessor conditional
directives. After all users are converted, the old interface can be removed.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/Kconfig |3 +
drivers/usb/gadget/Makefile |2 +
drivers/usb/gadget/acm_ms.c |1 +
drivers/usb/gadget/f_mass_storage.c | 217 +++---
drivers/usb/gadget/f_mass_storage.h |7 +
drivers/usb/gadget/mass_storage.c |1 +
drivers/usb/gadget/multi.c |1 +
7 files changed, 212 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 7d43750..1a06390 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -587,6 +587,9 @@ config USB_F_RNDIS
config USB_U_MS
tristate
+config USB_F_MASS_STORAGE
+ tristate
+
choice
tristate USB Gadget Drivers
default USB_ETH
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 5453908..a1088e8 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -64,6 +64,8 @@ usb_f_rndis-y := f_rndis.o
obj-$(CONFIG_USB_F_RNDIS) += usb_f_rndis.o
u_ms-y := storage_common.o
obj-$(CONFIG_USB_U_MS) += u_ms.o
+usb_f_mass_storage-y := f_mass_storage.o
+obj-$(CONFIG_USB_F_MASS_STORAGE)+= usb_f_mass_storage.o
#
# USB gadget drivers
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index 992ffb0..31aae8f 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -40,6 +40,7 @@
* the runtime footprint, and giving us at least some parts of what
* a gcc --combine ... part1.c part2.c part3.c ... build would.
*/
+#define USB_FMS_INCLUDED
#include f_mass_storage.c
/*-*/
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index ba24236..b56e27f 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -213,6 +213,7 @@
#include linux/spinlock.h
#include linux/string.h
#include linux/freezer.h
+#include linux/module.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -2591,11 +2592,17 @@ void fsg_common_get(struct fsg_common *common)
{
kref_get(common-ref);
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_get);
+#endif
void fsg_common_put(struct fsg_common *common)
{
kref_put(common-ref, fsg_common_release);
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_put);
+#endif
/* check if fsg_num_buffers is within a valid range */
static inline int fsg_num_buffers_validate(unsigned int fsg_num_buffers)
@@ -2633,6 +2640,9 @@ void fsg_common_set_sysfs(struct fsg_common *common, bool
sysfs)
{
common-sysfs = sysfs;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_set_sysfs);
+#endif
int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
{
@@ -2679,6 +2689,9 @@ error_release:
return -ENOMEM;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_set_num_buffers);
+#endif
void fsg_common_free_buffers(struct fsg_common *common)
{
@@ -2695,6 +2708,9 @@ void fsg_common_free_buffers(struct fsg_common *common)
kfree(common-buffhds);
common-buffhds = NULL;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_free_buffers);
+#endif
int fsg_common_set_nluns(struct fsg_common *common, int nluns)
{
@@ -2717,23 +2733,35 @@ int fsg_common_set_nluns(struct fsg_common *common, int
nluns)
return 0;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_set_nluns);
+#endif
void fsg_common_free_luns(struct fsg_common *common)
{
kfree(common-luns);
common-luns = NULL;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_free_luns);
+#endif
void fsg_common_set_ops(struct fsg_common *common,
const struct fsg_operations *ops)
{
common-ops = ops;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_set_ops);
+#endif
void fsg_common_set_private_data(struct fsg_common *common, void *priv)
{
common-private_data = priv;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_set_private_data);
+#endif
int fsg_common_set_cdev(struct fsg_common *common,
struct usb_composite_dev *cdev, bool can_stall)
@@ -2763,6 +2791,9 @@ int fsg_common_set_cdev(struct fsg_common *common,
return 0;
}
+#ifndef USB_FMS_INCLUDED
+EXPORT_SYMBOL(fsg_common_set_cdev);
+#endif
static inline
[PATCH v2 12/21] usb/gadget: f_mass_storage: use fsg_common_set_ops/_private_data in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions which cover all parts of it. Use them. Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/usb/gadget/f_mass_storage.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c index 9981694..b168a50 100644 --- a/drivers/usb/gadget/f_mass_storage.c +++ b/drivers/usb/gadget/f_mass_storage.c @@ -3020,8 +3020,9 @@ struct fsg_common *fsg_common_init(struct fsg_common *common, kfree(common); return ERR_PTR(rc); } - common-ops = cfg-ops; - common-private_data = cfg-private_data; + + fsg_common_set_ops(common, cfg-ops); + fsg_common_set_private_data(common, cfg-private_data); common-gadget = gadget; common-ep0 = gadget-ep0; -- 1.7.0.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 10/21] usb/gadget: f_mass_storage: use fsg_common_set_num_buffers in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 29 +++--
1 files changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 0f40d35..5a129c9 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3001,17 +3001,12 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
struct fsg_config *cfg)
{
struct usb_gadget *gadget = cdev-gadget;
- struct fsg_buffhd *bh;
struct fsg_lun **curlun_it;
struct fsg_lun_config *lcfg;
struct usb_string *us;
int nluns, i, rc;
char *pathbuf;
- rc = fsg_num_buffers_validate(cfg-fsg_num_buffers);
- if (rc != 0)
- return ERR_PTR(rc);
-
/* Find out how many LUNs there should be */
nluns = cfg-nluns;
if (nluns 1 || nluns FSG_MAX_LUNS) {
@@ -3025,15 +3020,12 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
common-sysfs = true;
common-state = FSG_STATE_IDLE;
- common-fsg_num_buffers = cfg-fsg_num_buffers;
- common-buffhds = kcalloc(common-fsg_num_buffers,
- sizeof *(common-buffhds), GFP_KERNEL);
- if (!common-buffhds) {
+ rc = fsg_common_set_num_buffers(common, cfg-fsg_num_buffers);
+ if (rc) {
if (common-free_storage_on_release)
kfree(common);
- return ERR_PTR(-ENOMEM);
+ return ERR_PTR(rc);
}
-
common-ops = cfg-ops;
common-private_data = cfg-private_data;
@@ -3126,21 +3118,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
}
common-nluns = nluns;
- /* Data buffers cyclic list */
- bh = common-buffhds;
- i = common-fsg_num_buffers;
- goto buffhds_first_it;
- do {
- bh-next = bh + 1;
- ++bh;
-buffhds_first_it:
- bh-buf = kmalloc(FSG_BUFLEN, GFP_KERNEL);
- if (unlikely(!bh-buf)) {
- rc = -ENOMEM;
- goto error_release;
- }
- } while (--i);
- bh-next = common-buffhds;
/* Prepare inquiryString */
i = get_default_bcdDevice();
--
1.7.0.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 11/21] usb/gadget: f_mass_storage: use fsg_common_set_nluns in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 22 +-
1 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 5a129c9..9981694 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3007,12 +3007,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
int nluns, i, rc;
char *pathbuf;
- /* Find out how many LUNs there should be */
- nluns = cfg-nluns;
- if (nluns 1 || nluns FSG_MAX_LUNS) {
- dev_err(gadget-dev, invalid number of LUNs: %u\n, nluns);
- return ERR_PTR(-EINVAL);
- }
common = fsg_common_setup(common, !!common);
if (IS_ERR(common))
@@ -3042,17 +3036,12 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
}
fsg_intf_desc.iInterface = us[FSG_STRING_INTERFACE].id;
- /*
-* Create the LUNs, open their backing files, and register the
-* LUN devices in sysfs.
-*/
- curlun_it = kcalloc(nluns, sizeof(*curlun_it), GFP_KERNEL);
- if (unlikely(!curlun_it)) {
- rc = -ENOMEM;
- goto error_release;
- }
- common-luns = curlun_it;
+ rc = fsg_common_set_nluns(common, cfg-nluns);
+ if (rc)
+ goto error_release;
+ curlun_it = common-luns;
+ nluns = cfg-nluns;
for (i = 0, lcfg = cfg-luns; i nluns; ++i, ++curlun_it, ++lcfg) {
struct fsg_lun *curlun;
@@ -3116,7 +3105,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
goto error_luns;
}
}
- common-nluns = nluns;
/* Prepare inquiryString */
--
1.7.0.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 21/21] usb/gadget: f_mass_storage: add configfs support
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../ABI/testing/configfs-usb-gadget-mass-storage | 31 ++
drivers/usb/gadget/Kconfig | 11 +
drivers/usb/gadget/f_mass_storage.c| 368
drivers/usb/gadget/f_mass_storage.h| 17 +
4 files changed, 427 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-mass-storage
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
b/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
new file mode 100644
index 000..e1e918e
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
@@ -0,0 +1,31 @@
+What: /config/usb-gadget/gadget/functions/mass_storage.name
+Date: Jul 2013
+KenelVersion: 3.12
+Description:
+ The attributes:
+
+ stall - Set to permit function to halt bulk endpoints.
+ Disabled on some USB devices known not to work
+ correctly. You should set it to true.
+ num_buffers - Number of pipeline buffers. Valid numbers
+ are 2..4. Available only if
+ CONFIG_USB_GADGET_DEBUG_FILES is set.
+
+What: /config/usb-gadget/gadget/functions/mass_storage.name/lun.name
+Date: Jul 2013
+KenelVersion: 3.12
+Description:
+ The attributes:
+
+ file- The path to the backing file for the LUN.
+ Required if LUN is not marked as removable.
+ ro - Flag specifying access to the LUN shall be
+ read-only. This is implied if CD-ROM emulation
+ is enabled as well as when it was impossible
+ to open filename in R/W mode.
+ removable - Flag specifying that LUN shall be indicated as
+ being removable.
+ cdrom - Flag specifying that LUN shall be reported as
+ being a CD-ROM.
+ nofua - Flag specifying that FUA flag
+ in SCSI WRITE(10,12)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index a678f35..8b0b965 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -727,6 +727,17 @@ config USB_CONFIGFS_PHONET
help
The Phonet protocol implementation for USB device.
+config USB_CONFIGFS_MASS_STORAGE
+ boolean Mass storage
+ depends on USB_CONFIGFS
+ select USB_U_MS
+ select USB_F_MASS_STORAGE
+ help
+ The Mass Storage Gadget acts as a USB Mass Storage disk drive.
+ As its storage repository it can use a regular file or a block
+ device (in much the same way as the loop device driver),
+ specified as a module parameter or sysfs option.
+
config USB_ZERO
tristate Gadget Zero (DEVELOPMENT)
select USB_LIBCOMPOSITE
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 1c5dc0a..70db26a 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -220,6 +220,7 @@
#include linux/usb/composite.h
#include gadget_chips.h
+#include configfs.h
/**/
@@ -3349,6 +3350,350 @@ static int fsg_bind_config(struct usb_composite_dev
*cdev,
#else
+static inline struct fsg_lun_opts *to_fsg_lun_opts(struct config_item *item)
+{
+ return container_of(to_config_group(item), struct fsg_lun_opts, group);
+}
+
+static inline struct fsg_opts *to_fsg_opts(struct config_item *item)
+{
+ return container_of(to_config_group(item), struct fsg_opts,
+ func_inst.group);
+}
+
+CONFIGFS_ATTR_STRUCT(fsg_lun_opts);
+CONFIGFS_ATTR_OPS(fsg_lun_opts);
+
+static void fsg_lun_attr_release(struct config_item *item)
+{
+ struct fsg_lun_opts *lun_opts;
+
+ lun_opts = to_fsg_lun_opts(item);
+ kfree(lun_opts);
+}
+
+static struct configfs_item_operations fsg_lun_item_ops = {
+ .release= fsg_lun_attr_release,
+ .show_attribute = fsg_lun_opts_attr_show,
+ .store_attribute = fsg_lun_opts_attr_store,
+};
+
+static ssize_t fsg_lun_opts_file_show(struct fsg_lun_opts *opts, char *page)
+{
+ struct fsg_opts *fsg_opts;
+
+ fsg_opts = to_fsg_opts(opts-group.cg_item.ci_parent);
+
+ return fsg_show_file(opts-lun, fsg_opts-common-filesem, page);
+}
+
+static ssize_t fsg_lun_opts_file_store(struct fsg_lun_opts *opts,
+ const char *page, size_t len)
+{
+ struct fsg_opts *fsg_opts;
+
+ fsg_opts =
[PATCH v2 00/21] Equivalent of g_mass_storage with configfs
This series aims at integrating configfs into mass storage, the way it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex and phonet. It contains everything that is required to provide the equivalent of g_mass_storage.ko with configfs. Mass storage itself is quite large, so the resulting patch series is a bit lengthy. However, it is supposed to be done in steps like this: 1) preliminary work, e.g. factoring out a header file, creating a utility u_ms.ko module, use usb_gstrings_attach 2) prepare for initializing the fsg_common structure in smaller steps instead of in one big step 3) usual stuff, similar to functions previously converted to configfs v1..v2: - simpified adding a level of indirection for fsg_lun storing - reworked lun debugging macros in order not to use struct device [both after discussion with Alan, thanks] A branch will be available here since 20th July: git://git.infradead.org/users/kmpark/linux-samsung usb-gadget-configfs BACKWARD COMPATIBILITY == Please note that the old g_mass_storage.ko is still available and works. USING THE NEW GADGET == Please refer to this post: http://www.spinics.net/lists/linux-usb/msg76388.html for general information from Sebastian on how to use configfs-based gadgets (*). With configfs the procedure is as follows, compared to the information mentioned above (*): instead of mkdir functions/acm.ttyS1 do mkdir functions/mass_storage.instance name e.g. mkdir functions/mass_storage.0 In functions/function.instance name there will be the following attribute files: stall - Set to permit function to halt bulk endpoints. Disabled on some USB devices known not to work correctly. You should set it to true. num_buffers - Number of pipeline buffers. Valid numbers are 2..4. Available only if CONFIG_USB_GADGET_DEBUG_FILES is set. and a default lun.0 directory corresponding to SCSI LUN #0. A new lun can be added with mkdir: $ mkdir functions/mass_storage.0/partition.5 Lun numbering does not have to be continuous, except for lun #0 which is created by default. A maximum of 8 luns can be specified and they all must be named following the name.number scheme. The numbers can be 0..8. Probably a good convention is to name the luns lun.number, although it is not mandatory. In each lun directory there are the following attribute files: file- The path to the backing file for the LUN. Required if LUN is not marked as removable. ro - Flag specifying access to the LUN shall be read-only. This is implied if CD-ROM emulation is enabled as well as when it was impossible to open filename in R/W mode. removable - Flag specifying that LUN shall be indicated as being removable. cdrom - Flag specifying that LUN shall be reported as being a CD-ROM. nofua - Flag specifying that FUA flag in SCSI WRITE(10,12) The rest of the procedure (*) remains the same. An example gadget with two luns: $ modprobe libcomposite $ mount none cfg -t configfs $ mkdir cfg/usb_gadget/g1 $ cd cfg/usb_gadget/g1 $ mkdir configs/c.1 $ mkdir functions/mass_storage.0 $ echo /root/lun0.img functions/mass_storage.0/lun.0/file $ mkdir functions/mass_storage.0/lun.1 $ echo /root/lun1.img functions/mass_storage.0/lun.1/file $ mkdir strings/0x409 $ mkdir configs/c.1/strings/0x409 $ echo 0xa4a2 idProduct $ echo 0x0525 idVendor $ echo my-serial-num strings/0x409/serialnumber $ echo my-manufacturer strings/0x409/manufacturer $ echo Mass Storage Gadget strings/0x409/product $ echo Conf 1 configs/c.1/strings/0x409/configuration $ echo 120 configs/c.1/MaxPower $ ln -s functions/mass_storage.0 configs/c.1 $ echo s3c-hsotg UDC After unbinding the gadget with echo UDC the symbolic links in the configuration directory can be removed, the strings/* subdirectories in the configuration directory can be removed, the strings/* subdirectories at the gadget level can be removed and the configs/* subdirectories can be removed. The functions/* subdirectories can be removed. After that the gadget directory can be removed. After that the respective modules can be unloaded. TESTING THE FUNCTIONS (actually there is only one) = mass_storage) device: connect the gadget, enable it host: dmesg, see the USB drives appear (if system configured to automatically mount) Andrzej Pietrasiewicz (21): usb/gadget: multi: fix error return code in cdc_do_config() usb/gadget: f_phonet: remove unused preprocessor conditional usb/gadget: configfs: add a method to unregister the gadget usb/gadget: create a utility module for mass_storage usb/gadget:
[PATCH v2 19/21] usb/gadget: storage_common: make attribute operations more generic
Show/store methods for sysfs attributes contain code which can be used
also by configfs. Make them abstract the source the lun and rw_semaphore
are taken from.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 67 ---
drivers/usb/gadget/storage_common.c | 36 --
drivers/usb/gadget/storage_common.h | 21 ++-
3 files changed, 81 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index b56e27f..1c5dc0a 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -2569,14 +2569,71 @@ static int fsg_main_thread(void *common_)
/*** DEVICE ATTRIBUTES ***/
-static DEVICE_ATTR(ro, 0644, fsg_show_ro, fsg_store_ro);
-static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, fsg_store_nofua);
-static DEVICE_ATTR(file, 0644, fsg_show_file, fsg_store_file);
+static ssize_t sysfs_fsg_show_ro(struct device *dev,
+struct device_attribute *attr,
+char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return fsg_show_ro(curlun, buf);
+}
+
+static ssize_t sysfs_fsg_show_nofua(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return fsg_show_nofua(curlun, buf);
+}
+
+static ssize_t sysfs_fsg_show_file(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ struct rw_semaphore *filesem = dev_get_drvdata(dev);
+
+ return fsg_show_file(curlun, filesem, buf);
+}
+
+static ssize_t sysfs_fsg_store_ro(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ struct rw_semaphore *filesem = dev_get_drvdata(dev);
+
+ return fsg_store_ro(curlun, filesem, buf, count);
+}
+
+static ssize_t sysfs_fsg_store_nofua(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+
+ return fsg_store_nofua(curlun, buf, count);
+}
+
+static ssize_t sysfs_fsg_store_file(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fsg_lun *curlun = fsg_lun_from_dev(dev);
+ struct rw_semaphore *filesem = dev_get_drvdata(dev);
+
+ return fsg_store_file(curlun, filesem, buf, count);
+}
+
+static DEVICE_ATTR(ro, 0644, sysfs_fsg_show_ro, sysfs_fsg_store_ro);
+static DEVICE_ATTR(nofua, 0644, sysfs_fsg_show_nofua, sysfs_fsg_store_nofua);
+static DEVICE_ATTR(file, 0644, sysfs_fsg_show_file, sysfs_fsg_store_file);
static struct device_attribute dev_attr_ro_cdrom =
- __ATTR(ro, 0444, fsg_show_ro, NULL);
+ __ATTR(ro, 0444, sysfs_fsg_show_ro, NULL);
static struct device_attribute dev_attr_file_nonremovable =
- __ATTR(file, 0444, fsg_show_file, NULL);
+ __ATTR(file, 0444, sysfs_fsg_show_file, NULL);
/** FSG COMMON **/
diff --git a/drivers/usb/gadget/storage_common.c
b/drivers/usb/gadget/storage_common.c
index 942324c..ab83d11 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -31,11 +31,6 @@
#include storage_common.h
-static inline struct fsg_lun *fsg_lun_from_dev(struct device *dev)
-{
- return container_of(dev, struct fsg_lun, dev);
-}
-
/* There is only one interface. */
struct usb_interface_descriptor fsg_intf_desc = {
@@ -324,31 +319,23 @@ EXPORT_SYMBOL(store_cdrom_address);
/*-*/
-ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
- char *buf)
+ssize_t fsg_show_ro(struct fsg_lun *curlun, char *buf)
{
- struct fsg_lun *curlun = fsg_lun_from_dev(dev);
-
return sprintf(buf, %d\n, fsg_lun_is_open(curlun)
? curlun-ro
: curlun-initially_ro);
}
EXPORT_SYMBOL(fsg_show_ro);
-ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
- char *buf)
+ssize_t fsg_show_nofua(struct fsg_lun *curlun, char *buf)
{
- struct fsg_lun *curlun = fsg_lun_from_dev(dev);
-
[PATCH v2 20/21] usb/gadget: storage_common: add methods to show/store 'cdrom' and 'removable'
This will be required by configfs integration.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/storage_common.c | 42 +++
drivers/usb/gadget/storage_common.h |5
2 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/storage_common.c
b/drivers/usb/gadget/storage_common.c
index ab83d11..d59b555 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -359,6 +359,17 @@ ssize_t fsg_show_file(struct fsg_lun *curlun, struct
rw_semaphore *filesem,
}
EXPORT_SYMBOL(fsg_show_file);
+ssize_t fsg_show_cdrom(struct fsg_lun *curlun, char *buf)
+{
+ return sprintf(buf, %u\n, curlun-cdrom);
+}
+EXPORT_SYMBOL(fsg_show_cdrom);
+
+ssize_t fsg_show_removable(struct fsg_lun *curlun, char *buf)
+{
+ return sprintf(buf, %u\n, curlun-removable);
+}
+EXPORT_SYMBOL(fsg_show_removable);
ssize_t fsg_store_ro(struct fsg_lun *curlun, struct rw_semaphore *filesem,
const char *buf, size_t count)
@@ -439,4 +450,35 @@ ssize_t fsg_store_file(struct fsg_lun *curlun, struct
rw_semaphore *filesem,
}
EXPORT_SYMBOL(fsg_store_file);
+ssize_t fsg_store_cdrom(struct fsg_lun *curlun, const char *buf, size_t count)
+{
+ unsignedcdrom;
+ int ret;
+
+ ret = kstrtouint(buf, 2, cdrom);
+ if (ret)
+ return ret;
+
+ curlun-cdrom = cdrom;
+
+ return count;
+}
+EXPORT_SYMBOL(fsg_store_cdrom);
+
+ssize_t fsg_store_removable(struct fsg_lun *curlun, const char *buf,
+ size_t count)
+{
+ unsignedremovable;
+ int ret;
+
+ ret = kstrtouint(buf, 2, removable);
+ if (ret)
+ return ret;
+
+ curlun-removable = removable;
+
+ return count;
+}
+EXPORT_SYMBOL(fsg_store_removable);
+
MODULE_LICENSE(GPL);
diff --git a/drivers/usb/gadget/storage_common.h
b/drivers/usb/gadget/storage_common.h
index e9d856b..b385240 100644
--- a/drivers/usb/gadget/storage_common.h
+++ b/drivers/usb/gadget/storage_common.h
@@ -236,10 +236,15 @@ ssize_t fsg_show_ro(struct fsg_lun *curlun, char *buf);
ssize_t fsg_show_nofua(struct fsg_lun *curlun, char *buf);
ssize_t fsg_show_file(struct fsg_lun *curlun, struct rw_semaphore *filesem,
char *buf);
+ssize_t fsg_show_cdrom(struct fsg_lun *curlun, char *buf);
+ssize_t fsg_show_removable(struct fsg_lun *curlun, char *buf);
ssize_t fsg_store_ro(struct fsg_lun *curlun, struct rw_semaphore *filesem,
const char *buf, size_t count);
ssize_t fsg_store_nofua(struct fsg_lun *curlun, const char *buf, size_t count);
ssize_t fsg_store_file(struct fsg_lun *curlun, struct rw_semaphore *filesem,
const char *buf, size_t count);
+ssize_t fsg_store_cdrom(struct fsg_lun *curlun, const char *buf, size_t count);
+ssize_t fsg_store_removable(struct fsg_lun *curlun, const char *buf,
+ size_t count);
#endif /* USB_STORAGE_COMMON_H */
--
1.7.0.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 02/21] usb/gadget: f_phonet: remove unused preprocessor conditional
The compatibility layer which the USBF_PHONET_INCLUDED was a part of is no longer present - the USBF_PHONET_INCLUDED is not #defined by anyone anymore, so the ifndef is always true. Removing it. Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/usb/gadget/f_phonet.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c index 7944fb0..3575427 100644 --- a/drivers/usb/gadget/f_phonet.c +++ b/drivers/usb/gadget/f_phonet.c @@ -488,7 +488,6 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f) struct usb_ep *ep; int status, i; -#ifndef USBF_PHONET_INCLUDED struct f_phonet_opts *phonet_opts; phonet_opts = container_of(f-fi, struct f_phonet_opts, func_inst); @@ -507,7 +506,6 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f) return status; phonet_opts-bound = true; } -#endif /* Reserve interface IDs */ status = usb_interface_id(c, f); -- 1.7.0.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 14/21] usb/gadget: f_mass_storage: use fsg_common_create_luns in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 101 ++-
1 files changed, 4 insertions(+), 97 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 9f36e12..3d4406a 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3000,12 +3000,7 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
struct usb_composite_dev *cdev,
struct fsg_config *cfg)
{
- struct usb_gadget *gadget = cdev-gadget;
- struct fsg_lun **curlun_it;
- struct fsg_lun_config *lcfg;
- int nluns, i, rc;
- char *pathbuf;
-
+ int i, rc;
common = fsg_common_setup(common, !!common);
if (IS_ERR(common))
@@ -3030,72 +3025,10 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
rc = fsg_common_set_nluns(common, cfg-nluns);
if (rc)
goto error_release;
- curlun_it = common-luns;
- nluns = cfg-nluns;
- for (i = 0, lcfg = cfg-luns; i nluns; ++i, ++curlun_it, ++lcfg) {
- struct fsg_lun *curlun;
-
- *curlun_it = kzalloc(sizeof(**curlun_it), GFP_KERNEL);
- curlun = *curlun_it;
- if (!curlun) {
- rc = -ENOMEM;
- common-nluns = i;
- goto error_release;
- }
- curlun-name = kzalloc(MAX_LUN_NAME_LEN, GFP_KERNEL);
- if (!curlun-name) {
- rc = -ENOMEM;
- common-nluns = i;
- goto error_release;
- }
- curlun-cdrom = !!lcfg-cdrom;
- curlun-ro = lcfg-cdrom || lcfg-ro;
- curlun-initially_ro = curlun-ro;
- curlun-removable = lcfg-removable;
- curlun-dev.release = fsg_lun_release;
- curlun-dev.parent = gadget-dev;
- /* curlun-dev.driver = fsg_driver.driver; XXX */
- dev_set_drvdata(curlun-dev, common-filesem);
- dev_set_name(curlun-dev, lun%d, i);
- snprintf(curlun-name, MAX_LUN_NAME_LEN,
-dev_name(curlun-dev));
-
- rc = device_register(curlun-dev);
- if (rc) {
- INFO(common, failed to register LUN%d: %d\n, i, rc);
- common-nluns = i;
- put_device(curlun-dev);
- kfree(curlun);
- goto error_release;
- }
-
- rc = device_create_file(curlun-dev,
- curlun-cdrom
- ? dev_attr_ro_cdrom
- : dev_attr_ro);
- if (rc)
- goto error_luns;
- rc = device_create_file(curlun-dev,
- curlun-removable
- ? dev_attr_file
- : dev_attr_file_nonremovable);
- if (rc)
- goto error_luns;
- rc = device_create_file(curlun-dev, dev_attr_nofua);
- if (rc)
- goto error_luns;
-
- if (lcfg-filename) {
- rc = fsg_lun_open(curlun, lcfg-filename);
- if (rc)
- goto error_luns;
- } else if (!curlun-removable) {
- ERROR(common, no file given for LUN%d\n, i);
- rc = -EINVAL;
- goto error_luns;
- }
- }
+ rc = fsg_common_create_luns(common, cfg);
+ if (rc)
+ goto error_release;
/* Prepare inquiryString */
i = get_default_bcdDevice();
@@ -3107,7 +3040,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
: File-Stor Gadget),
i);
-
/* Tell the thread to start working */
common-thread_task =
kthread_create(fsg_main_thread, common, file-storage);
@@ -3120,37 +3052,12 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
INFO(common, FSG_DRIVER_DESC , version: FSG_DRIVER_VERSION \n);
INFO(common, Number of LUNs=%d\n, common-nluns);
- pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
- for (i = 0, nluns = common-nluns, curlun_it = common-luns;
-i nluns;
-++curlun_it, ++i) {
- struct fsg_lun *curlun = *curlun_it;
- char *p = (no medium);
- if
[PATCH v2 07/21] usb/gadget: f_mass_storage: use usb_gstrings_attach
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 25 -
1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index d01d1dd..c56e24b 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -242,6 +242,11 @@ static struct usb_gadget_strings fsg_stringtab = {
.strings= fsg_strings,
};
+static struct usb_gadget_strings *fsg_strings_array[] = {
+ fsg_stringtab,
+ NULL,
+};
+
/*-*/
struct fsg_dev;
@@ -2609,6 +2614,7 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
struct fsg_buffhd *bh;
struct fsg_lun **curlun_it;
struct fsg_lun_config *lcfg;
+ struct usb_string *us;
int nluns, i, rc;
char *pathbuf;
@@ -2651,14 +2657,13 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
common-ep0req = cdev-req;
common-cdev = cdev;
- /* Maybe allocate device-global string IDs, and patch descriptors */
- if (fsg_strings[FSG_STRING_INTERFACE].id == 0) {
- rc = usb_string_id(cdev);
- if (unlikely(rc 0))
- goto error_release;
- fsg_strings[FSG_STRING_INTERFACE].id = rc;
- fsg_intf_desc.iInterface = rc;
+ us = usb_gstrings_attach(cdev, fsg_strings_array,
+ARRAY_SIZE(fsg_strings));
+ if (IS_ERR(us)) {
+ rc = PTR_ERR(us);
+ goto error_release;
}
+ fsg_intf_desc.iInterface = us[FSG_STRING_INTERFACE].id;
/*
* Create the LUNs, open their backing files, and register the
@@ -2951,11 +2956,6 @@ autoconf_fail:
/** ADD FUNCTION **/
-static struct usb_gadget_strings *fsg_strings_array[] = {
- fsg_stringtab,
- NULL,
-};
-
static int fsg_bind_config(struct usb_composite_dev *cdev,
struct usb_configuration *c,
struct fsg_common *common)
@@ -2968,7 +2968,6 @@ static int fsg_bind_config(struct usb_composite_dev *cdev,
return -ENOMEM;
fsg-function.name= FSG_DRIVER_DESC;
- fsg-function.strings = fsg_strings_array;
fsg-function.bind= fsg_bind;
fsg-function.unbind = fsg_unbind;
fsg-function.setup = fsg_setup;
--
1.7.0.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 13/21] usb/gadget: f_mass_storage: use fsg_common_set_cdev in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 23 ++-
1 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index b168a50..9f36e12 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3003,7 +3003,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
struct usb_gadget *gadget = cdev-gadget;
struct fsg_lun **curlun_it;
struct fsg_lun_config *lcfg;
- struct usb_string *us;
int nluns, i, rc;
char *pathbuf;
@@ -3024,19 +3023,9 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
fsg_common_set_ops(common, cfg-ops);
fsg_common_set_private_data(common, cfg-private_data);
- common-gadget = gadget;
- common-ep0 = gadget-ep0;
- common-ep0req = cdev-req;
- common-cdev = cdev;
-
- us = usb_gstrings_attach(cdev, fsg_strings_array,
-ARRAY_SIZE(fsg_strings));
- if (IS_ERR(us)) {
- rc = PTR_ERR(us);
+ rc = fsg_common_set_cdev(common, cdev, cfg-can_stall);
+ if (rc)
goto error_release;
- }
- fsg_intf_desc.iInterface = us[FSG_STRING_INTERFACE].id;
-
rc = fsg_common_set_nluns(common, cfg-nluns);
if (rc)
@@ -3118,14 +3107,6 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
: File-Stor Gadget),
i);
- /*
-* Some peripheral controllers are known not to be able to
-* halt bulk endpoints correctly. If one of them is present,
-* disable stalls.
-*/
- common-can_stall = cfg-can_stall
- !(gadget_is_at91(common-gadget));
-
/* Tell the thread to start working */
common-thread_task =
--
1.7.0.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 04/21] usb/gadget: create a utility module for mass_storage
Converting to configfs requires making the f_mass_storage.c a module.
But first we need to get rid of #include storage_common.c.
This patch makes storage_common.c a separately compiled file, which is
built as a utility module named u_ms.ko. After all mass storage users are
converted to the new function interface this module can be eliminated
by merging it with the mass storage function's module.
USB descriptors are exported so that they can be accessed from
f_mass_storage.
FSG_VENDOR_ID and FSG_PRODUCT_ID are moved to their only user.
Handling of CONFIG_USB_GADGET_DEBUG_FILES is moved to f_mass_storage.c.
The fsg_num_buffers static is moved to FSG_MODULE_PARAMETER users, so
instead of using a global variable the f_mass_storage introduces
fsg_num_buffers member in fsg_common (and fsg_config).
fsg_strings and fsg_stringtab are moved to f_mass_storage.c.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/Kconfig |6 +
drivers/usb/gadget/Makefile |2 +
drivers/usb/gadget/acm_ms.c | 17 ++-
drivers/usb/gadget/f_mass_storage.c | 71 ++--
drivers/usb/gadget/mass_storage.c | 25 +++-
drivers/usb/gadget/multi.c | 17 ++-
drivers/usb/gadget/storage_common.c | 319 ++-
drivers/usb/gadget/storage_common.h | 210 +++
8 files changed, 382 insertions(+), 285 deletions(-)
create mode 100644 drivers/usb/gadget/storage_common.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index abe905f..7d43750 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -584,6 +584,9 @@ config USB_F_SUBSET
config USB_F_RNDIS
tristate
+config USB_U_MS
+ tristate
+
choice
tristate USB Gadget Drivers
default USB_ETH
@@ -937,6 +940,7 @@ config USB_MASS_STORAGE
tristate Mass Storage Gadget
depends on BLOCK
select USB_LIBCOMPOSITE
+ select USB_U_MS
help
The Mass Storage Gadget acts as a USB Mass Storage disk drive.
As its storage repository it can use a regular file or a block
@@ -1060,6 +1064,7 @@ config USB_G_ACM_MS
select USB_LIBCOMPOSITE
select USB_U_SERIAL
select USB_F_ACM
+ select USB_U_MS
help
This driver provides two functions in one configuration:
a mass storage, and a CDC ACM (serial port) link.
@@ -1076,6 +1081,7 @@ config USB_G_MULTI
select USB_U_ETHER
select USB_U_RNDIS
select USB_F_ACM
+ select USB_U_MS
help
The Multifunction Composite Gadget provides Ethernet (RNDIS
and/or CDC Ethernet), mass storage and ACM serial link
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 80e1d17..5453908 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -62,6 +62,8 @@ usb_f_ecm_subset-y:= f_subset.o
obj-$(CONFIG_USB_F_SUBSET) += usb_f_ecm_subset.o
usb_f_rndis-y := f_rndis.o
obj-$(CONFIG_USB_F_RNDIS) += usb_f_rndis.o
+u_ms-y := storage_common.o
+obj-$(CONFIG_USB_U_MS) += u_ms.o
#
# USB gadget drivers
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index 4b947bb..992ffb0 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -104,6 +104,20 @@ static struct usb_gadget_strings *dev_strings[] = {
/** Configurations **/
static struct fsg_module_parameters fsg_mod_data = { .stall = 1 };
+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
+
+static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
+
+#else
+
+/*
+ * Number of buffers we will use.
+ * 2 is usually enough for good buffering pipeline
+ */
+#define fsg_num_buffersCONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
+
+#endif /* CONFIG_USB_DEBUG */
+
FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
static struct fsg_common fsg_common;
@@ -167,7 +181,8 @@ static int __init acm_ms_bind(struct usb_composite_dev
*cdev)
void*retp;
/* set up mass storage function */
- retp = fsg_common_from_params(fsg_common, cdev, fsg_mod_data);
+ retp = fsg_common_from_params(fsg_common, cdev, fsg_mod_data,
+ fsg_num_buffers);
if (IS_ERR(retp)) {
status = PTR_ERR(retp);
return PTR_ERR(retp);
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 56f1fd1..163d911 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -228,8 +228,18 @@
static const char fsg_string_interface[] = Mass Storage;
-#include storage_common.c
+#include storage_common.h
+/* Static strings, in UTF-8 (for simplicity we use only ASCII characters) */
[PATCH v2 15/21] usb/gadget: f_mass_storage: use fsg_common_set_inquiry_string in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 13 +++--
1 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 3d4406a..94b7bc3 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3000,7 +3000,7 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
struct usb_composite_dev *cdev,
struct fsg_config *cfg)
{
- int i, rc;
+ int rc;
common = fsg_common_setup(common, !!common);
if (IS_ERR(common))
@@ -3030,16 +3030,9 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
if (rc)
goto error_release;
- /* Prepare inquiryString */
- i = get_default_bcdDevice();
- snprintf(common-inquiry_string, sizeof common-inquiry_string,
-%-8s%-16s%04x, cfg-vendor_name ?: Linux,
-/* Assume product name dependent on the first LUN */
-cfg-product_name ?: ((*common-luns)-cdrom
-? File-CD Gadget
-: File-Stor Gadget),
-i);
+ fsg_common_set_inquiry_string(common, cfg-vendor_name,
+ cfg-product_name);
/* Tell the thread to start working */
common-thread_task =
kthread_create(fsg_main_thread, common, file-storage);
--
1.7.0.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 03/21] usb/gadget: configfs: add a method to unregister the gadget
Add a method to unregister the gadget using its config_item.
There can be functions (e.g. mass storage), which in some circumstances
need the gadget stopped. Add a method of stopping the gadget.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/configfs.c |8
drivers/usb/gadget/configfs.h |6 ++
2 files changed, 14 insertions(+), 0 deletions(-)
create mode 100644 drivers/usb/gadget/configfs.h
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 80e7f75..cda3ead 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -989,6 +989,14 @@ static struct configfs_subsystem gadget_subsys = {
.su_mutex = __MUTEX_INITIALIZER(gadget_subsys.su_mutex),
};
+void unregister_gadget_item(struct config_item *item)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+
+ unregister_gadget(gi);
+}
+EXPORT_SYMBOL(unregister_gadget_item);
+
static int __init gadget_cfs_init(void)
{
int ret;
diff --git a/drivers/usb/gadget/configfs.h b/drivers/usb/gadget/configfs.h
new file mode 100644
index 000..a7b564a
--- /dev/null
+++ b/drivers/usb/gadget/configfs.h
@@ -0,0 +1,6 @@
+#ifndef USB__GADGET__CONFIGFS__H
+#define USB__GADGET__CONFIGFS__H
+
+void unregister_gadget_item(struct config_item *item);
+
+#endif /* USB__GADGET__CONFIGFS__H */
--
1.7.0.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 08/21] usb/gadget: f_mass_storage: split fsg_common initialization into a number of functions
When configfs is in place, the things related to intialization
of struct fsg_common will be split over a number of places.
This patch adds several functions which together cover the former
intialization routine fsg_common_init.
When configfs is in place, the luns will not be represented in sysfs,
so there will be no struct device associated with a lun.
To prepare for this some debug macros need to be adjusted. Two new
fields are added to struct fsg_lun: name and name_pfx.
The name is for storing a string which is presented to the user
instead of the dev_name. The name_pfx, if non-NULL, is prepended
to the name at printing time.
The name_pfx is for a future lun.0, which will be a default group in
mass_storage.name. By design at USB function configfs group's creation
time its name is not known (but instead set a bit later in
drivers/usb/gadget/configfs.c:function_make) and it is this name that
serves the purpose of the said name prefix. So instead of copying
a yet-unknown string a pointer to it is stored in struct fsg_lun.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 430 +-
drivers/usb/gadget/f_mass_storage.h | 33 +++
drivers/usb/gadget/storage_common.h | 42 +++-
3 files changed, 488 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index c56e24b..f1b0da8 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -299,6 +299,7 @@ struct fsg_common {
unsigned intshort_packet_received:1;
unsigned intbad_lun_okay:1;
unsigned intrunning:1;
+ unsigned intsysfs:1;
int thread_wakeup_needed;
struct completion thread_notifier;
@@ -2606,6 +2607,395 @@ static inline int fsg_num_buffers_validate(unsigned int
fsg_num_buffers)
return -EINVAL;
}
+static struct fsg_common *fsg_common_setup(struct fsg_common *common, bool
zero)
+{
+ if (!common) {
+ common = kzalloc(sizeof(*common), GFP_KERNEL);
+ if (!common)
+ return ERR_PTR(-ENOMEM);
+ common-free_storage_on_release = 1;
+ } else {
+ if (zero)
+ memset(common, 0, sizeof(*common));
+ common-free_storage_on_release = 0;
+ }
+ init_rwsem(common-filesem);
+ spin_lock_init(common-lock);
+ kref_init(common-ref);
+ init_completion(common-thread_notifier);
+ init_waitqueue_head(common-fsg_wait);
+ common-state = FSG_STATE_TERMINATED;
+
+ return common;
+}
+
+void fsg_common_set_sysfs(struct fsg_common *common, bool sysfs)
+{
+ common-sysfs = sysfs;
+}
+
+int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
+{
+ struct fsg_buffhd *bh, *new_buffhds;
+ int i, rc;
+
+ rc = fsg_num_buffers_validate(n);
+ if (rc != 0)
+ return rc;
+
+ new_buffhds = kcalloc(n, sizeof *(new_buffhds), GFP_KERNEL);
+ if (!new_buffhds)
+ return -ENOMEM;
+
+ /* Data buffers cyclic list */
+ bh = new_buffhds;
+ i = n;
+ goto buffhds_first_it;
+ do {
+ bh-next = bh + 1;
+ ++bh;
+buffhds_first_it:
+ bh-buf = kmalloc(FSG_BUFLEN, GFP_KERNEL);
+ if (unlikely(!bh-buf))
+ goto error_release;
+ } while (--i);
+ bh-next = new_buffhds;
+
+ common-fsg_num_buffers = n;
+ kfree(common-buffhds);
+ common-buffhds = new_buffhds;
+
+ return 0;
+
+error_release:
+ bh = new_buffhds;
+ i = n - i;
+ while (i--) {
+ kfree(bh-buf);
+ bh++;
+ };
+
+ kfree(new_buffhds);
+
+ return -ENOMEM;
+}
+
+void fsg_common_free_buffers(struct fsg_common *common)
+{
+ struct fsg_buffhd *bh;
+ int i;
+
+ bh = common-buffhds;
+ i = common-fsg_num_buffers;
+ while (i--) {
+ kfree(bh-buf);
+ bh++;
+ };
+
+ kfree(common-buffhds);
+ common-buffhds = NULL;
+}
+
+int fsg_common_set_nluns(struct fsg_common *common, int nluns)
+{
+ struct fsg_lun **curlun;
+
+ /* Find out how many LUNs there should be */
+ if (nluns 1 || nluns FSG_MAX_LUNS) {
+ pr_err(invalid number of LUNs: %u\n, nluns);
+ return -EINVAL;
+ }
+
+ curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL);
+ if (unlikely(!curlun))
+ return -ENOMEM;
+
+ common-luns = curlun;
+ common-nluns = nluns;
+
+ pr_info(Number of LUNs=%d\n, common-nluns);
+
+ return 0;
+}
+
+void fsg_common_free_luns(struct fsg_common *common)
+{
+ kfree(common-luns);
+ common-luns
[PATCH v2 18/21] usb/gadget: mass_storage: convert to new interface of f_mass_storage
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/Kconfig|1 +
drivers/usb/gadget/mass_storage.c | 107 +++-
2 files changed, 81 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 1a06390..a678f35 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -944,6 +944,7 @@ config USB_MASS_STORAGE
depends on BLOCK
select USB_LIBCOMPOSITE
select USB_U_MS
+ select USB_F_MASS_STORAGE
help
The Mass Storage Gadget acts as a USB Mass Storage disk drive.
As its storage repository it can use a regular file or a block
diff --git a/drivers/usb/gadget/mass_storage.c
b/drivers/usb/gadget/mass_storage.c
index 6b79814..a9b5c95 100644
--- a/drivers/usb/gadget/mass_storage.c
+++ b/drivers/usb/gadget/mass_storage.c
@@ -46,17 +46,7 @@
#define FSG_VENDOR_ID 0x0525 /* NetChip */
#define FSG_PRODUCT_ID 0xa4a5 /* Linux-USB File-backed Storage Gadget */
-/*-*/
-
-/*
- * kbuild is not very cooperative with respect to linking separately
- * compiled library objects into one module. So for now we won't use
- * separate compilation ... ensuring init/exit sections work to shrink
- * the runtime footprint, and giving us at least some parts of what
- * a gcc --combine ... part1.c part2.c part3.c ... build would.
- */
-#define USB_FMS_INCLUDED
-#include f_mass_storage.c
+#include f_mass_storage.h
/*-*/
USB_GADGET_COMPOSITE_OPTIONS();
@@ -107,6 +97,9 @@ static struct usb_gadget_strings *dev_strings[] = {
NULL,
};
+static struct usb_function_instance *fi_msg;
+static struct usb_function *f_msg;
+
/** Configurations **/
static struct fsg_module_parameters mod_data = {
@@ -139,13 +132,7 @@ static int msg_thread_exits(struct fsg_common *common)
static int __init msg_do_config(struct usb_configuration *c)
{
- static const struct fsg_operations ops = {
- .thread_exits = msg_thread_exits,
- };
- static struct fsg_common common;
-
- struct fsg_common *retp;
- struct fsg_config config;
+ struct fsg_opts *opts;
int ret;
if (gadget_is_otg(c-cdev-gadget)) {
@@ -153,15 +140,24 @@ static int __init msg_do_config(struct usb_configuration
*c)
c-bmAttributes |= USB_CONFIG_ATT_WAKEUP;
}
- fsg_config_from_params(config, mod_data, fsg_num_buffers);
- config.ops = ops;
+ opts = container_of(fi_msg, struct fsg_opts, func_inst);
+
+ f_msg = usb_get_function(fi_msg);
+ if (IS_ERR(f_msg))
+ return PTR_ERR(f_msg);
- retp = fsg_common_init(common, c-cdev, config);
- if (IS_ERR(retp))
- return PTR_ERR(retp);
+ ret = fsg_common_run_thread(opts-common);
+ if (ret)
+ goto put_func;
- ret = fsg_bind_config(c-cdev, c, common);
- fsg_common_put(common);
+ ret = usb_add_function(c, f_msg);
+ if (ret)
+ goto put_func;
+
+ return 0;
+
+put_func:
+ usb_put_function(f_msg);
return ret;
}
@@ -176,23 +172,79 @@ static struct usb_configuration msg_config_driver = {
static int __init msg_bind(struct usb_composite_dev *cdev)
{
+ static const struct fsg_operations ops = {
+ .thread_exits = msg_thread_exits,
+ };
+ struct fsg_opts *opts;
+ struct fsg_config config;
int status;
+ fi_msg = usb_get_function_instance(mass_storage);
+ if (IS_ERR(fi_msg))
+ return PTR_ERR(fi_msg);
+
+ fsg_config_from_params(config, mod_data, fsg_num_buffers);
+ opts = container_of(fi_msg, struct fsg_opts, func_inst);
+
+ opts-no_configfs = true;
+ fsg_common_set_sysfs(opts-common, true);
+ status = fsg_common_set_num_buffers(opts-common, fsg_num_buffers);
+ if (status)
+ goto fail;
+
+ status = fsg_common_set_nluns(opts-common, config.nluns);
+ if (status)
+ goto fail_set_nluns;
+
+ fsg_common_set_ops(opts-common, ops);
+
+ status = fsg_common_set_cdev(opts-common, cdev, config.can_stall);
+ if (status)
+ goto fail_set_cdev;
+
+ status = fsg_common_create_luns(opts-common, config);
+ if (status)
+ goto fail_set_cdev;
+
+ fsg_common_set_inquiry_string(opts-common, config.vendor_name,
+ config.product_name);
+
status = usb_string_ids_tab(cdev, strings_dev);
if (status 0)
- return status;
+ goto fail_string_ids;
msg_device_desc.iProduct =
[PATCH v2 16/21] usb/gadget: f_mass_storage: use fsg_common_run_thread in fsg_common_init
fsg_common_init is a lengthy function. Now there are helper functions
which cover all parts of it. Use them.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 14 +++---
1 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 94b7bc3..ba24236 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -3033,21 +3033,13 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
fsg_common_set_inquiry_string(common, cfg-vendor_name,
cfg-product_name);
- /* Tell the thread to start working */
- common-thread_task =
- kthread_create(fsg_main_thread, common, file-storage);
- if (IS_ERR(common-thread_task)) {
- rc = PTR_ERR(common-thread_task);
- goto error_release;
- }
/* Information */
INFO(common, FSG_DRIVER_DESC , version: FSG_DRIVER_VERSION \n);
- INFO(common, Number of LUNs=%d\n, common-nluns);
- DBG(common, I/O thread pid: %d\n, task_pid_nr(common-thread_task));
-
- wake_up_process(common-thread_task);
+ rc = fsg_common_run_thread(common);
+ if (rc)
+ goto error_release;
return common;
--
1.7.0.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/21] usb/gadget: multi: fix error return code in cdc_do_config()
Fix to return a negative error code from the error handling
case instead of 0, as returned elsewhere in this function.
Introduced by commit 59835a (usb: gadget: multi: use
function framework for ACM.)
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/multi.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 032b96a..867db32 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -225,8 +225,10 @@ static __init int cdc_do_config(struct usb_configuration
*c)
/* implicit port_num is zero */
f_acm_multi = usb_get_function(fi_acm);
- if (IS_ERR(f_acm_multi))
+ if (IS_ERR(f_acm_multi)) {
+ ret = PTR_ERR(f_acm_multi);
goto err_func_acm;
+ }
ret = usb_add_function(c, f_acm_multi);
if (ret)
--
1.7.0.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 06/21] usb/gadget: f_mass_storage: add a level of indirection for luns storage
This is needed to prepare for configfs integration.
So far the luns have been allocated during gadget's initialization, based
on the nluns module parameter's value; the exact number is known when the
gadget is initialized and that number of luns is allocated in one go; they
all will be used.
When configfs is in place, the luns will be created one-by-one by the user.
Once the user is satisfied with the number of luns, they activate the
gadget. The number of luns must be = FSG_MAX_LUN (currently 8), but other
than that it is not known up front and the user need not use contiguous
numbering (apart from the default lun #0). On the other hand, the function
code uses lun numbers to identify them and the number needs to be used
as an index into an array.
Given the above, an array needs to be allocated, but it might happen that
7 out of its 8 elements will not be used. On my machine
sizeof(struct fsg_lun) == 462, so 3k of memory is allocated but not used
in the worst case.
By adding another level of indirection (allocating an array of pointers
to struct fsg_lun and then allocating individual luns instead of an array
of struct fsg_luns) at most 7 pointers are wasted, which is much less.
This patch also changes some for/while loops to cope with the fact
that in the luns array some entries are potentially empty.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 56 +++---
1 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index c36e208..d01d1dd 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -274,7 +274,7 @@ struct fsg_common {
unsigned intnluns;
unsigned intlun;
- struct fsg_lun *luns;
+ struct fsg_lun **luns;
struct fsg_lun *curlun;
unsigned intbulk_out_maxpacket;
@@ -2151,7 +2151,7 @@ static int received_cbw(struct fsg_dev *fsg, struct
fsg_buffhd *bh)
common-data_dir = DATA_DIR_NONE;
common-lun = cbw-Lun;
if (common-lun common-nluns)
- common-curlun = common-luns[common-lun];
+ common-curlun = common-luns[common-lun];
else
common-curlun = NULL;
common-tag = cbw-Tag;
@@ -2297,7 +2297,9 @@ reset:
common-running = 1;
for (i = 0; i common-nluns; ++i)
- common-luns[i].unit_attention_data = SS_RESET_OCCURRED;
+ if (common-luns[i])
+ common-luns[i]-unit_attention_data =
+ SS_RESET_OCCURRED;
return rc;
}
@@ -2397,7 +2399,9 @@ static void handle_exception(struct fsg_common *common)
common-state = FSG_STATE_STATUS_PHASE;
else {
for (i = 0; i common-nluns; ++i) {
- curlun = common-luns[i];
+ curlun = common-luns[i];
+ if (!curlun)
+ continue;
curlun-prevent_medium_removal = 0;
curlun-sense_data = SS_NO_SENSE;
curlun-unit_attention_data = SS_NO_SENSE;
@@ -2439,7 +2443,7 @@ static void handle_exception(struct fsg_common *common)
* CONFIG_CHANGE cases.
*/
/* for (i = 0; i common-nluns; ++i) */
- /* common-luns[i].unit_attention_data = */
+ /* common-luns[i]-unit_attention_data = */
/* SS_RESET_OCCURRED; */
break;
@@ -2536,12 +2540,13 @@ static int fsg_main_thread(void *common_)
if (!common-ops || !common-ops-thread_exits
|| common-ops-thread_exits(common) 0) {
- struct fsg_lun *curlun = common-luns;
+ struct fsg_lun **curlun_it = common-luns;
unsigned i = common-nluns;
down_write(common-filesem);
- for (; i--; ++curlun) {
- if (!fsg_lun_is_open(curlun))
+ for (; i--; ++curlun_it) {
+ struct fsg_lun *curlun = *curlun_it;
+ if (!curlun || !fsg_lun_is_open(curlun))
continue;
fsg_lun_close(curlun);
@@ -2602,7 +2607,7 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
{
struct usb_gadget *gadget = cdev-gadget;
struct fsg_buffhd *bh;
- struct fsg_lun *curlun;
+ struct fsg_lun **curlun_it;
struct fsg_lun_config *lcfg;
int nluns, i, rc;
char *pathbuf;
@@ -2659,16 +2664,25 @@ struct fsg_common *fsg_common_init(struct fsg_common
*common,
* Create the LUNs, open their backing files, and register the
*
[PATCH v2 05/21] usb/gadget: f_mass_storage: factor out a header file
In order to prepare for the new function interface the f_mass_storage.c
needs to be compiled as a module, and so a header file will be required.
This patch factors out some code to a new f_mass_storage.h.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/f_mass_storage.c | 117 ++--
drivers/usb/gadget/f_mass_storage.h | 126 +++
2 files changed, 133 insertions(+), 110 deletions(-)
create mode 100644 drivers/usb/gadget/f_mass_storage.h
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 163d911..c36e208 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -229,6 +229,7 @@
static const char fsg_string_interface[] = Mass Storage;
#include storage_common.h
+#include f_mass_storage.h
/* Static strings, in UTF-8 (for simplicity we use only ASCII characters) */
static struct usb_string fsg_strings[] = {
@@ -246,18 +247,6 @@ static struct usb_gadget_strings fsg_stringtab = {
struct fsg_dev;
struct fsg_common;
-/* FSF callback functions */
-struct fsg_operations {
- /*
-* Callback function to call when thread exits. If no
-* callback is set or it returns value lower then zero MSF
-* will force eject all LUNs it operates on (including those
-* marked as non-removable or with prevent_medium_removal flag
-* set).
-*/
- int (*thread_exits)(struct fsg_common *common);
-};
-
/* Data shared by all the FSG instances. */
struct fsg_common {
struct usb_gadget *gadget;
@@ -324,28 +313,6 @@ struct fsg_common {
struct kref ref;
};
-struct fsg_config {
- unsigned nluns;
- struct fsg_lun_config {
- const char *filename;
- char ro;
- char removable;
- char cdrom;
- char nofua;
- } luns[FSG_MAX_LUNS];
-
- /* Callback functions. */
- const struct fsg_operations *ops;
- /* Gadget's private data. */
- void*private_data;
-
- const char *vendor_name;/* 8 characters or less */
- const char *product_name; /* 16 characters or less */
-
- charcan_stall;
- unsigned intfsg_num_buffers;
-};
-
struct fsg_dev {
struct usb_function function;
struct usb_gadget *gadget;/* Copy of cdev-gadget */
@@ -2609,12 +2576,12 @@ static void fsg_lun_release(struct device *dev)
/* Nothing needs to be done */
}
-static inline void fsg_common_get(struct fsg_common *common)
+void fsg_common_get(struct fsg_common *common)
{
kref_get(common-ref);
}
-static inline void fsg_common_put(struct fsg_common *common)
+void fsg_common_put(struct fsg_common *common)
{
kref_put(common-ref, fsg_common_release);
}
@@ -2629,9 +2596,9 @@ static inline int fsg_num_buffers_validate(unsigned int
fsg_num_buffers)
return -EINVAL;
}
-static struct fsg_common *fsg_common_init(struct fsg_common *common,
- struct usb_composite_dev *cdev,
- struct fsg_config *cfg)
+struct fsg_common *fsg_common_init(struct fsg_common *common,
+ struct usb_composite_dev *cdev,
+ struct fsg_config *cfg)
{
struct usb_gadget *gadget = cdev-gadget;
struct fsg_buffhd *bh;
@@ -3008,62 +2975,8 @@ static int fsg_bind_config(struct usb_composite_dev
*cdev,
/* Module parameters */
-struct fsg_module_parameters {
- char*file[FSG_MAX_LUNS];
- boolro[FSG_MAX_LUNS];
- boolremovable[FSG_MAX_LUNS];
- boolcdrom[FSG_MAX_LUNS];
- boolnofua[FSG_MAX_LUNS];
-
- unsigned intfile_count, ro_count, removable_count, cdrom_count;
- unsigned intnofua_count;
- unsigned intluns; /* nluns */
- boolstall; /* can_stall */
-};
-
-#define _FSG_MODULE_PARAM_ARRAY(prefix, params, name, type, desc) \
- module_param_array_named(prefix ## name, params.name, type, \
-prefix ## params.name ## _count, \
-S_IRUGO); \
- MODULE_PARM_DESC(prefix ## name, desc)
-
-#define _FSG_MODULE_PARAM(prefix, params, name, type, desc)\
- module_param_named(prefix ## name, params.name, type, \
- S_IRUGO);\
- MODULE_PARM_DESC(prefix ## name, desc)
-
-#define __FSG_MODULE_PARAMETERS(prefix, params)
\
-
Re: How should we handle isochronous underruns?
Ming Lei wrote:
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
Not always, and the usb_set_interface() call can be done in a different
context.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()?
There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.
or delay URB resubmission to a workqueue?
Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
but it isn't a big deal:
But your proposed code would not be able to differentiate a workqueue
resubmission from a stream start.
Regards,
Clemens
--
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: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 6:30 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
Not always, and the usb_set_interface() call can be done in a different
context.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()?
There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.
That still can be covered easily by the approach since isoc direction is
different, :-)
or delay URB resubmission to a workqueue?
Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
but it isn't a big deal:
But your proposed code would not be able to differentiate a workqueue
resubmission from a stream start.
As I explained, it isn't a big deal. Either it is working well for long time
in case of underrun with empty queue, or we can change the resubmission
to complete(), right?
Thanks,
--
Ming Lei
--
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] [RFC] EHCI: add to memory barrier to updating hw_next
Thanks a lot for your replay. -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Thursday, July 18, 2013 11:09 PM To: Ming Lei Cc: Gioh Kim; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Mark Salter; namhyung@lge.com; Minchan Kim; Chanho Min; Jong-Sung Kim; linux-arm-kernel Subject: Re: [PATCH] [RFC] EHCI: add to memory barrier to updating hw_next On Thu, 18 Jul 2013, Ming Lei wrote: I guess that HC could have a use-after-free problem like following situation. 1. A qtd which is not at the queue head should be removed in qh_completions(). 2. The last-hw_next become be pointing at the next qtd but the hw_next value is delayed in write-buffer. 3. The qtd is removed in the list. 4. The qtd is freed into DMA pool and re-allocated for another urb. 5. HC try to process last-hw_next and it is pointing re-allocated qtd. What do you think about it? Is it possible? I understand it might not be possible because: when 'stopped' is set, that said the HC might not advance the queue. But I don't understand why 'last-hw_next' is patched here under 'stopped' situation. It should not be possible. When stopped is set, the QH gets unlinked and relinked before it can start up again. Relinking involves some memory barriers, so the qTD will not be accessed again by the HC. last-hw_next gets patched because the qTD might belong to some URB in the middle of the queue that is being unlinked. The URBs before it and after it will still be active, so the queue link has to be updated. You're right. I misunderstand those codes. Please forget about it. Even the 'stopped' case may be seldom triggered, do you know under which condition the stopped is triggered in your problem?(stall, short read or others) I was going to ask the same question. This particular piece of code gets executed _only_ when an URB is unlinked. Not during any other kind of error. I've got the problem when I listened to the mp3 file of USB HDD. I checked the urb data when the problem occurred, the last-status value of urb was EINPROGRESS and urb-unlinked was ECONNRESET. I think the 'stopped' case was occurred by the reset of USB port. The block device driver did reset USB port because there is no return from USB device. If I made block device driver could not reset USB port, the EHCI driver codes were not executed. Finally the halt of HC makes 'stopped' case. I think halt of the HC might be caused that store-buffer delays command for HC. When I applied the patch from https://lkml.org/lkml/2011/8/31/344 and added a mb() into hw_next updating to remove delay of store-buffer, My platform works well. Can the store-buffer delay halt HC? Is it possible? IMHO, if the qTD list is broken the HC think there is no qTD to send. So I added mb() at hw_next update code. 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: Audio I/O parameters
On Wed, Jul 17, 2013 at 7:48 PM, Alan Stern st...@rowland.harvard.edu wrote: The 32-frames/period trace looks just like the 64 frames/period, only worse. Each URB has only 4 packets, and the underrun problem is severe. This won't happen after the minimum pipeline length is fixed. The 256-frames/period duplex trace is different. There are 4 output URBs, alternating between 8 and 7 packets, so no underruns. On the other hand, the audio-in stream doesn't start up until 900 ms after the audio-out, and everything stops almost immediately afterward. It certainly looks like that delay is causing the problem, so I tracked it down. Before starting each stream, the ALSA driver sends several control requests. I'm not very familiar with the USB audio protocol; it looks like the requests are GET_CUR and SET_CUR for various clock-related controls. First there is Set-Interface for the audio-in interface, followed by: 88010586a3c0 3063689257 S Ci:1:004:0 s a1 01 0100 2800 0001 1 88010586a3c0 3063689324 C Ci:1:004:0 0 1 = 01 88010586a3c0 3063689355 S Ci:1:004:0 s a1 01 0200 2900 0001 1 88010586a3c0 3063689448 C Ci:1:004:0 0 1 = 01 88010586a3c0 3063689496 S Co:1:004:0 s 21 01 0100 2900 0004 4 = 44ac 88010586a3c0 3064139502 C Co:1:004:0 0 4 88010586a3c0 3064139605 S Ci:1:004:0 s a1 01 0100 2900 0004 4 88010586a3c0 3064139706 C Ci:1:004:0 0 4 = 44ac As far as I can tell, entity 0x28 is a CLOCK_SELECTOR and 0x29 is a CLOCK_SOURCE. The last two requests appear to set the clock frequency to 44100 (0x44ac in little-endian hex) and read it back. Then there is Set-Interface for the audio-out interface, followed by: 88010586ac00 3064140256 S Ci:1:004:0 s a1 01 0100 2800 0001 1 88010586ac00 3064140495 C Ci:1:004:0 0 1 = 01 88010586ac00 3064140593 S Ci:1:004:0 s a1 01 0200 2900 0001 1 88010586ac00 3064140706 C Ci:1:004:0 0 1 = 01 88010586ac00 3064140764 S Co:1:004:0 s 21 01 0100 2900 0004 4 = 44ac 88010586ac00 3064590952 C Co:1:004:0 0 4 88010586ac00 3064591055 S Ci:1:004:0 s a1 01 0100 2900 0004 4 88010586ac00 3064592201 C Ci:1:004:0 0 4 = 44ac These are exactly the same as the previous set of requests. I don't know why they are repeated. (It's worth pointing out that the same repeat also occurs in the simplex out-only trace.) Next, the audio-out URBs (including the synch URBs) begin, followed by some more control requests interspersed among them: 88010586ac00 3064593448 S Ci:1:004:0 s a1 01 0100 2800 0001 1 88010586ac00 3064593885 C Ci:1:004:0 0 1 = 01 88010586ac00 3064593964 S Ci:1:004:0 s a1 01 0200 2900 0001 1 88010586ac00 3064594185 C Ci:1:004:0 0 1 = 01 8800bd6db180 3064594238 S Co:1:004:0 s 21 01 0100 2900 0004 4 = 44ac 8800bd6db180 3065044098 C Co:1:004:0 0 4 88010586a240 3065044188 S Ci:1:004:0 s a1 01 0100 2900 0004 4 88010586a240 3065044350 C Ci:1:004:0 0 4 = 44ac 88010586a240 3065044412 S Ci:1:004:0 s a1 01 0100 2800 0001 1 88010586a240 3065044474 C Ci:1:004:0 0 1 = 01 88010586a240 3065044510 S Ci:1:004:0 s a1 01 0200 2900 0001 1 88010586a240 3065044598 C Ci:1:004:0 0 1 = 01 88010586a240 3065044631 S Co:1:004:0 s 21 01 0100 2900 0004 4 = 44ac 88010586a240 3065494566 C Co:1:004:0 0 4 88010586a240 3065494651 S Ci:1:004:0 s a1 01 0100 2900 0004 4 88010586a240 3065494814 C Ci:1:004:0 0 4 = 44ac after which the audio-in URBs start up. This is the same four requests as before, repeated as before. But notice the timestamps (second column, values are in microseconds). Each of the SET_CUR requests for the clock frequency requires 450 ms to complete! That accounts for the 900 ms delay. The same 450-ms delay can be seen in the earlier requests too, but there they didn't matter. It appears that the delay in starting up the second stream annoys JACK, causing it to abort the session. After everything stops, another attempt is made. This time there is only one SET_CUR between the start of the audio-out and the start of the audio-in. Even so, a 450-ms delay evidently is enough to cause JACK to abort. Several more attempts were made, each with the same result. The questions now are: Why are the same requests sent over and over again? Why does the ALSA driver attempt to set the clock frequency while the clock is actively in use? Has this behavior changed since the 3.5 kernel? Well, I think these requests may correspond to the lights flashing on and off on the front of the device. When starting the device in 3.5 at 256 frames/period (duplex), the lights flash on and off 2 times, in the current patched 3.8 version I have been using, the device lights flash on and off 4 times before starting jack with exactly the same settings - so it seems for some reason, the requests are going through multiple times on the 3.8 kernel but
Re: [PATCH] usb: xhci: add the suspend/resume functionality
Hi, On Thu, Jul 18, 2013 at 04:18:08PM -0700, Sarah Sharp wrote: Hi Felipe, My bad, this patch got dropped. I've refreshed it against Greg's usb-next branch. Do you still want me to push this? Again, my apologies for dropping this. Sure, this is necessary, at least, for the Exynos5-based Chromebook folks. OMAP5/AM437x/DRA7xxx will also make use of the same calls when operating as host. Thanks -- balbi signature.asc Description: Digital signature
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 6:36 PM, Ming Lei tom.leim...@gmail.com wrote:
On Fri, Jul 19, 2013 at 6:30 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
Not always, and the usb_set_interface() call can be done in a different
context.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()?
There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.
That still can be covered easily by the approach since isoc direction is
different, :-)
Even we can introduce one flag of 'completing' in 'struct urb' to check if
the USB's submit is called inside its own complete(), by which we can
check if resubmission is in underrun easily.
Consider that isoc URB's resubmission in completion handler should
cover 99% cases, I think this approach is doable.
For other resubmission from tasklet or workque cases, either let them alone
or move the resubmission inside completion() handler, or introduce simple
helper to mark start of frame only for them. Anyway, there are very few isoc
resubmissions from non-completion handler (only two drivers found in my
urb complet() cleanup work), so it shouldn't be a big deal.
What do you think about this approach?
Thanks,
--
Ming Lei
--
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 0/3] staging: dwc2: Add some dma-related defensive programming
Hi Greg, here's three patches for dwc2, acked by Paul. They're not critical, so no need to push them to 3.11. Gr. Matthijs Matthijs Kooijman (3): staging: dwc2: disable dma when no dma_mask was setup staging: dwc2: when dma is disabled, clear hcd-self.uses_dma staging: dwc2: Don't touch the dma_mask when dma is disabled drivers/staging/dwc2/hcd.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) -- 1.8.3.rc1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] staging: dwc2: Don't touch the dma_mask when dma is disabled
There was some code that cleared the dma_mask when dma was disabled in
the driver. Given that clearing the mask doesn't actually tell the usb
core we're not using dma, and a previous commit explicitely sets the
hcd-self.uses_dma value, it seems these values are unneeded and can
only potentially cause problems (when reloading a module, for example).
Signed-off-by: Matthijs Kooijman matth...@stdin.nl
Acked-by: Paul Zimmerman paul.zimmer...@synopsys.com
---
drivers/staging/dwc2/hcd.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c
index 50a379d..075e53b 100644
--- a/drivers/staging/dwc2/hcd.c
+++ b/drivers/staging/dwc2/hcd.c
@@ -2817,9 +2817,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
if (dma_set_coherent_mask(hsotg-dev, DMA_BIT_MASK(31)) 0)
dev_warn(hsotg-dev,
can't enable workaround for 2GB RAM\n);
- } else {
- dma_set_mask(hsotg-dev, 0);
- dma_set_coherent_mask(hsotg-dev, 0);
}
hcd = usb_create_hcd(dwc2_hc_driver, hsotg-dev, dev_name(hsotg-dev));
--
1.8.3.rc1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] staging: dwc2: disable dma when no dma_mask was setup
If the platform or bus driver failed to setup a dma_mask, but the
hardware advertises support for DMA, before DMA would be enabled in
dwc2, but disabled in the usb core, making all connectivity break.
With this commit, the dwc2 driver will emit a warning and fall back to
slave mode in this case.
Note that since commit 642f2ec (staging: dwc2: Fix dma-enabled platform
devices using a default dma_mask) the platform bindings make sure a DMA
mask is always present, but having this check here anyway is probably a
good from a defensive programming standpoint (in case of changes to
platform.c or addition of new glue layers).
Signed-off-by: Matthijs Kooijman matth...@stdin.nl
Acked-by: Paul Zimmerman paul.zimmer...@synopsys.com
---
drivers/staging/dwc2/hcd.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c
index 2ed54b1..b5b229a 100644
--- a/drivers/staging/dwc2/hcd.c
+++ b/drivers/staging/dwc2/hcd.c
@@ -2801,6 +2801,15 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
/* Validate parameter values */
dwc2_set_parameters(hsotg, params);
+ /* Check if the bus driver or platform code has setup a dma_mask */
+ if (hsotg-core_params-dma_enable 0
+ hsotg-dev-dma_mask == NULL) {
+ dev_warn(hsotg-dev,
+dma_mask not set, disabling DMA\n);
+ hsotg-core_params-dma_enable = 0;
+ hsotg-core_params-dma_desc_enable = 0;
+ }
+
/* Set device flags indicating whether the HCD supports DMA */
if (hsotg-core_params-dma_enable 0) {
if (dma_set_mask(hsotg-dev, DMA_BIT_MASK(32)) 0)
--
1.8.3.rc1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] staging: dwc2: when dma is disabled, clear hcd-self.uses_dma
When dma is disabled inside dwc2 (because the hardware does not support it, or the code was changed to disable it for testing), let the usb core know about this by clearing hcd-self.uses_dma. By default, the usb core assumes that dma is used when a dma_mask is set, but this might not always match the dma_enable value in dwc2. To prevent problems resulting from a mismatch, better to explicitely disable dma in this case (though everything seemed to work with the wrong value of uses_dma as well, probably only resulted in some unneeded work). Signed-off-by: Matthijs Kooijman matth...@stdin.nl Acked-by: Paul Zimmerman paul.zimmer...@synopsys.com --- drivers/staging/dwc2/hcd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c index b5b229a..50a379d 100644 --- a/drivers/staging/dwc2/hcd.c +++ b/drivers/staging/dwc2/hcd.c @@ -2826,6 +2826,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, if (!hcd) goto error1; + if (hsotg-core_params-dma_enable = 0) + hcd-self.uses_dma = 0; + hcd-has_tt = 1; spin_lock_init(hsotg-lock); -- 1.8.3.rc1 -- 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] [RFC] EHCI: add to memory barrier to updating hw_next
On Fri, Jul 19, 2013 at 11:50 AM, Ming Lei tom.leim...@gmail.com wrote:
On Thu, Jul 18, 2013 at 10:08 PM, Alan Stern st...@rowland.harvard.edu
wrote:
On Thu, 18 Jul 2013, Ming Lei wrote:
I guess that HC could have a use-after-free problem like following
situation.
1. A qtd which is not at the queue head should be removed in
qh_completions().
2. The last-hw_next become be pointing at the next qtd but the hw_next
value is delayed in write-buffer.
3. The qtd is removed in the list.
4. The qtd is freed into DMA pool and re-allocated for another urb.
5. HC try to process last-hw_next and it is pointing re-allocated qtd.
What do you think about it? Is it possible?
I understand it might not be possible because: when 'stopped' is set, that
said the HC might not advance the queue. But I don't understand why
'last-hw_next' is patched here under 'stopped' situation.
It should not be possible. When stopped is set, the QH gets unlinked
and relinked before it can start up again. Relinking involves some
memory barriers, so the qTD will not be accessed again by the HC.
last-hw_next gets patched because the qTD might belong to some URB in
the middle of the queue that is being unlinked. The URBs before it and
after it will still be active, so the queue link has to be updated.
'stopped' is set under below situations:
- unlink over or shutdown
- halt
- short packet(URB_SHORT_NOT_OK)
Looks EHCI won't advance the queue(qh) any more on above situations, so I
think last-hw_next might not need patching.
Even the 'stopped' case may be seldom triggered, do you know under
which condition the stopped is triggered in your problem?(stall, short read
or others)
I was going to ask the same question. This particular piece of code
gets executed _only_ when an URB is unlinked. Not during any other
kind of error.
The code may run under 'halt' or short packet(URB_SHORT_NOT_OK) too.
If Gioh's problem falls to these two situations, below patch might be helpful.
Because the qh will be unlinked when there is fault in the endpoint(halt,
short
packet), maybe it is safer to complete these URBs after the qh becomes
unlinked, like what the blew patch does:
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index b637a65..6a65e0a 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -454,6 +454,10 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh
*qh)
}
}
+ /* complete URBs after unlinking */
+ if (stopped state != QH_STATE_IDLE)
+ goto exit;
+
/* unless we already know the urb's status, collect qtd status
* and update count of bytes transferred. in common short
read
* cases with only one data qtd (including control transfers),
@@ -489,15 +493,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh
*qh)
}
}
- /* if we're removing something not at the queue head,
-* patch the hardware queue pointer.
-*/
- if (stopped qtd-qtd_list.prev != qh-qtd_list) {
- last = list_entry (qtd-qtd_list.prev,
- struct ehci_qtd, qtd_list);
- last-hw_next = qtd-hw_next;
- }
-
/* remove qtd; it's recycled after possible urb completion */
list_del (qtd-qtd_list);
last = qtd;
@@ -520,7 +515,7 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
/* Otherwise the caller must unlink the QH. */
}
-
+ exit:
/* restore original state; caller must unlink or relink */
qh-qh_state = state;
Please ignore this email, and looks I understand this piece of code totally
wrong: active URBs and its qtds should survive qh unlink relink.
Sorry for the noise, :-(
Thanks,
--
Ming Lei
--
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 4/4] usb: musb: dsps: Remove the phy control from glue and add phy driver APIs
Remove usb phy control module access from platform glue.
The same is now done using am phy driver and phy-omap-control.
Adapt the driver to the split dt nodes.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/usb/musb/musb_dsps.c | 150 ---
1 file changed, 55 insertions(+), 95 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 41f135a..6bec458 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -43,6 +43,7 @@
#include linux/of.h
#include linux/of_device.h
#include linux/of_address.h
+#include linux/of_irq.h
#include musb_core.h
@@ -110,8 +111,6 @@ struct dsps_musb_wrapper {
/* miscellaneous stuff */
u32 musb_core_offset;
u8 poll_seconds;
- /* number of musb instances */
- u8 instances;
};
/**
@@ -123,49 +122,11 @@ struct dsps_glue {
const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
struct timer_list timer[2]; /* otg_workaround timer */
unsigned long last_timer[2];/* last timer data for each instance */
- u32 __iomem *usb_ctrl[2];
};
-#defineDSPS_AM33XX_CONTROL_MODULE_PHYS_0 0x44e10620
-#defineDSPS_AM33XX_CONTROL_MODULE_PHYS_1 0x44e10628
+#define glue_to_musb(g, i) platform_get_drvdata(g-musb[i])
-static const resource_size_t dsps_control_module_phys[] = {
- DSPS_AM33XX_CONTROL_MODULE_PHYS_0,
- DSPS_AM33XX_CONTROL_MODULE_PHYS_1,
-};
-
-#define USBPHY_CM_PWRDN(1 0)
-#define USBPHY_OTG_PWRDN (1 1)
-#define USBPHY_OTGVDET_EN (1 19)
-#define USBPHY_OTGSESSEND_EN (1 20)
-
-/**
- * musb_dsps_phy_control - phy on/off
- * @glue: struct dsps_glue *
- * @id: musb instance
- * @on: flag for phy to be switched on or off
- *
- * This is to enable the PHY using usb_ctrl register in system control
- * module space.
- *
- * XXX: This function will be removed once we have a seperate driver for
- * control module
- */
-static void musb_dsps_phy_control(struct dsps_glue *glue, u8 id, u8 on)
-{
- u32 usbphycfg;
-
- usbphycfg = readl(glue-usb_ctrl[id]);
-
- if (on) {
- usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
- usbphycfg |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN;
- } else {
- usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN;
- }
- writel(usbphycfg, glue-usb_ctrl[id]);
-}
/**
* dsps_musb_enable - enable interrupts
*/
@@ -411,16 +372,28 @@ static int dsps_musb_init(struct musb *musb)
void __iomem *reg_base = musb-ctrl_base;
u32 rev, val;
int status;
+ char phyname[12];
/* mentor core register starts at offset of 0x400 from musb base */
musb-mregs += wrp-musb_core_offset;
- /* NOP driver needs change if supporting dual instance */
- usb_nop_xceiv_register();
- musb-xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
+ snprintf(phyname , 12, am335x-usb%d, pdev-id);
+
+ if (dev-of_node) {
+ musb-phy = devm_phy_get(dev, phyname);
+ musb-xceiv = devm_usb_get_phy_by_phandle(dev,
+ phys, 0);
+ } else {
+ musb-phy = devm_phy_get(dev, musb-phy_label);
+ musb-xceiv = devm_usb_get_phy_dev(dev, pdev-id);
+ }
+
if (IS_ERR_OR_NULL(musb-xceiv))
return -EPROBE_DEFER;
+ if (IS_ERR_OR_NULL(musb-phy))
+ return -EPROBE_DEFER;
+
/* Returns zero if e.g. not clocked */
rev = dsps_readl(reg_base, wrp-revision);
if (!rev) {
@@ -434,7 +407,8 @@ static int dsps_musb_init(struct musb *musb)
dsps_writel(reg_base, wrp-control, (1 wrp-reset));
/* Start the on-chip PHY and its PLL. */
- musb_dsps_phy_control(glue, pdev-id, 1);
+ phy_init(musb-phy);
+ phy_power_on(musb-phy);
musb-isr = dsps_interrupt;
@@ -448,8 +422,7 @@ static int dsps_musb_init(struct musb *musb)
return 0;
err0:
- usb_put_phy(musb-xceiv);
- usb_nop_xceiv_unregister();
+ devm_phy_put(dev-parent, musb-phy);
return status;
}
@@ -462,11 +435,8 @@ static int dsps_musb_exit(struct musb *musb)
del_timer_sync(glue-timer[pdev-id]);
/* Shutdown the on-chip PHY and its PLL. */
- musb_dsps_phy_control(glue, pdev-id, 0);
-
- /* NOP driver needs change if supporting dual instance */
- usb_put_phy(musb-xceiv);
- usb_nop_xceiv_unregister();
+ phy_power_off(musb-phy);
+ phy_exit(musb-phy);
return 0;
}
@@ -483,49 +453,34 @@ static struct musb_platform_ops dsps_ops = {
static u64 musb_dmamask = DMA_BIT_MASK(32);
-static int dsps_create_musb_pdev(struct dsps_glue *glue, u8 id)
+static int
[PATCH v2 3/4] arm: dts: Add USB phy nodes for AM33XX
Add phy nodes for AM33XX platform and split the musb nodes
per instance.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: George Cherian george.cher...@ti.com
---
arch/arm/boot/dts/am33xx.dtsi | 68 +--
1 file changed, 53 insertions(+), 15 deletions(-)
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 8e1248f..e3890c4 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -326,21 +326,59 @@
status = disabled;
};
- usb@4740 {
- compatible = ti,musb-am33xx;
- reg = 0x4740 0x1000/* usbss */
- 0x47401000 0x800 /* musb instance 0 */
- 0x47401800 0x800; /* musb instance 1 */
- interrupts = 17/* usbss */
- 18/* musb instance 0 */
- 19; /* musb instance 1 */
- multipoint = 1;
- num-eps = 16;
- ram-bits = 12;
- port0-mode = 3;
- port1-mode = 3;
- power = 250;
- ti,hwmods = usb_otg_hs;
+ phy1: am335x-usb0@44e10620 {
+ compatible = ti,am335x-usb2;
+ #phy-cells = 0;
+ id= 0;
+ };
+
+ phy2: am335x-usb1@44e10628 {
+ compatible = ti,am335x-usb2;
+ #phy-cells = 0;
+ id= 1;
+ };
+
+ omap_control_usb: omap-control-usb@44e10620 {
+ compatible = ti,omap-control-usb;
+ reg = 0x44e10620 0x10;
+ reg-names = control_dev_conf;
+ ti,type = 3;
+ };
+
+musb: usb@4740 {
+compatible = ti,musb-am33xx;
+reg = 0x4740 0x1000;
+ranges;
+#address-cells = 1;
+#size-cells = 1;
+interrupts = 17;
+ti,hwmods = usb_otg_hs;
+
+usb0@47401000 {
+reg = 0x47401000 0x800;
+interrupts = 18;
+interrupt-names = mc;
+multipoint = 1;
+num-eps = 16;
+ram-bits = 12;
+port-mode = 3;
+power = 250;
+phys = phy1;
+ phy-names = am335x-usb0;
+};
+
+usb1@47401800 {
+reg = 0x47401800 0x800;
+interrupts = 19;
+interrupt-names = mc;
+multipoint = 1;
+num-eps = 16;
+ram-bits = 12;
+port-mode = 3;
+power = 250;
+phys = phy2;
+ phy-names = am335x-usb1;
+ };
};
mac: ethernet@4a10 {
--
1.8.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 0/4] Add phy support for AM335X platform using Generic PHy framework
This patch series adds phy support for AM335X platform. This patch series is based on Generic PHY framework [1]. This series has - adds dual musb instances support for am335x platform - adds phy-am-usb driver used in AM platforms - adds OTG callbacks - adds PHY wakeup enable/disable - adds dt bindings for the phys - removes usb-phy and replaced with generic phy apis in glue layer All these changes are avilable at [2] [1] - http://marc.info/?l=linux-usbm=137224750928570w=2 [2] - git://git.ti.com/~georgecherian/ti-linux-kernel/georgec-connectivity-linux-feature-tree.git am335x-phy-driver-v2 George Cherian (4): usb: phy: phy-omap-control: Add API to power and wakeup phy: phy-am-usb: Add PHY driver for am platform arm: dts: Add USB phy nodes for AM33XX usb: musb: dsps: Remove the phy control from glue and add phy driver APIs arch/arm/boot/dts/am33xx.dtsi| 68 --- drivers/phy/Kconfig | 12 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-am-usb.c | 222 +++ drivers/usb/musb/musb_dsps.c | 150 +-- drivers/usb/phy/phy-omap-control.c | 67 +++ include/linux/usb/omap_control_usb.h | 24 7 files changed, 434 insertions(+), 110 deletions(-) create mode 100644 drivers/phy/phy-am-usb.c -- 1.8.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 1/4] usb: phy: phy-omap-control: Add API to power and wakeup
on/off USB PHY for AM335X
Add APIs to
-power on/off USB PHY for AM335X
-enable/disable PHY wakeup.
This API will be called from phy-am-usb driver.
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/usb/phy/phy-omap-control.c | 67
include/linux/usb/omap_control_usb.h | 24 +
2 files changed, 91 insertions(+)
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 1419ced..4f2502c 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -46,6 +46,73 @@ struct device *omap_get_control_dev(void)
EXPORT_SYMBOL_GPL(omap_get_control_dev);
/**
+ * omap_control_am335x_phy_wkup - PHY wakeup on/off using control
+ * module
+ * @dev: the control module device
+ * @on: 0 to off and 1 to on PHY wakeup feature
+ * @id: phy id 0 or 1 for phy instance 0 and 1 repectively
+ *
+ * AM PHY driver should call this API to enable or disable PHY wakeup.
+ */
+void omap_control_am335x_phy_wkup(struct device *dev, bool on, u8 id)
+{
+ u32 val;
+ u32 *phy_wkup_reg;
+ struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+
+ if (control_usb-type != OMAP_CTRL_DEV_TYPE3 || id 0 || id 1)
+ return;
+
+ phy_wkup_reg = control_usb-dev_conf + AM335X_USB_WKUP_OFFSET;
+ val = readl(phy_wkup_reg);
+
+ if (on)
+ val |= id ? AM335X_USB1_WKUP_CTRL_ENABLE :
+ AM335X_USB0_WKUP_CTRL_ENABLE;
+else
+ val = id ? ~AM335X_USB1_WKUP_CTRL_ENABLE :
+ ~AM335X_USB0_WKUP_CTRL_ENABLE;
+
+
+ writel(val, phy_wkup_reg);
+}
+EXPORT_SYMBOL_GPL(omap_control_am335x_phy_wkup);
+
+/**
+ * omap_control_am335x_phy_power - power on/off the serializer using control
+ * module
+ * @dev: the control module device
+ * @on: 0 to off and 1 to on based on powering on or off the PHY
+ * @id: phy id 0 or 1 for phy instance 0 and 1 repectively
+ *
+ * AM PHY driver should call this API to power on or off the PHY.
+ */
+void omap_control_am335x_phy_power(struct device *dev, bool on, u8 id)
+{
+ u32 val;
+ u32 *phy_pwr_reg;
+ struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+
+ if (control_usb-type != OMAP_CTRL_DEV_TYPE3 || id 0 || id 1)
+ return;
+
+ phy_pwr_reg = GET_PHY_POWER_REG(control_usb-dev_conf, id);
+ val = readl(phy_pwr_reg);
+
+ if (on) {
+ val = ~(AM335X_USBPHY_CM_PWRDN | AM335X_USBPHY_OTG_PWRDN);
+ val |= AM335X_USBPHY_OTGVDET_EN |
+ AM335X_USBPHY_OTGSESSEND_EN;
+ } else
+ val |= AM335X_USBPHY_CM_PWRDN | AM335X_USBPHY_OTG_PWRDN;
+
+
+ writel(val, phy_pwr_reg);
+}
+EXPORT_SYMBOL_GPL(omap_control_am335x_phy_power);
+
+
+/**
* omap_control_usb3_phy_power - power on/off the serializer using control
* module
* @dev: the control module device
diff --git a/include/linux/usb/omap_control_usb.h
b/include/linux/usb/omap_control_usb.h
index 27b5b8c..a92b417 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -45,6 +45,7 @@ enum omap_control_usb_mode {
/* To differentiate ctrl module IP having either mailbox or USB3 PHY power */
#defineOMAP_CTRL_DEV_TYPE1 0x1
#defineOMAP_CTRL_DEV_TYPE2 0x2
+#defineOMAP_CTRL_DEV_TYPE3 0x3
#defineOMAP_CTRL_DEV_PHY_PDBIT(0)
@@ -63,10 +64,23 @@ enum omap_control_usb_mode {
#defineOMAP_CTRL_USB3_PHY_TX_RX_POWERON0x3
#defineOMAP_CTRL_USB3_PHY_TX_RX_POWEROFF 0x0
+#define GET_PHY_POWER_REG(base, id)((u32 *)base + (id * 2))
+#define AM335X_USBPHY_CM_PWRDN (1 0)
+#define AM335X_USBPHY_OTG_PWRDN(1 1)
+#define AM335X_USBPHY_OTGVDET_EN (1 19)
+#define AM335X_USBPHY_OTGSESSEND_EN(1 20)
+#define AM335X_USB_WKUP_OFFSET 0xA
+#define AM335X_USB0_WKUP_CTRL_ENABLE (1 0)
+#define AM335X_USB1_WKUP_CTRL_ENABLE (1 8)
+
+
+
#if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
extern struct device *omap_get_control_dev(void);
extern void omap_control_usb_phy_power(struct device *dev, int on);
extern void omap_control_usb3_phy_power(struct device *dev, bool on);
+extern void omap_control_am335x_phy_power(struct device *dev, bool on, u8 id);
+extern void omap_control_am335x_phy_wkup(struct device *dev, bool on, u8 id);
extern void omap_control_usb_set_mode(struct device *dev,
enum omap_control_usb_mode mode);
#else
@@ -87,6 +101,16 @@ static inline void omap_control_usb_set_mode(struct device
*dev,
enum omap_control_usb_mode mode)
{
}
+
+static inline void omap_control_am335x_phy_power(struct device *dev,
+bool on, u8 id)
+{
+}
+
+static inline void omap_control_am335x_phy_wkup(struct device *dev,
+bool on, u8 id)
+{
+}
#endif
[PATCH v2 2/4] phy: phy-amxxxx-usb: Add PHY driver for amxxxx platform
Adds phy driver support for am33xx platform, the host/device
peripheral controller shall get this phy object to control the phy
operations.
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/phy/Kconfig | 12 +++
drivers/phy/Makefile | 1 +
drivers/phy/phy-am-usb.c | 222 +++
3 files changed, 235 insertions(+)
create mode 100644 drivers/phy/phy-am-usb.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5f85909..55087ab 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -11,3 +11,15 @@ menuconfig GENERIC_PHY
devices present in the kernel. This layer will have the generic
API by which phy drivers can create PHY using the phy framework and
phy users can obtain reference to the PHY.
+if GENERIC_PHY
+
+config PHY_AM_USB
+ tristate AM USB2 PHY Driver
+ depends on SOC_AM33XX
+ help
+ Enable this to support the transceiver that is part of SOC. This
+ phy supports all LS/FS/HS speed and also supports OTG functionality.
+ The USB OTG controller communicates with this phy through stand UTMI
+ interface.
+
+endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..471c525 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_PHY_AM_USB) += phy-am-usb.o
diff --git a/drivers/phy/phy-am-usb.c b/drivers/phy/phy-am-usb.c
new file mode 100644
index 000..d0b0749
--- /dev/null
+++ b/drivers/phy/phy-am-usb.c
@@ -0,0 +1,222 @@
+/*
+ * phy-am-usb.c - USB PHY, talking to usb controller in AMXXX.
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Author: George Cherian george.cher...@ti.com
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/of.h
+#include linux/usb/otg.h
+#include linux/clk.h
+#include linux/pm_runtime.h
+#include linux/usb/omap_control_usb.h
+#include linux/phy/phy.h
+
+struct am_usb {
+ struct usb_phy phy;
+ struct device *dev;
+ struct device *control_dev;
+ struct clk *optclk;
+ u32 id;
+};
+
+#define phy_to_amphy(x)container_of((x), struct am_phy, phy)
+
+
+static int am_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+ struct usb_phy *phy = otg-phy;
+
+ otg-host = host;
+ if (!host)
+ phy-state = OTG_STATE_UNDEFINED;
+
+ return 0;
+}
+
+static int am_usb_set_peripheral(struct usb_otg *otg,
+ struct usb_gadget *gadget)
+{
+ struct usb_phy *phy = otg-phy;
+
+ otg-gadget = gadget;
+ if (!gadget)
+ phy-state = OTG_STATE_UNDEFINED;
+
+ return 0;
+}
+
+
+
+static int am_usb_power_off(struct phy *x)
+{
+ struct am_usb *phy = phy_get_drvdata(x);
+
+ omap_control_am335x_phy_power(phy-control_dev, 0, phy-id);
+
+ return 0;
+}
+
+static int am_usb_power_on(struct phy *x)
+{
+ struct am_usb *phy = phy_get_drvdata(x);
+
+ omap_control_am335x_phy_power(phy-control_dev, 1, phy-id);
+
+ return 0;
+}
+
+static struct phy_ops ops = {
+ .power_on = am_usb_power_on,
+ .power_off = am_usb_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int am_usb2_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev-dev.of_node;
+ struct am_usb *phy;
+ struct phy *generic_phy;
+ struct usb_otg *otg;
+ struct phy_provider *phy_provider;
+
+ phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
+
+ otg = devm_kzalloc(pdev-dev, sizeof(*otg), GFP_KERNEL);
+
+ phy-dev= pdev-dev;
+
+ if (np)
+ of_property_read_u32(np, id, phy-id);
+
+ phy-phy.dev= phy-dev;
+ phy-phy.label = am-usb2;
+ phy-phy.otg= otg;
+ phy-phy.type = USB_PHY_TYPE_USB2;
+
+ phy_provider = devm_of_phy_provider_register(phy-dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy-control_dev = omap_get_control_dev();
+ if (IS_ERR(phy-control_dev)) {
+
Re: [PATCH v2 0/4] Add phy support for AM335X platform using Generic PHy framework
Hi, On Friday 19 July 2013 06:04 PM, George Cherian wrote: This patch series adds phy support for AM335X platform. This patch series is based on Generic PHY framework [1]. There are some comments on the generic PHY framework. So you might have to adapt it once those comments are fixed. Thanks Kishon -- 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/4] usb: phy: phy-omap-control: Add API to power and wakeup
Hi,
On Friday 19 July 2013 06:04 PM, George Cherian wrote:
on/off USB PHY for AM335X
Add APIs to
-power on/off USB PHY for AM335X
-enable/disable PHY wakeup.
This API will be called from phy-am-usb driver.
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/usb/phy/phy-omap-control.c | 67
include/linux/usb/omap_control_usb.h | 24 +
2 files changed, 91 insertions(+)
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 1419ced..4f2502c 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -46,6 +46,73 @@ struct device *omap_get_control_dev(void)
EXPORT_SYMBOL_GPL(omap_get_control_dev);
/**
+ * omap_control_am335x_phy_wkup - PHY wakeup on/off using control
+ * module
+ * @dev: the control module device
+ * @on: 0 to off and 1 to on PHY wakeup feature
%s/off/disable %s/on/enable
+ * @id: phy id 0 or 1 for phy instance 0 and 1 repectively
+ *
+ * AM PHY driver should call this API to enable or disable PHY wakeup.
+ */
+void omap_control_am335x_phy_wkup(struct device *dev, bool on, u8 id)
+{
+ u32 val;
+ u32 *phy_wkup_reg;
+ struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+
+ if (control_usb-type != OMAP_CTRL_DEV_TYPE3 || id 0 || id 1)
+ return;
+
+ phy_wkup_reg = control_usb-dev_conf + AM335X_USB_WKUP_OFFSET;
+ val = readl(phy_wkup_reg);
+
+ if (on)
+ val |= id ? AM335X_USB1_WKUP_CTRL_ENABLE :
+ AM335X_USB0_WKUP_CTRL_ENABLE;
+ else
+ val = id ? ~AM335X_USB1_WKUP_CTRL_ENABLE :
+ ~AM335X_USB0_WKUP_CTRL_ENABLE;
+
+
You really need two blank spaces here?
+ writel(val, phy_wkup_reg);
+}
+EXPORT_SYMBOL_GPL(omap_control_am335x_phy_wkup);
+
+/**
+ * omap_control_am335x_phy_power - power on/off the serializer using control
+ * module
+ * @dev: the control module device
+ * @on: 0 to off and 1 to on based on powering on or off the PHY
+ * @id: phy id 0 or 1 for phy instance 0 and 1 repectively
+ *
+ * AM PHY driver should call this API to power on or off the PHY.
+ */
+void omap_control_am335x_phy_power(struct device *dev, bool on, u8 id)
+{
+ u32 val;
+ u32 *phy_pwr_reg;
+ struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+
+ if (control_usb-type != OMAP_CTRL_DEV_TYPE3 || id 0 || id 1)
+ return;
+
+ phy_pwr_reg = GET_PHY_POWER_REG(control_usb-dev_conf, id);
+ val = readl(phy_pwr_reg);
+
+ if (on) {
+ val = ~(AM335X_USBPHY_CM_PWRDN | AM335X_USBPHY_OTG_PWRDN);
+ val |= AM335X_USBPHY_OTGVDET_EN |
+ AM335X_USBPHY_OTGSESSEND_EN;
+ } else
+ val |= AM335X_USBPHY_CM_PWRDN | AM335X_USBPHY_OTG_PWRDN;
+
+
ditto.
+ writel(val, phy_pwr_reg);
+}
+EXPORT_SYMBOL_GPL(omap_control_am335x_phy_power);
+
+
ditto.
+/**
* omap_control_usb3_phy_power - power on/off the serializer using control
* module
* @dev: the control module device
diff --git a/include/linux/usb/omap_control_usb.h
b/include/linux/usb/omap_control_usb.h
index 27b5b8c..a92b417 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -45,6 +45,7 @@ enum omap_control_usb_mode {
/* To differentiate ctrl module IP having either mailbox or USB3 PHY power */
#define OMAP_CTRL_DEV_TYPE1 0x1
#define OMAP_CTRL_DEV_TYPE2 0x2
+#define OMAP_CTRL_DEV_TYPE3 0x3
#define OMAP_CTRL_DEV_PHY_PDBIT(0)
@@ -63,10 +64,23 @@ enum omap_control_usb_mode {
#define OMAP_CTRL_USB3_PHY_TX_RX_POWERON0x3
#define OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF 0x0
+#define GET_PHY_POWER_REG(base, id) ((u32 *)base + (id * 2))
how about
#define GET_PHY_POWER_REG(base, id) ((u32 *)(base) + ((id) * 2))
+#define AM335X_USBPHY_CM_PWRDN (1 0)
+#define AM335X_USBPHY_OTG_PWRDN (1 1)
+#define AM335X_USBPHY_OTGVDET_EN (1 19)
+#define AM335X_USBPHY_OTGSESSEND_EN (1 20)
+#define AM335X_USB_WKUP_OFFSET 0xA
+#define AM335X_USB0_WKUP_CTRL_ENABLE (1 0)
+#define AM335X_USB1_WKUP_CTRL_ENABLE (1 8)
+
+
+
too many blank lines...
Thanks
Kishon
--
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 2/4] phy: phy-amxxxx-usb: Add PHY driver for amxxxx platform
Hi,
On Friday 19 July 2013 06:04 PM, George Cherian wrote:
Adds phy driver support for am33xx platform, the host/device
peripheral controller shall get this phy object to control the phy
operations.
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/phy/Kconfig | 12 +++
drivers/phy/Makefile | 1 +
drivers/phy/phy-am-usb.c | 222
+++
3 files changed, 235 insertions(+)
create mode 100644 drivers/phy/phy-am-usb.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5f85909..55087ab 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -11,3 +11,15 @@ menuconfig GENERIC_PHY
devices present in the kernel. This layer will have the generic
API by which phy drivers can create PHY using the phy framework and
phy users can obtain reference to the PHY.
+if GENERIC_PHY
Greg has some comments about the generic PHY framework. So instead of having
like this, you might have to select GENERIC_PHY in the Kconfig of your phy
driver.
+
+config PHY_AM_USB
+ tristate AM USB2 PHY Driver
+ depends on SOC_AM33XX
+ help
+ Enable this to support the transceiver that is part of SOC. This
+ phy supports all LS/FS/HS speed and also supports OTG functionality.
+ The USB OTG controller communicates with this phy through stand UTMI
+ interface.
+
+endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..471c525 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_GENERIC_PHY)+= phy-core.o
+obj-$(CONFIG_PHY_AM_USB) += phy-am-usb.o
diff --git a/drivers/phy/phy-am-usb.c b/drivers/phy/phy-am-usb.c
new file mode 100644
index 000..d0b0749
--- /dev/null
+++ b/drivers/phy/phy-am-usb.c
@@ -0,0 +1,222 @@
+/*
+ * phy-am-usb.c - USB PHY, talking to usb controller in AMXXX.
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Author: George Cherian george.cher...@ti.com
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/of.h
+#include linux/usb/otg.h
+#include linux/clk.h
+#include linux/pm_runtime.h
+#include linux/usb/omap_control_usb.h
+#include linux/phy/phy.h
+
+struct am_usb {
+ struct usb_phy phy;
+ struct device *dev;
+ struct device *control_dev;
+ struct clk *optclk;
+ u32 id;
+};
+
+#define phy_to_amphy(x) container_of((x), struct am_phy, phy)
+
+
additional blank line..
+static int am_usb_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+ struct usb_phy *phy = otg-phy;
+
+ otg-host = host;
+ if (!host)
+ phy-state = OTG_STATE_UNDEFINED;
+
+ return 0;
+}
+
+static int am_usb_set_peripheral(struct usb_otg *otg,
+ struct usb_gadget *gadget)
+{
+ struct usb_phy *phy = otg-phy;
+
+ otg-gadget = gadget;
+ if (!gadget)
+ phy-state = OTG_STATE_UNDEFINED;
+
+ return 0;
+}
+
+
+
ditto..
+static int am_usb_power_off(struct phy *x)
+{
+ struct am_usb *phy = phy_get_drvdata(x);
+
+ omap_control_am335x_phy_power(phy-control_dev, 0, phy-id);
+
+ return 0;
+}
+
+static int am_usb_power_on(struct phy *x)
+{
+ struct am_usb *phy = phy_get_drvdata(x);
+
+ omap_control_am335x_phy_power(phy-control_dev, 1, phy-id);
+
+ return 0;
+}
+
+static struct phy_ops ops = {
+ .power_on = am_usb_power_on,
+ .power_off = am_usb_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int am_usb2_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev-dev.of_node;
+ struct am_usb *phy;
+ struct phy *generic_phy;
+ struct usb_otg *otg;
+ struct phy_provider *phy_provider;
+
+ phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
You might have to bail out of memory allocation fails.
+
+ otg = devm_kzalloc(pdev-dev, sizeof(*otg), GFP_KERNEL);
ditto.
+
+ phy-dev= pdev-dev;
+
+ if (np)
+ of_property_read_u32(np, id, phy-id);
+
+ phy-phy.dev= phy-dev;
+ phy-phy.label = am-usb2;
+
Re: [PATCH v2 3/4] arm: dts: Add USB phy nodes for AM33XX
Hi,
On Friday 19 July 2013 06:04 PM, George Cherian wrote:
Add phy nodes for AM33XX platform and split the musb nodes
per instance.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: George Cherian george.cher...@ti.com
---
arch/arm/boot/dts/am33xx.dtsi | 68
+--
1 file changed, 53 insertions(+), 15 deletions(-)
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 8e1248f..e3890c4 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -326,21 +326,59 @@
status = disabled;
};
- usb@4740 {
- compatible = ti,musb-am33xx;
- reg = 0x4740 0x1000/* usbss */
-0x47401000 0x800 /* musb instance 0 */
-0x47401800 0x800; /* musb instance 1 */
- interrupts = 17/* usbss */
- 18/* musb instance 0 */
- 19; /* musb instance 1 */
- multipoint = 1;
- num-eps = 16;
- ram-bits = 12;
- port0-mode = 3;
- port1-mode = 3;
- power = 250;
- ti,hwmods = usb_otg_hs;
+ phy1: am335x-usb0@44e10620 {
*44e10620* is not needed if you are not having the reg property.
+ compatible = ti,am335x-usb2;
+ #phy-cells = 0;
+ id= 0;
+ };
+
+ phy2: am335x-usb1@44e10628 {
+ compatible = ti,am335x-usb2;
ditto..
+ #phy-cells = 0;
+ id= 1;
+ };
+
+ omap_control_usb: omap-control-usb@44e10620 {
+ compatible = ti,omap-control-usb;
+ reg = 0x44e10620 0x10;
+ reg-names = control_dev_conf;
+ ti,type = 3;
+ };
+
+musb: usb@4740 {
+compatible = ti,musb-am33xx;
+reg = 0x4740 0x1000;
+ranges;
+#address-cells = 1;
+#size-cells = 1;
+interrupts = 17;
+ti,hwmods = usb_otg_hs;
+
+usb0@47401000 {
+reg = 0x47401000 0x800;
+interrupts = 18;
+interrupt-names = mc;
+multipoint = 1;
+num-eps = 16;
+ram-bits = 12;
+port-mode = 3;
+power = 250;
+phys = phy1;
+ phy-names = am335x-usb0;
Looks like alignment has gone wrong here.
+};
+
+usb1@47401800 {
+reg = 0x47401800 0x800;
+interrupts = 19;
+interrupt-names = mc;
+multipoint = 1;
+num-eps = 16;
+ram-bits = 12;
+port-mode = 3;
+power = 250;
+phys = phy2;
+ phy-names = am335x-usb1;
ditto.
Thanks
Kishon
--
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 4/4] usb: musb: dsps: Remove the phy control from glue and add phy driver APIs
On Friday 19 July 2013 06:04 PM, George Cherian wrote:
Remove usb phy control module access from platform glue.
The same is now done using am phy driver and phy-omap-control.
Adapt the driver to the split dt nodes.
This patch should be split.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: George Cherian george.cher...@ti.com
---
drivers/usb/musb/musb_dsps.c | 150
---
1 file changed, 55 insertions(+), 95 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 41f135a..6bec458 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -43,6 +43,7 @@
#include linux/of.h
#include linux/of_device.h
#include linux/of_address.h
+#include linux/of_irq.h
#include musb_core.h
@@ -110,8 +111,6 @@ struct dsps_musb_wrapper {
/* miscellaneous stuff */
u32 musb_core_offset;
u8 poll_seconds;
- /* number of musb instances */
- u8 instances;
};
/**
@@ -123,49 +122,11 @@ struct dsps_glue {
const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
struct timer_list timer[2]; /* otg_workaround timer */
unsigned long last_timer[2];/* last timer data for each instance */
- u32 __iomem *usb_ctrl[2];
};
-#define DSPS_AM33XX_CONTROL_MODULE_PHYS_0 0x44e10620
-#define DSPS_AM33XX_CONTROL_MODULE_PHYS_1 0x44e10628
+#define glue_to_musb(g, i) platform_get_drvdata(g-musb[i])
-static const resource_size_t dsps_control_module_phys[] = {
- DSPS_AM33XX_CONTROL_MODULE_PHYS_0,
- DSPS_AM33XX_CONTROL_MODULE_PHYS_1,
-};
-
-#define USBPHY_CM_PWRDN (1 0)
-#define USBPHY_OTG_PWRDN (1 1)
-#define USBPHY_OTGVDET_EN(1 19)
-#define USBPHY_OTGSESSEND_EN (1 20)
-
-/**
- * musb_dsps_phy_control - phy on/off
- * @glue: struct dsps_glue *
- * @id: musb instance
- * @on: flag for phy to be switched on or off
- *
- * This is to enable the PHY using usb_ctrl register in system control
- * module space.
- *
- * XXX: This function will be removed once we have a seperate driver for
- * control module
- */
-static void musb_dsps_phy_control(struct dsps_glue *glue, u8 id, u8 on)
-{
- u32 usbphycfg;
-
- usbphycfg = readl(glue-usb_ctrl[id]);
-
- if (on) {
- usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
- usbphycfg |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN;
- } else {
- usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN;
- }
- writel(usbphycfg, glue-usb_ctrl[id]);
-}
/**
* dsps_musb_enable - enable interrupts
*/
@@ -411,16 +372,28 @@ static int dsps_musb_init(struct musb *musb)
void __iomem *reg_base = musb-ctrl_base;
u32 rev, val;
int status;
+ char phyname[12];
/* mentor core register starts at offset of 0x400 from musb base */
musb-mregs += wrp-musb_core_offset;
- /* NOP driver needs change if supporting dual instance */
- usb_nop_xceiv_register();
- musb-xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
+ snprintf(phyname , 12, am335x-usb%d, pdev-id);
+
+ if (dev-of_node) {
+ musb-phy = devm_phy_get(dev, phyname);
+ musb-xceiv = devm_usb_get_phy_by_phandle(dev,
+ phys, 0);
+ } else {
+ musb-phy = devm_phy_get(dev, musb-phy_label);
+ musb-xceiv = devm_usb_get_phy_dev(dev, pdev-id);
+ }
+
if (IS_ERR_OR_NULL(musb-xceiv))
return -EPROBE_DEFER;
+ if (IS_ERR_OR_NULL(musb-phy))
+ return -EPROBE_DEFER;
+
/* Returns zero if e.g. not clocked */
rev = dsps_readl(reg_base, wrp-revision);
if (!rev) {
@@ -434,7 +407,8 @@ static int dsps_musb_init(struct musb *musb)
dsps_writel(reg_base, wrp-control, (1 wrp-reset));
/* Start the on-chip PHY and its PLL. */
- musb_dsps_phy_control(glue, pdev-id, 1);
+ phy_init(musb-phy);
+ phy_power_on(musb-phy);
musb-isr = dsps_interrupt;
@@ -448,8 +422,7 @@ static int dsps_musb_init(struct musb *musb)
return 0;
err0:
- usb_put_phy(musb-xceiv);
- usb_nop_xceiv_unregister();
+ devm_phy_put(dev-parent, musb-phy);
return status;
}
@@ -462,11 +435,8 @@ static int dsps_musb_exit(struct musb *musb)
del_timer_sync(glue-timer[pdev-id]);
/* Shutdown the on-chip PHY and its PLL. */
- musb_dsps_phy_control(glue, pdev-id, 0);
-
- /* NOP driver needs change if supporting dual instance */
- usb_put_phy(musb-xceiv);
- usb_nop_xceiv_unregister();
+ phy_power_off(musb-phy);
+ phy_exit(musb-phy);
return 0;
}
@@ -483,49 +453,34 @@ static struct musb_platform_ops dsps_ops = {
static u64 musb_dmamask =
Re: [PATCH v2 3/4] arm: dts: Add USB phy nodes for AM33XX
Hello.
On 19-07-2013 16:34, George Cherian wrote:
Add phy nodes for AM33XX platform and split the musb nodes
per instance.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
Signed-off-by: George Cherian george.cher...@ti.com
---
arch/arm/boot/dts/am33xx.dtsi | 68 +--
1 file changed, 53 insertions(+), 15 deletions(-)
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 8e1248f..e3890c4 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -326,21 +326,59 @@
status = disabled;
};
- usb@4740 {
- compatible = ti,musb-am33xx;
- reg = 0x4740 0x1000 /* usbss */
- 0x47401000 0x800 /* musb instance 0 */
- 0x47401800 0x800;/* musb instance 1 */
- interrupts = 17 /* usbss */
- 18/* musb instance 0 */
- 19; /* musb instance 1 */
- multipoint = 1;
- num-eps = 16;
- ram-bits = 12;
- port0-mode = 3;
- port1-mode = 3;
- power = 250;
- ti,hwmods = usb_otg_hs;
+ phy1: am335x-usb0@44e10620 {
Shouldn't the PHYs be *under* the usb0/1 device nodes in the hierarchy?
They're not on the same bus as the MUSB controllers for sure.
+ compatible = ti,am335x-usb2;
+ #phy-cells = 0;
+ id= 0;
Forgot space before =.
+ };
+
+ phy2: am335x-usb1@44e10628 {
+ compatible = ti,am335x-usb2;
+ #phy-cells = 0;
+ id= 1;
Same here.
+ };
+
+ omap_control_usb: omap-control-usb@44e10620 {
+ compatible = ti,omap-control-usb;
+ reg = 0x44e10620 0x10;
+ reg-names = control_dev_conf;
+ ti,type = 3;
+ };
+
+musb: usb@4740 {
+compatible = ti,musb-am33xx;
+reg = 0x4740 0x1000;
+ranges;
+#address-cells = 1;
+#size-cells = 1;
+interrupts = 17;
+ti,hwmods = usb_otg_hs;
+
+usb0@47401000 {
I don't think you need index in the name since you have the address as a
part of the name anyway. That way it's closer to the ePAPR spec.
+reg = 0x47401000 0x800;
+interrupts = 18;
+interrupt-names = mc;
+multipoint = 1;
+num-eps = 16;
+ram-bits = 12;
+port-mode = 3;
+power = 250;
+phys = phy1;
The above lines are indented with spaces, the below one with tabs. Use
tabs please.
+ phy-names = am335x-usb0;
+};
+
+usb1@47401800 {
+reg = 0x47401800 0x800;
+interrupts = 19;
+interrupt-names = mc;
+multipoint = 1;
+num-eps = 16;
+ram-bits = 12;
+port-mode = 3;
+power = 250;
+phys = phy2;
The above lines are indented with spaces, the below one with tabs. Use
tabs please.
+ phy-names = am335x-usb1;
+ };
};
WBR, Sergei
--
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 3/4] arm: dts: Add USB phy nodes for AM33XX
On 07/19/2013 03:56 PM, Sergei Shtylyov wrote:
On 19-07-2013 16:34, George Cherian wrote:
diff --git a/arch/arm/boot/dts/am33xx.dtsi
b/arch/arm/boot/dts/am33xx.dtsi
index 8e1248f..e3890c4 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -326,21 +326,59 @@
status = disabled;
};
-usb@4740 {
-compatible = ti,musb-am33xx;
-reg = 0x4740 0x1000/* usbss */
- 0x47401000 0x800/* musb instance 0 */
- 0x47401800 0x800;/* musb instance 1 */
-interrupts = 17/* usbss */
- 18/* musb instance 0 */
- 19;/* musb instance 1 */
-multipoint = 1;
-num-eps = 16;
-ram-bits = 12;
-port0-mode = 3;
-port1-mode = 3;
-power = 250;
-ti,hwmods = usb_otg_hs;
+phy1: am335x-usb0@44e10620 {
Shouldn't the PHYs be *under* the usb0/1 device nodes in the hierarchy?
They're not on the same bus as the MUSB controllers for sure.
I redo the complete thing. I have now:
usb: usb@4740 {
compatible = ti,am33xx-usb;
usb0_phy: phy@47401300 {
compatible = ti,am335x-usb-phy;
}
usb0: usb@47401000 {
musb0: usb@47401400 {
compatible = mg,musbmhdrc;
}
}
usb1_phy: phy@47402300 {
compatible = ti,am335x-usb-phy;
}
usb1: usb@47402000 {
musb1: usb@47402400 {
compatible = mg,musbmhdrc;
}
}
}
And you want usb0_phy to be child of usb0? In the TRM they are all in
the same block.
WBR, Sergei
Sebastian
--
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 driver problem
On Thu, 18 Jul 2013, Sarah Sharp wrote: Then try using your other camera. If that doesn't work, you may have to unconfigure the device by running something like: root@xanatos:/sys/bus/usb/devices/3-1.6# echo 0 bConfigurationValue Alan, is that the right command? It fails on my machine with -EINVAL. Yes, it is the right command. You may need to do a little debugging to find out where in usb_set_configuration() the error occurs. 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] backports: backport drvdata = NULL core driver fixes
On Thu, 18 Jul 2013, Luis R. Rodriguez wrote: From: Luis R. Rodriguez mcg...@do-not-panic.com The Linux kernel had tons of code which at times cleared the drvdata upon probe failure or release. There are however a bunch of drivers that didn't clear this. Commit 0998d063 implmented clearing this upon device_release_driver() and dealt with probe failure on driver_probe_device(). After this the kernel was cleaned up separately with *tons* of patches to remove all these driver specific settings given that the clearing is now done internally by the device core. Instead of ifdef'ing code back in for older code where it was properly in place backport this by piggy backing the new required code upon the calls used in place. There is a small race here upon device_release_driver() but we can live with that theoretical race. Due to the way we hack this backport we can't use a separate namespace as we have with other symbols. mcgrof@frijol ~/linux-stable (git::master)$ git describe --contains \ 0998d0631001288a5974afc0b2a5f568bcdecb4d v3.6-rc1~99^2~14^2~17 commit 0998d0631001288a5974afc0b2a5f568bcdecb4d Author: Hans de Goede hdego...@redhat.com Date: Wed May 23 00:09:34 2012 +0200 device-core: Ensure drvdata = NULL when no driver is bound 1) drvdata is for a driver to store a pointer to driver specific data 2) If no driver is bound, there is no driver specific data associated with the device 3) Thus logically drvdata should be NULL if no driver is bound. But many drivers don't clear drvdata on device_release, or set drvdata early on in probe and leave it set on probe error. Both of which results in a dangling pointer in drvdata. This patch enforce for drvdata to be NULL after device_release or on probe failure. Signed-off-by: Hans de Goede hdego...@redhat.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org This is not a very good idea. Although setting drvdata to NULL allowed a lot of code to be removed, it also exposed a bunch of hidden bugs -- drivers were using the drvdata value even after their remove function returned. Several commits have been applied to fix those bugs. Finding and backporting them and their dependencies will not be easy. I suggest this patch not be applied. 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 v12 00/13] Add tested id switch and vbus connect detect support for Chipidea
Hi Alexander, On Thu, Jul 11, 2013 at 3:27 AM, Peter Chen peter.c...@freescale.com wrote: This patchset adds tested otg id switch function and vbus connect and disconnect detection for chipidea driver. And fix kinds of bugs found at chipidea drivers after enabling id and vbus detection. This patch is fully tested at imx6 sabresd platform. My chipidea repo: https://github.com/hzpeterchen/linux-usb.git Changes for v12: - Rebased greg's usb-next tree (3.10.0-rc7+) - Split more small patches for single function and fix. Any comments about this series? -- 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/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
On 07/18/2013 07:37 PM, Peter Hurley wrote: On 07/18/2013 06:09 PM, Sarah Sharp wrote: On Thu, Jul 18, 2013 at 04:28:01PM -0400, Peter Hurley wrote: [ +cc Sarah Sharp, linux-usb ] On 07/18/2013 09:21 AM, Nestor Lopez Casado wrote: This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887. This patch re-adds the workaround introduced by 596264082f10dd4 which was reverted by 8af6c08830b1ae114. The original patch 596264 was needed to overcome a situation where the hid-core would drop incoming reports while probe() was being executed. This issue was solved by c849a6143bec520af which added hid_device_io_start() and hid_device_io_stop() that enable a specific hid driver to opt-in for input reports while its probe() is being executed. Commit a9dd22b730857347 modified hid-logitech-dj so as to use the functionality added to hid-core. Having done that, workaround 596264 was no longer necessary and was reverted by 8af6c08. We now encounter a different problem that ends up 'again' thwarting the Unifying receiver enumeration. The problem is time and usb controller dependent. Ocasionally the reports sent to the usb receiver to start the paired devices enumeration fail with -EPIPE and the receiver never gets to enumerate the paired devices. With dcd9006b1b053c7b1c the problem was hidden as the call to the usb driver became asynchronous and none was catching the error from the failing URB. As the root cause for this failing SET_REPORT is not understood yet, -possibly a race on the usb controller drivers or a problem with the Unifying receiver- reintroducing this workaround solves the problem. Before we revert to using the workaround, I'd like to suggest that this new hidden problem may be an interaction with the xhci_hcd host controller driver only. Looking at the related bug, the OP indicates the machine only has USB3 ports. Additionally, comments #7, #100, and #104 of the original bug report [1] add additional information that would seem to confirm this suspicion. Question: does this USB device need a control transfer to reset its endpoints when the endpoints are not actually halted? If so, yes, that is a known xHCI driver bug that needs to be fixed. The xHCI host will not accept a Reset Endpoint command when the endpoints are not actually halted, but the USB core will send the control transfer to reset the endpoint. That means the device and host toggles will be out of sync, and all messages will start to fail with -EPIPE. Can the OP capture a usbmon trace when the device starts failing? That will reveal whether this actually is the issue. dmesg output with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on would also be helpful. Sarah, I forwarded your usbmon capture request to the OP in the bug report (I don't have an email address for the reporter). As far as getting printk output from a custom kernel, I think that may be beyond the reporter's capability. Perhaps one of the Ubuntu devs triaging this bug could provide a test kernel for the OP with those options on. Joseph, would you be willing to do that? Sure thing. I'll build a kernel and request that the bug reporter collect usbmon data. Thanks everyone for the feedback! Regards, Peter Hurley -- 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/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
On Thu, 18 Jul 2013, Sarah Sharp wrote: Question: does this USB device need a control transfer to reset its endpoints when the endpoints are not actually halted? If so, yes, that is a known xHCI driver bug that needs to be fixed. The xHCI host will not accept a Reset Endpoint command when the endpoints are not actually halted, but the USB core will send the control transfer to reset the endpoint. That means the device and host toggles will be out of sync, and all messages will start to fail with -EPIPE. Why -EPIPE? Isn't that code reserved to indicate a STALL? In fact, there's no way to detect a toggle mismatch problem with a USB-2 device. Packets with the wrong toggle value are simply ignored (or acknowledged and ignored). The problem ends up appearing as an indefinite delay (for an IN transfer) or as data lost (for an OUT transfer). I don't know what happens with USB-3 devices when the sequence numbers get out of alignment. 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: Audio I/O parameters
On Fri, 19 Jul 2013, James Stone wrote: The questions now are: Why are the same requests sent over and over again? Why does the ALSA driver attempt to set the clock frequency while the clock is actively in use? Has this behavior changed since the 3.5 kernel? Well, I think these requests may correspond to the lights flashing on and off on the front of the device. When starting the device in 3.5 at 256 frames/period (duplex), the lights flash on and off 2 times, in the current patched 3.8 version I have been using, the device lights flash on and off 4 times before starting jack with exactly the same settings - so it seems for some reason, the requests are going through multiple times on the 3.8 kernel but not on 3.5. I will send a 3.5 usbmon trace of a successful start off list (plus the same for 3.8?) if it would be useful. I don't know -- it's up to Clemens. 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: How should we handle isochronous underruns?
On Fri, 19 Jul 2013, Ming Lei wrote: Even we can introduce one flag of 'completing' in 'struct urb' to check if the USB's submit is called inside its own complete(), by which we can check if resubmission is in underrun easily. Consider that isoc URB's resubmission in completion handler should cover 99% cases, I think this approach is doable. For other resubmission from tasklet or workque cases, either let them alone or move the resubmission inside completion() handler, or introduce simple helper to mark start of frame only for them. Anyway, there are very few isoc resubmissions from non-completion handler (only two drivers found in my urb complet() cleanup work), so it shouldn't be a big deal. What do you think about this approach? Why go to all this trouble? We already have the URB_ISO_ASAP flag. Trying to check the context of an URB submission strikes me as not robust at all. Who knows what tricks drivers will get up to in the future? Adding a new API seems like the cleanest solution. (Although I still think that usb_reset_endpoint() would be appropriate.) Actually 'start of stream' should be done inside usbcore because it is HCD-independent function, so it is better to provide the information from usbcore and let HCD use it if HCD needs the flag. I'm not quite sure what you mean. Note that usb_reset_endpoint() _is_ a function in usbcore. But never mind. For a new API, how about void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *); Does that look okay? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [RFC] EHCI: add to memory barrier to updating hw_next
On Fri, 19 Jul 2013, Gioh Kim wrote: I was going to ask the same question. This particular piece of code gets executed _only_ when an URB is unlinked. Not during any other kind of error. I've got the problem when I listened to the mp3 file of USB HDD. I checked the urb data when the problem occurred, the last-status value of urb was EINPROGRESS and urb-unlinked was ECONNRESET. Ah, so the URB _was_ unlinked. I think the 'stopped' case was occurred by the reset of USB port. The block device driver did reset USB port because there is no return from USB device. Okay. If I made block device driver could not reset USB port, the EHCI driver codes were not executed. Finally the halt of HC makes 'stopped' case. Why was the HC halted? That should happen only when there is an extremely severe error. I think halt of the HC might be caused that store-buffer delays command for HC. When I applied the patch from https://lkml.org/lkml/2011/8/31/344 and added a mb() into hw_next updating to remove delay of store-buffer, My platform works well. Can the store-buffer delay halt HC? Is it possible? I don't see how. It could slow things down but it should not cause any errors. IMHO, if the qTD list is broken the HC think there is no qTD to send. So I added mb() at hw_next update code. At the time when the hw_next update gets executed, what is the value of state? It should be QH_STATE_IDLE. 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 01/15] drivers: phy: add generic PHY framework
On 07/19/2013 12:36 AM, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:59 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:13 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id); Your naming is odd, no phy anywhere in it? You rely on the sender to never send a duplicate name.id pair? Why not create your own ids based on the number of phys in the system, like almost all other classes and subsystems do? hmm.. some PHY drivers use the id they provide to perform some of their internal operation as in [1] (This is used only if a single PHY provider implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO to give the PHY drivers an option to use auto id. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, who cares about the id? No one outside of the phy core ever should, because you pass back the only pointer that they really do care about, if they need to do anything with the device. Use that, and then you can hmm.. ok. rip out all of the search for a phy by a string logic, as that's not Actually this is needed for non-dt boot case. In the case of dt boot, we use a phandle by which the controller can get a reference to the phy. But in the case of non-dt boot, the controller can get a reference to the phy only by label. I don't understand. They registered the phy, and got back a pointer to it. Why can't they save it in their local structure to use it again later if needed? They should never have to ask for the device, as the One is a *PHY provider* driver which is a driver for some PHY device. This will use phy_create to create the phy. The other is a *PHY consumer* driver which might be any controller driver (can be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the phy (by *phandle* in the case of dt boot and *label* in the case of non-dt boot). device id might be unknown if there are multiple devices in the system. I agree with you on the device id part. That need not be known to the PHY driver. How does a consumer know which label to use in a non-dt system if there are multiple PHYs in the system? That should be passed using platform data. I don't think that's right. That's just like passing clock names in platform data, which I believe is frowned upon. Instead, why not take the approach that e.g. regulators have taken? Each driver defines the names of the objects that it requires. There is a table (registered by a board file) that has lookup key (device name, regulator name), and the output is the regulator device (or global regulator name). This is almost the same way that DT works, except that in DT, the table is represented as properties in the DT. The DT binding defines the names of those properties, or the strings that must be in the foo-names properties, just like a driver in non-DT Linux is going to define the names it expects to be provided with. That way, you don't need platform data to define the names. -- 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/2] Revert Revert HID: Fix logitech-dj: missing Unifying device issue
Hi Sarah, On Fri, Jul 19, 2013 at 12:09 AM, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: On Thu, Jul 18, 2013 at 04:28:01PM -0400, Peter Hurley wrote: [ +cc Sarah Sharp, linux-usb ] On 07/18/2013 09:21 AM, Nestor Lopez Casado wrote: This reverts commit 8af6c08830b1ae114d1a8b548b1f8b056e068887. This patch re-adds the workaround introduced by 596264082f10dd4 which was reverted by 8af6c08830b1ae114. The original patch 596264 was needed to overcome a situation where the hid-core would drop incoming reports while probe() was being executed. This issue was solved by c849a6143bec520af which added hid_device_io_start() and hid_device_io_stop() that enable a specific hid driver to opt-in for input reports while its probe() is being executed. Commit a9dd22b730857347 modified hid-logitech-dj so as to use the functionality added to hid-core. Having done that, workaround 596264 was no longer necessary and was reverted by 8af6c08. We now encounter a different problem that ends up 'again' thwarting the Unifying receiver enumeration. The problem is time and usb controller dependent. Ocasionally the reports sent to the usb receiver to start the paired devices enumeration fail with -EPIPE and the receiver never gets to enumerate the paired devices. With dcd9006b1b053c7b1c the problem was hidden as the call to the usb driver became asynchronous and none was catching the error from the failing URB. As the root cause for this failing SET_REPORT is not understood yet, -possibly a race on the usb controller drivers or a problem with the Unifying receiver- reintroducing this workaround solves the problem. Before we revert to using the workaround, I'd like to suggest that this new hidden problem may be an interaction with the xhci_hcd host controller driver only. Looking at the related bug, the OP indicates the machine only has USB3 ports. Additionally, comments #7, #100, and #104 of the original bug report [1] add additional information that would seem to confirm this suspicion. Question: does this USB device need a control transfer to reset its endpoints when the endpoints are not actually halted? If so, yes, that is a known xHCI driver bug that needs to be fixed. The xHCI host will not accept a Reset Endpoint command when the endpoints are not actually halted, but the USB core will send the control transfer to reset the endpoint. That means the device and host toggles will be out of sync, and all messages will start to fail with -EPIPE. Could you re-phrase your question providing a bit more detail? I don't quite get the idea. Can the OP capture a usbmon trace when the device starts failing? That will reveal whether this actually is the issue. dmesg output with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on would also be helpful. Sarah Sharp Cheers, Nestor -- 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 3/4] arm: dts: Add USB phy nodes for AM33XX
Hello.
On 07/19/2013 06:20 PM, Sebastian Andrzej Siewior wrote:
diff --git a/arch/arm/boot/dts/am33xx.dtsi
b/arch/arm/boot/dts/am33xx.dtsi
index 8e1248f..e3890c4 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -326,21 +326,59 @@
status = disabled;
};
-usb@4740 {
-compatible = ti,musb-am33xx;
-reg = 0x4740 0x1000/* usbss */
- 0x47401000 0x800/* musb instance 0 */
- 0x47401800 0x800;/* musb instance 1 */
-interrupts = 17/* usbss */
- 18/* musb instance 0 */
- 19;/* musb instance 1 */
-multipoint = 1;
-num-eps = 16;
-ram-bits = 12;
-port0-mode = 3;
-port1-mode = 3;
-power = 250;
-ti,hwmods = usb_otg_hs;
+phy1: am335x-usb0@44e10620 {
Shouldn't the PHYs be *under* the usb0/1 device nodes in the hierarchy?
They're not on the same bus as the MUSB controllers for sure.
I redo the complete thing. I have now:
usb: usb@4740 {
compatible = ti,am33xx-usb;
usb0_phy: phy@47401300 {
compatible = ti,am335x-usb-phy;
}
usb0: usb@47401000 {
musb0: usb@47401400 {
compatible = mg,musbmhdrc;
}
}
usb1_phy: phy@47402300 {
compatible = ti,am335x-usb-phy;
}
usb1: usb@47402000 {
musb1: usb@47402400 {
compatible = mg,musbmhdrc;
}
}
}
And you want usb0_phy to be child of usb0? In the TRM they are all in
the same block.
Ah, the fact that PHYs didn't have the reg property got me muddled, I
didn't pay attention to the address part of the node names... BTW, where is
the reg prop? I see PHYs share the address space with
omap-control-usb@44e10620 device -- what's the point with this?
Sebastian
WBR, Sergei
--
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: Add Device Tree support to XHCI Platform driver
Hello.
On 07/19/2013 02:40 AM, Sergei Shtylyov wrote:
Add Device Tree match table. Setup dma_mask and coherent_dma_mask
if they're not already set.
Signed-off-by: Al Cooper alcoop...@gmail.com
[...]
@@ -186,11 +198,17 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+{ .compatible = xhci-hcd },
I think just xhci would be enough. We're describing the device, not its
driver with this prop.
BTW, you need to describe the xHCI DT binding somewhere in
Documentation/devicetree/bindings/usb/ in the same patch.
PS: Sarah, sorry for commenting on xHCI patches, I hope my comments were more
to the point this time. :-)
WBR, Sergei
--
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 3/4] arm: dts: Add USB phy nodes for AM33XX
On 07/19/2013 08:33 PM, Sergei Shtylyov wrote:
Hello.
Hello,
usb: usb@4740 {
compatible = ti,am33xx-usb;
usb0_phy: phy@47401300 {
compatible = ti,am335x-usb-phy;
}
usb0: usb@47401000 {
musb0: usb@47401400 {
compatible = mg,musbmhdrc;
}
}
usb1_phy: phy@47402300 {
compatible = ti,am335x-usb-phy;
}
usb1: usb@47402000 {
musb1: usb@47402400 {
compatible = mg,musbmhdrc;
}
}
}
And you want usb0_phy to be child of usb0? In the TRM they are all in
the same block.
Ah, the fact that PHYs didn't have the reg property got me muddled,
I didn't pay attention to the address part of the node names... BTW,
where is the reg prop?
I skipped it for the general idea. I planned to repost is today but I
messed up dsps and need to get it working first…
I see PHYs share the address space with
omap-control-usb@44e10620 device -- what's the point with this?
I decided to get rid of this. Both phys have 8 bytes (2 registers)
which are exclusive for them.
There is one register for the wakeup which is shared by both.
I changed this to limit it only to the 8bytes per phy. I care about
wakeup later - hopefully George will take care of this :)
Sebastian
WBR, Sergei
Sebastian
--
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] usbserial: append Petatel NP10T device to GSM modems list
On Fri, Jul 19, 2013 at 10:21:23AM +0300, Daniil Bolsun wrote: This patch was tested on 3.10.1 kernel. Same models of Petatel NP10T modems have different device IDs. Unfortunately they have no additional revision information on a board which may treat them as different devices. Currently I've seen only two NP10T devices with various IDs. Possibly Petatel NP10T list will be appended upon devices with new IDs will appear. Signed-off-by: Daniil Bolsun dan.bol...@gmail.com Much better resubmission, thanks. I'll queue it up in a few days and you will get an automated email from my system then. 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] staging: dwc2: fix thinko in dwc2_hc_set_even_odd_frame()
On 07/19/2013 01:55 PM, Paul Zimmerman wrote: From: Stephen Warren [mailto:swar...@wwwdotorg.org] Sent: Thursday, July 18, 2013 11:01 PM ... I then wanted to try WiFi, so I plugged in a USB mouse/keyboard, and started X, trying to use GUI tools. Then I saw some issues. With just the USB mouse/keyboard attached (via a powered hub), and no WiFi device yet, they would work for a while, but pretty soon I kept seeing all USB devices just disappear; only the Linux Foundation 2.0 root hub would be left. Unplugging and replugging didn't fix this; I had to power-cycle. I wonder if there are issues with just USB interrupt transfers, which I assume both HID devices and the USB SD card plug/unplug notifications use?? Hi Stephen, Are you sure you added both of the new driver parameters to platform.c? I was seeing the same kind of hangs before I set .ahbcfg = 0x10. I'm afraid so. You can see the exact code I ran at: https://github.com/swarren/linux-rpi/commits/rpi_dev -- 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] staging: dwc2: fix thinko in dwc2_hc_set_even_odd_frame()
From: Stephen Warren [mailto:swar...@wwwdotorg.org] Sent: Thursday, July 18, 2013 11:01 PM ... I then wanted to try WiFi, so I plugged in a USB mouse/keyboard, and started X, trying to use GUI tools. Then I saw some issues. With just the USB mouse/keyboard attached (via a powered hub), and no WiFi device yet, they would work for a while, but pretty soon I kept seeing all USB devices just disappear; only the Linux Foundation 2.0 root hub would be left. Unplugging and replugging didn't fix this; I had to power-cycle. I wonder if there are issues with just USB interrupt transfers, which I assume both HID devices and the USB SD card plug/unplug notifications use?? Hi Stephen, Are you sure you added both of the new driver parameters to platform.c? I was seeing the same kind of hangs before I set .ahbcfg = 0x10. -- Paul -- 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] backports: backport drvdata = NULL core driver fixes
On Fri, Jul 19, 2013 at 2:07 PM, Luis R. Rodriguez mcg...@do-not-panic.com wrote: This is not a very good idea. Although setting drvdata to NULL allowed a lot of code to be removed, it also exposed a bunch of hidden bugs -- drivers were using the drvdata value even after their remove function returned. Eek, have we not SmPL'ify'd a proof yet to ensure code like this no longer exists? Julia? :) Come to think of it, perhaps we should require *proof* with SmPL like this in future to avoid regressions ? Luis -- 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] backports: backport drvdata = NULL core driver fixes
On Fri, Jul 19, 2013 at 7:17 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 18 Jul 2013, Luis R. Rodriguez wrote: From: Luis R. Rodriguez mcg...@do-not-panic.com The Linux kernel had tons of code which at times cleared the drvdata upon probe failure or release. There are however a bunch of drivers that didn't clear this. Commit 0998d063 implmented clearing this upon device_release_driver() and dealt with probe failure on driver_probe_device(). After this the kernel was cleaned up separately with *tons* of patches to remove all these driver specific settings given that the clearing is now done internally by the device core. Instead of ifdef'ing code back in for older code where it was properly in place backport this by piggy backing the new required code upon the calls used in place. There is a small race here upon device_release_driver() but we can live with that theoretical race. Due to the way we hack this backport we can't use a separate namespace as we have with other symbols. mcgrof@frijol ~/linux-stable (git::master)$ git describe --contains \ 0998d0631001288a5974afc0b2a5f568bcdecb4d v3.6-rc1~99^2~14^2~17 commit 0998d0631001288a5974afc0b2a5f568bcdecb4d Author: Hans de Goede hdego...@redhat.com Date: Wed May 23 00:09:34 2012 +0200 device-core: Ensure drvdata = NULL when no driver is bound 1) drvdata is for a driver to store a pointer to driver specific data 2) If no driver is bound, there is no driver specific data associated with the device 3) Thus logically drvdata should be NULL if no driver is bound. But many drivers don't clear drvdata on device_release, or set drvdata early on in probe and leave it set on probe error. Both of which results in a dangling pointer in drvdata. This patch enforce for drvdata to be NULL after device_release or on probe failure. Signed-off-by: Hans de Goede hdego...@redhat.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org This is not a very good idea. Although setting drvdata to NULL allowed a lot of code to be removed, it also exposed a bunch of hidden bugs -- drivers were using the drvdata value even after their remove function returned. Eek, have we not SmPL'ify'd a proof yet to ensure code like this no longer exists? Julia? :) Several commits have been applied to fix those bugs. I actualy found *tons* ! Finding and backporting them and their dependencies will not be easy. May the SmPL proof be with us. I suggest this patch not be applied. Thanks for the review Alan! Luis -- 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] backports: backport drvdata = NULL core driver fixes
On Fri, 19 Jul 2013, Luis R. Rodriguez wrote: On Fri, Jul 19, 2013 at 2:07 PM, Luis R. Rodriguez mcg...@do-not-panic.com wrote: This is not a very good idea. Although setting drvdata to NULL allowed a lot of code to be removed, it also exposed a bunch of hidden bugs -- drivers were using the drvdata value even after their remove function returned. Eek, have we not SmPL'ify'd a proof yet to ensure code like this no longer exists? Julia? :) Come to think of it, perhaps we should require *proof* with SmPL like this in future to avoid regressions ? Is it a concurrency problem? SmPL is not so good at that in the general case. One would have to know a specific case where other functions of the driver can be invoked after remove. julia -- 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] backports: backport drvdata = NULL core driver fixes
On Fri, Jul 19, 2013 at 2:27 PM, Julia Lawall julia.law...@lip6.fr wrote: On Fri, 19 Jul 2013, Luis R. Rodriguez wrote: On Fri, Jul 19, 2013 at 2:07 PM, Luis R. Rodriguez mcg...@do-not-panic.com wrote: This is not a very good idea. Although setting drvdata to NULL allowed a lot of code to be removed, it also exposed a bunch of hidden bugs -- drivers were using the drvdata value even after their remove function returned. Eek, have we not SmPL'ify'd a proof yet to ensure code like this no longer exists? Julia? :) Come to think of it, perhaps we should require *proof* with SmPL like this in future to avoid regressions ? Is it a concurrency problem? SmPL is not so good at that in the general case. One would have to know a specific case where other functions of the driver can be invoked after remove. Thanks Julia. In that case I'm going to just leave this in place given that if there's a bug upstream we'll get it fixed as soon as a respective patch gets upstream as well. That is, we are not using old drivers, we use the same upstream drivers so if a regression was found in backports the fix must go upstream s well. This is one of the benefits of backporting -- the range of users and testers increases and we still benefit from the upstream bandwagon. Luis -- 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: Add Device Tree support to XHCI Platform driver
On Fri, Jul 19, 2013 at 10:42:50PM +0400, Sergei Shtylyov wrote:
Hello.
On 07/19/2013 02:40 AM, Sergei Shtylyov wrote:
Add Device Tree match table. Setup dma_mask and coherent_dma_mask
if they're not already set.
Signed-off-by: Al Cooper alcoop...@gmail.com
[...]
@@ -186,11 +198,17 @@ static int xhci_plat_remove(struct platform_device
*dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+{ .compatible = xhci-hcd },
I think just xhci would be enough. We're describing the device, not its
driver with this prop.
BTW, you need to describe the xHCI DT binding somewhere in
Documentation/devicetree/bindings/usb/ in the same patch.
PS: Sarah, sorry for commenting on xHCI patches, I hope my comments
were more to the point this time. :-)
Yes, they were appropriate. Thank you for increasing your signal to
noise ratio. :)
Sarah Sharp
--
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/10] backports: backport drvdata = NULL core driver fixes
From: Luis R. Rodriguez mcg...@do-not-panic.com
The Linux kernel had tons of code which at times cleared the
drvdata upon probe failure or release. There are however a bunch
of drivers that didn't clear this.
Commit 0998d063 implmented clearing this upon device_release_driver()
and dealt with probe failure on driver_probe_device(). After this the
kernel was cleaned up separately with *tons* of patches to remove all
these driver specific settings given that the clearing is now done
internally by the device core.
Instead of ifdef'ing code back in for older code where it was properly
in place backport this by piggy backing the new required code upon the
calls used in place. There is a small race here upon device_release_driver()
but we can live with that theoretical race.
Due to the way we hack this backport we can't use a separate namespace
as we have with other symbols.
mcgrof@frijol ~/linux-stable (git::master)$ git describe --contains \
0998d0631001288a5974afc0b2a5f568bcdecb4d
v3.6-rc1~99^2~14^2~17
I count 65 patches implemented after this:
mcgrof@frijol ~/linux-stable (git::master)$ git format-patch \
--grep=device-core: Ensure drvdata = NULL when no driver is bound \
-o null-drv-fix v3.6-rc1~99^2~14^2~17..
TL;DR
Alan Stern argued that perhaps applying this to backports wasn't a good
idea given that evidence shows that the original patch actually exposed
tons of bugs in driver code where they were doing the wrong thing.
While this may be true if the original patch was a bad idea it should
be reverted, and if a bug is found upstream, then by all means
finding it through backports will only accelerate the pace at which
we fix these exposed bugs. That is, if a bug is found due to this on
backports then a respective fix for it should go upstream, not to
backports. This is the benefit of providing backports releases: keep
your users engaged on upstream fixes.
Furthermore I am in hopes that perhaps we can SmPL'ify the bugs
instead and in the future perhaps require SmPL to proove that
the what the original patch was doing won't affect the inverse
of what the patch was trying to do -- that is drivers doing the
wrong thing.
commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
Author: Hans de Goede hdego...@redhat.com
Date: Wed May 23 00:09:34 2012 +0200
device-core: Ensure drvdata = NULL when no driver is bound
1) drvdata is for a driver to store a pointer to driver specific data
2) If no driver is bound, there is no driver specific data associated with
the device
3) Thus logically drvdata should be NULL if no driver is bound.
But many drivers don't clear drvdata on device_release, or set drvdata
early on in probe and leave it set on probe error. Both of which results
in a dangling pointer in drvdata.
This patch enforce for drvdata to be NULL after device_release or on probe
failure.
Signed-off-by: Hans de Goede hdego...@redhat.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Tested with ckmake against next-20130618:
1 2.6.24 [ OK ]
2 2.6.25 [ OK ]
3 2.6.26 [ OK ]
4 2.6.27 [ OK ]
5 2.6.28 [ OK ]
6 2.6.29 [ OK ]
7 2.6.30 [ OK ]
8 2.6.31 [ OK ]
9 2.6.32 [ OK ]
10 2.6.33 [ OK ]
11 2.6.34 [ OK ]
12 2.6.35 [ OK ]
13 2.6.36 [ OK ]
14 2.6.37 [ OK ]
15 2.6.38 [ OK ]
16 2.6.39 [ OK ]
17 3.0.79 [ OK ]
18 3.1.10 [ OK ]
19 3.10-rc1[ OK ]
20 3.2.45 [ OK ]
21 3.3.8 [ OK ]
22 3.4.46 [ OK ]
23 3.5.7 [ OK ]
24 3.6.11 [ OK ]
25 3.7.10 [ OK ]
26 3.8.13 [ OK ]
27 3.9.3 [ OK ]
real32m2.332s
user860m23.688s
sys 121m20.840s
Cc: Hans de Goede hdego...@redhat.com
Cc: Julia Lawall julia.law...@lip6.fr
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Jiri Slaby jsl...@suse.cz
Cc: Jiri Kosina jkos...@suse.cz
Cc: Felix Fietkau n...@openwrt.org
Signed-off-by: Luis R. Rodriguez mcg...@do-not-panic.com
---
backport/backport-include/linux/device.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/backport/backport-include/linux/device.h
b/backport/backport-include/linux/device.h
index c2f80e2..ba55d0e 100644
--- a/backport/backport-include/linux/device.h
+++ b/backport/backport-include/linux/device.h
@@ -176,4 +176,23 @@ extern int dev_set_name(struct device *dev, const char
*name, ...)
__attribute__((format(printf, 2, 3)));
#endif
+#if LINUX_VERSION_CODE = KERNEL_VERSION(3,6,0)
+#define driver_probe_device(__drv, __dev) \
+({ \
+ int ret;
Re: Sound Blaster USB X-Fi configuration problem
Hi Alan,
thanks for the fast response and useful infos. Unfortunately your patch
does not work but triggered by it I made it working.
The changes as as follows:
First we should diverse between Audigy and X-Fi in snd_usb_apply_boot_quirk:
case USB_ID(0x041e, 0x30df):
/* X-Fi Surround 5.1 */
return snd_usb_sb_x_fi_boot_quirk(dev);
and then additional function
static int snd_usb_sb_x_fi_boot_quirk(struct usb_device *dev)
{
u16 buf = 1;
snd_printk(KERN_ERR X-Fi Surround 5.1 newer quirk\n);
snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
0, 0, buf, 2);
snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
0, 0, buf, 2);
if (buf == 0) {
snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), 0x29,
USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
1, 2000, NULL, 0);
usb_reset_device(dev);
return -EAGAIN; //-ENODEV;
}
return 0;
}
about this quirk function I am not sure. I tried to follow recorded
initialization process from WinXP running on virtual machine. I made it
finally running but without real understanding why it should be like
that. I noticed that initialization process takes a while (~2 sec.). I
do not care about that. If you think there is a smarter way to do it
just suggest and I will test it.
The patch combining these two is as follows:
===
mgrecki@mgpc:/usr/src/linux-3.9.2/sound/usb$ diff -c quirks.c.org quirks.c
===
*** quirks.c.org2013-07-20 00:06:51.246927975 +0200
--- quirks.c2013-07-20 00:07:28.067690027 +0200
***
*** 374,379
--- 374,401
return 0;
}
+ static int snd_usb_sb_x_fi_boot_quirk(struct usb_device *dev)
+ {
+ u16 buf = 1;
+
+ snd_printk(KERN_ERR X-Fi Surround 5.1 newer quirk\n);
+
+ snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+ 0, 0, buf, 2);
+ snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+ 0, 0, buf, 2);
+ if (buf == 0) {
+ snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), 0x29,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+ 1, 2000, NULL, 0);
+ usb_reset_device(dev);
+ return -EAGAIN; //-ENODEV;
+ }
+ return 0;
+ }
+
static int snd_usb_fasttrackpro_boot_quirk(struct usb_device *dev)
{
int err;
***
*** 733,738
--- 755,764
/* SB Audigy 2 NX needs its own boot-up magic, too */
return snd_usb_audigy2nx_boot_quirk(dev);
+ case USB_ID(0x041e, 0x30df):
+ /* X-Fi Surround 5.1 quirk for HS operation*/
+ return snd_usb_sb_x_fi_boot_quirk(dev);
+
case USB_ID(0x10f5, 0x0200):
/* C-Media CM106 / Turtle Beach Audio Advantage Roadie */
return snd_usb_cm106_boot_quirk(dev);
===
In case you would like me to perform additional tests just let me know.
Many thanks for your help. That was exactly what I was looking for.
Mariusz Grecki
PS. I still do not understand why the card was initialized to HS without
that. I really seen that once... But this is probably not important,
just a curiosity...
On 04.07.2013 18:26, Alan Stern wrote:
As Oliver Neukum suggested earlier, your device appears to need the
same magic message as the SoundBlaster Audigy 2 NX. This patch for
3.10 should do it. Let me know how it works.
Index: usb-3.10/sound/usb/quirks.c
===
--- usb-3.10.orig/sound/usb/quirks.c
+++ usb-3.10/sound/usb/quirks.c
@@ -744,6 +744,8 @@ int snd_usb_apply_boot_quirk(struct usb_
case USB_ID(0x041e, 0x3020):
/* SB Audigy 2 NX needs its own boot-up magic, too */
+ case USB_ID(0x041e, 0x30df):
+ /* Same for the X-Fi Surround 5.1 */
return snd_usb_audigy2nx_boot_quirk(dev);
case USB_ID(0x10f5, 0x0200):
--
Best regards
Mariusz Grecki
_
| |
| Dr. Mariusz Grecki |
| Deutsches Elektronen-Synchrotron DESY |
| Notkestraße 85, 22607 Hamburg, Geb. 3/109a |
| tel.: +49 40 89985489 fax: +49 40 89983282
[PATCH] usb: cp210x support SEL C662 Vendor/Device
From: Barry Grussling ba...@grussling.com
This patch adds support for the Schweitzer Engineering Laboratories
C662 USB cable based off the CP210x driver.
Signed-off-by: Barry Grussling ba...@grussling.com
---
I have tested this patch and verified the C662 works properly with it.
drivers/usb/serial/cp210x.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index d6ef2f8..8c109c6 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -148,6 +148,7 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x17F4, 0x) }, /* Wavesense Jazz blood glucose meter */
{ USB_DEVICE(0x1843, 0x0200) }, /* Vaisala USB Instrument Cable */
{ USB_DEVICE(0x18EF, 0xE00F) }, /* ELV USB-I2C-Interface */
+ { USB_DEVICE(0x1ADB, 0x0001) }, /* Schweitzer Engineering C662 Cable */
{ USB_DEVICE(0x1BE3, 0x07A6) }, /* WAGO 750-923 USB Service Cable */
{ USB_DEVICE(0x1E29, 0x0102) }, /* Festo CPX-USB */
{ USB_DEVICE(0x1E29, 0x0501) }, /* Festo CMSP */
--
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
[PROBLEM/BUG] mouse polling at only half rate on ohci_hcd
This is https://bugzilla.kernel.org/show_bug.cgi?id=60586 - I'm posting it here as directed by Greg Kroah-Hartman. -- usbhid's mousepoll parameter is setting all of the gaming mice I've tested to exactly 1/2 of the expected polling rate that I specify, but only if the gaming mice are using the ohci-hcd driver. To test polling rates, I use the evhz utility: http://web.archive.org/web/20060623094750/http://homepages.nildram.co.uk/~kial/evhz.c Here is an example using a Zowie EC1 eVo gaming mouse with usbhid.mousepoll=1: [ohci] From dmesg: [ 9998.795207] usb 7-3: new full-speed USB device number 2 using ohci_hcd From evhz: event3: latest hz = 500 (average hz = 500) event3: latest hz = 500 (average hz = 500) event3: latest hz = 500 (average hz = 500) event3: latest hz = 500 (average hz = 500) [ehci] From dmesg: [10217.625617] usb 2-1.1: new full-speed USB device number 5 using ehci-pci From evhz: event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) My motherboard is an Asus Sabertooth 990FX/GEN3 R2.0, which uses the SB950 south bridge chip to provide USB. I've also tested a Gigabyte GA-990FXA-UD3 which uses the same south bridge chip and have the exact same problem. -- 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: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 10:53 PM, Alan Stern st...@rowland.harvard.edu wrote:
On Fri, 19 Jul 2013, Ming Lei wrote:
Even we can introduce one flag of 'completing' in 'struct urb' to check if
the USB's submit is called inside its own complete(), by which we can
check if resubmission is in underrun easily.
Consider that isoc URB's resubmission in completion handler should
cover 99% cases, I think this approach is doable.
For other resubmission from tasklet or workque cases, either let them alone
or move the resubmission inside completion() handler, or introduce simple
helper to mark start of frame only for them. Anyway, there are very few
isoc
resubmissions from non-completion handler (only two drivers found in my
urb complet() cleanup work), so it shouldn't be a big deal.
What do you think about this approach?
Why go to all this trouble? We already have the URB_ISO_ASAP flag.
Trying to check the context of an URB submission strikes me as not
robust at all. Who knows what tricks drivers will get up to in the
future?
Firstly, it shouldn't be fragile as you image, with the below simple script, we
can find almost all isoc completion handler(276, but most of them are
false positive).
#/bin/sh
USB=drivers/usb
dirs=`ls -d drivers/* | grep -v $USB`
dirs=$dirs $USB/storage $USB/serial $USB/image $USB/class $USB/atm
$USB/misc sound/usb
export dir
for dir in ${dirs}; do
ss=$(git grep -l -w -E iso_frame_desc $dir)
if [ -n $ss ]; then
git grep -n struct\surb $dir | grep -E \(|\) | grep -v -E
;|,
fi
done
Secondly, from the above script, only very few drivers resubmits isoc URB
in tasklet or other context, and most of them do it in complete()
handler directly.
Finally, suppose drivers do some trick to resubmit URBs in tasklet, workqueue
or other context, we even can ignore them since they worked well for long time
in case of underrun with empty queue, right?
Adding a new API seems like the cleanest solution. (Although I still
think that usb_reset_endpoint() would be appropriate.)
After adding a new API, we still need to change every ISOC driver, and it
won't be simpler than only checking the completion handler, will it?
Actually 'start of stream' should be done inside usbcore because it
is HCD-independent function, so it is better to provide the information
from usbcore and let HCD use it if HCD needs the flag.
I'm not quite sure what you mean. Note that usb_reset_endpoint() _is_
a function in usbcore.
But never mind. For a new API, how about
void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *);
Does that look okay?
Looks you missed one parameter of 'int on'.
But we need to change every ISOC driver to use the API, that is why I
suggest to add urb-completing to minimize changes on drivers.
So how about only using the API on drivers which don't resubmit isoc URBs
in its completion handler()? And I think we can document this usage.
Thanks,
--
Ming Lei
--
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 01/15] drivers: phy: add generic PHY framework
Hi, On Friday 19 July 2013 09:24 PM, Stephen Warren wrote: On 07/19/2013 12:36 AM, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:59 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:13 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id); Your naming is odd, no phy anywhere in it? You rely on the sender to never send a duplicate name.id pair? Why not create your own ids based on the number of phys in the system, like almost all other classes and subsystems do? hmm.. some PHY drivers use the id they provide to perform some of their internal operation as in [1] (This is used only if a single PHY provider implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO to give the PHY drivers an option to use auto id. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, who cares about the id? No one outside of the phy core ever should, because you pass back the only pointer that they really do care about, if they need to do anything with the device. Use that, and then you can hmm.. ok. rip out all of the search for a phy by a string logic, as that's not Actually this is needed for non-dt boot case. In the case of dt boot, we use a phandle by which the controller can get a reference to the phy. But in the case of non-dt boot, the controller can get a reference to the phy only by label. I don't understand. They registered the phy, and got back a pointer to it. Why can't they save it in their local structure to use it again later if needed? They should never have to ask for the device, as the One is a *PHY provider* driver which is a driver for some PHY device. This will use phy_create to create the phy. The other is a *PHY consumer* driver which might be any controller driver (can be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the phy (by *phandle* in the case of dt boot and *label* in the case of non-dt boot). device id might be unknown if there are multiple devices in the system. I agree with you on the device id part. That need not be known to the PHY driver. How does a consumer know which label to use in a non-dt system if there are multiple PHYs in the system? That should be passed using platform data. I don't think that's right. That's just like passing clock names in platform data, which I believe is frowned upon. Instead, why not take the approach that e.g. regulators have taken? Each driver defines the names of the objects that it requires. There is a table (registered by a board file) that has lookup key (device name, We were using a similar approach with USB PHY layer but things started breaking after the device names are created using PLATFORM_DEVID_AUTO. Now theres no way to reliably know the device names in advance. Btw I had such device name binding in my v3 of this patch series [1] and had some discussion with Grant during that time [2]. [1] - http://archive.arm.linux.org.uk/lurker/message/20130320.091200.721a6fb5.hu.html [2] - https://lkml.org/lkml/2013/4/22/26 Thanks Kishon -- 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 01/15] drivers: phy: add generic PHY framework
Hi, On Saturday 20 July 2013 05:20 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 12:06:01PM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:59 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:13 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id); Your naming is odd, no phy anywhere in it? You rely on the sender to never send a duplicate name.id pair? Why not create your own ids based on the number of phys in the system, like almost all other classes and subsystems do? hmm.. some PHY drivers use the id they provide to perform some of their internal operation as in [1] (This is used only if a single PHY provider implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO to give the PHY drivers an option to use auto id. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, who cares about the id? No one outside of the phy core ever should, because you pass back the only pointer that they really do care about, if they need to do anything with the device. Use that, and then you can hmm.. ok. rip out all of the search for a phy by a string logic, as that's not Actually this is needed for non-dt boot case. In the case of dt boot, we use a phandle by which the controller can get a reference to the phy. But in the case of non-dt boot, the controller can get a reference to the phy only by label. I don't understand. They registered the phy, and got back a pointer to it. Why can't they save it in their local structure to use it again later if needed? They should never have to ask for the device, as the One is a *PHY provider* driver which is a driver for some PHY device. This will use phy_create to create the phy. The other is a *PHY consumer* driver which might be any controller driver (can be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the phy (by *phandle* in the case of dt boot and *label* in the case of non-dt boot). device id might be unknown if there are multiple devices in the system. I agree with you on the device id part. That need not be known to the PHY driver. How does a consumer know which label to use in a non-dt system if there are multiple PHYs in the system? That should be passed using platform data. Ick, don't pass strings around, pass pointers. If you have platform data you can get to, then put the pointer there, don't use a name. I don't think I understood you here :-s We wont have phy pointer when we create the device for the controller no?(it'll be done in board file). Probably I'm missing something. Thanks Kishon -- 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] backports: backport drvdata = NULL core driver fixes
On Fri, 19 Jul 2013, Luis R. Rodriguez wrote: Thanks Julia. In that case I'm going to just leave this in place given that if there's a bug upstream we'll get it fixed as soon as a respective patch gets upstream as well. That is, we are not using old drivers, we use the same upstream drivers so if a regression was found in backports the fix must go upstream s well. This is one of the benefits of backporting -- the range of users and testers increases and we still benefit from the upstream bandwagon. I don't understand. If you're not using old drivers, and you incorporate all the upstream patches, then what's the difference between a backport and the current kernel? In fact, if you're incorporating all the upstream driver patches, then why haven't you already got the drvdata change? As one example of the sort of subtle problem exposed by the drvdata change, take a look at commit b2ca69907657. For more examples, see commits bf90ff5f3b8f, 638b9e15233c, 51ef847df746, 289b076f89c2, 53636555b919, 99a6f73c495c, 003615302a16, 94ab71ce2889, 3124d1d71d3d, c27f3efc5608, 95940a04bfe8, 5c1a0f418d8d, db5c8b52, 8bf769eb5f6e, 4295fe7791a1, fa919751a2d2, a9556040119a, 7bdce71822f4, and a1028f0abfb3. Admittedly, these are all related problems in a single subsystem, but it gives you a little idea of how far this goes. 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 v2 3/4] arm: dts: Add USB phy nodes for AM33XX
On 7/20/2013 12:03 AM, Sergei Shtylyov wrote:
Hello.
On 07/19/2013 06:20 PM, Sebastian Andrzej Siewior wrote:
diff --git a/arch/arm/boot/dts/am33xx.dtsi
b/arch/arm/boot/dts/am33xx.dtsi
index 8e1248f..e3890c4 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -326,21 +326,59 @@
status = disabled;
};
-usb@4740 {
-compatible = ti,musb-am33xx;
-reg = 0x4740 0x1000/* usbss */
- 0x47401000 0x800/* musb instance 0 */
- 0x47401800 0x800;/* musb instance 1 */
-interrupts = 17/* usbss */
- 18/* musb instance 0 */
- 19;/* musb instance 1 */
-multipoint = 1;
-num-eps = 16;
-ram-bits = 12;
-port0-mode = 3;
-port1-mode = 3;
-power = 250;
-ti,hwmods = usb_otg_hs;
+phy1: am335x-usb0@44e10620 {
Shouldn't the PHYs be *under* the usb0/1 device nodes in the
hierarchy?
They're not on the same bus as the MUSB controllers for sure.
I redo the complete thing. I have now:
usb: usb@4740 {
compatible = ti,am33xx-usb;
usb0_phy: phy@47401300 {
compatible = ti,am335x-usb-phy;
}
usb0: usb@47401000 {
musb0: usb@47401400 {
compatible = mg,musbmhdrc;
}
}
usb1_phy: phy@47402300 {
compatible = ti,am335x-usb-phy;
}
usb1: usb@47402000 {
musb1: usb@47402400 {
compatible = mg,musbmhdrc;
}
}
}
And you want usb0_phy to be child of usb0? In the TRM they are all in
the same block.
Ah, the fact that PHYs didn't have the reg property got me
muddled, I didn't pay attention to the address part of the node
names... BTW, where is the reg prop? I see PHYs share the address
space with omap-control-usb@44e10620 device -- what's the point with
this?
In control module(CM) each USB has got 2 registers in which one is a
shared register( wakeup register)
So all the CM access are done using the control-usb driver
(phy-omap-control-usb.c). Same reason why phy's dont have a reg property.
Sebastian
WBR, Sergei
--
-George
--
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 3/4] arm: dts: Add USB phy nodes for AM33XX
On 7/20/2013 12:12 AM, Sebastian Andrzej Siewior wrote:
On 07/19/2013 08:33 PM, Sergei Shtylyov wrote:
Hello.
Hello,
usb: usb@4740 {
compatible = ti,am33xx-usb;
usb0_phy: phy@47401300 {
compatible = ti,am335x-usb-phy;
}
usb0: usb@47401000 {
musb0: usb@47401400 {
compatible = mg,musbmhdrc;
}
}
usb1_phy: phy@47402300 {
compatible = ti,am335x-usb-phy;
}
usb1: usb@47402000 {
musb1: usb@47402400 {
compatible = mg,musbmhdrc;
}
}
}
And you want usb0_phy to be child of usb0? In the TRM they are all in
the same block.
Ah, the fact that PHYs didn't have the reg property got me muddled,
I didn't pay attention to the address part of the node names... BTW,
where is the reg prop?
I skipped it for the general idea. I planned to repost is today but I
messed up dsps and need to get it working first…
I see PHYs share the address space with
omap-control-usb@44e10620 device -- what's the point with this?
I decided to get rid of this. Both phys have 8 bytes (2 registers)
which are exclusive for them.
There is one register for the wakeup which is shared by both.
I changed this to limit it only to the 8bytes per phy. I care about
wakeup later - hopefully George will take care of this :)
But for wakeup how can we map it since its the same register. That is
the main reason i took the
omap-control-usb route.
Sebastian
WBR, Sergei
Sebastian
--
-George
--
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: Sound Blaster USB X-Fi configuration problem
On Sat, 20 Jul 2013, Mariusz Grecki wrote:
Hi Alan,
thanks for the fast response and useful infos. Unfortunately your patch
does not work but triggered by it I made it working.
The changes as as follows:
First we should diverse between Audigy and X-Fi in snd_usb_apply_boot_quirk:
case USB_ID(0x041e, 0x30df):
/* X-Fi Surround 5.1 */
return snd_usb_sb_x_fi_boot_quirk(dev);
and then additional function
static int snd_usb_sb_x_fi_boot_quirk(struct usb_device *dev)
{
u16 buf = 1;
snd_printk(KERN_ERR X-Fi Surround 5.1 newer quirk\n);
snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
0, 0, buf, 2);
snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
0, 0, buf, 2);
if (buf == 0) {
snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), 0x29,
USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
1, 2000, NULL, 0);
usb_reset_device(dev);
return -EAGAIN; //-ENODEV;
}
return 0;
}
This is the same as the Audigy function, except for the printk string
and the 2-byte buffer instead of the 1-byte buffer, right? Therefore I
suggest combining them into a single function. You can pass the string
and the buffer size as arguments.
about this quirk function I am not sure. I tried to follow recorded
initialization process from WinXP running on virtual machine. I made it
finally running but without real understanding why it should be like
that. I noticed that initialization process takes a while (~2 sec.). I
do not care about that. If you think there is a smarter way to do it
just suggest and I will test it.
Doing what Windows does is usually the best strategy.
+ static int snd_usb_sb_x_fi_boot_quirk(struct usb_device *dev)
+ {
+ u16 buf = 1;
+
+ snd_printk(KERN_ERR X-Fi Surround 5.1 newer quirk\n);
+
+ snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+ 0, 0, buf, 2);
There's a serious problem here. It's present in most or all of the
quirk routines in this file, not just yours.
Namely, buffers used for USB transfers must not be allocated on the
stack; they must be allocated using kmalloc or a related function. The
reason is because some architectures are not capable of performing DMA
to addresses on the stack.
Do you feel like fixing up all those routines? I suggest allocating
and deallocating a buffer in the function that calls the quirk
routines, and have it pass the buffer as an extra argument.
Many thanks for your help. That was exactly what I was looking for.
You're welcome.
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: [PROBLEM/BUG] mouse polling at only half rate on ohci_hcd
On Fri, 19 Jul 2013 sgtcapsl...@lavabit.com wrote: This is https://bugzilla.kernel.org/show_bug.cgi?id=60586 - I'm posting it here as directed by Greg Kroah-Hartman. -- usbhid's mousepoll parameter is setting all of the gaming mice I've tested to exactly 1/2 of the expected polling rate that I specify, but only if the gaming mice are using the ohci-hcd driver. To test polling rates, I use the evhz utility: http://web.archive.org/web/20060623094750/http://homepages.nildram.co.uk/~kial/evhz.c Here is an example using a Zowie EC1 eVo gaming mouse with usbhid.mousepoll=1: [ohci] From dmesg: [ 9998.795207] usb 7-3: new full-speed USB device number 2 using ohci_hcd From evhz: event3: latest hz = 500 (average hz = 500) event3: latest hz = 500 (average hz = 500) event3: latest hz = 500 (average hz = 500) event3: latest hz = 500 (average hz = 500) [ehci] From dmesg: [10217.625617] usb 2-1.1: new full-speed USB device number 5 using ehci-pci From evhz: event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) My motherboard is an Asus Sabertooth 990FX/GEN3 R2.0, which uses the SB950 south bridge chip to provide USB. I've also tested a Gigabyte GA-990FXA-UD3 which uses the same south bridge chip and have the exact same problem. Can you provide usbmon traces showing what happens when the mouse is plugged in to an OHCI controller and when it is plugged in to an EHCI controller? Instructions are in Documentation/usb/usbmon.txt. 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: How should we handle isochronous underruns?
On Sat, 20 Jul 2013, Ming Lei wrote:
Trying to check the context of an URB submission strikes me as not
robust at all. Who knows what tricks drivers will get up to in the
future?
Firstly, it shouldn't be fragile as you image, with the below simple script,
we
can find almost all isoc completion handler(276, but most of them are
false positive).
#/bin/sh
USB=drivers/usb
dirs=`ls -d drivers/* | grep -v $USB`
dirs=$dirs $USB/storage $USB/serial $USB/image $USB/class $USB/atm
$USB/misc sound/usb
export dir
for dir in ${dirs}; do
ss=$(git grep -l -w -E iso_frame_desc $dir)
if [ -n $ss ]; then
git grep -n struct\surb $dir | grep -E \(|\) | grep -v -E
;|,
fi
done
Secondly, from the above script, only very few drivers resubmits isoc URB
in tasklet or other context, and most of them do it in complete()
handler directly.
This is irrelevant. We have to handle drivers that haven't been
written yet, as well as drivers that exist now.
Finally, suppose drivers do some trick to resubmit URBs in tasklet, workqueue
or other context, we even can ignore them since they worked well for long time
in case of underrun with empty queue, right?
No. This is an example of trying to be too clever. Keep it simple.
Adding a new API seems like the cleanest solution. (Although I still
think that usb_reset_endpoint() would be appropriate.)
After adding a new API, we still need to change every ISOC driver, and it
won't be simpler than only checking the completion handler, will it?
No, we don't have to change every driver using isochronous URBs. Many
of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio
is the only one that has been fixed not to do this.
We also don't have to change the isochronous code in every HCD. Many
of them treat every URB as though URB_ISO_ASAP was set. Only a few
make an attempt to implement proper handling of isochronous underruns
-- and those have to be changed anyway.
But never mind. For a new API, how about
void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *);
Does that look okay?
Looks you missed one parameter of 'int on'.
I don't understand. Why is another parameter needed?
But we need to change every ISOC driver to use the API, that is why I
suggest to add urb-completing to minimize changes on drivers.
So how about only using the API on drivers which don't resubmit isoc URBs
in its completion handler()? And I think we can document this usage.
I do not like the idea of adding a complex and non-robust mechanism
(with extra overhead for every URB!) to fix a problem that ought to be
fixed by changing drivers.
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: [PROBLEM/BUG] mouse polling at only half rate on ohci_hcd
Can you provide usbmon traces showing what happens when the mouse is plugged in to an OHCI controller and when it is plugged in to an EHCI controller? Instructions are in Documentation/usb/usbmon.txt. Alan Stern Absolutely. Ahh, I'm not really sure if I should send them as attachments to this message or not, so I've decided to put them up on a paste site. Please let me know if you want them here instead. Here's that same mouse (Zowie EC1 eVo) plugged into a [native ehci] port on the board which hands it off to ohci: http://pastebin.com/auM1MavY And here it is when plugged into an external USB hub on one of the same ports, which causes it to use the ehci driver instead: http://pastebin.com/1r7BQSBw -- 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
Attaching an usb 1.1 (sound ) device to an usb3 port results in not enough bandwidth
Hi all, Attaching an usb 1.1 device ( Funcube dongle pro+ ) results in an not enough bandwidth and some error messages from the xhci driver. (See attachment in bugzilla #60581 ) The device is not usable. The device works without any problems with ehci or ohci drivers. Regards Volker Schroer -- 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: How should we handle isochronous underruns?
On Sat, Jul 20, 2013 at 11:57 AM, Alan Stern st...@rowland.harvard.edu wrote:
On Sat, 20 Jul 2013, Ming Lei wrote:
Trying to check the context of an URB submission strikes me as not
robust at all. Who knows what tricks drivers will get up to in the
future?
Firstly, it shouldn't be fragile as you image, with the below simple script,
we
can find almost all isoc completion handler(276, but most of them are
false positive).
#/bin/sh
USB=drivers/usb
dirs=`ls -d drivers/* | grep -v $USB`
dirs=$dirs $USB/storage $USB/serial $USB/image $USB/class $USB/atm
$USB/misc sound/usb
export dir
for dir in ${dirs}; do
ss=$(git grep -l -w -E iso_frame_desc $dir)
if [ -n $ss ]; then
git grep -n struct\surb $dir | grep -E \(|\) | grep -v -E
;|,
fi
done
Secondly, from the above script, only very few drivers resubmits isoc URB
in tasklet or other context, and most of them do it in complete()
handler directly.
This is irrelevant. We have to handle drivers that haven't been
written yet, as well as drivers that exist now.
Finally, suppose drivers do some trick to resubmit URBs in tasklet, workqueue
or other context, we even can ignore them since they worked well for long
time
in case of underrun with empty queue, right?
No. This is an example of trying to be too clever. Keep it simple.
Adding a new API seems like the cleanest solution. (Although I still
think that usb_reset_endpoint() would be appropriate.)
After adding a new API, we still need to change every ISOC driver, and it
won't be simpler than only checking the completion handler, will it?
No, we don't have to change every driver using isochronous URBs. Many
of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio
is the only one that has been fixed not to do this.
OK, if you are sure just snd-usb-audio and very less drivers need the change,
using the API only is correct way.
We also don't have to change the isochronous code in every HCD. Many
of them treat every URB as though URB_ISO_ASAP was set. Only a few
make an attempt to implement proper handling of isochronous underruns
-- and those have to be changed anyway.
Yes.
But never mind. For a new API, how about
void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *);
Does that look okay?
Looks you missed one parameter of 'int on'.
I don't understand. Why is another parameter needed?
So do you only want to set streaming on and not streaming off?
If so, that looks a bit weird, could you describe its usage?
But we need to change every ISOC driver to use the API, that is why I
suggest to add urb-completing to minimize changes on drivers.
So how about only using the API on drivers which don't resubmit isoc URBs
in its completion handler()? And I think we can document this usage.
I do not like the idea of adding a complex and non-robust mechanism
IMO, it is very simple, and only one flag inside urb is required for meeting
the demand.
For the non-robust part(new drivers don't resubmit URBs in complete()), we
can let the new introduced API cover it.
Looks it should cover all old drivers automatically.
(with extra overhead for every URB!) to fix a problem that ought to be
In a normal system, the URB count won't be very much( 1000), and if
you mean the overhead is from memory, I guess it won't cause real memory
increase per URB for slab's sake. If you mean performance loss with
set/get the single lockless/common variable, maybe we should focus on
the big HCD lock, which might be the biggest bottle of USB protocol, :-)
fixed by changing drivers.
If only very less drivers need the change, I agree with you.
Thanks,
--
Ming Lei
--
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
