Re: [v2] usb: UHCI: fix pkt size in TD for a non-aligned sg element
On Sun, 23 Jun 2013, Ming Lei wrote: > >> Since you mentioned it doesn't make sense to generate short packet > >> in the middle of transfer, also it may not be what the driver/device > >> expected, > >> I suggest to add a check in usb_submit_urb() for this case and returns > >> failure on it simply, because all HCDs shouldn't support this sort of > >> thing. > >> The check in usb_submit_urb() can avoid unnecessary change in HCD. > >> > >> Any comments on the idea? > > > > Wireless USB has some strange features, one of which is that the > > maxpacket sizes for endpoints can be changed. I'm afraid that adding > > this check in usb_submit_urb() would not work right for wireless USB. > > See > > > > http://marc.info/?l=linux-usb&m=136934531624850&w=2 > > From the Thomas's last reply in the discussion, looks the device's maxp > need to be set as one value which is divided evenly into 4k, so looks the > check might be OK for wireless USB too. Maybe. Thomas should be able to tell us. > > That's why, if the check is checked, I feel it should be added to each > > HCD driver separately. Maybe I'm wrong. But before doing anything, > > you should check with Thomas Pugliese. He recently added SG support to > > the wireless USB driver. > > Suppose wireless USB is one exception, it should the only one, so we can > rule it out easily by using hcd->wireless, right? Yes, that's true. 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
URB completion order, normal behavior or bug?
So I'm working on this MCP2210 driver which talks in 64-byte request/response pairs using interrupt URBs (it can be used with hid-generic, but I don't want to do that). I'm testing on a Raspberry Pi Model B using the Raspbian 3.9.7 kernel. My usual order of things is: I submit an out URB (to rx my response) I submit an in URB (with my request) My in URB completion handler is called My out URB completion handler is called. But occasionally (more so when my system is heavily loaded, spamming printks & such), the out URB completion handler is called first. I initially thought this was due to the device chattering or some electrical noise as I have all of this open with wires in bread-boards & such, but then I dumped the contents of the out URB and discovered that it was a valid response for the given request. So the transaction must have completed normally & correctly, but my completion handlers weren't called in the same order as the actual communications occurred. I'm (obviously) new to device drivers, but I wasn't able to find any information saying that this is normal. Is it? Also, again usually when I have debug enabled, it is common for me to have a stalled out URB (that I detect with a manager thread running in a delayed work queue) that I have to kill and re-submit. This almost always stalls as well, so I have to re-submit the in URB (request) as well. I'm not sure of the cause of that either, but it causes me to think that some of my responses are being dropped on the floor, which wont allow me to build a reliable device since double-sending some requests will cause undesired results. Anyway, I guess I'll test on my workstation for now, but it would be nice to understand what's happening. Daniel -- 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: URB completion order, normal behavior or bug?
On Sun, Jun 23, 2013 at 01:13:34PM -0500, Daniel Santos wrote: > So I'm working on this MCP2210 driver which talks in 64-byte > request/response pairs using interrupt URBs (it can be used with > hid-generic, but I don't want to do that). I'm testing on a > Raspberry Pi Model B using the Raspbian 3.9.7 kernel. My deepest sympathies, the USB host controller driver on the Raspberry Pi is a huge shop of horrors, it's amazing that it works at all, and when it does, it eats huge amounts of CPU for no known good reason. Good luck with it, and be happy that it's working for you in the first place :) Anyway... > My usual order > of things is: > > I submit an out URB (to rx my response) > I submit an in URB (with my request) > My in URB completion handler is called > My out URB completion handler is called. > > But occasionally (more so when my system is heavily loaded, spamming > printks & such), the out URB completion handler is called first. I > initially thought this was due to the device chattering or some > electrical noise as I have all of this open with wires in > bread-boards & such, but then I dumped the contents of the out URB > and discovered that it was a valid response for the given request. > So the transaction must have completed normally & correctly, but my > completion handlers weren't called in the same order as the actual > communications occurred. > > I'm (obviously) new to device drivers, but I wasn't able to find any > information saying that this is normal. Is it? Yes it is, there is no guarantee in which order an urb callback happens, as the hardware itself can answer them in different order (the hardware in the client device, not the host.) > Also, again usually when I have debug enabled, it is common for me > to have a stalled out URB (that I detect with a manager thread > running in a delayed work queue) that I have to kill and re-submit. > This almost always stalls as well, so I have to re-submit the in URB > (request) as well. I'm not sure of the cause of that either, but it > causes me to think that some of my responses are being dropped on > the floor, which wont allow me to build a reliable device since > double-sending some requests will cause undesired results. That's probably due to the crappy USB host driver on the rpi. Try it on a "normal" machine and see what happens there. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: URB completion order, normal behavior or bug?
It would be good to try it with the fiq_split branch of the kernel, that has much better scheduling to stop split transactions from going wrong In this situation what happens is a split transaction fails (the device you are using is full speed and therefore uses split transactions) and the transaction will get lost in the hub. Gordon On 23/06/2013 19:43, "Greg KH" wrote: >On Sun, Jun 23, 2013 at 01:13:34PM -0500, Daniel Santos wrote: >> So I'm working on this MCP2210 driver which talks in 64-byte >> request/response pairs using interrupt URBs (it can be used with >> hid-generic, but I don't want to do that). I'm testing on a >> Raspberry Pi Model B using the Raspbian 3.9.7 kernel. > >My deepest sympathies, the USB host controller driver on the Raspberry >Pi is a huge shop of horrors, it's amazing that it works at all, and >when it does, it eats huge amounts of CPU for no known good reason. > >Good luck with it, and be happy that it's working for you in the first >place :) > >Anyway... > >> My usual order >> of things is: >> >> I submit an out URB (to rx my response) >> I submit an in URB (with my request) >> My in URB completion handler is called >> My out URB completion handler is called. >> >> But occasionally (more so when my system is heavily loaded, spamming >> printks & such), the out URB completion handler is called first. I >> initially thought this was due to the device chattering or some >> electrical noise as I have all of this open with wires in >> bread-boards & such, but then I dumped the contents of the out URB >> and discovered that it was a valid response for the given request. >> So the transaction must have completed normally & correctly, but my >> completion handlers weren't called in the same order as the actual >> communications occurred. >> >> I'm (obviously) new to device drivers, but I wasn't able to find any >> information saying that this is normal. Is it? > >Yes it is, there is no guarantee in which order an urb callback happens, >as the hardware itself can answer them in different order (the hardware >in the client device, not the host.) > >> Also, again usually when I have debug enabled, it is common for me >> to have a stalled out URB (that I detect with a manager thread >> running in a delayed work queue) that I have to kill and re-submit. >> This almost always stalls as well, so I have to re-submit the in URB >> (request) as well. I'm not sure of the cause of that either, but it >> causes me to think that some of my responses are being dropped on >> the floor, which wont allow me to build a reliable device since >> double-sending some requests will cause undesired results. > >That's probably due to the crappy USB host driver on the rpi. Try it on >a "normal" machine and see what happens there. > >thanks, > >greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] net: add dm9620 net usb driver
> "Joseph" == Joseph CHANG writes:
Joseph> DM9620 is an USB2.0 network adapter rather than DM9601 USB1.1. This
Joseph> driver processed the RX data 4 bytes header, TX data 2 bytes header,
Joseph> make the control bit exactly right in PHY write function, and optional
Joseph> IFF_ALLMUTLI bit for RX control.
But dm9601.c already supports the dm9620 based devices. Why another
driver for the same hardware?
Please CC me on dm9601 related patches.
Joseph> Tested good for many platforms, include X86 desktop and ARM embedded.
Joseph> +static struct usb_driver dm9620_driver = {
Joseph> + .name = "dm9620",
Joseph> + .id_table = products,
Joseph> + .probe = usbnet_probe,
Joseph> + .disconnect = usbnet_disconnect,
Joseph> + .suspend = usbnet_suspend,
Joseph> + .resume = usbnet_resume,
Joseph> + .disable_hub_initiated_lpm = 1,
Joseph> +};
Joseph> +
Joseph> +module_usb_driver(dm9620_driver);
Joseph> +
Joseph> +MODULE_AUTHOR("Peter Korsgaard ");
I'm not the author of this file.
--
Bye, Peter Korsgaard
--
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: URB completion order, normal behavior or bug?
Ahh, thank you both so very much for the responses, at least I know this isn't because of my code! :) On 06/23/2013 02:03 PM, Gordon Hollingworth wrote: It would be good to try it with the fiq_split branch of the kernel, that has much better scheduling to stop split transactions from going wrongŠ OK, I'll check this out later. My main concern is getting my driver sane for now and then working *planned* platform (which would be the Pi Model A, but I'm getting scared :) In this situation what happens is a split transaction fails (the device you are using is full speed and therefore uses split transactions) and the transaction will get lost in the hub. Once again, it's good to have an explanation! Daniel -- 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: URB completion order, normal behavior or bug?
On 06/23/2013 01:43 PM, Greg KH wrote: On Sun, Jun 23, 2013 at 01:13:34PM -0500, Daniel Santos wrote: So I'm working on this MCP2210 driver which talks in 64-byte request/response pairs using interrupt URBs (it can be used with hid-generic, but I don't want to do that). I'm testing on a Raspberry Pi Model B using the Raspbian 3.9.7 kernel. My deepest sympathies, the USB host controller driver on the Raspberry Pi is a huge shop of horrors, it's amazing that it works at all, and when it does, it eats huge amounts of CPU for no known good reason. Good luck with it, and be happy that it's working for you in the first place :) Anyway... Well, our project needs more than two USB ports, and we were looking at the model A anyway, so maybe I should just find some nice USB hub chip and/or PIC that uses SPI and has a good stable driver and hook it up via the Pi's SPI interface? I know that sounds kinda stupid when it already has the USB interface on it, but if it's an unstable thing, it may be good since we're going to be moving big telescopes around with these things and it would be bad if it suddenly whacked somebody in the head because of a bad USB message. :) My usual order of things is: I submit an out URB (to rx my response) I submit an in URB (with my request) My in URB completion handler is called My out URB completion handler is called. But occasionally (more so when my system is heavily loaded, spamming printks & such), the out URB completion handler is called first. I initially thought this was due to the device chattering or some electrical noise as I have all of this open with wires in bread-boards & such, but then I dumped the contents of the out URB and discovered that it was a valid response for the given request. So the transaction must have completed normally & correctly, but my completion handlers weren't called in the same order as the actual communications occurred. I'm (obviously) new to device drivers, but I wasn't able to find any information saying that this is normal. Is it? Yes it is, there is no guarantee in which order an urb callback happens, as the hardware itself can answer them in different order (the hardware in the client device, not the host.) Ahh, this is so good to know. This shouldn't be too hard of a change to make in my driver. Also, again usually when I have debug enabled, it is common for me to have a stalled out URB (that I detect with a manager thread running in a delayed work queue) that I have to kill and re-submit. This almost always stalls as well, so I have to re-submit the in URB (request) as well. I'm not sure of the cause of that either, but it causes me to think that some of my responses are being dropped on the floor, which wont allow me to build a reliable device since double-sending some requests will cause undesired results. That's probably due to the crappy USB host driver on the rpi. Try it on a "normal" machine and see what happens there. thanks, greg k-h Well on the bright side, this is a good training ground -- a nice unstable USB interface that fails a lot in a lot of various ways -- at least I get to test my error recovery code a lot! I ran it (reluctantly) on my workstation and it worked flawlessly! At least until about the 5th SPI message and then my machine hung (I didn't expect my driver to be perfect yet :). So I guess I'll try it on my laptop, which doesn't have 5TB of storage to recover when it crashes. Thanks for the help!! Daniel -- 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: URB completion order, normal behavior or bug?
On Sun, Jun 23, 2013 at 04:56:22PM -0500, Daniel Santos wrote: > On 06/23/2013 01:43 PM, Greg KH wrote: > >On Sun, Jun 23, 2013 at 01:13:34PM -0500, Daniel Santos wrote: > >>So I'm working on this MCP2210 driver which talks in 64-byte > >>request/response pairs using interrupt URBs (it can be used with > >>hid-generic, but I don't want to do that). I'm testing on a > >>Raspberry Pi Model B using the Raspbian 3.9.7 kernel. > >My deepest sympathies, the USB host controller driver on the Raspberry > >Pi is a huge shop of horrors, it's amazing that it works at all, and > >when it does, it eats huge amounts of CPU for no known good reason. > > > >Good luck with it, and be happy that it's working for you in the first > >place :) > > > >Anyway... > > Well, our project needs more than two USB ports, and we were looking > at the model A anyway, so maybe I should just find some nice USB hub > chip and/or PIC that uses SPI and has a good stable driver and hook > it up via the Pi's SPI interface? I know that sounds kinda stupid > when it already has the USB interface on it, but if it's an unstable > thing, it may be good since we're going to be moving big telescopes > around with these things and it would be bad if it suddenly whacked > somebody in the head because of a bad USB message. :) I would trust SPI over USB, especially as you have more "direct" control over SPI than USB. But if you only have USB, then perhaps you should use a different device than the RPI? Like a BeagleBoneBlack? Or some other tiny Linux machine, there are a lot of them under 100 USD these days. Best of luck, 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
[RESEND PATCH v2 1/1] usb: fix build error without CONFIG_USB_PHY
on i386:
drivers/built-in.o: In function `ci_hdrc_probe':
core.c:(.text+0x20446b): undefined reference to `of_usb_get_phy_mode'
Signed-off-by: Peter Chen
Reported-by: Randy Dunlap
Acked-by: Randy Dunlap
---
Changes for v2:
- Using IS_ENABLED to MACRO define
include/linux/usb/of.h | 16 ++--
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index e460a24..a0ef405 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -10,19 +10,23 @@
#include
#include
-#ifdef CONFIG_OF
-enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+#if IS_ENABLED(CONFIG_OF)
enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np);
#else
-static inline enum usb_phy_interface of_usb_get_phy_mode(struct device_node
*np)
+static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
{
- return USBPHY_INTERFACE_MODE_UNKNOWN;
+ return USB_DR_MODE_UNKNOWN;
}
+#endif
-static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_PHY)
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+#else
+static inline enum usb_phy_interface of_usb_get_phy_mode(struct device_node
*np)
{
- return USB_DR_MODE_UNKNOWN;
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
}
+
#endif
#endif /* __LINUX_USB_OF_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
RE: Chipidea usb otg support for IMX/MXS (device functionality)
> Hi Peter, > > > > Peter, I dunno if you are already aware of it, but the USB peripheral > > > mode hangs > > > on MX233. It's easy to replicate for example if you try to run CDC > > > ethernet over > > > the USB peripheral mode, then telnet into the board and run "dmesg" . > > > This will > > > trigger a "larger" data transfer which will make the UDC driver hang. > > > This > > > doesn't happen on MX28 so it must be some MX233-specific thing. > > > > Marek, Have you tried below? > > > > http://marc.info/?l=linux-usb&m=136537318510847&w=2 > > > > It may not the USB itself problem, the mx23's usb is almost > > the same with mx28's. > > This also happens on a different MX23-based board for me. This other > board I > tried has mDDR DRAM and the DRAM is operating correctly. The 96MHz thing > people > complained about in the Olimex forum is resolved now. > Add shawn. Marek, have you tried mx23 evk? Shawn, marek reported the udc function at mx23 works abnormal, but it works good at mx28. Have you tried mx23 udc recently? Best regards, Peter -- 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: URB completion order, normal behavior or bug?
On Sun, 23 Jun 2013, Daniel Santos wrote: > So I'm working on this MCP2210 driver which talks in 64-byte > request/response pairs using interrupt URBs (it can be used with > hid-generic, but I don't want to do that). I'm testing on a Raspberry > Pi Model B using the Raspbian 3.9.7 kernel. My usual order of things is: Maybe you don't care about this any more, but in case you still do, here's my perspective. This applies to the drivers in the standard kernel; I don't know anything in particular about the driver used in the RPi. > I submit an out URB (to rx my response) No, an OUT URB transfers data _out_ from the computer to the device. The response goes from the device _in_ to the computer. > I submit an in URB (with my request) Likewise, your request goes _out_ from the computer to the device. > My in URB completion handler is called > My out URB completion handler is called. Because of the confusion in the terminology, it's hard to tell what this means. I take it that normally, the request completes before the response does (which is what you would expect). > But occasionally (more so when my system is heavily loaded, spamming > printks & such), the out URB completion handler is called first. I > initially thought this was due to the device chattering or some > electrical noise as I have all of this open with wires in bread-boards & > such, but then I dumped the contents of the out URB and discovered that > it was a valid response for the given request. So the transaction must > have completed normally & correctly, but my completion handlers weren't > called in the same order as the actual communications occurred. That can happen. URBs for any single endpoint always complete in order of submission (unless one of them terminates prematurely because it was cancelled), but this is not guaranteed for URBs to different endpoints. If both URBs happen to complete in the same frame or microframe, the order in which the handlers get called is entirely up to the host controller driver. Of course, if you didn't submit the response URB until after the request URB completed, then everything would always go in the expected order. (Just stating the obvious...) > I'm (obviously) new to device drivers, but I wasn't able to find any > information saying that this is normal. Is it? It is a little unusual, but it can happen. > Also, again usually when I have debug enabled, it is common for me to > have a stalled out URB (that I detect with a manager thread running in a > delayed work queue) that I have to kill and re-submit. This almost > always stalls as well, so I have to re-submit the in URB (request) as > well. I'm not sure of the cause of that either, but it causes me to > think that some of my responses are being dropped on the floor, which > wont allow me to build a reliable device since double-sending some > requests will cause undesired results. > > Anyway, I guess I'll test on my workstation for now, but it would be > nice to understand what's happening. Things will probably make more sense on a workstation. Hope this helps, Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 5/6] USB: EHCI: improve interrupt qh unlink
On Sat, Jun 22, 2013 at 4:16 AM, Alan Stern wrote:
> On Thu, 20 Jun 2013, Ming Lei wrote:
>
>> IMO, there is no any side effect when we change the state to
>> QH_STATE_UNLINK_WAIT and later change back to QH_STATE_LINKED
>> later under this situation.
>
> I don't like the idea of changing an invariant.
>
>> The reason why I use QH_STATE_UNLINK_WAIT here is for avoiding
>> unnecessary CPU wakeup. Without the state, the unlink wait timer
>> is still triggered to check if there are intr QHs to be unlinked, but in
>> most of
>> situations, there aren't QHs to be unlinked since tasklet is surely run
>> before the wait timer is expired. So generally, without knowing the wait
>> state,
>> CPU wakeup events may be introduced unnecessarily.
>
> Avoiding unnecessary timer events is a good thing, but I don't
> understand how it is connected with QH_STATE_UNLINK_WAIT. Doesn't this
> revised patch avoid the events without using this state?
>
> Besides, don't you already know the wait state by checking whether
> qh->unlink_node is empty?
>
>> Considered that the interval of interrupt endpoint might be very
>> small(e.g. 125us,
>> 1ms) and some devices have very frequent intr event, I think we
>> need to pay attention to the problem.
>
> Yes, we do. The hrtimer code in ehci-timer is written specifically to
> handle that sort of situation.
>
>> Even on async QH situation, we might need to consider the problem too
>> since some application(eg. bulk only mass storage on xhci) may have
>> thousands of interrupts per seconds during data transfer, so CPU wakeup
>> events could be increased much by letting wait timer expire unnecessarily.
>
> I suspect it's the other way around. Letting the timer expire
> _reduces_ the amount of work, because we don't have to start and stop
> the timer as often.
>
> It's a tradeoff. One approach starts and cancels the timer N times
> (where N can be fairly large); the other approach starts the timer
> once and lets it expire, and then the handler routine does almost no
> work. Which approach uses more CPU time? I don't know; I haven't
> measured it.
>
>> Also the async QH unlink approach has another disadvantage:
>>
>> - if there are several QHs which are become empty during one wait period,
>> the hrtimer has to be scheduled several times for starting unlink one qh
>> each time.
>
> That's because the driver unlinks only one async QH at a time. It is
> unavoidable. In earlier kernels the driver would unlink multiple async
> QHs simultaneously, and it needed to schedule the timer only once.
> For some reason (I still don't understand why), this did not work on
> some systems.
>
>> And after introducing the unlink wait list like the patch, we only
>> need schedule the hrtimer one time for unlinking all these empty QHs.
>
> The async code used to work that way, before I changed it to unlink
> only one async QH at a time.
>
>> Finally, unlink wait for intr qh is only needed when the qh is completed
>> right now, and other cases(eg. unlink) needn't the wait.
>
> Right.
>
>> The attached patch removes the QH_STATE_UNLINK_WAIT, and can
>> avoid the above disadvantages of async QH unlink, could you comment
>> on the new patch?
>
> Okay...
Thanks for your review.
>
>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> index 246e124..35b4148 100644
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>> @@ -304,7 +304,8 @@ static void end_unlink_async(struct ehci_hcd *ehci);
>> static void unlink_empty_async(struct ehci_hcd *ehci);
>> static void unlink_empty_async_suspended(struct ehci_hcd *ehci);
>> static void ehci_work(struct ehci_hcd *ehci);
>> -static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh);
>> +static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh,
>> + bool wait);
>
> Adding a "wait" argument is a bad approach. You should create a new
> function: start_unlink_intr_wait() or something like that. After all,
> it is used in only one place.
OK.
>
>> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
>> index acff5b8..5dfda56 100644
>> --- a/drivers/usb/host/ehci-sched.c
>> +++ b/drivers/usb/host/ehci-sched.c
>> @@ -548,6 +548,9 @@ static void qh_link_periodic(struct ehci_hcd
>> *ehci, struct ehci_qh *qh)
>>
>> list_add(&qh->intr_node, &ehci->intr_qh_list);
>>
>> + /* unlink need this node to be initialized */
>> + INIT_LIST_HEAD(&qh->unlink_node);
>
> This should be done only once, when the QH is created. And the comment
> is unnecessary.
OK, I will move it into ehci_qh_alloc().
>
>> @@ -98,6 +99,17 @@ static void ehci_enable_event(struct ehci_hcd
>> *ehci, unsigned event,
>> }
>> }
>>
>> +/* Warning: don't call this function from hrtimer handler context */
>> +static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event)
>> +{
>> + if (!hcd_giveback_urb_in_bh(ehci_to_hcd(ehci)))
>> + return;
>
Re: [PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework
Hi,
On Tuesday 18 June 2013 03:17 PM, Felipe Balbi wrote:
Hi,
On Thu, Jun 13, 2013 at 02:13:57PM +0530, Kishon Vijay Abraham I wrote:
Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
and usb_phy_set_suspend is replaced with power_on/get_sync and
power_off/put_sync to align with the new PHY framework.
musb->xceiv can't be removed as of now because musb core uses xceiv.state and
xceiv.otg. Once there is a separate state machine to handle otg, these can be
moved out of xceiv and then we can start using the generic PHY framework.
Signed-off-by: Kishon Vijay Abraham I
---
drivers/usb/musb/musb_core.c |1 +
drivers/usb/musb/musb_core.h |3 +++
drivers/usb/musb/omap2430.c | 29 +
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 37a261a..f732bcc 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1864,6 +1864,7 @@ musb_init_controller(struct device *dev, int nIrq, void
__iomem *ctrl)
musb->board_set_power = plat->set_power;
musb->min_power = plat->min_power;
musb->ops = plat->platform_ops;
+ musb->phy_label = plat->phy_label;
/* The musb_platform_init() call:
* - adjusts musb->mregs
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7fb4819..498ae21 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -46,6 +46,7 @@
#include
#include
#include
+#include
struct musb;
struct musb_hw_ep;
@@ -357,6 +358,7 @@ struct musb {
u16 int_tx;
struct usb_phy *xceiv;
+ struct phy *phy;
int nIrq;
unsignedirq_wake:1;
@@ -434,6 +436,7 @@ struct musb {
unsigneddouble_buffer_not_ok:1;
struct musb_hdrc_config *config;
+ const char *phy_label;
#ifdef MUSB_CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 628b93f..c62a004 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -348,14 +348,24 @@ static int omap2430_musb_init(struct musb *musb)
* up through ULPI. TWL4030-family PMICs include one,
* which needs a driver, drivers aren't always needed.
*/
- if (dev->parent->of_node)
+ if (dev->parent->of_node) {
+ musb->phy = devm_phy_get(dev->parent, "usb2-phy");
+
+ /* We can't totally remove musb->xceiv as of now because
+* musb core uses xceiv.state and xceiv.otg. Once we have
+* a separate state machine to handle otg, these can be moved
+* out of xceiv and then we can start using the generic PHY
+* framework
+*/
musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
"usb-phy", 0);
- else
+ } else {
musb->xceiv = devm_usb_get_phy_dev(dev, 0);
+ musb->phy = devm_phy_get(dev, musb->phy_label);
+ }
- if (IS_ERR(musb->xceiv)) {
- status = PTR_ERR(musb->xceiv);
+ if (IS_ERR(musb->xceiv) || IS_ERR(musb->phy)) {
+ status = PTR_ERR(musb->xceiv) | PTR_ERR(musb->phy);
if (status == -ENXIO)
return status;
@@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb)
if (glue->status != OMAP_MUSB_UNKNOWN)
omap_musb_set_mailbox(glue);
- usb_phy_init(musb->xceiv);
+ phy_init(musb->phy);
pm_runtime_put_noidle(musb->controller);
+ phy_pm_runtime_put(musb->phy);
see, weird unbalanced calls :-)
Make it all explicit:
phy_pm_runtime_get_sync(phy);
phy_init(phy);
phy_pm_runtime_put(phy);
I think then it makes sense to drop get_sync from phy_init()?
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: Linux USB file storage gadget with new UDC
Hi, >> The problem is in UDC driver. i made the change, it is ok now. > > Good. I noticed that the usb_ep_set_wedge routine still isn't working > right. You might try to fix that. > > Alan Stern > Ok, is the usb_ep_set_wedge routine not working? I can't see that in the log file. Now, in USB 2.0 CV test, there is an error about GET_STATUS request, as shown below. g_file_storage gadget: ep0-setup, length 8: : 82 00 00 00 81 00 02 00 g_file_storage gadget: unknown control req 82.00 v i0081 l2 handle_setup status -95 I think the GET_STATUS request is not handled by the gadget driver. Isn't it so? thanks, victor -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework
Hi, On Mon, Jun 24, 2013 at 11:01:56AM +0530, Kishon Vijay Abraham I wrote: > >>@@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb) > >>if (glue->status != OMAP_MUSB_UNKNOWN) > >>omap_musb_set_mailbox(glue); > >> > >>- usb_phy_init(musb->xceiv); > >>+ phy_init(musb->phy); > >> > >>pm_runtime_put_noidle(musb->controller); > >>+ phy_pm_runtime_put(musb->phy); > > > >see, weird unbalanced calls :-) > > > >Make it all explicit: > > > >phy_pm_runtime_get_sync(phy); > >phy_init(phy); > >phy_pm_runtime_put(phy); > > I think then it makes sense to drop get_sync from phy_init()? maybe not, you don't know if the phy has already autosuspended or not. -- balbi signature.asc Description: Digital signature
Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework
On Wednesday 19 June 2013 02:52 AM, Sylwester Nawrocki wrote: Hi Kishon, I've noticed there is a little inconsistency between the code and documentation. On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote: +3. Creating the PHY + +The PHY driver should create the PHY in order for other peripheral controllers +to make use of it. The PHY framework provides 2 APIs to create the PHY. + +struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops, +void *priv); +struct phy *devm_phy_create(struct device *dev, int id, +const struct phy_ops *ops, void *priv); The 'label' argument is missing in those function prototypes. It seems the labels must be unique, I guess this needs to be documented. And do we allow NULL labels ? If so, then there is probably a check for NULL label needed in phy_lookup() and if not, then phy_create() should fail when NULL label is passed ? Yeah. Label is used only for non-dt case so NULL should be allowed while phy_create(). 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
