[PATCH 6/6] USB: gadget: mv_udc: fix corner case for missiong dTD
When the missing dTD issue is triggerred, queue_dtd may prime the new request instead of the missing dTD. We can just add the request to the queue end and jump out if there are more than one request in the queue already. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index 8df8606..918b603 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -295,13 +295,23 @@ static int queue_dtd(struct mv_ep *ep, struct mv_req *req) /* check if the pipe is empty */ if (!(list_empty(ep-queue))) { - struct mv_req *lastreq; + struct mv_req *firstreq, *lastreq; + firstreq = list_entry(ep-queue.next, struct mv_req, queue); lastreq = list_entry(ep-queue.prev, struct mv_req, queue); lastreq-tail-dtd_next = req-head-td_dma EP_QUEUE_HEAD_NEXT_POINTER_MASK; wmb(); + /* +* If there are more than one reqs in the queue, +* then it's safe to just add it to the list end. +* It should be able to handle the missing DTD issue. +* And suppose it can improve the performance too. +*/ + if (firstreq != lastreq) + goto done; + if (readl(udc-op_regs-epprime) bit_pos) goto done; -- 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
[PATCH 3/6] usb: gadget: mv_udc: clear corresponding interrupt when flush fifo
The interruts are useless when this endpoint is going to be flushed. Especially in the enumeration phase, let's take get device description for example. 1. Device doesn't ACK the status package from host in time due to irq is disabled by some module. 2. Host find no ACK in time, and send another request. 3. Device gets the chance to handle the interrupt, the sequence is as following. a. Flush pending requests in ep0. b. Queue a request in ep0 according to host's request and change the ep0 state to DATA_STATE_XMIT. c. Handle the pending interrupt for the last status package. But actually it will check the new added request instead of the status package and because of wront ep0 state, device will request an unexpected status package from host. d. The ep0 state is going mad. The solution is that we need clear the pending corresponding interrupt as well as flush endpoint. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index 657ac5c..d5a9bdf 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -692,6 +692,8 @@ static void mv_ep_fifo_flush(struct usb_ep *_ep) } loops--; } while (readl(udc-op_regs-epstatus) bit_pos); + + writel(bit_pos, udc-op_regs-epcomplete); } /* queues (submits) an I/O request to an endpoint */ -- 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
[PATCH 1/6] usb: gadget: mv_udc: remove redundant pull up in udc_start
Romove redundant pull up in udc_start since function udc_bind_to_driver in udc-core.c will do it for us. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index fcff3a5..ebc0dfd 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -1365,9 +1365,6 @@ static int mv_udc_start(struct usb_gadget *gadget, } } - /* pullup is always on */ - mv_udc_pullup(udc-gadget, 1); - /* When boot with cable attached, there will be no vbus irq occurred */ if (udc-qwork) queue_work(udc-qwork, udc-vbus_work); -- 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
[PATCH 4/6] usb: gadget: mv_udc: check endpoint before queue dtd
There is a corner case that endpoint is disabled by system shutdown between check ep-desc and hold spin lock in mv_ep_queue. In this case ep-ep.desc will be NULL and occur kernel panic when access it in build_dtd. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index d5a9bdf..a620cff 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -734,6 +734,14 @@ mv_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags) spin_lock_irqsave(udc-lock, flags); + if (!ep-ep.desc) { + spin_unlock_irqrestore(udc-lock, flags); + dev_info(udc-dev-dev, + %s is already disabled!\n, ep-name); + retval = -EINVAL; + goto err_unmap_dma; + } + /* build dtds and push them to device queue */ if (!req_to_dtd(req)) { retval = queue_dtd(ep, req); -- 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
[PATCH 5/6] USB: gadget: mv_udc: workaroud for missing dTD
There is an issue with the add dTD tripwire semaphore (ATDTW bit in USBCMD register) that can cause the controller to ignore a dTD that is added to a primed endpoint. When this happens, the software can read the tripwire bit and the status bit at '1' even though the endpoint is unprimed. After executing a dTD, the device controller endpoint state machine executes a final read of the dTD terminate bit to check if the application added a dTD to the linked list at the last moment. This read is done in the finpkt_read_latest_next_td (44) state. After the read is performed, if the terminate bit is still set, the state machine moves to unprime the endpoint. The decision to unprime the endpoint is done in the checkqh_decision (59) state, based on the value of the terminate bit. Before reaching the checkqh_decision state, the state machine traverses the writeqhtd_status (57), writeqh_status (56), and release_prime_mask (42) states. As shown in the waveform, the ep_addtd_tripwire_clr signal is not set to clear the tripwire bit in these states. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index a620cff..8df8606 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -196,7 +196,27 @@ static int process_ep_req(struct mv_udc *udc, int index, while (readl(udc-op_regs-epstatus) bit_pos) udelay(1); break; + } else { + if (!(readl(udc-op_regs-epstatus) bit_pos)) { + /* The DMA engine thinks there is no more dTD */ + curr_dqh-next_dtd_ptr = curr_dtd-dtd_next +EP_QUEUE_HEAD_NEXT_POINTER_MASK; + + /* clear active and halt bit */ + curr_dqh-size_ioc_int_sts = + ~(DTD_STATUS_ACTIVE + | DTD_STATUS_HALTED); + + /* Do prime again */ + wmb(); + writel(bit_pos, udc-op_regs-epprime); + while (readl(udc-op_regs-epprime) bit_pos) + cpu_relax(); + + break; + } } + udelay(1); } -- 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
[PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0
Hardware zlt will try to send the zero length packet automatically when the data transferd is multiple times of max packet, this will cause issues on Windows. So let's disable HW zlt by default. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index ebc0dfd..657ac5c 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -89,7 +89,7 @@ static void ep0_reset(struct mv_udc *udc) /* configure ep0 endpoint capabilities in dQH */ ep-dqh-max_packet_length = (EP0_MAX_PKT_SIZE EP_QUEUE_HEAD_MAX_PKT_LEN_POS) - | EP_QUEUE_HEAD_IOS; + | EP_QUEUE_HEAD_IOS | EP_QUEUE_HEAD_ZLT_SEL; ep-dqh-next_dtd_ptr = EP_QUEUE_HEAD_NEXT_TERMINATE; -- 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
[PATCH 0/6] bug fix for mv_udc_core.c
This patch set is mainly for bug fix. Neil Zhang (6): usb: gadget: mv_udc: remove redundant pull up in udc_start usb: gadget: mv_udc: disable HW zlt for ep0 usb: gadget: mv_udc: clear corresponding interrupt when flush fifo usb: gadget: mv_udc: check endpoint before queue dtd USB: gadget: mv_udc: workaroud for missing dTD USB: gadget: mv_udc: fix corner case for missiong dTD drivers/usb/gadget/mv_udc_core.c | 47 ++ 1 file changed, 42 insertions(+), 5 deletions(-) -- 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
RE: [PATCH RESEND v2 1/3] usb: chipidea: msm: Add device tree binding information
+CI13xxx (Chipidea) USB controllers + +Required properties: +- compatible: should contain qcom,ci-hdrc +- reg: offset and length of the register set in the memory map +- interrupts: interrupt-specifier for the controller interrupt. +- usb-phy: phandle for the PHY device +- dr_mode: Sould be peripheral + + gadget@f9a55000 { + compatible = qcom,ci-hdrc; + reg = 0xf9a55000 0x400; + dr_mode = peripheral; + interrupts = 0 134 0; + usb-phy = usb_otg; + }; + usb_otg is not a good name for the node of usb phy. 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: [PATCH] usb: chipidea: imx: Use dev_name() for ci_hdrc name to distinguish USBs
On Mon, Feb 24, 2014 at 11:49:40AM +0800, Peter Chen wrote: On Mon, Feb 24, 2014 at 08:32:14AM +0400, Alexander Shiyan wrote: Понедельник, 24 февраля 2014, 10:02 +08:00 от Peter Chen peter.c...@freescale.com: On Sat, Feb 22, 2014 at 05:17:58PM +0400, Alexander Shiyan wrote: Signed-off-by: Alexander Shiyan shc_w...@mail.ru --- drivers/usb/chipidea/ci_hdrc_imx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index c00f772..2e58f8d 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -96,7 +96,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) { struct ci_hdrc_imx_data *data; struct ci_hdrc_platform_data pdata = { - .name = ci_hdrc_imx, + .name = dev_name(pdev-dev), .capoffset = DEF_CAPOFFSET, .flags = CI_HDRC_REQUIRE_TRANSCEIVER | CI_HDRC_DISABLE_STREAMING, -- ... I don't see too much meaningful for this change, and I don't see the meaning of this entry (.name) and no user is using it. Field name is used in the function ci_hdrc_probe() (core.c) for request_irq(). That's what will result: # cat /proc.interrupts ... 71: 0 mxc-avic 55 10024400.usb 72: 0 mxc-avic 56 10024000.usb --- Get it, thanks. Please add commit log for this patch, then I can apply it. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usbserial_generic, idVendor=1a28, idProduct=6010
On Mon, Feb 24, 2014 at 01:46:59AM +0100, Emanuel Koczwara wrote: Hi, W dniu 23.02.2014 21:21, Johan Hovold pisze: You might need to mount debugfs: mount -t debugfs none /sys/kernel/debug and make sure your kernel is compiled with CONFIG_DYNAMIC_DEBUG (and CONFIG_DEBUG_FS) enabled. Johan -- 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 Here is my debug output: http://nopaste.info/50a31e9ca7.html This is interesting: usb_serial_generic_read_bulk_callback - urb 0, len 2 usb_serial_generic_read_bulk_callback - length = 2, data = 31 60 I'm getting this 31 60 as 1` when I read from /dev/ttyUSB0. Yes, that was expected. The printer is returning what appears to be empty status-messages every 110ms. I also wanted to make sure that all bulk data had this two-byte header. Is that also the case for /dev/ttyUSB1? The printer has a serial interface as well? Which behaves exactly the same but does not return any 1`? I can write up a small driver which strips the header from incoming data for you to test, but it would be nice to know what the header actually is (and if it ever changes). There are no hints in the (Polish) manual? Thanks, Johan -- 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: usbserial_generic, idVendor=1a28, idProduct=6010
Hi, W dniu 24.02.2014 09:30, Johan Hovold pisze: On Mon, Feb 24, 2014 at 01:46:59AM +0100, Emanuel Koczwara wrote: Hi, W dniu 23.02.2014 21:21, Johan Hovold pisze: You might need to mount debugfs: mount -t debugfs none /sys/kernel/debug and make sure your kernel is compiled with CONFIG_DYNAMIC_DEBUG (and CONFIG_DEBUG_FS) enabled. Johan -- 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 Here is my debug output: http://nopaste.info/50a31e9ca7.html This is interesting: usb_serial_generic_read_bulk_callback - urb 0, len 2 usb_serial_generic_read_bulk_callback - length = 2, data = 31 60 I'm getting this 31 60 as 1` when I read from /dev/ttyUSB0. Yes, that was expected. The printer is returning what appears to be empty status-messages every 110ms. I also wanted to make sure that all bulk data had this two-byte header. Is that also the case for /dev/ttyUSB1? Yes. The printer has a serial interface as well? Which behaves exactly the same but does not return any 1`? I will check it. I don't have a computer with rs232 at this moment. I can write up a small driver which strips the header from incoming data for you to test, but it would be nice to know what the header actually is (and if it ever changes). There are no hints in the (Polish) manual? What kind of header? Some low level bytes? Manual contains only application level protocol description. There are 2 kinds of messages incoming from the printer: 1) single byte status 2) message starting with 0x1b 0x50 and it ends with 0x1b 0x5c. To recognize these 2 types of messages I need that 0x1b 0x50 header. Thanks, Emanuel -- 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] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Roger, On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. snip This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? right, but the PHY is connected to the dwc3 core and not to the glue. I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. Just to summarize this thread on what we need Thanks for summarizing. 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or not. It should be as generic as possible. 2) dwc3 core should continue probe even if PHY layer is not enabled, as not all platforms need it. 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) 4) dwc3 core should error out on any error condition if PHY device is available and caused some error, e.g. init error. 5) dwc3 core should return EPROBE_DEFER if PHY device is available but device driver is not yet loaded. 6) platform glue should do the necessary sanity checks for availability of all resources like PHY device, PHY layer, etc, before populating the dwc3 device. e.g. in OMAP5
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. snip This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? right, but the PHY is connected to the dwc3 core and not to the glue. I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. Just to summarize this thread on what we need Thanks for summarizing. 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or not. It should be as generic as possible. I think this contradicts with Felipe's requirement of dwc3 core bailing out if a particular platform needs a PHY but it's not able to get it. 2) dwc3 core should continue probe even if PHY layer is not enabled, as not all platforms need it. 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) 4) dwc3 core should error out on any error condition if PHY device is available and caused some error, e.g. init error. 5) dwc3 core should return EPROBE_DEFER if PHY device is available but device driver is not yet loaded. 6) platform glue should do the necessary
RE: [PATCH RESEND v2 1/3] usb: chipidea: msm: Add device tree binding information
On Mon, 2014-02-24 at 08:15 +, Peter Chen wrote: +CI13xxx (Chipidea) USB controllers + +Required properties: +- compatible: should contain qcom,ci-hdrc +- reg: offset and length of the register set in the memory map +- interrupts: interrupt-specifier for the controller interrupt. +- usb-phy: phandle for the PHY device +- dr_mode: Sould be peripheral + + gadget@f9a55000 { + compatible = qcom,ci-hdrc; + reg = 0xf9a55000 0x400; + dr_mode = peripheral; + interrupts = 0 134 0; + usb-phy = usb_otg; + }; + usb_otg is not a good name for the node of usb phy. Well, this is just an example. Is this better phy0? Regards, Ivan 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: SPDX-License-Identifier
On 02/21/2014 08:01 PM, Theodore Ts'o wrote: On Fri, Feb 21, 2014 at 09:57:20AM -0800, Greg Kroah-Hartman wrote: But shouldn't we at least write somewhere that it has connection to spdx.org where you can find out that licenses. Why? Are these licenses so unknown that no one knows what they are? And, as part of the kernel-as-a-whole-work, they all resolve to GPLv2 anyway, and we have that license in the source tree, so nothing else should be needed. Note that not all lawyers are in agreement about this, so if this is a driver being developed by a company, you may want to ask your corporate counsel if they have an opinion about this. I've received advice of the form that it's not obvious that regardless of whether or not us *engineers* understand what all of the licensing terms mean, what's important is whether someone who is accused of borrowing GPL'ed code and dropping it in a driver for some other OS can convince a judge whether or not it's considered obvious from a legal perspective what an SPDX header means, and what is implied by an SPDX license identifer. Also note that with the advent of web sites that allow people to do web searches and turn up a singleton file via some gitweb interface, the fact that the full license text is distributed alongside the tarball might or might have as much legal significance as it once had. But of course, I'm not a lawyer, and if your company has is paying for the development of the driver, the Golden Rule applies (he who has the Gold, makes the Rules), and each of our respective corporate lawyers may have different opinions about what might happen if the question was ever to be adjudicated in court. Thanks Ted. Aren't all these points already answered by SPDX project? I believe that they should know how this should be handled properly. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP - KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [PATCH v5 14/16] usb: documentation for usb port power off mechanisms
On Fri, 2014-02-21 at 16:10 -0800, Dan Williams wrote: From: Lan Tianyu tianyu@intel.com describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum oneu...@suse.de Signed-off-by: Lan Tianyu tianyu@intel.com [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/usb/power-management.txt | 237 1 files changed, 237 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..e67c1d4d1994 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -5,6 +5,25 @@ October 28, 2010 + Contents: + - + * What is Power Management? + * What is Remote Wakeup? + * When is a USB device idle? + * Forms of dynamic PM + * The user interface for dynamic PM + * Changing the default idle-delay time + * Warnings + * The driver interface for Power Management + * The driver interface for autosuspend and autoresume + * Other parts of the driver interface + * Mutual exclusion + * Interaction between dynamic PM and system PM + * xHCI hardware link PM + * USB Port Power Control + * User Interface for Port Power Control + * Suggested Userspace Port Power Policy + What is Power Management? - @@ -516,3 +535,221 @@ relevant attribute files is usb2_hardware_lpm. driver will enable hardware LPM for the device. You can write y/Y/1 or n/N/0 to the file to enable/disable USB2 hardware LPM manually. This is for test purpose mainly. + + + USB Port Power Control + -- + +In addition to suspending endpoint devices and enabling hardware +controlled link power management, the USB subsystem also has the +capability to disable power to individual ports. Power is controlled Not necessary individual ports. You explain the limitations of ganged switching further below. I'd prefer ports under some conditions +* wakeup note: the implementation does not allow a port connected to a + device with wakeup capability to be powered off. The capability may be there. It just mustn't be enabled. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RESEND v2 1/3] usb: chipidea: msm: Add device tree binding information
On Mon, 2014-02-24 at 08:15 +, Peter Chen wrote: +CI13xxx (Chipidea) USB controllers + +Required properties: +- compatible: should contain qcom,ci-hdrc +- reg: offset and length of the register set in the memory map +- interrupts: interrupt-specifier for the controller interrupt. +- usb-phy: phandle for the PHY device +- dr_mode: Sould be peripheral + + gadget@f9a55000 { + compatible = qcom,ci-hdrc; + reg = 0xf9a55000 0x400; + dr_mode = peripheral; + interrupts = 0 134 0; + usb-phy = usb_otg; + }; + usb_otg is not a good name for the node of usb phy. Well, this is just an example. Is this better phy0? usbphy0 is better since we have ethernet, sata, pcie phy. 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
[Patch V1 0/2] usb: g_zero: Add support for interrupt EP
This patchset adds support for interrupt EP and the corresponding test cases to gadget zero. The code has been rebased and tested on Kernel v3.14-rc3 RFC - V1 - Added support for configuring interrupt EP attributes from configfs interface Amit Virdi (2): usb: gadget: zero: Add support for interrupt EP usbtest: Add interrupt EP testcases drivers/usb/gadget/f_loopback.c | 3 +- drivers/usb/gadget/f_sourcesink.c | 519 -- drivers/usb/gadget/g_zero.h | 13 +- drivers/usb/gadget/zero.c | 21 ++ drivers/usb/misc/usbtest.c| 112 ++-- 5 files changed, 630 insertions(+), 38 deletions(-) -- 1.8.0 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Patch V1 1/2] usb: gadget: zero: Add support for interrupt EP
Interrupt endpoints behave quite similar to the bulk endpoints with the difference that the endpoints expect data sending/reception request at particular intervals till the whole data has not been transmitted. The interrupt EP support is added to gadget zero. A new alternate setting (=2) has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The default parameters are set as: bInterval: 4 wMaxPacketSize: 1024 However, the same can be overridden through the module parameter interface. The code is tested for HS and SS on a platform having DWC3 controller. Signed-off-by: Amit Virdi amit.vi...@st.com --- drivers/usb/gadget/f_loopback.c | 3 +- drivers/usb/gadget/f_sourcesink.c | 519 -- drivers/usb/gadget/g_zero.h | 13 +- drivers/usb/gadget/zero.c | 21 ++ 4 files changed, 533 insertions(+), 23 deletions(-) diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c index 4557cd0..bf04389 100644 --- a/drivers/usb/gadget/f_loopback.c +++ b/drivers/usb/gadget/f_loopback.c @@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop) struct usb_composite_dev*cdev; cdev = loop-function.config-cdev; - disable_endpoints(cdev, loop-in_ep, loop-out_ep, NULL, NULL); + disable_endpoints(cdev, loop-in_ep, loop-out_ep, NULL, NULL, NULL, + NULL); VDBG(cdev, %s disabled\n, loop-function.name); } diff --git a/drivers/usb/gadget/f_sourcesink.c b/drivers/usb/gadget/f_sourcesink.c index d3cd52d..306de39 100644 --- a/drivers/usb/gadget/f_sourcesink.c +++ b/drivers/usb/gadget/f_sourcesink.c @@ -23,6 +23,13 @@ #include gadget_chips.h #include u_f.h +enum eptype { + EP_CONTROL = 0, + EP_BULK, + EP_ISOC, + EP_INTERRUPT, +}; + /* * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral * controller drivers. @@ -55,6 +62,8 @@ struct f_sourcesink { struct usb_ep *out_ep; struct usb_ep *iso_in_ep; struct usb_ep *iso_out_ep; + struct usb_ep *int_in_ep; + struct usb_ep *int_out_ep; int cur_alt; }; @@ -68,6 +77,10 @@ static unsigned isoc_interval; static unsigned isoc_maxpacket; static unsigned isoc_mult; static unsigned isoc_maxburst; +static unsigned int_interval; +static unsigned int_maxpacket; +static unsigned int_mult; +static unsigned int_maxburst; static unsigned buflen; /*-*/ @@ -92,6 +105,16 @@ static struct usb_interface_descriptor source_sink_intf_alt1 = { /* .iInterface = DYNAMIC */ }; +static struct usb_interface_descriptor source_sink_intf_alt2 = { + .bLength = USB_DT_INTERFACE_SIZE, + .bDescriptorType = USB_DT_INTERFACE, + + .bAlternateSetting =2, + .bNumEndpoints =6, + .bInterfaceClass = USB_CLASS_VENDOR_SPEC, + /* .iInterface = DYNAMIC */ +}; + /* full speed support: */ static struct usb_endpoint_descriptor fs_source_desc = { @@ -130,6 +153,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = { .bInterval =4, }; +static struct usb_endpoint_descriptor fs_int_source_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + + .bEndpointAddress = USB_DIR_IN, + .bmAttributes = USB_ENDPOINT_XFER_INT, + .wMaxPacketSize = cpu_to_le16(1023), + .bInterval =4, +}; + +static struct usb_endpoint_descriptor fs_int_sink_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + + .bEndpointAddress = USB_DIR_OUT, + .bmAttributes = USB_ENDPOINT_XFER_INT, + .wMaxPacketSize = cpu_to_le16(1023), + .bInterval =4, +}; + static struct usb_descriptor_header *fs_source_sink_descs[] = { (struct usb_descriptor_header *) source_sink_intf_alt0, (struct usb_descriptor_header *) fs_sink_desc, @@ -140,6 +183,14 @@ static struct usb_descriptor_header *fs_source_sink_descs[] = { (struct usb_descriptor_header *) fs_source_desc, (struct usb_descriptor_header *) fs_iso_sink_desc, (struct usb_descriptor_header *) fs_iso_source_desc, + (struct usb_descriptor_header *) source_sink_intf_alt2, +#define FS_ALT_IFC_2_OFFSET8 + (struct usb_descriptor_header *) fs_sink_desc, + (struct usb_descriptor_header *) fs_source_desc, + (struct usb_descriptor_header *) fs_iso_sink_desc, + (struct usb_descriptor_header *) fs_iso_source_desc, + (struct usb_descriptor_header *) fs_int_sink_desc, + (struct usb_descriptor_header *) fs_int_source_desc, NULL, }; @@ -179,6 +230,24 @@ static struct
[Patch V1 2/2] usbtest: Add interrupt EP testcases
Two simple test cases for interrupt endpoints are added to the usbtest.c file. These are simple non-queued interrupt IN and interrupt OUT transfers. Currently, only gadget zero is capable of executing the interrupt EP test cases. However, extending the same to other gadgets is extremely simple and can be done on-demand. This code has been tested only with gadget zero and care has been taken so as to not break the existing functionality. However, if anyone can test with other gadgets then that would be great! Signed-off-by: Amit Virdi amit.vi...@st.com --- drivers/usb/misc/usbtest.c | 112 +++-- 1 file changed, 97 insertions(+), 15 deletions(-) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index f6568b5..8b96235 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -54,6 +54,7 @@ struct usbtest_info { unsignedautoconf:1; unsignedctrl_out:1; unsignediso:1; /* try iso in/out */ + unsignedintr:1; /* try interrupt in/out */ int alt; }; @@ -70,7 +71,10 @@ struct usbtest_dev { int out_pipe; int in_iso_pipe; int out_iso_pipe; + int in_int_pipe; + int out_int_pipe; struct usb_endpoint_descriptor *iso_in, *iso_out; + struct usb_endpoint_descriptor *int_in, *int_out; struct mutexlock; #define TBUF_SIZE 256 @@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) struct usb_host_interface *alt; struct usb_host_endpoint*in, *out; struct usb_host_endpoint*iso_in, *iso_out; + struct usb_host_endpoint*int_in, *int_out; struct usb_device *udev; for (tmp = 0; tmp intf-num_altsetting; tmp++) { @@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) in = out = NULL; iso_in = iso_out = NULL; + int_in = int_out = NULL; alt = intf-altsetting + tmp; if (override_alt = 0 @@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) switch (usb_endpoint_type(e-desc)) { case USB_ENDPOINT_XFER_BULK: break; + case USB_ENDPOINT_XFER_INT: + if (dev-info-intr) + goto try_intr; case USB_ENDPOINT_XFER_ISOC: if (dev-info-iso) goto try_iso; @@ -139,6 +148,14 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) out = e; } continue; +try_intr: + if (usb_endpoint_dir_in(e-desc)) { + if (!int_in) + int_in = e; + } else { + if (!int_out) + int_out = e; + } try_iso: if (usb_endpoint_dir_in(e-desc)) { if (!iso_in) @@ -148,7 +165,7 @@ try_iso: iso_out = e; } } - if ((in out) || iso_in || iso_out) + if ((in out) || iso_in || iso_out || int_in || int_out) goto found; } return -EINVAL; @@ -183,6 +200,20 @@ found: iso_out-desc.bEndpointAddress USB_ENDPOINT_NUMBER_MASK); } + + if (int_in) { + dev-int_in = int_in-desc; + dev-in_int_pipe = usb_rcvintpipe(udev, + int_in-desc.bEndpointAddress +USB_ENDPOINT_NUMBER_MASK); + } + + if (int_out) { + dev-int_out = int_out-desc; + dev-out_int_pipe = usb_sndintpipe(udev, + int_out-desc.bEndpointAddress +USB_ENDPOINT_NUMBER_MASK); + } return 0; } @@ -205,14 +236,22 @@ static struct urb *usbtest_alloc_urb( int pipe, unsigned long bytes, unsignedtransfer_flags, - unsignedoffset) + unsignedoffset, + u8 bInterval) { struct urb *urb; urb = usb_alloc_urb(0, GFP_KERNEL); if (!urb) return
Re: usbserial_generic, idVendor=1a28, idProduct=6010
On Mon, Feb 24, 2014 at 10:28:05AM +0100, Emanuel Koczwara wrote: Hi, W dniu 24.02.2014 09:30, Johan Hovold pisze: On Mon, Feb 24, 2014 at 01:46:59AM +0100, Emanuel Koczwara wrote: Hi, W dniu 23.02.2014 21:21, Johan Hovold pisze: You might need to mount debugfs: mount -t debugfs none /sys/kernel/debug and make sure your kernel is compiled with CONFIG_DYNAMIC_DEBUG (and CONFIG_DEBUG_FS) enabled. Johan -- 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 Here is my debug output: http://nopaste.info/50a31e9ca7.html This is interesting: usb_serial_generic_read_bulk_callback - urb 0, len 2 usb_serial_generic_read_bulk_callback - length = 2, data = 31 60 I'm getting this 31 60 as 1` when I read from /dev/ttyUSB0. Yes, that was expected. The printer is returning what appears to be empty status-messages every 110ms. I also wanted to make sure that all bulk data had this two-byte header. Is that also the case for /dev/ttyUSB1? Yes. The printer has a serial interface as well? Which behaves exactly the same but does not return any 1`? I will check it. I don't have a computer with rs232 at this moment. I can write up a small driver which strips the header from incoming data for you to test, but it would be nice to know what the header actually is (and if it ever changes). There are no hints in the (Polish) manual? What kind of header? Some low level bytes? Manual contains only application level protocol description. There are 2 kinds of messages incoming from the printer: 1) single byte status 2) message starting with 0x1b 0x50 and it ends with 0x1b 0x5c. To recognize these 2 types of messages I need that 0x1b 0x50 header. Yes, I meant the lower-level two-byte 1` (0x31 0x60) that starts every bulk-in message. Hey, wait a minute. This looks like one of those ftdi_sio bulk-in headers: 0x31: DSR, CTS 0x60: Transmitter Empty, Transmitter Holding Register Empty This is likely an ftdi-device. Care to try the patch below? You probably also want to unset the low_latency-flag (e.g. using setserial) to avoid having the printer sending these status messages every millisecond when using the ftdi_driver. You can then increase the latency timer through sysfs to what appears to be your device's default of 110ms (or up to 255ms it seems): echo 110 /sys/bus/usb-serial/devices/ttyUSB0/latency_timer Thanks, Johan From 19e5289d621e062b8aa49a7c9dbaa7120f3d57dc Mon Sep 17 00:00:00 2001 From: Johan Hovold jhov...@gmail.com Date: Mon, 24 Feb 2014 11:20:15 +0100 Subject: [PATCH] USB: ftdi_sio: add support for NOVITUS Bono E thermal printer Add device id for NOVITUS Bono E thermal printer. --- drivers/usb/serial/ftdi_sio.c | 1 + drivers/usb/serial/ftdi_sio_ids.h | 6 ++ 2 files changed, 7 insertions(+) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index ee1f00f03c43..255f4756c08d 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -735,6 +735,7 @@ static const struct usb_device_id id_table_combined[] = { { USB_DEVICE(FTDI_VID, FTDI_NDI_AURORA_SCU_PID), .driver_info = (kernel_ulong_t)ftdi_NDI_device_quirk }, { USB_DEVICE(TELLDUS_VID, TELLDUS_TELLSTICK_PID) }, + { USB_DEVICE(NOVITUS_VID, NOVITUS_BONO_E_PID) }, { USB_DEVICE(RTSYSTEMS_VID, RTSYSTEMS_USB_S03_PID) }, { USB_DEVICE(RTSYSTEMS_VID, RTSYSTEMS_USB_59_PID) }, { USB_DEVICE(RTSYSTEMS_VID, RTSYSTEMS_USB_57A_PID) }, diff --git a/drivers/usb/serial/ftdi_sio_ids.h b/drivers/usb/serial/ftdi_sio_ids.h index 1e2d369df86e..049c1d75ea30 100644 --- a/drivers/usb/serial/ftdi_sio_ids.h +++ b/drivers/usb/serial/ftdi_sio_ids.h @@ -820,6 +820,12 @@ #define TELLDUS_TELLSTICK_PID 0x0C30 /* RF control dongle 433 MHz using FT232RL */ /* + * NOVITUS printers + */ +#define NOVITUS_VID0x1a28 +#define NOVITUS_BONO_E_PID 0x6010 + +/* * RT Systems programming cables for various ham radios */ #define RTSYSTEMS_VID 0x2100 /* Vendor ID */ -- 1.8.3.2 -- 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: sisusb: Use static const, fix typo
From: Joe Perches Reduce text a bit by using static const. If you want to save a few bytes remove the pointers. (and the fixed RAM text to get below 7 chars). eg: - const char *ramtypetext2[] = { SDR SDRAM, SDR SGRAM, - DDR SDRAM, DDR SGRAM }; static const char ramtypetext2[8][] = { SDR SD, SDR SG, DDR SD, DDR SG ... - dev_info(sisusb-sisusb_dev-dev, %dMB %s %s, bus width %d\n, (sisusb-vramsize 20), ramtypetext1, - ramtypetext2[ramtype], bw); dev_info(sisusb-sisusb_dev-dev, %dMB %s %sRAM, bus width %d\n, sisusb-vramsize 20, ramtypetext1, ramtypetext2[ramtype], bw); David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB to Serial converter code pl2303
[ Please don't drop the lists from CC. ] On Mon, Feb 24, 2014 at 11:07:50AM +0100, Magnus wrote: It doesnt work with my servo controller chip at 2400 Baud. I get some reply back at 115200 Baud by using gtkterm so it probably works at that speed but not lower speeds. I will get a null-modem cable so that i can test it better and ill let you know the exact results. Ok. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] bug fix for mv_udc_core.c
Le Mon, 24 Feb 2014 16:03:10 +0800, Neil Zhang zhan...@marvell.com a écrit : This patch set is mainly for bug fix. Neil Zhang (6): usb: gadget: mv_udc: remove redundant pull up in udc_start usb: gadget: mv_udc: disable HW zlt for ep0 usb: gadget: mv_udc: clear corresponding interrupt when flush fifo usb: gadget: mv_udc: check endpoint before queue dtd USB: gadget: mv_udc: workaroud for missing dTD USB: gadget: mv_udc: fix corner case for missiong dTD drivers/usb/gadget/mv_udc_core.c | 47 ++ 1 file changed, 42 insertions(+), 5 deletions(-) Do you have plan to move to the chipidea udc driver ? You could have some bug fix for free or share your bug fix with others. Matthieu -- 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: usbserial_generic, idVendor=1a28, idProduct=6010
Hi, W dniu 24.02.2014 11:25, Johan Hovold pisze: On Mon, Feb 24, 2014 at 10:28:05AM +0100, Emanuel Koczwara wrote: Hi, W dniu 24.02.2014 09:30, Johan Hovold pisze: On Mon, Feb 24, 2014 at 01:46:59AM +0100, Emanuel Koczwara wrote: Hi, W dniu 23.02.2014 21:21, Johan Hovold pisze: You might need to mount debugfs: mount -t debugfs none /sys/kernel/debug and make sure your kernel is compiled with CONFIG_DYNAMIC_DEBUG (and CONFIG_DEBUG_FS) enabled. Johan -- 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 Here is my debug output: http://nopaste.info/50a31e9ca7.html This is interesting: usb_serial_generic_read_bulk_callback - urb 0, len 2 usb_serial_generic_read_bulk_callback - length = 2, data = 31 60 I'm getting this 31 60 as 1` when I read from /dev/ttyUSB0. Yes, that was expected. The printer is returning what appears to be empty status-messages every 110ms. I also wanted to make sure that all bulk data had this two-byte header. Is that also the case for /dev/ttyUSB1? Yes. The printer has a serial interface as well? Which behaves exactly the same but does not return any 1`? I will check it. I don't have a computer with rs232 at this moment. I can write up a small driver which strips the header from incoming data for you to test, but it would be nice to know what the header actually is (and if it ever changes). There are no hints in the (Polish) manual? What kind of header? Some low level bytes? Manual contains only application level protocol description. There are 2 kinds of messages incoming from the printer: 1) single byte status 2) message starting with 0x1b 0x50 and it ends with 0x1b 0x5c. To recognize these 2 types of messages I need that 0x1b 0x50 header. Yes, I meant the lower-level two-byte 1` (0x31 0x60) that starts every bulk-in message. Hey, wait a minute. This looks like one of those ftdi_sio bulk-in headers: 0x31:DSR, CTS 0x60:Transmitter Empty, Transmitter Holding Register Empty This is likely an ftdi-device. Care to try the patch below? Thanks, I'll try. You probably also want to unset the low_latency-flag (e.g. using setserial) to avoid having the printer sending these status messages every millisecond when using the ftdi_driver. You can then increase the latency timer through sysfs to what appears to be your device's default of 110ms (or up to 255ms it seems): echo 110 /sys/bus/usb-serial/devices/ttyUSB0/latency_timer Ok. Thanks, Emanuel -- 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] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On 02/24/2014 11:51 AM, Kishon Vijay Abraham I wrote: Hi Roger, On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. snip This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? right, but the PHY is connected to the dwc3 core and not to the glue. I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. Just to summarize this thread on what we need Thanks for summarizing. 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or not. It should be as generic as possible. 2) dwc3 core should continue probe even if PHY layer is not enabled, as not all platforms need it. 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) 4) dwc3 core should error out on any error condition if PHY device is available and caused some error, e.g. init error. 5) dwc3 core should return EPROBE_DEFER if PHY device is available but device driver is not yet loaded. 6) platform glue should do the necessary sanity checks for availability of all resources like PHY device, PHY layer,
Re: is it possible to temporarily not let user mode get hid input event
hi David: 2014-02-23 23:16 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Sun, Feb 23, 2014 at 7:52 AM, loody milo...@gmail.com wrote: hi David: Thanks for your suggestion. 2014-02-23 0:56 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Sat, Feb 22, 2014 at 5:35 PM, loody milo...@gmail.com wrote: hi all: is there any kernel hid module parameter or test program can temporarily not letting user mode program not receiving hid event? 1. My hid kos are still inserted in. 2. the kernel usb driver is working well; that mean kernel usb driver still handle interrupt transaction. I just not want user mode program see the hid event for a while, For each connected HID device, there is a driver bound to it that reads the events and forwards them to HID core. What you can do, is to unbind a driver on a given device: echo your-device-name /sys/bus/hid/drivers/driver-name/unbind The device-name is the directory name in: /sys/bus/hid/devices/ The driver name is usually hid-generic but can be figured out for each device by looking at the driver symlink in it's directry. However, this is *really* just meant for debugging. This is not recommended for anything serious. There is no support for that and if you don't know what all this does, you shouldn't use it. There is no proper way to disable a single device in the kernel. User-space is supposed to control device-access so we probably won't add such features to the kernel. If you describe your use-case in more details, we can try to give hints how to get that working. Sorry for not describing our situation clearer previously, The problem we met like below a. once plug in usb hid mouse and fast moving mouse b. the screen will get blur. We want to know whether the screen blur is caused by 1. the interrupt frequency of usb mouse is too high for our embedded system that make video decode slow 2. something wrong between hw cursor and video overlay. if we can deceive user mode program there is no mouse event, but kernel usb level still get hid interrupt transaction. We may clarify whether above 1) conclusion is correct. Appreciate your kind help :-) You can unload the HID driver as described above, but that's unlikely to fix any interrupt issues. How about you compile your kernel without usbhid support? (CONFIG_USB_HID) BTW, is there any fake virtual mouse to use? That mean user doesn't plug in any usb hid mouse, but user mode program can receive hid mouse event? -- Regards, -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: is it possible to temporarily not let user mode get hid input event
Hi On Mon, Feb 24, 2014 at 12:20 PM, loody milo...@gmail.com wrote: hi David: 2014-02-23 23:16 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Sun, Feb 23, 2014 at 7:52 AM, loody milo...@gmail.com wrote: hi David: Thanks for your suggestion. 2014-02-23 0:56 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Sat, Feb 22, 2014 at 5:35 PM, loody milo...@gmail.com wrote: hi all: is there any kernel hid module parameter or test program can temporarily not letting user mode program not receiving hid event? 1. My hid kos are still inserted in. 2. the kernel usb driver is working well; that mean kernel usb driver still handle interrupt transaction. I just not want user mode program see the hid event for a while, For each connected HID device, there is a driver bound to it that reads the events and forwards them to HID core. What you can do, is to unbind a driver on a given device: echo your-device-name /sys/bus/hid/drivers/driver-name/unbind The device-name is the directory name in: /sys/bus/hid/devices/ The driver name is usually hid-generic but can be figured out for each device by looking at the driver symlink in it's directry. However, this is *really* just meant for debugging. This is not recommended for anything serious. There is no support for that and if you don't know what all this does, you shouldn't use it. There is no proper way to disable a single device in the kernel. User-space is supposed to control device-access so we probably won't add such features to the kernel. If you describe your use-case in more details, we can try to give hints how to get that working. Sorry for not describing our situation clearer previously, The problem we met like below a. once plug in usb hid mouse and fast moving mouse b. the screen will get blur. We want to know whether the screen blur is caused by 1. the interrupt frequency of usb mouse is too high for our embedded system that make video decode slow 2. something wrong between hw cursor and video overlay. if we can deceive user mode program there is no mouse event, but kernel usb level still get hid interrupt transaction. We may clarify whether above 1) conclusion is correct. Appreciate your kind help :-) You can unload the HID driver as described above, but that's unlikely to fix any interrupt issues. How about you compile your kernel without usbhid support? (CONFIG_USB_HID) BTW, is there any fake virtual mouse to use? That mean user doesn't plug in any usb hid mouse, but user mode program can receive hid mouse event? Yes. For instance, you can run ./samples/uhid/uhid-example.c via ssh to emulate a mouse. ssh keyboard input can then be used to generate mouse-movement events. Thanks David -- 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: is it possible to temporarily not let user mode get hid input event
hi David: 2014-02-24 19:35 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Mon, Feb 24, 2014 at 12:20 PM, loody milo...@gmail.com wrote: hi David: 2014-02-23 23:16 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Sun, Feb 23, 2014 at 7:52 AM, loody milo...@gmail.com wrote: hi David: Thanks for your suggestion. 2014-02-23 0:56 GMT+08:00 David Herrmann dh.herrm...@gmail.com: Hi On Sat, Feb 22, 2014 at 5:35 PM, loody milo...@gmail.com wrote: hi all: is there any kernel hid module parameter or test program can temporarily not letting user mode program not receiving hid event? 1. My hid kos are still inserted in. 2. the kernel usb driver is working well; that mean kernel usb driver still handle interrupt transaction. I just not want user mode program see the hid event for a while, For each connected HID device, there is a driver bound to it that reads the events and forwards them to HID core. What you can do, is to unbind a driver on a given device: echo your-device-name /sys/bus/hid/drivers/driver-name/unbind The device-name is the directory name in: /sys/bus/hid/devices/ The driver name is usually hid-generic but can be figured out for each device by looking at the driver symlink in it's directry. However, this is *really* just meant for debugging. This is not recommended for anything serious. There is no support for that and if you don't know what all this does, you shouldn't use it. There is no proper way to disable a single device in the kernel. User-space is supposed to control device-access so we probably won't add such features to the kernel. If you describe your use-case in more details, we can try to give hints how to get that working. Sorry for not describing our situation clearer previously, The problem we met like below a. once plug in usb hid mouse and fast moving mouse b. the screen will get blur. We want to know whether the screen blur is caused by 1. the interrupt frequency of usb mouse is too high for our embedded system that make video decode slow 2. something wrong between hw cursor and video overlay. if we can deceive user mode program there is no mouse event, but kernel usb level still get hid interrupt transaction. We may clarify whether above 1) conclusion is correct. Appreciate your kind help :-) You can unload the HID driver as described above, but that's unlikely to fix any interrupt issues. How about you compile your kernel without usbhid support? (CONFIG_USB_HID) BTW, is there any fake virtual mouse to use? That mean user doesn't plug in any usb hid mouse, but user mode program can receive hid mouse event? Yes. For instance, you can run ./samples/uhid/uhid-example.c via ssh to emulate a mouse. ssh keyboard input can then be used to generate mouse-movement events. There is one embarrassing question :-) Would you please let me know how to cross-compile it ? I try below commands but failed #cd samples/uhid/ #make CROSS_COMPILE=arm-v7a8v4r2-linux-gnueabi- make: *** No targets. Stop. Thanks for your kind help, -- 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: is it possible to temporarily not let user mode get hid input event
Hi On Mon, Feb 24, 2014 at 1:34 PM, loody milo...@gmail.com wrote: Yes. For instance, you can run ./samples/uhid/uhid-example.c via ssh to emulate a mouse. ssh keyboard input can then be used to generate mouse-movement events. There is one embarrassing question :-) Would you please let me know how to cross-compile it ? I try below commands but failed #cd samples/uhid/ #make CROSS_COMPILE=arm-v7a8v4r2-linux-gnueabi- make: *** No targets. Stop. Thanks for your kind help, Don't use the Makefile. Use gcc directly: gcc -o uhid-example -Wall samples/uhid/uhid-example.c It's just one source file, you should be able to compile it on the device. No idea how your cross-compiler works, so cannot help you there. Thanks David -- 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: SPDX-License-Identifier
On 02/24/2014 02:41 PM, Theodore Ts'o wrote: On Mon, Feb 24, 2014 at 11:12:53AM +0100, Michal Simek wrote: But of course, I'm not a lawyer, and if your company has is paying for the development of the driver, the Golden Rule applies (he who has the Gold, makes the Rules), and each of our respective corporate lawyers may have different opinions about what might happen if the question was ever to be adjudicated in court. Aren't all these points already answered by SPDX project? I believe that they should know how this should be handled properly. The SPDX can not give legal advice; not to you, and not to your company. One lawyer might believe that /* * SPDX-License-Identifier: GPL-2.0 */ Might be sufficient. Others might believe you need to do: /* * Copyright Ty Coon, 2012. * * SPDX-License-Identifier: GPL-2.0 */ Still others might believe you need at the very least: /* * Copyright Ty Coon, 2012. * * All Rights Reserved. * * SPDX-License-Identifier: GPL-2.0 */ Aren't these differences already present in the header? As far as I know, there is no case law on point about whether or not SPDX-License-Identifier has legal significance, or whether the court would consider that to be a valid copyright permission statement. So any answers made by any lawyer would be guesses. Of course, an guess by a lawyer which is retained by *you* or your company and fully informed with the unique parameters of your situation would constitute legal advice. Anything else, including anything any of us could say on this mailing list, would be biovating. :-) I think make sense to wait for Wolfgang about his experience because I believe he has considered it before u-boot change. BTW: Isn't this a good topic for kernel-summit? :-) Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP - KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: Kernel OOPS with g_ffs module loaded
On Mon, Feb 24, 2014 at 10:05 AM, Marco Zamponi mzamp...@gmail.com wrote: I get a kernel panic with a null pointer dereference. Both on Kernel 3.6.9 and 3.0.35 1e00: ba8e0200 80771e18 805a3b40 7f000b9c 6193 [8009a850] (__dabt_svc+0x70/0xa0) from [7f000b9c] (ffs_func_set_alt+0xd4/0x1a4 [g_ffs]) [7f000b9c] (ffs_func_set_alt+0xd4/0x1a4 [g_ffs]) from [7f003f68] (composite_setup+0x6f0/0xb64 [g_ffs]) [7f003f68] (composite_setup+0x6f0/0xb64 [g_ffs]) from [803be7cc] (fsl_udc_irq+0x9c8/0xc60) [803be7cc] (fsl_udc_irq+0x9c8/0xc60) from [8010c630] What is your hardware? You should use the chipidea driver instead of the old fsl udc one. Please try it with 3.13 or 3.14-rc4. Regards, Fabio Estevam -- 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: SPDX-License-Identifier
On Mon, Feb 24, 2014 at 03:03:25PM +0100, Michal Simek wrote: BTW: Isn't this a good topic for kernel-summit? :-) No, lawyers don't go to the summit, developers do. -- 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] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Monday 24 February 2014 04:35 PM, Roger Quadros wrote: On 02/24/2014 11:51 AM, Kishon Vijay Abraham I wrote: Hi Roger, On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. snip This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? right, but the PHY is connected to the dwc3 core and not to the glue. I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. Just to summarize this thread on what we need Thanks for summarizing. 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or not. It should be as generic as possible. 2) dwc3 core should continue probe even if PHY layer is not enabled, as not all platforms need it. 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) 4) dwc3 core should error out on any error condition if PHY device is available and caused some error, e.g. init error. 5) dwc3 core should return EPROBE_DEFER if PHY device is available but device driver is not yet loaded. 6) platform glue should do the necessary
Re: [PATCH v2] u_ether: move hardware transmit to RX workqueue
On 2/22/2014 11:11 AM, Clanlab (Taiwan) Linux Project wrote: In order to reduce the interrupt times in the embedded system, a receiving workqueue is introduced. This modification also enhanced the overall throughput as the benefits of reducing interrupt occurrence. This patch looks to be derived from: https://android.googlesource.com/kernel/msm/+/08eb78ebbee92e4f133fc75237cf6af6356c38b6 -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] storage: accept some UAS devices if streams are unavailable
On Fri, 2014-02-21 at 15:00 -0800, Sarah Sharp wrote: On Tue, Feb 11, 2014 at 08:36:04PM +0100, oli...@neukum.org wrote: From: Oliver Neukum oneu...@suse.de On some older XHCIs streams are not supported and the UAS driver will fail at probe time. For those devices storage should try to bind to UAS devices. This patch adds a flag for stream support to HCDs and evaluates it. Oliver, the logic in this patch is off. When I plug it into an Intel Panther Point xHCI host that has hcc_params set to 0x20007181, which translates to 2^(7+1) stream IDs, the device comes up as usb-storage, not uas: Thank you, this is subtle. I'd test this but only whether it fixed the error case. I shall test this on all configurations. It'll take a few hours. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Mon, Feb 24, 2014 at 03:21:05PM +0530, Kishon Vijay Abraham I wrote: Hi Roger, On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. snip This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? right, but the PHY is connected to the dwc3 core and not to the glue. I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. Just to summarize this thread on what we need Thanks for summarizing. 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or not. It should be as generic as possible. 2) dwc3 core should continue probe even if PHY layer is not enabled, as not all platforms need it. 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) 4) dwc3 core should error out on any error condition if PHY device is available and caused some error, e.g. init error. 5) dwc3 core should return EPROBE_DEFER if PHY device is available but device
Re: [Patch V1 0/2] usb: g_zero: Add support for interrupt EP
Hi, On Mon, Feb 24, 2014 at 03:55:09PM +0530, Amit Virdi wrote: This patchset adds support for interrupt EP and the corresponding test cases to gadget zero. The code has been rebased and tested on Kernel v3.14-rc3 RFC - V1 - Added support for configuring interrupt EP attributes from configfs interface Amit Virdi (2): usb: gadget: zero: Add support for interrupt EP usbtest: Add interrupt EP testcases Cc maintainers if you want to get your patches reviewed. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
On 02/23/2014 12:51 AM, Alan Stern wrote: Right, but I assume you'd want to hold the reference until after the hub is registered, otherwise there's still a chance we suspend right before register. So I'm saying hold the reference until the registration process takes its own. To be really safe about it, you should call pm_runtime_get_noresume at the start of xhci_pci_probe, and pm_runtime_put_noidle at the end Why _put_noidle? The fact that we suspend between registrations means the controller is getting suspend requests that we will block with _get_noresume. However, once probe is done our final put might be the only one to trigger the deferred suspension. I.e. just worried about the case where the controller's usage count is 0, but it remains active indefinitely. Either _put_sync or _put_noidle would be okay. The reason is because the device core always calls pm_request_idle after probing a driver (see driver_probe_device()). Thanks Dan and Alan for the input, I did the following changes: - Take and release the reference in xhci_pci_probe to avoid releasing the reference for a moment just before usb3 roothub registration. - Make sure we release the reference in error cases - Still use _put_noidle as driver_probe_device calls pm_request_idle -Mathias -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2] xhci: Prevent runtime pm from autosuspending during initialization
xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready. Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready. xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe. Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com --- drivers/usb/host/xhci-pci.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 04f986d..ea7158b 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) struct usb_hcd *hcd; driver = (struct hc_driver *)id-driver_data; + + /* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */ + pm_runtime_get_noresume(dev-dev); + /* Register the USB 2.0 roothub. * FIXME: USB core must know to register the USB 2.0 roothub first. * This is sort of silly, because we could just set the HCD driver flags @@ -199,7 +203,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) retval = usb_hcd_pci_probe(dev, id); if (retval) - return retval; + goto put_runtime_pm; /* USB 2.0 roothub is stored in the PCI device now. */ hcd = dev_get_drvdata(dev-dev); @@ -228,12 +232,17 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) if (xhci-quirks XHCI_LPM_SUPPORT) hcd_to_bus(xhci-shared_hcd)-root_hub-lpm_capable = 1; + /* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */ + pm_runtime_put_noidle(dev-dev); + return 0; put_usb3_hcd: usb_put_hcd(xhci-shared_hcd); dealloc_usb2_hcd: usb_hcd_pci_remove(dev); +put_runtime_pm: + pm_runtime_put_noidle(dev-dev); return retval; } -- 1.8.1.2 -- 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] xhci: Prevent runtime pm from autosuspending during initialization
On Mon, Feb 24, 2014 at 8:22 AM, Mathias Nyman mathias.ny...@linux.intel.com wrote: On 02/23/2014 12:51 AM, Alan Stern wrote: Right, but I assume you'd want to hold the reference until after the hub is registered, otherwise there's still a chance we suspend right before register. So I'm saying hold the reference until the registration process takes its own. To be really safe about it, you should call pm_runtime_get_noresume at the start of xhci_pci_probe, and pm_runtime_put_noidle at the end Why _put_noidle? The fact that we suspend between registrations means the controller is getting suspend requests that we will block with _get_noresume. However, once probe is done our final put might be the only one to trigger the deferred suspension. I.e. just worried about the case where the controller's usage count is 0, but it remains active indefinitely. Either _put_sync or _put_noidle would be okay. The reason is because the device core always calls pm_request_idle after probing a driver (see driver_probe_device()). Thanks Dan and Alan for the input, I did the following changes: - Take and release the reference in xhci_pci_probe to avoid releasing the reference for a moment just before usb3 roothub registration. - Make sure we release the reference in error cases - Still use _put_noidle as driver_probe_device calls pm_request_idle Sounds ok, and it is documented that the core calls pm_request_idle() on a driver's behalf. But it still seems a small layering violation and more future-proof to call _put_sync(). -- 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 v2] xhci: Prevent runtime pm from autosuspending during initialization
On Mon, Feb 24, 2014 at 8:29 AM, Mathias Nyman mathias.ny...@linux.intel.com wrote: xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready. Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready. xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe. Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com Acked-by: Dan Williams dan.j.willi...@intel.com -- 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 v5 14/16] usb: documentation for usb port power off mechanisms
On Mon, Feb 24, 2014 at 2:17 AM, Oliver Neukum oneu...@suse.de wrote: On Fri, 2014-02-21 at 16:10 -0800, Dan Williams wrote: From: Lan Tianyu tianyu@intel.com describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum oneu...@suse.de Signed-off-by: Lan Tianyu tianyu@intel.com [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/usb/power-management.txt | 237 1 files changed, 237 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..e67c1d4d1994 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -5,6 +5,25 @@ October 28, 2010 + Contents: + - + * What is Power Management? + * What is Remote Wakeup? + * When is a USB device idle? + * Forms of dynamic PM + * The user interface for dynamic PM + * Changing the default idle-delay time + * Warnings + * The driver interface for Power Management + * The driver interface for autosuspend and autoresume + * Other parts of the driver interface + * Mutual exclusion + * Interaction between dynamic PM and system PM + * xHCI hardware link PM + * USB Port Power Control + * User Interface for Port Power Control + * Suggested Userspace Port Power Policy + What is Power Management? - @@ -516,3 +535,221 @@ relevant attribute files is usb2_hardware_lpm. driver will enable hardware LPM for the device. You can write y/Y/1 or n/N/0 to the file to enable/disable USB2 hardware LPM manually. This is for test purpose mainly. + + + USB Port Power Control + -- + +In addition to suspending endpoint devices and enabling hardware +controlled link power management, the USB subsystem also has the +capability to disable power to individual ports. Power is controlled Not necessary individual ports. You explain the limitations of ganged switching further below. I'd prefer ports under some conditions Ok. +* wakeup note: the implementation does not allow a port connected to a + device with wakeup capability to be powered off. The capability may be there. It just mustn't be enabled. I'm not sure I grok this comment? Specifically I am referring to this logic: usb_port_suspend() { [..] if (status == 0 !udev-do_remote_wakeup udev-persist_enabled) { pm_runtime_put_sync(port_dev-dev); port_dev-did_runtime_put = true; } [..] } I have a follow-on patch to go clear -do_remote_wakeup on a hub once all its downstream ports are suspended, but until then the simply does not allow such ports to suspend. -- 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] introduce assign_if() macros in attempt to reduce ifdeffery
Based on the observation that given the following: static void my_callback(void) { } void (*callback)(void) = 0 ? my_callback : NULL; GCC will, 1.) Not emit 'defined but unused' warnings for 'my_callback' 2.) Typecheck my_callback against typeof(*callback) 3.) Eliminate my_callback as dead code (assuming it's unused elsewhere) this patchset explores where/how this might be used to reduce the amount of ifdeffed code in device drivers. Patch 1 in this series introduces two friendly-named helper macros: assign_if(const_expr, fn): A friendlier form of (const_expr ? fn : NULL) assign_if_enabled(conf, fn): Composition of assign_if() and IS_ENABLED(). In order to show a good target usecase for these macros, patch 2 introduces a new set of PM-related macros, similar to SET_*_PM_OPS, using assign_if_enabled() under the hood. In particular, this is aimed at reducing code like the following: #ifdef CONFIG_PM_SLEEP static int foo_suspend(struct device *dev) { /* ... */ } #else #define foo_suspend NULL #endif static const struct dev_pm_ops foo_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(foo_suspend, NULL) }; With the ifdefless: static int foo_suspend(struct device *dev) { /* ... */ } static const struct dev_pm_ops foo_pm_ops = { ASSIGN_SYSTEM_SLEEP_PM_OPS(foo_suspend, NULL), }; For patch 3, an existing consumer of SET_*_PM_OPS(), the MSM USB phy driver[1] is modified to use the newly introduced ASSIGN_*_PM_OPS() macros to show in a real case what simplifications could be gained. For testing that GCC is actually eliminating the PM-related callbacks when it can, the MSM USB phy driver was built before and after patch 3, in 4 different configurations, and object sizes compared[2]: textdata bss dec hex filename 11008 488 20 115162cfc phy_objects_before/phy-msm-usb.nopm.o 11008 488 20 115162cfc phy_objects_after/phy-msm-usb.nopm.o 13528 584 20 141323734 phy_objects_before/phy-msm-usb.runtime.o 13528 584 20 141323734 phy_objects_after/phy-msm-usb.runtime.o 13828 632 20 144803890 phy_objects_before/phy-msm-usb.runtime+sleep.o 13828 632 20 144803890 phy_objects_after/phy-msm-usb.runtime+sleep.o 13072 560 20 136523554 phy_objects_before/phy-msm-usb.sleep.o 13072 560 20 136523554 phy_objects_after/phy-msm-usb.sleep.o [1]: Chosen because it has recently broken randconfig builds due to improper #ifdeffery using CONFIG_PM* defines [2]: $ ${CROSS_COMPILE}gcc --version arm-linux-gnueabihf-gcc (crosstool-NG linaro-1.13.1-4.8-2013.07-1 - Linaro GCC 2013.07) 4.8.2 20130624 (prerelease) Copyright (C) 2013 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Josh Cartwright (3): typecheck: introduce assign_if() and assign_if_enabled() PM: define new ASSIGN_*_PM_OPS macros based on assign_if usb: phy: msm: use ASSIGN_*_PM_OPS variants drivers/usb/phy/phy-msm-usb.c | 13 +++-- include/linux/pm.h| 39 +++ include/linux/typecheck.h | 18 ++ 3 files changed, 60 insertions(+), 10 deletions(-) -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] usb: phy: msm: use ASSIGN_*_PM_OPS variants
Use ASSIGN_SYSTEM_SLEEP_PM_OPS and ASSIGN_RUNTIME_PM_OPS in the initializer for msm_otg_dev_pm_ops. Doing so allows us to eliminate preprocessor conditionals around the specified callbacks. Signed-off-by: Josh Cartwright jo...@codeaurora.org --- drivers/usb/phy/phy-msm-usb.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 5b37b81..c04f2e3 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -414,8 +414,6 @@ static int msm_otg_reset(struct usb_phy *phy) #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000) #define PHY_RESUME_TIMEOUT_USEC(100 * 1000) -#ifdef CONFIG_PM - #define USB_PHY_SUSP_DIG_VOL 50 static int msm_hsusb_config_vddcx(int high) { @@ -609,7 +607,6 @@ skip_phy_resume: return 0; } -#endif static void msm_otg_notify_charger(struct msm_otg *motg, unsigned mA) { @@ -1664,7 +1661,6 @@ static int msm_otg_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_RUNTIME static int msm_otg_runtime_idle(struct device *dev) { struct msm_otg *motg = dev_get_drvdata(dev); @@ -1699,9 +1695,7 @@ static int msm_otg_runtime_resume(struct device *dev) dev_dbg(dev, OTG runtime resume\n); return msm_otg_resume(motg); } -#endif -#ifdef CONFIG_PM_SLEEP static int msm_otg_pm_suspend(struct device *dev) { struct msm_otg *motg = dev_get_drvdata(dev); @@ -1731,12 +1725,11 @@ static int msm_otg_pm_resume(struct device *dev) return 0; } -#endif static const struct dev_pm_ops msm_otg_dev_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(msm_otg_pm_suspend, msm_otg_pm_resume) - SET_RUNTIME_PM_OPS(msm_otg_runtime_suspend, msm_otg_runtime_resume, - msm_otg_runtime_idle) + ASSIGN_SYSTEM_SLEEP_PM_OPS(msm_otg_pm_suspend, msm_otg_pm_resume) + ASSIGN_RUNTIME_PM_OPS(msm_otg_runtime_suspend, msm_otg_runtime_resume, + msm_otg_runtime_idle) }; static struct platform_driver msm_otg_driver = { -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 09/15] usb: phy: Add set_wakeup API
Hello. On 02/24/2014 05:21 AM, Peter Chen wrote: This API is used to set wakeup enable at PHY registers, in that case, the PHY can be waken up from suspend due to external events, like vbus change, dp/dm change and id change. Signed-off-by: Peter Chen peter.c...@freescale.com --- include/linux/usb/phy.h | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index 6c0b1c5..c2c6f49 100644 --- a/include/linux/usb/phy.h +++ b/include/linux/usb/phy.h @@ -111,6 +111,13 @@ struct usb_phy { int (*set_suspend)(struct usb_phy *x, int suspend); + /* +* Set wakeup enable for PHY, in that case, the PHY can be +* waken up from suspend status due to external events, s/waken/woken/ 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: [RFC PATCH v2] xhci: Prevent runtime pm from autosuspending during initialization
On Mon, 24 Feb 2014, Mathias Nyman wrote: xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready. Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready. xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe. Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com --- drivers/usb/host/xhci-pci.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 04f986d..ea7158b 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) struct usb_hcd *hcd; driver = (struct hc_driver *)id-driver_data; + + /* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */ + pm_runtime_get_noresume(dev-dev); Strictly speaking, this prevents the _controller_ from going into runtime suspend -- not the root hub. + /* Register the USB 2.0 roothub. * FIXME: USB core must know to register the USB 2.0 roothub first. * This is sort of silly, because we could just set the HCD driver flags @@ -199,7 +203,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) retval = usb_hcd_pci_probe(dev, id); if (retval) - return retval; + goto put_runtime_pm; /* USB 2.0 roothub is stored in the PCI device now. */ hcd = dev_get_drvdata(dev-dev); @@ -228,12 +232,17 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) if (xhci-quirks XHCI_LPM_SUPPORT) hcd_to_bus(xhci-shared_hcd)-root_hub-lpm_capable = 1; + /* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */ + pm_runtime_put_noidle(dev-dev); + return 0; put_usb3_hcd: usb_put_hcd(xhci-shared_hcd); dealloc_usb2_hcd: usb_hcd_pci_remove(dev); +put_runtime_pm: + pm_runtime_put_noidle(dev-dev); return retval; } Apart from nit about the comment, Acked-by: Alan Stern st...@rowland.harvard.edu -- 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] xhci: Prevent runtime pm from autosuspending during initialization
On Mon, 24 Feb 2014, Dan Williams wrote: Thanks Dan and Alan for the input, I did the following changes: - Take and release the reference in xhci_pci_probe to avoid releasing the reference for a moment just before usb3 roothub registration. - Make sure we release the reference in error cases - Still use _put_noidle as driver_probe_device calls pm_request_idle Sounds ok, and it is documented that the core calls pm_request_idle() on a driver's behalf. But it still seems a small layering violation and more future-proof to call _put_sync(). Actually, _put() would be preferable to _put_sync(), because it gives you an async suspend -- the probe sequence isn't delayed. I don't see this as a layering violation. Bind and unbind are important aspects of a device's lifecycle, and they clearly are intimately related to power management. Besides, one of the reasons for having a core is so that it can do things on behalf of drivers, even if the drivers could just as easily do these things on their own. This is related to the Don't-Repeat-Yourself principle. 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: sisusb: Use static const, fix typo
On Mon, 2014-02-24 at 10:26 +, David Laight wrote: From: Joe Perches Reduce text a bit by using static const. If you want to save a few bytes remove the pointers. (and the fixed RAM text to get below 7 chars). Hi David. eg: - const char *ramtypetext2[] = { SDR SDRAM, SDR SGRAM, - DDR SDRAM, DDR SGRAM }; The idea was use static to avoid the array reload on each function entry. static const char ramtypetext2[8][] = { SDR SD, SDR SG, DDR SD, DDR SG 8 is an odd number here. That could be done with char like: static const char ram_datarate[4] = {'S', 'S', 'D', 'D'}; static const char ram_dynamictype[4] = {'D', 'G', 'D', 'G'}; ... - dev_info(sisusb-sisusb_dev-dev, %dMB %s %s, bus width %d\n, (sisusb-vramsize 20), ramtypetext1, - ramtypetext2[ramtype], bw); dev_info(sisusb-sisusb_dev-dev, %dMB %s %sRAM, bus width %d\n, sisusb-vramsize 20, ramtypetext1, ramtypetext2[ramtype], bw); dev_info(sisusb-sisusb_dev-dev, %dMB %s %c%cRAM, bus width %d\n, sisusb-vramsize 20, ramtypetext1, ram_datarate[ramtype], ram_dynamictype[ramtype], bw); So something like: Convert 1 char * array to 2 char arrays to reduce size. Use static const to avoid array reloads on function entry. Fix asymmetric typo. $ size drivers/usb/misc/sisusbvga/sisusb.o* textdata bss dec hex filename 2997148419180 43992abd8 drivers/usb/misc/sisusbvga/sisusb.o.new 3008348419180 44104ac48 drivers/usb/misc/sisusbvga/sisusb.o.old Signed-off-by: Joe Perches j...@perches.com --- drivers/usb/misc/sisusbvga/sisusb.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c index de98906..06b5d77 100644 --- a/drivers/usb/misc/sisusbvga/sisusb.c +++ b/drivers/usb/misc/sisusbvga/sisusb.c @@ -2123,8 +2123,8 @@ sisusb_get_ramconfig(struct sisusb_usb_data *sisusb) u8 tmp8, tmp82, ramtype; int bw = 0; char *ramtypetext1 = NULL; - const char *ramtypetext2[] = { SDR SDRAM, SDR SGRAM, - DDR SDRAM, DDR SGRAM }; + static const char ram_datarate[4] = {'S', 'S', 'D', 'D'}; + static const char ram_dynamictype[4] = {'D', 'G', 'D', 'G'}; static const int busSDR[4] = {64, 64, 128, 128}; static const int busDDR[4] = {32, 32, 64, 64}; static const int busDDRA[4] = {64+32, 64+32 , (64+32)*2, (64+32)*2}; @@ -2156,8 +2156,10 @@ sisusb_get_ramconfig(struct sisusb_usb_data *sisusb) break; } - dev_info(sisusb-sisusb_dev-dev, %dMB %s %s, bus width %d\n, (sisusb-vramsize 20), ramtypetext1, - ramtypetext2[ramtype], bw); + + dev_info(sisusb-sisusb_dev-dev, %dMB %s %cDR S%cRAM, bus width %d\n, +sisusb-vramsize 20, ramtypetext1, +ram_datarate[ramtype], ram_dynamictype[ramtype], bw); } static int -- 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 v5 14/16] usb: documentation for usb port power off mechanisms
On Mon, 2014-02-24 at 08:55 -0800, Dan Williams wrote: On Mon, Feb 24, 2014 at 2:17 AM, Oliver Neukum oneu...@suse.de wrote: On Fri, 2014-02-21 at 16:10 -0800, Dan Williams wrote: From: Lan Tianyu tianyu@intel.com +* wakeup note: the implementation does not allow a port connected to a + device with wakeup capability to be powered off. The capability may be there. It just mustn't be enabled. I'm not sure I grok this comment? Specifically I am referring to this logic: usb_port_suspend() { [..] if (status == 0 !udev-do_remote_wakeup udev-persist_enabled) { do_remote_wakeup does not mean that the device can do remote wakeup. It means that some interface driver has requested remote wakeup to be enabled at the next suspend. Some drivers don't use remote wakeup under some circumstances even though the device does support it. The best example for this is btusb. If you do hciconfig down remote wakeup will not be requested. Your description suggests that it depends on the device's capabilities. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 14/16] usb: documentation for usb port power off mechanisms
On Mon, Feb 24, 2014 at 9:58 AM, Oliver Neukum oneu...@suse.de wrote: On Mon, 2014-02-24 at 08:55 -0800, Dan Williams wrote: On Mon, Feb 24, 2014 at 2:17 AM, Oliver Neukum oneu...@suse.de wrote: On Fri, 2014-02-21 at 16:10 -0800, Dan Williams wrote: From: Lan Tianyu tianyu@intel.com +* wakeup note: the implementation does not allow a port connected to a + device with wakeup capability to be powered off. The capability may be there. It just mustn't be enabled. I'm not sure I grok this comment? Specifically I am referring to this logic: usb_port_suspend() { [..] if (status == 0 !udev-do_remote_wakeup udev-persist_enabled) { do_remote_wakeup does not mean that the device can do remote wakeup. It means that some interface driver has requested remote wakeup to be enabled at the next suspend. Some drivers don't use remote wakeup under some circumstances even though the device does support it. The best example for this is btusb. If you do hciconfig down remote wakeup will not be requested. Your description suggests that it depends on the device's capabilities. Got it now, thanks for the clarification. -- 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 v5 14/16] usb: documentation for usb port power off mechanisms
On Mon, 2014-02-24 at 18:58 +0100, Oliver Neukum wrote: On Mon, 2014-02-24 at 08:55 -0800, Dan Williams wrote: On Mon, Feb 24, 2014 at 2:17 AM, Oliver Neukum oneu...@suse.de wrote: On Fri, 2014-02-21 at 16:10 -0800, Dan Williams wrote: From: Lan Tianyu tianyu@intel.com +* wakeup note: the implementation does not allow a port connected to a + device with wakeup capability to be powered off. The capability may be there. It just mustn't be enabled. I'm not sure I grok this comment? Specifically I am referring to this logic: usb_port_suspend() { [..] if (status == 0 !udev-do_remote_wakeup udev-persist_enabled) { do_remote_wakeup does not mean that the device can do remote wakeup. It means that some interface driver has requested remote wakeup to be enabled at the next suspend. Some drivers don't use remote wakeup under some circumstances even though the device does support it. The best example for this is btusb. If you do hciconfig down remote wakeup will not be requested. Your description suggests that it depends on the device's capabilities. 8--- Subject: usb: documentation for usb port power off mechanisms From: Lan Tianyu tianyu@intel.com describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum oneu...@suse.de [oliver]: fixes, clarification of wakeup vs port-power-control Signed-off-by: Lan Tianyu tianyu@intel.com [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/usb/power-management.txt | 238 1 files changed, 238 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..5cfd7cb23d23 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -5,6 +5,25 @@ October 28, 2010 + Contents: + - + * What is Power Management? + * What is Remote Wakeup? + * When is a USB device idle? + * Forms of dynamic PM + * The user interface for dynamic PM + * Changing the default idle-delay time + * Warnings + * The driver interface for Power Management + * The driver interface for autosuspend and autoresume + * Other parts of the driver interface + * Mutual exclusion + * Interaction between dynamic PM and system PM + * xHCI hardware link PM + * USB Port Power Control + * User Interface for Port Power Control + * Suggested Userspace Port Power Policy + What is Power Management? - @@ -516,3 +535,222 @@ relevant attribute files is usb2_hardware_lpm. driver will enable hardware LPM for the device. You can write y/Y/1 or n/N/0 to the file to enable/disable USB2 hardware LPM manually. This is for test purpose mainly. + + + USB Port Power Control + -- + +In addition to suspending endpoint devices and enabling hardware +controlled link power management, the USB subsystem also has the +capability to disable power to ports under some conditions. Power is +controlled through Set/ClearPortFeature(PORT_POWER) requests to a hub. +In the case of a root or platform-internal hub the host controller +driver translates PORT_POWER requests into platform firmware (ACPI) +method calls to set the port power state. For more background see the +Linux Plumbers Conference 2012 slides [1] and video [2]: + +Upon receiving a ClearPortFeature(PORT_POWER) request a USB port is +logically off, and may trigger the actual loss of VBUS to the port [3]. +VBUS may be maintained in the case where a hub gangs multiple ports into +a shared power well causing power to remain until all ports in the gang +are turned off. VBUS may also be maintained by hub ports configured for +a charging application. In any event a logically off port will lose +connection with its device, not respond to hotplug events, and not +respond to remote wakeup events*. + +WARNING: turning off a port may result in the inability to hot add a device. +Please see User Interface for Port Power Control for details. + +As far as the effect on the device itself it is similar to what a device +goes through during system suspend, i.e. the power session is lost. Any +USB device or driver that misbehaves with system suspend will be +similarly affected by a port power cycle event. For this reason the +implementation shares the same device recovery path (and honors the same +quirks) as the system resume path for the hub. + +[1]: http://dl.dropbox.com/u/96820575/sarah-sharp-lpt-port-power-off2-mini.pdf +[2]: http://linuxplumbers.ubicast.tv/videos/usb-port-power-off-kerneluserspace-api/ +[3]: USB 3.1 Section 10.12 +* wakeup note: if a device is configured
Re: [PATCH v5 14/16] usb: documentation for usb port power off mechanisms
On Mon, 24 Feb 2014, Dan Williams wrote: 8--- Subject: usb: documentation for usb port power off mechanisms From: Lan Tianyu tianyu@intel.com describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum oneu...@suse.de [oliver]: fixes, clarification of wakeup vs port-power-control Signed-off-by: Lan Tianyu tianyu@intel.com [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/usb/power-management.txt | 238 1 files changed, 238 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..5cfd7cb23d23 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -5,6 +5,25 @@ October 28, 2010 With all the patches flying around, I hope that one of them will change the date in this document. :-) Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: 'noxhci_port_switch' kernel parameter
Add a command line switch for disabling ehci port switchover. Useful for working around / debugging xhci incompatibilities where ehci operation is available. Reference: http://marc.info/?l=linux-usbm=138920063001509w=2 Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Mathias Nyman mathias.ny...@linux.intel.com Cc: Holger Hans Peter Freyther hol...@moiji-mobile.com Suggested-by: Alan Stern st...@rowland.harvard.edu Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/kernel-parameters.txt |3 +++ drivers/usb/host/pci-quirks.c | 15 +-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 7116fda7077f..126c6e064a1c 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -2232,6 +2232,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted. nox2apic[X86-64,APIC] Do not enable x2APIC mode. + noxhci_port_switch + [USB] Use EHCI instead of XHCI where available + cpu0_hotplug[X86] Turn on CPU0 hotplug feature when CONFIG_BOOTPARAM_HOTPLUG_CPU0 is off. Some features depend on CPU0. Known dependencies are: diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 00661d305143..38bfe3dfc0bb 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -823,6 +823,15 @@ static int handshake(void __iomem *ptr, u32 mask, u32 done, return -ETIMEDOUT; } +static int noxhci_port_switch; + +static int __init noxhci_port_switch_setup(char *str) +{ + noxhci_port_switch = 1; + return 0; +} +early_param(noxhci_port_switch, noxhci_port_switch_setup); + /* * Intel's Panther Point chipset has two host controllers (EHCI and xHCI) that * share some number of ports. These ports can be switched between either @@ -860,8 +869,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev) return; /* Don't switchover the ports if the user hasn't compiled the xHCI -* driver. Otherwise they will see dead USB ports that don't power -* the devices. +* driver or has explicitly disabled switchover */ if (!IS_ENABLED(CONFIG_USB_XHCI_HCD)) { dev_warn(xhci_pdev-dev, @@ -871,6 +879,9 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev) USB 3.0 devices will work at USB 2.0 speeds.\n); usb_disable_xhci_ports(xhci_pdev); return; + } else if (noxhci_port_switch) { + usb_disable_xhci_ports(xhci_pdev); + return; } /* Read USB3PRM, the USB 3.0 Port Routing Mask Register -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer
Save someone else the debug cycles of figuring out why a driver's transfer request is failing or causing undefined system behavior. Buffers submitted for dma must come from GFP allocated / DMA-able memory. Return -EAGAIN matching the return value for dma_mapping_error() cases. Cc: Alan Stern st...@rowland.harvard.edu Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Mathias Nyman mathias.ny...@linux.intel.com Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hcd.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c3250750..ff7cd489f55d 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, ret = -EAGAIN; else urb-transfer_flags |= URB_DMA_MAP_PAGE; + } else if (is_vmalloc_addr(urb-transfer_buffer)) { + WARN_ONCE(1, transfer buffer not dma capable\n); + return -EAGAIN; } else { urb-transfer_dma = dma_map_single( hcd-self.controller, -- 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 v5 14/16] usb: documentation for usb port power off mechanisms
On Mon, 2014-02-24 at 14:11 -0500, Alan Stern wrote: On Mon, 24 Feb 2014, Dan Williams wrote: 8--- Subject: usb: documentation for usb port power off mechanisms From: Lan Tianyu tianyu@intel.com describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum oneu...@suse.de [oliver]: fixes, clarification of wakeup vs port-power-control Signed-off-by: Lan Tianyu tianyu@intel.com [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/usb/power-management.txt | 238 1 files changed, 238 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..5cfd7cb23d23 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -5,6 +5,25 @@ October 28, 2010 With all the patches flying around, I hope that one of them will change the date in this document. :-) I guess it is time. 8- Subject: usb: documentation for usb port power off mechanisms From: Lan Tianyu tianyu@intel.com describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum oneu...@suse.de [oliver]: fixes, clarification of wakeup vs port-power-control Signed-off-by: Lan Tianyu tianyu@intel.com [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams dan.j.willi...@intel.com --- Documentation/usb/power-management.txt | 242 1 files changed, 240 insertions(+), 2 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..6c3e0f04f6cb 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -2,8 +2,27 @@ Alan Stern st...@rowland.harvard.edu - October 28, 2010 - + Last-updated: February 2014 + + + Contents: + - + * What is Power Management? + * What is Remote Wakeup? + * When is a USB device idle? + * Forms of dynamic PM + * The user interface for dynamic PM + * Changing the default idle-delay time + * Warnings + * The driver interface for Power Management + * The driver interface for autosuspend and autoresume + * Other parts of the driver interface + * Mutual exclusion + * Interaction between dynamic PM and system PM + * xHCI hardware link PM + * USB Port Power Control + * User Interface for Port Power Control + * Suggested Userspace Port Power Policy What is Power Management? @@ -516,3 +535,222 @@ relevant attribute files is usb2_hardware_lpm. driver will enable hardware LPM for the device. You can write y/Y/1 or n/N/0 to the file to enable/disable USB2 hardware LPM manually. This is for test purpose mainly. + + + USB Port Power Control + -- + +In addition to suspending endpoint devices and enabling hardware +controlled link power management, the USB subsystem also has the +capability to disable power to ports under some conditions. Power is +controlled through Set/ClearPortFeature(PORT_POWER) requests to a hub. +In the case of a root or platform-internal hub the host controller +driver translates PORT_POWER requests into platform firmware (ACPI) +method calls to set the port power state. For more background see the +Linux Plumbers Conference 2012 slides [1] and video [2]: + +Upon receiving a ClearPortFeature(PORT_POWER) request a USB port is +logically off, and may trigger the actual loss of VBUS to the port [3]. +VBUS may be maintained in the case where a hub gangs multiple ports into +a shared power well causing power to remain until all ports in the gang +are turned off. VBUS may also be maintained by hub ports configured for +a charging application. In any event a logically off port will lose +connection with its device, not respond to hotplug events, and not +respond to remote wakeup events*. + +WARNING: turning off a port may result in the inability to hot add a device. +Please see User Interface for Port Power Control for details. + +As far as the effect on the device itself it is similar to what a device +goes through during system suspend, i.e. the power session is lost. Any +USB device or driver that misbehaves with system suspend will be +similarly affected by a port power cycle event. For this reason the +implementation shares the same device recovery path (and honors the same +quirks) as the system resume path for the hub. + +[1]: http://dl.dropbox.com/u/96820575/sarah-sharp-lpt-port-power-off2-mini.pdf +[2]:
Re: [PATCH] xhci: 'noxhci_port_switch' kernel parameter
On Mon, Feb 24, 2014 at 11:37:35AM -0800, Dan Williams wrote: Add a command line switch for disabling ehci port switchover. Useful for working around / debugging xhci incompatibilities where ehci operation is available. Reference: http://marc.info/?l=linux-usbm=138920063001509w=2 What about if xhci is a kernel module? -- 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: catch attempts to submit urbs with a vmalloc'd transfer buffer
On Mon, 24 Feb 2014, Dan Williams wrote: Save someone else the debug cycles of figuring out why a driver's transfer request is failing or causing undefined system behavior. Buffers submitted for dma must come from GFP allocated / DMA-able memory. Return -EAGAIN matching the return value for dma_mapping_error() cases. Cc: Alan Stern st...@rowland.harvard.edu Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Mathias Nyman mathias.ny...@linux.intel.com Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hcd.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c3250750..ff7cd489f55d 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, ret = -EAGAIN; else urb-transfer_flags |= URB_DMA_MAP_PAGE; + } else if (is_vmalloc_addr(urb-transfer_buffer)) { + WARN_ONCE(1, transfer buffer not dma capable\n); + return -EAGAIN; } else { urb-transfer_dma = dma_map_single( hcd-self.controller, You mustn't just return -EAGAIN. Set ret = -EAGAIN and fall through to the error-handling pathway. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gadgetfs: Initialize CHIP to NULL before UDC probe
Otherwise the value from the last probe would be retained that possibly is freed since (the UDC is removed) and therefore no longer relevant. Reproducible with the dummy UDC: modprobe dummy_hcd mount -t gadgetfs gadgetfs /dev/gadget umount /dev/gadget rmmod dummy_hcd mount -t gadgetfs gadgetfs /dev/gadget BUG: unable to handle kernel paging request at a066fd9d Call Trace: [811d0cd2] ? d_alloc_name+0x22/0x50 [812b74dc] ? selinux_d_instantiate+0x1c/0x20 [a067d687] gadgetfs_create_file+0x27/0xa0 [gadgetfs] [a067da70] ? setup_req.isra.4+0x80/0x80 [gadgetfs] [a067dbac] gadgetfs_fill_super+0x13c/0x180 [gadgetfs] [811bc832] mount_single+0x92/0xc0 [a067d0f8] gadgetfs_mount+0x18/0x20 [gadgetfs] [811bc8f9] mount_fs+0x39/0x1b0 [8116b220] ? __alloc_percpu+0x10/0x20 [811d6da3] vfs_kern_mount+0x63/0xf0 [811d93be] do_mount+0x23e/0xac0 [811660eb] ? strndup_user+0x4b/0xf0 [811d9f63] SyS_mount+0x83/0xc0 [81695b69] system_call_fastpath+0x16/0x1b Signed-off-by: Lubomir Rintel lkund...@v3.sk --- drivers/usb/gadget/inode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c index b94c049..ee15628 100644 --- a/drivers/usb/gadget/inode.c +++ b/drivers/usb/gadget/inode.c @@ -2046,6 +2046,7 @@ gadgetfs_fill_super (struct super_block *sb, void *opts, int silent) return -ESRCH; /* fake probe to determine $CHIP */ + CHIP = NULL; usb_gadget_probe_driver(probe_driver); if (!CHIP) return -ENODEV; -- 1.8.5.3 -- 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] xhci: 'noxhci_port_switch' kernel parameter
On Mon, Feb 24, 2014 at 12:37 PM, Greg KH gre...@linuxfoundation.org wrote: On Mon, Feb 24, 2014 at 11:37:35AM -0800, Dan Williams wrote: Add a command line switch for disabling ehci port switchover. Useful for working around / debugging xhci incompatibilities where ehci operation is available. Reference: http://marc.info/?l=linux-usbm=138920063001509w=2 What about if xhci is a kernel module? Should still be ok because drivers/usb/host/pci-quirks.c is always built-in: drivers/usb/host/Makefile:26:obj-$(CONFIG_PCI) += pci-quirks.o As far as the xhci driver is concerned the port switches are read-only. -- 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 v5 13/16] usb: force warm reset to break link re-connect livelock
Acked-by: Julius Werner jwer...@chromium.org -- 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 v5 02/16] usb: assign default peer ports for root hubs
On Fri, 21 Feb 2014, Dan Williams wrote: Assume that the peer of a superspeed port is the port with the same id on the shared_hcd root hub. This identification scheme is required of external hubs by the USB3 spec [1]. However, for root hubs, tier mismatch may be in effect [2]. Tier mismatch can only be enumerated via platform firmware. For now, simply perform the nominal association. [1]: usb 3.1 section 10.3.3 [2]: xhci 1.1 appendix D Signed-off-by: Dan Williams dan.j.willi...@intel.com +/* + * Set the default peer port for root hubs. Platform firmware will have + * already set the peer if tier-mismatch is present. Assumes the + * primary_hcd is registered first + */ +static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) The second sentence isn't accurate, at least, not as of this patch. +static void link_peers(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev; + struct device *ldev; + For safety, add if (left-peer == right right-peer == left) return; + if (left-peer) { + right = left-peer; + ldev = left-dev.parent-parent; + rdev = right-dev.parent-parent; At this point we don't know if left-peer points to anything valid. It would be better to print out the names of left and right rather than left and left-peer. For debugging, print the value of left-peer as well. + + WARN_ONCE(1, %s port%d already peered with %s %d\n, + dev_name(ldev), left-portnum, dev_name(rdev), + right-portnum); Since this isn't expected ever to happen, I think WARN is more appropriate than WARN_ONCE. Also, the gyrations you have to go through here and elsewhere to print useful names indicate that we should set up better names for the port devices. How about: dev_set_name(port_dev-dev, %s-port%d, dev_name(hub-hdev-dev), port1); + return; + } else if (right-peer) { + left = right-peer; + ldev = left-dev.parent-parent; + rdev = right-dev.parent-parent; + + WARN_ONCE(1, %s port%d already peered with %s %d\n, + dev_name(rdev), right-portnum, dev_name(ldev), + left-portnum); Similar comments. + return; + } + + get_device(right-dev); + left-peer = right; + get_device(left-dev); + right-peer = left; Add dev_dbg(left-dev, (%p) peered with %s (%p)\n, left, dev_name(right-dev), right); +} + +static void unlink_peers(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev = right-dev.parent-parent; + struct device *ldev = left-dev.parent-parent; + + WARN_ONCE(right-peer != left || left-peer != right, + %s port%d and %s port%d are not peers?\n, + dev_name(ldev), left-portnum, dev_name(rdev), right-portnum); Include left-peer and right-peer for debugging and use WARN. + Add dev_dbg(left-dev, (%p) unpeered from %s (%p)\n, left, dev_name(right-dev), right); + put_device(left-dev); + right-peer = NULL; + put_device(right-dev); + left-peer = NULL; +} @@ -190,9 +270,15 @@ exit: return retval; } -void usb_hub_remove_port_device(struct usb_hub *hub, -int port1) +void usb_hub_remove_port_device(struct usb_hub *hub, int port1) { - device_unregister(hub-ports[port1 - 1]-dev); -} + struct usb_port *port_dev = hub-ports[port1 - 1]; + struct usb_port *peer = port_dev-peer; + mutex_lock(peer_lock); + if (peer) + unlink_peers(port_dev, peer); + mutex_unlock(peer_lock); + There's a race; another thread could peer port_dev to something else right here. One possible solution: Add a flag to the port structure indicating that it is being removed, and check the peer's flag when setting the peer. + device_unregister(port_dev-dev); +} 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 v5 02/16] usb: assign default peer ports for root hubs
On Mon, Feb 24, 2014 at 1:46 PM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 21 Feb 2014, Dan Williams wrote: Assume that the peer of a superspeed port is the port with the same id on the shared_hcd root hub. This identification scheme is required of external hubs by the USB3 spec [1]. However, for root hubs, tier mismatch may be in effect [2]. Tier mismatch can only be enumerated via platform firmware. For now, simply perform the nominal association. [1]: usb 3.1 section 10.3.3 [2]: xhci 1.1 appendix D Signed-off-by: Dan Williams dan.j.willi...@intel.com +/* + * Set the default peer port for root hubs. Platform firmware will have + * already set the peer if tier-mismatch is present. Assumes the + * primary_hcd is registered first + */ +static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) The second sentence isn't accurate, at least, not as of this patch. +static void link_peers(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev; + struct device *ldev; + For safety, add if (left-peer == right right-peer == left) return; + if (left-peer) { + right = left-peer; + ldev = left-dev.parent-parent; + rdev = right-dev.parent-parent; At this point we don't know if left-peer points to anything valid. It would be better to print out the names of left and right rather than left and left-peer. For debugging, print the value of left-peer as well. + + WARN_ONCE(1, %s port%d already peered with %s %d\n, + dev_name(ldev), left-portnum, dev_name(rdev), + right-portnum); Since this isn't expected ever to happen, I think WARN is more appropriate than WARN_ONCE. Also, the gyrations you have to go through here and elsewhere to print useful names indicate that we should set up better names for the port devices. How about: dev_set_name(port_dev-dev, %s-port%d, dev_name(hub-hdev-dev), port1); I like it, however isn't this an ABI change that could mess up existing scripts? What's worse, in-kernel gymnastics to print out useful names, or adding sysfs links from the old short name to the new name? + return; + } else if (right-peer) { + left = right-peer; + ldev = left-dev.parent-parent; + rdev = right-dev.parent-parent; + + WARN_ONCE(1, %s port%d already peered with %s %d\n, + dev_name(rdev), right-portnum, dev_name(ldev), + left-portnum); Similar comments. + return; + } + + get_device(right-dev); + left-peer = right; + get_device(left-dev); + right-peer = left; Add dev_dbg(left-dev, (%p) peered with %s (%p)\n, left, dev_name(right-dev), right); +} + +static void unlink_peers(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev = right-dev.parent-parent; + struct device *ldev = left-dev.parent-parent; + + WARN_ONCE(right-peer != left || left-peer != right, + %s port%d and %s port%d are not peers?\n, + dev_name(ldev), left-portnum, dev_name(rdev), right-portnum); Include left-peer and right-peer for debugging and use WARN. + Add dev_dbg(left-dev, (%p) unpeered from %s (%p)\n, left, dev_name(right-dev), right); + put_device(left-dev); + right-peer = NULL; + put_device(right-dev); + left-peer = NULL; +} @@ -190,9 +270,15 @@ exit: return retval; } -void usb_hub_remove_port_device(struct usb_hub *hub, -int port1) +void usb_hub_remove_port_device(struct usb_hub *hub, int port1) { - device_unregister(hub-ports[port1 - 1]-dev); -} + struct usb_port *port_dev = hub-ports[port1 - 1]; + struct usb_port *peer = port_dev-peer; + mutex_lock(peer_lock); + if (peer) + unlink_peers(port_dev, peer); + mutex_unlock(peer_lock); + There's a race; another thread could peer port_dev to something else right here. One possible solution: Add a flag to the port structure indicating that it is being removed, and check the peer's flag when setting the peer. I'll take a look. Ack on all the other comments. -- 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: catch attempts to submit urbs with a vmalloc'd transfer buffer
On Mon, 24 Feb 2014, Dan Williams wrote: diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c3250750..ff7cd489f55d 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, ret = -EAGAIN; else urb-transfer_flags |= URB_DMA_MAP_PAGE; + } else if (is_vmalloc_addr(urb-transfer_buffer)) { + WARN_ONCE(1, transfer buffer not dma capable\n); + return -EAGAIN; } else { urb-transfer_dma = dma_map_single( hcd-self.controller, You mustn't just return -EAGAIN. Set ret = -EAGAIN and fall through to the error-handling pathway. Alan, I'm simply trying to trade one undefined behavior for another ;-). Good catch. Although the result would be the same in this case, it's something subtle for someone to trip over later. I don't know what you mean. Think about what would happen to the DMA mapping for the SETUP buffer in a control transfer if the data buffer was a vmalloc'ed region. Hmm, this check above also short-circuits the error fall-through: /* We don't support sg for isoc transfers ! */ if (usb_endpoint_xfer_isoc(urb-ep-desc)) { WARN_ON(1); return -EINVAL; } True, but in this case it doesn't matter because an URB for an isochronous endpoint will never get a DMA mapping for a SETUP buffer. 8 Subject: usb: catch attempts to submit urbs with a vmalloc'd transfer buffer From: Dan Williams dan.j.willi...@intel.com Save someone else the debug cycles of figuring out why a driver's transfer request is failing or causing undefined system behavior. Buffers submitted for dma must come from GFP allocated / DMA-able memory. Return -EAGAIN matching the return value for dma_mapping_error() cases. Cc: Alan Stern st...@rowland.harvard.edu Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Mathias Nyman mathias.ny...@linux.intel.com Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hcd.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c3250750..a8a8cc94b6e3 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, ret = -EAGAIN; else urb-transfer_flags |= URB_DMA_MAP_PAGE; + } else if (is_vmalloc_addr(urb-transfer_buffer)) { + WARN_ONCE(1, transfer buffer not dma capable\n); + ret = -EAGAIN; } else { urb-transfer_dma = dma_map_single( hcd-self.controller, Acked-by: Alan Stern st...@rowland.harvard.edu -- 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 v5 02/16] usb: assign default peer ports for root hubs
On Mon, 24 Feb 2014, Dan Williams wrote: Also, the gyrations you have to go through here and elsewhere to print useful names indicate that we should set up better names for the port devices. How about: dev_set_name(port_dev-dev, %s-port%d, dev_name(hub-hdev-dev), port1); I like it, however isn't this an ABI change that could mess up existing scripts? What existing scripts? Right now the only useful information is the connect_type attribute. Unless you think people are already using the power-off facility? What's worse, in-kernel gymnastics to print out useful names, or adding sysfs links from the old short name to the new name? Normally I'd agree, but this is a marginal case. What do other people think? 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 v5 02/16] usb: assign default peer ports for root hubs
On Mon, Feb 24, 2014 at 2:12 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 24 Feb 2014, Dan Williams wrote: Also, the gyrations you have to go through here and elsewhere to print useful names indicate that we should set up better names for the port devices. How about: dev_set_name(port_dev-dev, %s-port%d, dev_name(hub-hdev-dev), port1); I like it, however isn't this an ABI change that could mess up existing scripts? What existing scripts? Right now the only useful information is the connect_type attribute. Unless you think people are already using the power-off facility? Or someone is walking portX to find the child device... What's worse, in-kernel gymnastics to print out useful names, or adding sysfs links from the old short name to the new name? Normally I'd agree, but this is a marginal case. What do other people think? How about this, make the dev_name change now and we can add the links later if someone screams. -- 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] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
The Lenovo OneLink dock includes a USB ethernet adapter using the AX88179 chip, but with a different USB ID. Add this new USB id to the driver so that it will autodetect the adapter correctly. Signed-off-by: Keith Packard kei...@keithp.com Tested-by: Carl Worth cwo...@cworth.org --- drivers/net/usb/ax88179_178a.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 8e8d0fc..dcf974f 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -1418,6 +1418,19 @@ static const struct driver_info samsung_info = { .tx_fixup = ax88179_tx_fixup, }; +static const struct driver_info lenovo_info = { + .description = ThinkPad OneLink Dock USB GigaLAN, + .bind = ax88179_bind, + .unbind = ax88179_unbind, + .status = ax88179_status, + .link_reset = ax88179_link_reset, + .reset = ax88179_reset, + .stop = ax88179_stop, + .flags = FLAG_ETHER | FLAG_FRAMING_AX, + .rx_fixup = ax88179_rx_fixup, + .tx_fixup = ax88179_tx_fixup, +}; + static const struct usb_device_id products[] = { { /* ASIX AX88179 10/100/1000 */ @@ -1435,6 +1448,10 @@ static const struct usb_device_id products[] = { /* Samsung USB Ethernet Adapter */ USB_DEVICE(0x04e8, 0xa100), .driver_info = (unsigned long)samsung_info, +}, { + /* Lenovo ThinkPad OneLink GigaLAN */ + USB_DEVICE(0x17ef, 0x304b), + .driver_info = (unsigned long)samsung_info, }, { }, }; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
On Mon, Feb 24, 2014 at 03:13:49PM -0800, Keith Packard wrote: +static const struct driver_info lenovo_info = { snip +}, { + /* Lenovo ThinkPad OneLink GigaLAN */ + USB_DEVICE(0x17ef, 0x304b), + .driver_info = (unsigned long)samsung_info, lenovo_info, surely. --Kyle -- 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] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
The Lenovo OneLink dock includes a USB ethernet adapter using the AX88179 chip, but with a different USB ID. Add this new USB id to the driver so that it will autodetect the adapter correctly. Signed-off-by: Keith Packard kei...@keithp.com Tested-by: Carl Worth cwo...@cworth.org --- Kyle McMartin says: lenovo_info, surely. Yeah, Daniel Stone caught the same bug. Not a big deal; the only difference between any of these elements is the string produced by the kernel. And that's odd because when we tested it, we saw the expected name... drivers/net/usb/ax88179_178a.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 8e8d0fc..7707b4e 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -1418,6 +1418,19 @@ static const struct driver_info samsung_info = { .tx_fixup = ax88179_tx_fixup, }; +static const struct driver_info lenovo_info = { + .description = ThinkPad OneLink Dock USB GigaLAN, + .bind = ax88179_bind, + .unbind = ax88179_unbind, + .status = ax88179_status, + .link_reset = ax88179_link_reset, + .reset = ax88179_reset, + .stop = ax88179_stop, + .flags = FLAG_ETHER | FLAG_FRAMING_AX, + .rx_fixup = ax88179_rx_fixup, + .tx_fixup = ax88179_tx_fixup, +}; + static const struct usb_device_id products[] = { { /* ASIX AX88179 10/100/1000 */ @@ -1435,6 +1448,10 @@ static const struct usb_device_id products[] = { /* Samsung USB Ethernet Adapter */ USB_DEVICE(0x04e8, 0xa100), .driver_info = (unsigned long)samsung_info, +}, { + /* Lenovo ThinkPad OneLink GigaLAN */ + USB_DEVICE(0x17ef, 0x304b), + .driver_info = (unsigned long)lenovo_info, }, { }, }; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v11 09/15] usb: phy: Add set_wakeup API
On 02/24/2014 05:21 AM, Peter Chen wrote: This API is used to set wakeup enable at PHY registers, in that case, the PHY can be waken up from suspend due to external events, like vbus change, dp/dm change and id change. Signed-off-by: Peter Chen peter.c...@freescale.com --- include/linux/usb/phy.h | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index 6c0b1c5..c2c6f49 100644 --- a/include/linux/usb/phy.h +++ b/include/linux/usb/phy.h @@ -111,6 +111,13 @@ struct usb_phy { int (*set_suspend)(struct usb_phy *x, int suspend); + /* +* Set wakeup enable for PHY, in that case, the PHY can be +* waken up from suspend status due to external events, s/waken/woken/ WBR, Sergei Thanks, will change. 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: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0
On Mon, Feb 24, 2014 at 04:03:12PM +0800, Neil Zhang wrote: Hardware zlt will try to send the zero length packet automatically when the data transferd is multiple times of max packet, this will cause issues on Windows. So let's disable HW zlt by default. Would you have description that what kinds of issue on Windows if zlt is is selected? Peter Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index ebc0dfd..657ac5c 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -89,7 +89,7 @@ static void ep0_reset(struct mv_udc *udc) /* configure ep0 endpoint capabilities in dQH */ ep-dqh-max_packet_length = (EP0_MAX_PKT_SIZE EP_QUEUE_HEAD_MAX_PKT_LEN_POS) - | EP_QUEUE_HEAD_IOS; + | EP_QUEUE_HEAD_IOS | EP_QUEUE_HEAD_ZLT_SEL; ep-dqh-next_dtd_ptr = EP_QUEUE_HEAD_NEXT_TERMINATE; -- 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 -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/16] usb: assign default peer ports for root hubs
On Mon, 2014-02-24 at 16:46 -0500, Alan Stern wrote: On Fri, 21 Feb 2014, Dan Williams wrote: Assume that the peer of a superspeed port is the port with the same id on the shared_hcd root hub. This identification scheme is required of external hubs by the USB3 spec [1]. However, for root hubs, tier mismatch may be in effect [2]. Tier mismatch can only be enumerated via platform firmware. For now, simply perform the nominal association. [1]: usb 3.1 section 10.3.3 [2]: xhci 1.1 appendix D Signed-off-by: Dan Williams dan.j.willi...@intel.com +/* + * Set the default peer port for root hubs. Platform firmware will have + * already set the peer if tier-mismatch is present. Assumes the + * primary_hcd is registered first + */ +static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) The second sentence isn't accurate, at least, not as of this patch. Moved it to patch 4 usb: find internal hub tier mismatch via acpi +static void link_peers(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev; + struct device *ldev; + For safety, add if (left-peer == right right-peer == left) return; Done. + if (left-peer) { + right = left-peer; + ldev = left-dev.parent-parent; + rdev = right-dev.parent-parent; At this point we don't know if left-peer points to anything valid. It would be better to print out the names of left and right rather than left and left-peer. For debugging, print the value of left-peer as well. + + WARN_ONCE(1, %s port%d already peered with %s %d\n, + dev_name(ldev), left-portnum, dev_name(rdev), + right-portnum); Since this isn't expected ever to happen, I think WARN is more appropriate than WARN_ONCE. Also, the gyrations you have to go through here and elsewhere to print useful names indicate that we should set up better names for the port devices. How about: dev_set_name(port_dev-dev, %s-port%d, dev_name(hub-hdev-dev), port1); + return; + } else if (right-peer) { + left = right-peer; + ldev = left-dev.parent-parent; + rdev = right-dev.parent-parent; + + WARN_ONCE(1, %s port%d already peered with %s %d\n, + dev_name(rdev), right-portnum, dev_name(ldev), + left-portnum); Similar comments. Replaced with: + if (left-peer || right-peer) { + struct usb_port *lpeer = left-peer; + struct usb_port *rpeer = right-peer; + + WARN(1, failed to peer %s and %s (%s - %s) (%s - %s)\n, + dev_name(left-dev), dev_name(right-dev), + dev_name(left-dev), + lpeer ? dev_name(lpeer-dev) : [none], + dev_name(right-dev), + rpeer ? dev_name(rpeer-dev) : [none]); + return; + } + return; + } + + get_device(right-dev); + left-peer = right; + get_device(left-dev); + right-peer = left; Add dev_dbg(left-dev, (%p) peered with %s (%p)\n, left, dev_name(right-dev), right); Deferred this to patch 5 usb: sysfs link peer ports which adds a general facility for reporting the result of link_peers. +} + +static void unlink_peers(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev = right-dev.parent-parent; + struct device *ldev = left-dev.parent-parent; + + WARN_ONCE(right-peer != left || left-peer != right, + %s port%d and %s port%d are not peers?\n, + dev_name(ldev), left-portnum, dev_name(rdev), right-portnum); Include left-peer and right-peer for debugging and use WARN. Done. + Add dev_dbg(left-dev, (%p) unpeered from %s (%p)\n, left, dev_name(right-dev), right); Done. + put_device(left-dev); + right-peer = NULL; + put_device(right-dev); + left-peer = NULL; +} @@ -190,9 +270,15 @@ exit: return retval; } -void usb_hub_remove_port_device(struct usb_hub *hub, - int port1) +void usb_hub_remove_port_device(struct usb_hub *hub, int port1) { - device_unregister(hub-ports[port1 - 1]-dev); -} + struct usb_port *port_dev = hub-ports[port1 - 1]; + struct usb_port *peer = port_dev-peer; + mutex_lock(peer_lock); + if (peer) + unlink_peers(port_dev, peer); + mutex_unlock(peer_lock); + There's a race; another thread could peer port_dev to something else right here. One possible solution: Add a flag to the port structure indicating that it is being removed, and check the peer's flag when setting the peer. I think it's enough to just de-reference port_dev-peer under the lock. We
Re: [PATCH 3/6] usb: gadget: mv_udc: clear corresponding interrupt when flush fifo
On Mon, Feb 24, 2014 at 04:03:13PM +0800, Neil Zhang wrote: The interruts are useless when this endpoint is going to be flushed. Especially in the enumeration phase, let's take get device description for example. 1. Device doesn't ACK the status package from host in time due to irq is disabled by some module. 2. Host find no ACK in time, and send another request. Why device does not send NAK at that time? Besides, you can try to prime status at data stage, it can also avoid the problem usb 2.0 8.5.3.3 Error Handling on the Last Data Transaction described. 3. Device gets the chance to handle the interrupt, the sequence is as following. a. Flush pending requests in ep0. b. Queue a request in ep0 according to host's request and change the ep0 state to DATA_STATE_XMIT. c. Handle the pending interrupt for the last status package. But actually it will check the new added request instead of the status package and because of wront ep0 state, device will request an %s/wront/wrong Peter unexpected status package from host. d. The ep0 state is going mad. The solution is that we need clear the pending corresponding interrupt as well as flush endpoint. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index 657ac5c..d5a9bdf 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -692,6 +692,8 @@ static void mv_ep_fifo_flush(struct usb_ep *_ep) } loops--; } while (readl(udc-op_regs-epstatus) bit_pos); + + writel(bit_pos, udc-op_regs-epcomplete); } /* queues (submits) an I/O request to an endpoint */ -- 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 -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 05/16] usb: sysfs link peer ports
On Fri, 2014-02-21 at 16:09 -0800, Dan Williams wrote: The usb topology after this change will have symlinks between usb3 ports and their usb2 peers, for example: usb2/2-1/2-1:1.0/port1/peer = ../../../../usb3/3-1/3-1:1.0/port1 usb2/2-1/2-1:1.0/port2/peer = ../../../../usb3/3-1/3-1:1.0/port2 usb2/2-1/2-1:1.0/port3/peer = ../../../../usb3/3-1/3-1:1.0/port3 usb2/2-1/2-1:1.0/port4/peer = ../../../../usb3/3-1/3-1:1.0/port4 usb2/2-0:1.0/port1/peer = ../../../usb3/3-0:1.0/port1 usb2/2-0:1.0/port2/peer = ../../../usb3/3-0:1.0/port2 usb2/2-0:1.0/port3/peer = ../../../usb3/3-0:1.0/port3 usb2/2-0:1.0/port4/peer = ../../../usb3/3-0:1.0/port4 usb3/3-1/3-1:1.0/port1/peer = ../../../../usb2/2-1/2-1:1.0/port1 usb3/3-1/3-1:1.0/port2/peer = ../../../../usb2/2-1/2-1:1.0/port2 usb3/3-1/3-1:1.0/port3/peer = ../../../../usb2/2-1/2-1:1.0/port3 usb3/3-1/3-1:1.0/port4/peer = ../../../../usb2/2-1/2-1:1.0/port4 usb3/3-0:1.0/port1/peer = ../../../usb2/2-0:1.0/port1 usb3/3-0:1.0/port2/peer = ../../../usb2/2-0:1.0/port2 usb3/3-0:1.0/port3/peer = ../../../usb2/2-0:1.0/port3 usb3/3-0:1.0/port4/peer = ../../../usb2/2-0:1.0/port4 Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/port.c | 45 +++-- 1 files changed, 39 insertions(+), 6 deletions(-) diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 0b8ae73b0466..86e78bbd2e4e 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -196,10 +196,11 @@ static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) return peer; } -static void link_peers(struct usb_port *left, struct usb_port *right) +static int link_peers(struct usb_port *left, struct usb_port *right) { struct device *rdev; struct device *ldev; + int rc; if (left-peer) { right = left-peer; @@ -209,7 +210,7 @@ static void link_peers(struct usb_port *left, struct usb_port *right) WARN_ONCE(1, %s port%d already peered with %s %d\n, dev_name(ldev), left-portnum, dev_name(rdev), right-portnum); - return; + return -EBUSY; } else if (right-peer) { left = right-peer; ldev = left-dev.parent-parent; @@ -218,13 +219,43 @@ static void link_peers(struct usb_port *left, struct usb_port *right) WARN_ONCE(1, %s port%d already peered with %s %d\n, dev_name(rdev), right-portnum, dev_name(ldev), left-portnum); - return; + return -EBUSY; + } + + rc = sysfs_create_link(left-dev.kobj, right-dev.kobj, peer); + if (rc) + return rc; + rc = sysfs_create_link(right-dev.kobj, left-dev.kobj, peer); + if (rc) { + sysfs_remove_link(left-dev.kobj, peer); + return rc; } get_device(right-dev); left-peer = right; get_device(left-dev); right-peer = left; + + return 0; +} + +static void link_peers_report(struct usb_port *left, struct usb_port *right) +{ + struct device *rdev = right-dev.parent-parent; + struct device *ldev = left-dev.parent-parent; + int rc; + + rc = link_peers(left, right); + if (rc == 0) { + pr_debug(usb: link %s port%d to %s port%d\n, + dev_name(ldev), left-portnum, + dev_name(rdev), right-portnum); + } else { + pr_warn(usb: failed to link %s port%d to %s port%d (%d)\n, + dev_name(ldev), left-portnum, + dev_name(rdev), right-portnum, rc); + pr_warn_once(usb: port power management may be unreliable\n); + } } Fix this up to use dev_name(port_dev-dev) now that the name is no longer ambiguous. 8 Subject: usb: sysfs link peer ports From: Dan Williams dan.j.willi...@intel.com The usb topology after this change will have symlinks between usb3 ports and their usb2 peers, for example: usb2/2-1/2-1:1.0/port1/peer = ../../../../usb3/3-1/3-1:1.0/port1 usb2/2-1/2-1:1.0/port2/peer = ../../../../usb3/3-1/3-1:1.0/port2 usb2/2-1/2-1:1.0/port3/peer = ../../../../usb3/3-1/3-1:1.0/port3 usb2/2-1/2-1:1.0/port4/peer = ../../../../usb3/3-1/3-1:1.0/port4 usb2/2-0:1.0/port1/peer = ../../../usb3/3-0:1.0/port1 usb2/2-0:1.0/port2/peer = ../../../usb3/3-0:1.0/port2 usb2/2-0:1.0/port3/peer = ../../../usb3/3-0:1.0/port3 usb2/2-0:1.0/port4/peer = ../../../usb3/3-0:1.0/port4 usb3/3-1/3-1:1.0/port1/peer = ../../../../usb2/2-1/2-1:1.0/port1 usb3/3-1/3-1:1.0/port2/peer = ../../../../usb2/2-1/2-1:1.0/port2 usb3/3-1/3-1:1.0/port3/peer = ../../../../usb2/2-1/2-1:1.0/port3 usb3/3-1/3-1:1.0/port4/peer = ../../../../usb2/2-1/2-1:1.0/port4 usb3/3-0:1.0/port1/peer = ../../../usb2/2-0:1.0/port1
Re: [PATCH v5 06/16] usb: defer suspension of superspeed port while peer is powered
On Fri, 2014-02-21 at 16:09 -0800, Dan Williams wrote: ClearPortFeature(PORT_POWER) on a usb3 port places the port in either a DSPORT.Powered-off-detect / DSPORT.Powered-off-reset loop, or the DSPORT.Powered-off state. There is no way to ensure that RX terminations will persist in this state, so it is possible a device will degrade to its usb2 connection. Prevent this by blocking power-off of a usb3 port while its usb2 peer is active, and powering on a usb3 port before its usb2 peer. Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hub.c |5 - drivers/usb/core/hub.h |5 + drivers/usb/core/port.c | 50 +++ 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4e967f613e70..479abbe0ba09 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -36,11 +36,6 @@ #define USB_VENDOR_GENESYS_LOGIC 0x05e3 #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01 -static inline int hub_is_superspeed(struct usb_device *hdev) -{ - return (hdev-descriptor.bDeviceProtocol == USB_HUB_PR_SS); -} - /* Protect struct usb_device-state and -children members * Note: Both are also protected by -dev.sem, except that -state can * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index e7a9666e7cd6..87efea1424d3 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -129,6 +129,11 @@ static inline bool hub_is_port_power_switchable(struct usb_hub *hub) return (le16_to_cpu(hcs) HUB_CHAR_LPSM) HUB_CHAR_NO_LPSM; } +static inline int hub_is_superspeed(struct usb_device *hdev) +{ + return hdev-descriptor.bDeviceProtocol == USB_HUB_PR_SS; +} + static inline int hub_port_debounce_be_connected(struct usb_hub *hub, int port1) { diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 86e78bbd2e4e..1e38f123ed8c 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -77,12 +77,20 @@ static int usb_port_runtime_resume(struct device *dev) struct usb_device *hdev = to_usb_device(dev-parent-parent); struct usb_interface *intf = to_usb_interface(dev-parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_port *peer = port_dev-peer; int port1 = port_dev-portnum; int retval; if (!hub) return -EINVAL; + /* + * Power on our usb3 peer before this usb2 port to prevent a usb3 + * device from degrading to its usb2 connection + */ + if (!hub_is_superspeed(hdev) peer) + pm_runtime_get_sync(peer-dev); + usb_autopm_get_interface(intf); set_bit(port1, hub-busy_bits); @@ -104,6 +112,7 @@ static int usb_port_runtime_resume(struct device *dev) clear_bit(port1, hub-busy_bits); usb_autopm_put_interface(intf); + return retval; } @@ -113,6 +122,7 @@ static int usb_port_runtime_suspend(struct device *dev) struct usb_device *hdev = to_usb_device(dev-parent-parent); struct usb_interface *intf = to_usb_interface(dev-parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_port *peer = port_dev-peer; int port1 = port_dev-portnum; int retval; @@ -130,6 +140,15 @@ static int usb_port_runtime_suspend(struct device *dev) usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); clear_bit(port1, hub-busy_bits); usb_autopm_put_interface(intf); + + /* + * Our peer usb3 port may now be able to suspend, asynchronously + * queue a suspend request to observe that this usb2 peer port + * is now off. + */ + if (!hub_is_superspeed(hdev) peer) + pm_runtime_put(peer-dev); + return retval; } #endif @@ -198,13 +217,12 @@ static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) static int link_peers(struct usb_port *left, struct usb_port *right) { - struct device *rdev; - struct device *ldev; + struct device *ldev = left-dev.parent-parent; + struct device *rdev = right-dev.parent-parent; int rc; if (left-peer) { right = left-peer; - ldev = left-dev.parent-parent; rdev = right-dev.parent-parent; WARN_ONCE(1, %s port%d already peered with %s %d\n, @@ -214,7 +232,6 @@ static int link_peers(struct usb_port *left, struct usb_port *right) } else if (right-peer) { left = right-peer; ldev = left-dev.parent-parent; - rdev = right-dev.parent-parent; WARN_ONCE(1, %s port%d already peered with %s %d\n, dev_name(rdev), right-portnum, dev_name(ldev), @@ -236,6 +253,19 @@ static int
Re: [PATCH v5 11/16] usb: introduce port status lock
On Fri, 2014-02-21 at 16:10 -0800, Dan Williams wrote: In general we do not want khubd to act on port status changes that are the result of in progress resets or USB runtime PM operations. Specifically port power control testing has been able to trigger an unintended disconnect in hub_port_connect_change(), paraphrasing: if ((portstatus USB_PORT_STAT_CONNECTION) udev udev-state != USB_STATE_NOTATTACHED) { if (portstatus USB_PORT_STAT_ENABLE) { /* Nothing to do */ } else if (udev-state == USB_STATE_SUSPENDED udev-persist_enabled) { ... } else { /* Don't resuscitate */; } } ...by falling to the Don't resuscitate path or missing USB_PORT_STAT_CONNECTION because usb_port_resume() was in the middle of modifying the port status. So, we want a new lock to hold off khubd for a given port while the child device is being suspended, resumed, or reset. The lock ordering rules are now usb_lock_device() = usb_lock_port(). This is mandated by the device core which may hold the device_lock on the usb_device before invoking usb_port_{suspend|resume} which in turn take the status_lock on the usb_port. We attempt to hold the status_lock for the duration of a port_event() run, and drop/re-acquire it when needing to take the device_lock. The lock is also dropped/re-acquired during hub_port_reconnect(). This patch also deletes hub-busy_bits as all use cases are now covered by port PM runtime synchronization or the port-status_lock. Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hcd.c |2 - drivers/usb/core/hub.c | 102 +-- drivers/usb/core/hub.h |4 +- drivers/usb/core/port.c |7 +-- drivers/usb/core/usb.h |6 +++ 5 files changed, 82 insertions(+), 39 deletions(-) The dev_set_name() update in patch 2 collided with the addition of -status_lock. Simply rebase to resolve the collision, no other changes are made. 8--- Subject: usb: introduce port status lock From: Dan Williams dan.j.willi...@intel.com In general we do not want khubd to act on port status changes that are the result of in progress resets or USB runtime PM operations. Specifically port power control testing has been able to trigger an unintended disconnect in hub_port_connect_change(), paraphrasing: if ((portstatus USB_PORT_STAT_CONNECTION) udev udev-state != USB_STATE_NOTATTACHED) { if (portstatus USB_PORT_STAT_ENABLE) { /* Nothing to do */ } else if (udev-state == USB_STATE_SUSPENDED udev-persist_enabled) { ... } else { /* Don't resuscitate */; } } ...by falling to the Don't resuscitate path or missing USB_PORT_STAT_CONNECTION because usb_port_resume() was in the middle of modifying the port status. So, we want a new lock to hold off khubd for a given port while the child device is being suspended, resumed, or reset. The lock ordering rules are now usb_lock_device() = usb_lock_port(). This is mandated by the device core which may hold the device_lock on the usb_device before invoking usb_port_{suspend|resume} which in turn take the status_lock on the usb_port. We attempt to hold the status_lock for the duration of a port_event() run, and drop/re-acquire it when needing to take the device_lock. The lock is also dropped/re-acquired during hub_port_reconnect(). This patch also deletes hub-busy_bits as all use cases are now covered by port PM runtime synchronization or the port-status_lock. Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hcd.c |2 - drivers/usb/core/hub.c | 102 +-- drivers/usb/core/hub.h |4 +- drivers/usb/core/port.c |7 +-- drivers/usb/core/usb.h |6 +++ 5 files changed, 82 insertions(+), 39 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c3250750..b3ecaf32f2aa 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2246,9 +2246,7 @@ static void hcd_resume_work(struct work_struct *work) struct usb_hcd *hcd = container_of(work, struct usb_hcd, wakeup_work); struct usb_device *udev = hcd-self.root_hub; - usb_lock_device(udev); usb_remote_wakeup(udev); - usb_unlock_device(udev); } /** diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index bb6ebde85192..5b4a5e7b3762 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2737,6 +2737,20 @@ static int port_is_power_on(struct usb_hub *hub, unsigned portstatus) return ret; } +static void usb_lock_port(struct usb_port *port_dev) +
Re: [PATCH v5 06/16] usb: defer suspension of superspeed port while peer is powered
On Mon, 2014-02-24 at 19:01 -0800, Dan Williams wrote: As mentioned in the comments on patch 2, while -peer is being modified we don't want usb_port_runtime_{suspend|resume} to run. Introduce pre_modify_peers() and post_modify_peers() to close that hole. ...thinking about it further, when we unlink the peers it might be the final put on the device, so {pre|post}_modify_peers() need to take and drop a device reference. 8- Subject: usb: defer suspension of superspeed port while peer is powered From: Dan Williams dan.j.willi...@intel.com ClearPortFeature(PORT_POWER) on a usb3 port places the port in either a DSPORT.Powered-off-detect / DSPORT.Powered-off-reset loop, or the DSPORT.Powered-off state. There is no way to ensure that RX terminations will persist in this state, so it is possible a device will degrade to its usb2 connection. Prevent this by blocking power-off of a usb3 port while its usb2 peer is active, and powering on a usb3 port before its usb2 peer. Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/usb/core/hub.c |5 --- drivers/usb/core/hub.h |5 +++ drivers/usb/core/port.c | 76 +++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4e967f613e70..479abbe0ba09 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -36,11 +36,6 @@ #define USB_VENDOR_GENESYS_LOGIC 0x05e3 #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01 -static inline int hub_is_superspeed(struct usb_device *hdev) -{ - return (hdev-descriptor.bDeviceProtocol == USB_HUB_PR_SS); -} - /* Protect struct usb_device-state and -children members * Note: Both are also protected by -dev.sem, except that -state can * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index e7a9666e7cd6..87efea1424d3 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -129,6 +129,11 @@ static inline bool hub_is_port_power_switchable(struct usb_hub *hub) return (le16_to_cpu(hcs) HUB_CHAR_LPSM) HUB_CHAR_NO_LPSM; } +static inline int hub_is_superspeed(struct usb_device *hdev) +{ + return hdev-descriptor.bDeviceProtocol == USB_HUB_PR_SS; +} + static inline int hub_port_debounce_be_connected(struct usb_hub *hub, int port1) { diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 2a560cd64758..cd1937fc8763 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -77,12 +77,20 @@ static int usb_port_runtime_resume(struct device *dev) struct usb_device *hdev = to_usb_device(dev-parent-parent); struct usb_interface *intf = to_usb_interface(dev-parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_port *peer = port_dev-peer; int port1 = port_dev-portnum; int retval; if (!hub) return -EINVAL; + /* +* Power on our usb3 peer before this usb2 port to prevent a usb3 +* device from degrading to its usb2 connection +*/ + if (!hub_is_superspeed(hdev) peer) + pm_runtime_get_sync(peer-dev); + usb_autopm_get_interface(intf); set_bit(port1, hub-busy_bits); @@ -104,6 +112,7 @@ static int usb_port_runtime_resume(struct device *dev) clear_bit(port1, hub-busy_bits); usb_autopm_put_interface(intf); + return retval; } @@ -113,6 +122,7 @@ static int usb_port_runtime_suspend(struct device *dev) struct usb_device *hdev = to_usb_device(dev-parent-parent); struct usb_interface *intf = to_usb_interface(dev-parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_port *peer = port_dev-peer; int port1 = port_dev-portnum; int retval; @@ -130,6 +140,15 @@ static int usb_port_runtime_suspend(struct device *dev) usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); clear_bit(port1, hub-busy_bits); usb_autopm_put_interface(intf); + + /* +* Our peer usb3 port may now be able to suspend, asynchronously +* queue a suspend request to observe that this usb2 peer port +* is now off. +*/ + if (!hub_is_superspeed(hdev) peer) + pm_runtime_put(peer-dev); + return retval; } #endif @@ -196,8 +215,30 @@ static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) return peer; } +/* + * Modifying -peer affects usb_port_runtime_{suspend|resume} so make + * sure devices are active before the change and re-evaluate + * afterwards + */ +static void pre_modify_peers(struct usb_port *left, struct usb_port *right) +{ + get_device(left-dev); + get_device(right-dev); + pm_runtime_get_sync(left-dev); + pm_runtime_get_sync(right-dev); +} + +static void
RE: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0
-Original Message- From: Peter Chen [mailto:peter.c...@freescale.com] Sent: 2014年2月25日 9:19 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0 On Mon, Feb 24, 2014 at 04:03:12PM +0800, Neil Zhang wrote: Hardware zlt will try to send the zero length packet automatically when the data transferd is multiple times of max packet, this will cause issues on Windows. So let's disable HW zlt by default. Would you have description that what kinds of issue on Windows if zlt is is selected? Enumeration will fail. Peter Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index ebc0dfd..657ac5c 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -89,7 +89,7 @@ static void ep0_reset(struct mv_udc *udc) /* configure ep0 endpoint capabilities in dQH */ ep-dqh-max_packet_length = (EP0_MAX_PKT_SIZE EP_QUEUE_HEAD_MAX_PKT_LEN_POS) - | EP_QUEUE_HEAD_IOS; + | EP_QUEUE_HEAD_IOS | EP_QUEUE_HEAD_ZLT_SEL; ep-dqh-next_dtd_ptr = EP_QUEUE_HEAD_NEXT_TERMINATE; -- 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 -- Best Regards, Peter Chen Best Regards, Neil Zhang N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��Ⅷ�G���h�(�茛j���m赇z罐��帼f"�h���~�m�
Re: [PATCH v5 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode
On 12/24/2013 3:00 PM, Manu Gautam wrote: Allow userspace to pass SuperSpeed descriptors and handle them in the driver accordingly. This change doesn't modify existing desc_header and thereby keeps the ABI changes backward compatible i.e. existing userspace drivers compiled with old header (functionfs.h) would continue to work with the updated kernel. Signed-off-by: Manu Gautam mgau...@codeaurora.org Acked-by: Michal Nazarewicz min...@mina86.com --- Hi Felipe, Is this patch good now to be added to your queue (if not already in)? It applied cleanly on your next branch. Thanks, Manu -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] bug fix for mv_udc_core.c
On Mon, Feb 24, 2014 at 04:42:40AM -0800, Neil Zhang wrote: -Original Message- From: Matthieu CASTET [mailto:matthieu.cas...@parrot.com] Sent: 2014年2月24日 18:32 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 0/6] bug fix for mv_udc_core.c Le Mon, 24 Feb 2014 16:03:10 +0800, Neil Zhang zhan...@marvell.com a écrit : This patch set is mainly for bug fix. Neil Zhang (6): usb: gadget: mv_udc: remove redundant pull up in udc_start usb: gadget: mv_udc: disable HW zlt for ep0 usb: gadget: mv_udc: clear corresponding interrupt when flush fifo usb: gadget: mv_udc: check endpoint before queue dtd USB: gadget: mv_udc: workaroud for missing dTD USB: gadget: mv_udc: fix corner case for missiong dTD drivers/usb/gadget/mv_udc_core.c | 47 ++ 1 file changed, 42 insertions(+), 5 deletions(-) Do you have plan to move to the chipidea udc driver ? You could have some bug fix for free or share your bug fix with others. Not yet. Will estimate it later. Freescale uses mv udc driver at u-boot, so the chipidea IP should be compatible for two SoCs , both Intel and msm are using chipidea driver at linux kernel, it should be not difficult for marvell to use it. For this patchset, some of them may already be fixed at chipidea driver (3/6, 4/6), some of them may not (5/6, 6/6). -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] USB: gadget: mv_udc: workaroud for missing dTD
On Mon, Feb 24, 2014 at 04:03:15PM +0800, Neil Zhang wrote: There is an issue with the add dTD tripwire semaphore (ATDTW bit in USBCMD register) that can cause the controller to ignore a dTD that is added to a primed endpoint. When this happens, the software can read the tripwire bit and the status bit at '1' even though the endpoint is unprimed. After executing a dTD, the device controller endpoint state machine executes a final read of the dTD terminate bit to check if the application added a dTD to the linked list at the last moment. This read is done in the finpkt_read_latest_next_td (44) state. After the read is performed, if the terminate bit is still set, the state machine moves to unprime the endpoint. The decision to unprime the endpoint is done in the checkqh_decision (59) state, based on the value of the terminate bit. Before reaching the checkqh_decision state, the state machine traverses the writeqhtd_status (57), writeqh_status (56), and release_prime_mask (42) states. As shown in the waveform, the ep_addtd_tripwire_clr signal is not set to clear the tripwire bit in these states. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index a620cff..8df8606 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -196,7 +196,27 @@ static int process_ep_req(struct mv_udc *udc, int index, while (readl(udc-op_regs-epstatus) bit_pos) udelay(1); break; + } else { + if (!(readl(udc-op_regs-epstatus) bit_pos)) { + /* The DMA engine thinks there is no more dTD */ + curr_dqh-next_dtd_ptr = curr_dtd-dtd_next + EP_QUEUE_HEAD_NEXT_POINTER_MASK; + + /* clear active and halt bit */ + curr_dqh-size_ioc_int_sts = + ~(DTD_STATUS_ACTIVE + | DTD_STATUS_HALTED); + + /* Do prime again */ + wmb(); + writel(bit_pos, udc-op_regs-epprime); + while (readl(udc-op_regs-epprime) bit_pos) + cpu_relax(); + + break; + } } + udelay(1); } Is it a chipidea IP issue? Any errate number for this issue? Does we need it for chipidea driver? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0
On Mon, Feb 24, 2014 at 04:03:12PM +0800, Neil Zhang wrote: Hardware zlt will try to send the zero length packet automatically when the data transferd is multiple times of max packet, this will cause issues on Windows. So let's disable HW zlt by default. Would you have description that what kinds of issue on Windows if zlt is is selected? Enumeration will fail. What causes enumeration fail, why it does not occur before? 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: [Patch V1 2/2] usbtest: Add interrupt EP testcases
ccing: Felipe Balbi, Alan Stern On 2/24/2014 3:55 PM, Amit VIRDI wrote: Two simple test cases for interrupt endpoints are added to the usbtest.c file. These are simple non-queued interrupt IN and interrupt OUT transfers. Currently, only gadget zero is capable of executing the interrupt EP test cases. However, extending the same to other gadgets is extremely simple and can be done on-demand. This code has been tested only with gadget zero and care has been taken so as to not break the existing functionality. However, if anyone can test with other gadgets then that would be great! Signed-off-by: Amit Virdi amit.vi...@st.com --- drivers/usb/misc/usbtest.c | 112 +++-- 1 file changed, 97 insertions(+), 15 deletions(-) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index f6568b5..8b96235 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -54,6 +54,7 @@ struct usbtest_info { unsignedautoconf:1; unsignedctrl_out:1; unsignediso:1; /* try iso in/out */ + unsignedintr:1; /* try interrupt in/out */ int alt; }; @@ -70,7 +71,10 @@ struct usbtest_dev { int out_pipe; int in_iso_pipe; int out_iso_pipe; + int in_int_pipe; + int out_int_pipe; struct usb_endpoint_descriptor *iso_in, *iso_out; + struct usb_endpoint_descriptor *int_in, *int_out; struct mutexlock; #define TBUF_SIZE 256 @@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) struct usb_host_interface *alt; struct usb_host_endpoint*in, *out; struct usb_host_endpoint*iso_in, *iso_out; + struct usb_host_endpoint*int_in, *int_out; struct usb_device *udev; for (tmp = 0; tmp intf-num_altsetting; tmp++) { @@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) in = out = NULL; iso_in = iso_out = NULL; + int_in = int_out = NULL; alt = intf-altsetting + tmp; if (override_alt = 0 @@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) switch (usb_endpoint_type(e-desc)) { case USB_ENDPOINT_XFER_BULK: break; + case USB_ENDPOINT_XFER_INT: + if (dev-info-intr) + goto try_intr; case USB_ENDPOINT_XFER_ISOC: if (dev-info-iso) goto try_iso; @@ -139,6 +148,14 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) out = e; } continue; +try_intr: + if (usb_endpoint_dir_in(e-desc)) { + if (!int_in) + int_in = e; + } else { + if (!int_out) + int_out = e; + } try_iso: if (usb_endpoint_dir_in(e-desc)) { if (!iso_in) @@ -148,7 +165,7 @@ try_iso: iso_out = e; } } - if ((in out) || iso_in || iso_out) + if ((in out) || iso_in || iso_out || int_in || int_out) goto found; } return -EINVAL; @@ -183,6 +200,20 @@ found: iso_out-desc.bEndpointAddress USB_ENDPOINT_NUMBER_MASK); } + + if (int_in) { + dev-int_in = int_in-desc; + dev-in_int_pipe = usb_rcvintpipe(udev, + int_in-desc.bEndpointAddress +USB_ENDPOINT_NUMBER_MASK); + } + + if (int_out) { + dev-int_out = int_out-desc; + dev-out_int_pipe = usb_sndintpipe(udev, + int_out-desc.bEndpointAddress +USB_ENDPOINT_NUMBER_MASK); + } return 0; } @@ -205,14 +236,22 @@ static struct urb *usbtest_alloc_urb( int pipe, unsigned long bytes, unsignedtransfer_flags, - unsignedoffset) + unsignedoffset, + u8 bInterval) { struct urb *urb; urb =
Re: [Patch V1 1/2] usb: gadget: zero: Add support for interrupt EP
ccing: Felipe Balbi, Alen Stern On 2/24/2014 3:55 PM, Amit VIRDI wrote: Interrupt endpoints behave quite similar to the bulk endpoints with the difference that the endpoints expect data sending/reception request at particular intervals till the whole data has not been transmitted. The interrupt EP support is added to gadget zero. A new alternate setting (=2) has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The default parameters are set as: bInterval: 4 wMaxPacketSize: 1024 However, the same can be overridden through the module parameter interface. The code is tested for HS and SS on a platform having DWC3 controller. Signed-off-by: Amit Virdi amit.vi...@st.com --- drivers/usb/gadget/f_loopback.c | 3 +- drivers/usb/gadget/f_sourcesink.c | 519 -- drivers/usb/gadget/g_zero.h | 13 +- drivers/usb/gadget/zero.c | 21 ++ 4 files changed, 533 insertions(+), 23 deletions(-) diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c index 4557cd0..bf04389 100644 --- a/drivers/usb/gadget/f_loopback.c +++ b/drivers/usb/gadget/f_loopback.c @@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop) struct usb_composite_dev*cdev; cdev = loop-function.config-cdev; - disable_endpoints(cdev, loop-in_ep, loop-out_ep, NULL, NULL); + disable_endpoints(cdev, loop-in_ep, loop-out_ep, NULL, NULL, NULL, + NULL); VDBG(cdev, %s disabled\n, loop-function.name); } diff --git a/drivers/usb/gadget/f_sourcesink.c b/drivers/usb/gadget/f_sourcesink.c index d3cd52d..306de39 100644 --- a/drivers/usb/gadget/f_sourcesink.c +++ b/drivers/usb/gadget/f_sourcesink.c @@ -23,6 +23,13 @@ #include gadget_chips.h #include u_f.h +enum eptype { + EP_CONTROL = 0, + EP_BULK, + EP_ISOC, + EP_INTERRUPT, +}; + /* * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral * controller drivers. @@ -55,6 +62,8 @@ struct f_sourcesink { struct usb_ep *out_ep; struct usb_ep *iso_in_ep; struct usb_ep *iso_out_ep; + struct usb_ep *int_in_ep; + struct usb_ep *int_out_ep; int cur_alt; }; @@ -68,6 +77,10 @@ static unsigned isoc_interval; static unsigned isoc_maxpacket; static unsigned isoc_mult; static unsigned isoc_maxburst; +static unsigned int_interval; +static unsigned int_maxpacket; +static unsigned int_mult; +static unsigned int_maxburst; static unsigned buflen; /*-*/ @@ -92,6 +105,16 @@ static struct usb_interface_descriptor source_sink_intf_alt1 = { /* .iInterface = DYNAMIC */ }; +static struct usb_interface_descriptor source_sink_intf_alt2 = { + .bLength = USB_DT_INTERFACE_SIZE, + .bDescriptorType = USB_DT_INTERFACE, + + .bAlternateSetting =2, + .bNumEndpoints =6, + .bInterfaceClass = USB_CLASS_VENDOR_SPEC, + /* .iInterface = DYNAMIC */ +}; + /* full speed support: */ static struct usb_endpoint_descriptor fs_source_desc = { @@ -130,6 +153,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = { .bInterval =4, }; +static struct usb_endpoint_descriptor fs_int_source_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + + .bEndpointAddress = USB_DIR_IN, + .bmAttributes = USB_ENDPOINT_XFER_INT, + .wMaxPacketSize = cpu_to_le16(1023), + .bInterval =4, +}; + +static struct usb_endpoint_descriptor fs_int_sink_desc = { + .bLength = USB_DT_ENDPOINT_SIZE, + .bDescriptorType = USB_DT_ENDPOINT, + + .bEndpointAddress = USB_DIR_OUT, + .bmAttributes = USB_ENDPOINT_XFER_INT, + .wMaxPacketSize = cpu_to_le16(1023), + .bInterval =4, +}; + static struct usb_descriptor_header *fs_source_sink_descs[] = { (struct usb_descriptor_header *) source_sink_intf_alt0, (struct usb_descriptor_header *) fs_sink_desc, @@ -140,6 +183,14 @@ static struct usb_descriptor_header *fs_source_sink_descs[] = { (struct usb_descriptor_header *) fs_source_desc, (struct usb_descriptor_header *) fs_iso_sink_desc, (struct usb_descriptor_header *) fs_iso_source_desc, + (struct usb_descriptor_header *) source_sink_intf_alt2, +#define FS_ALT_IFC_2_OFFSET8 + (struct usb_descriptor_header *) fs_sink_desc, + (struct usb_descriptor_header *) fs_source_desc, + (struct usb_descriptor_header *) fs_iso_sink_desc, + (struct usb_descriptor_header *) fs_iso_source_desc, + (struct usb_descriptor_header *) fs_int_sink_desc, + (struct
RE: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0
-Original Message- From: Peter Chen [mailto:peter.c...@freescale.com] Sent: 2014年2月25日 13:15 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: RE: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0 On Mon, Feb 24, 2014 at 04:03:12PM +0800, Neil Zhang wrote: Hardware zlt will try to send the zero length packet automatically when the data transferd is multiple times of max packet, this will cause issues on Windows. So let's disable HW zlt by default. Would you have description that what kinds of issue on Windows if zlt is is selected? Enumeration will fail. What causes enumeration fail, why it does not occur before? A unexpected zero packet cause enumeration fail. It's not easy that the descriptor is actually 1024 bytes, so not easy to be found. Peter Best Regards, Neil Zhang N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��Ⅷ�G���h�(�茛j���m赇z罐��帼f"�h���~�m�
RE: [PATCH 5/6] USB: gadget: mv_udc: workaroud for missing dTD
-Original Message- From: Peter Chen [mailto:peter.c...@freescale.com] Sent: 2014年2月25日 12:19 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 5/6] USB: gadget: mv_udc: workaroud for missing dTD On Mon, Feb 24, 2014 at 04:03:15PM +0800, Neil Zhang wrote: There is an issue with the add dTD tripwire semaphore (ATDTW bit in USBCMD register) that can cause the controller to ignore a dTD that is added to a primed endpoint. When this happens, the software can read the tripwire bit and the status bit at '1' even though the endpoint is unprimed. After executing a dTD, the device controller endpoint state machine executes a final read of the dTD terminate bit to check if the application added a dTD to the linked list at the last moment. This read is done in the finpkt_read_latest_next_td (44) state. After the read is performed, if the terminate bit is still set, the state machine moves to unprime the endpoint. The decision to unprime the endpoint is done in the checkqh_decision (59) state, based on the value of the terminate bit. Before reaching the checkqh_decision state, the state machine traverses the writeqhtd_status (57), writeqh_status (56), and release_prime_mask (42) states. As shown in the waveform, the ep_addtd_tripwire_clr signal is not set to clear the tripwire bit in these states. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index a620cff..8df8606 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -196,7 +196,27 @@ static int process_ep_req(struct mv_udc *udc, int index, while (readl(udc-op_regs-epstatus) bit_pos) udelay(1); break; + } else { + if (!(readl(udc-op_regs-epstatus) bit_pos)) { + /* The DMA engine thinks there is no more dTD */ + curr_dqh-next_dtd_ptr = curr_dtd-dtd_next +EP_QUEUE_HEAD_NEXT_POINTER_MASK; + + /* clear active and halt bit */ + curr_dqh-size_ioc_int_sts = + ~(DTD_STATUS_ACTIVE + | DTD_STATUS_HALTED); + + /* Do prime again */ + wmb(); + writel(bit_pos, udc-op_regs-epprime); + while (readl(udc-op_regs-epprime) bit_pos) + cpu_relax(); + + break; + } } + udelay(1); } Is it a chipidea IP issue? Any errate number for this issue? Does we need it for chipidea driver? Yes, I think so. 9000531823 B2-Medium Adding a dTD to a Primed Endpoint May Not Get Recognized Impacted Configuration: All device mode configurations. -- Best Regards, Peter Chen Best Regards, Neil Zhang N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��Ⅷ�G���h�(�茛j���m赇z罐��帼f"�h���~�m�
RE: [PATCH 0/6] bug fix for mv_udc_core.c
-Original Message- From: Peter Chen [mailto:peter.c...@freescale.com] Sent: 2014年2月25日 12:18 To: Neil Zhang Cc: Matthieu CASTET; ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 0/6] bug fix for mv_udc_core.c On Mon, Feb 24, 2014 at 04:42:40AM -0800, Neil Zhang wrote: -Original Message- From: Matthieu CASTET [mailto:matthieu.cas...@parrot.com] Sent: 2014年2月24日 18:32 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 0/6] bug fix for mv_udc_core.c Le Mon, 24 Feb 2014 16:03:10 +0800, Neil Zhang zhan...@marvell.com a écrit : This patch set is mainly for bug fix. Neil Zhang (6): usb: gadget: mv_udc: remove redundant pull up in udc_start usb: gadget: mv_udc: disable HW zlt for ep0 usb: gadget: mv_udc: clear corresponding interrupt when flush fifo usb: gadget: mv_udc: check endpoint before queue dtd USB: gadget: mv_udc: workaroud for missing dTD USB: gadget: mv_udc: fix corner case for missiong dTD drivers/usb/gadget/mv_udc_core.c | 47 ++ 1 file changed, 42 insertions(+), 5 deletions(-) Do you have plan to move to the chipidea udc driver ? You could have some bug fix for free or share your bug fix with others. Not yet. Will estimate it later. Freescale uses mv udc driver at u-boot, so the chipidea IP should be compatible for two SoCs , both Intel and msm are using chipidea driver at linux kernel, it should be not difficult for marvell to use it. For this patchset, some of them may already be fixed at chipidea driver (3/6, 4/6), some of them may not (5/6, 6/6). -- Thanks for the comments. Since we are using it in our product, so it's not that easy to switch to a new driver. Best Regards, Peter Chen Best Regards, Neil Zhang
RE: [PATCH 5/6] USB: gadget: mv_udc: workaroud for missing dTD
From: Peter Chen [mailto:peter.c...@freescale.com] Sent: 2014年2月25日 12:19 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 5/6] USB: gadget: mv_udc: workaroud for missing dTD On Mon, Feb 24, 2014 at 04:03:15PM +0800, Neil Zhang wrote: There is an issue with the add dTD tripwire semaphore (ATDTW bit in USBCMD register) that can cause the controller to ignore a dTD that is added to a primed endpoint. When this happens, the software can read the tripwire bit and the status bit at '1' even though the endpoint is unprimed. After executing a dTD, the device controller endpoint state machine executes a final read of the dTD terminate bit to check if the application added a dTD to the linked list at the last moment. This read is done in the finpkt_read_latest_next_td (44) state. After the read is performed, if the terminate bit is still set, the state machine moves to unprime the endpoint. The decision to unprime the endpoint is done in the checkqh_decision (59) state, based on the value of the terminate bit. Before reaching the checkqh_decision state, the state machine traverses the writeqhtd_status (57), writeqh_status (56), and release_prime_mask (42) states. As shown in the waveform, the ep_addtd_tripwire_clr signal is not set to clear the tripwire bit in these states. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index a620cff..8df8606 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -196,7 +196,27 @@ static int process_ep_req(struct mv_udc *udc, int index, while (readl(udc-op_regs-epstatus) bit_pos) udelay(1); break; + } else { + if (!(readl(udc-op_regs-epstatus) bit_pos)) { + /* The DMA engine thinks there is no more dTD */ + curr_dqh-next_dtd_ptr = curr_dtd-dtd_next + EP_QUEUE_HEAD_NEXT_POINTER_MASK; + + /* clear active and halt bit */ + curr_dqh-size_ioc_int_sts = + ~(DTD_STATUS_ACTIVE + | DTD_STATUS_HALTED); + + /* Do prime again */ + wmb(); + writel(bit_pos, udc-op_regs-epprime); + while (readl(udc-op_regs-epprime) bit_pos) + cpu_relax(); + + break; + } } + udelay(1); } Is it a chipidea IP issue? Any errate number for this issue? Does we need it for chipidea driver? Yes, I think so. 9000531823B2-Medium Adding a dTD to a Primed Endpoint May Not Get Recognized Impacted Configuration: All device mode configurations. Thanks for your information. 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: [PATCH 3/6] usb: gadget: mv_udc: clear corresponding interrupt when flush fifo
-Original Message- From: Peter Chen [mailto:peter.c...@freescale.com] Sent: 2014年2月25日 9:59 To: Neil Zhang Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH 3/6] usb: gadget: mv_udc: clear corresponding interrupt when flush fifo On Mon, Feb 24, 2014 at 04:03:13PM +0800, Neil Zhang wrote: The interruts are useless when this endpoint is going to be flushed. Especially in the enumeration phase, let's take get device description for example. 1. Device doesn't ACK the status package from host in time due to irq is disabled by some module. 2. Host find no ACK in time, and send another request. Why device does not send NAK at that time? Besides, you can try to prime status at data stage, it can also avoid the problem usb 2.0 8.5.3.3 Error Handling on the Last Data Transaction described. 3. Device gets the chance to handle the interrupt, the sequence is as following. a. Flush pending requests in ep0. b. Queue a request in ep0 according to host's request and change the ep0 state to DATA_STATE_XMIT. c. Handle the pending interrupt for the last status package. But actually it will check the new added request instead of the status package and because of wront ep0 state, device will request an %s/wront/wrong Thanks for the catch. Peter unexpected status package from host. d. The ep0 state is going mad. The solution is that we need clear the pending corresponding interrupt as well as flush endpoint. Signed-off-by: Neil Zhang zhan...@marvell.com --- drivers/usb/gadget/mv_udc_core.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c index 657ac5c..d5a9bdf 100644 --- a/drivers/usb/gadget/mv_udc_core.c +++ b/drivers/usb/gadget/mv_udc_core.c @@ -692,6 +692,8 @@ static void mv_ep_fifo_flush(struct usb_ep *_ep) } loops--; } while (readl(udc-op_regs-epstatus) bit_pos); + + writel(bit_pos, udc-op_regs-epcomplete); } /* queues (submits) an I/O request to an endpoint */ -- 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 Best Regards, Neil Zhang -- Best Regards, Peter Chen
RE: [PATCH 2/6] usb: gadget: mv_udc: disable HW zlt for ep0
On Mon, Feb 24, 2014 at 04:03:12PM +0800, Neil Zhang wrote: Hardware zlt will try to send the zero length packet automatically when the data transferd is multiple times of max packet, this will cause issues on Windows. So let's disable HW zlt by default. Would you have description that what kinds of issue on Windows if zlt is is selected? Enumeration will fail. What causes enumeration fail, why it does not occur before? A unexpected zero packet cause enumeration fail. It's not easy that the descriptor is actually 1024 bytes, so not easy to be found. Chipidea bug too? Does it follow ch 8.5.3.2 Variable-length Data Stage, USB 2.0 spec? 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