[PATCH v2] usb: ehci-sh: Fix build error due to comma has been deleted
By commit 39d3568 (USB: remove incorrect __exit markups), comma following ehci_hcd_sh_remove has been deleted. This fixes the error by the correction. Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com CC: Dmitry Torokhov dmitry.torok...@gmail.com --- v2: Added commit's summary and fixed sentence. drivers/usb/host/ehci-sh.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ehci-sh.c b/drivers/usb/host/ehci-sh.c index e30e396..b0f2268 100644 --- a/drivers/usb/host/ehci-sh.c +++ b/drivers/usb/host/ehci-sh.c @@ -196,7 +196,7 @@ static void ehci_hcd_sh_shutdown(struct platform_device *pdev) static struct platform_driver ehci_hcd_sh_driver = { .probe = ehci_hcd_sh_probe, - .remove = ehci_hcd_sh_remove + .remove = ehci_hcd_sh_remove, .shutdown = ehci_hcd_sh_shutdown, .driver = { .name = sh_ehci, -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/12] usb: chipidea: udc: add the define TD_PAGE_COUNT and fix all users
HI, On Tue, Mar 26, 2013 at 09:24:30PM +0100, Michael Grzeschik wrote: On Tue, Mar 26, 2013 at 07:55:46PM +0200, Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 05:58:45PM +0100, Michael Grzeschik wrote: A static count of transfer descriptors was used everywhere in the driver with the fixed number 5. This patch adds a define, named TD_PAGE_COUNT, and replaces all users of this value. This way its possible to have only one parameter to change and limit the amount of buffer pointers per TD. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- Changes since v1: - renamed TD_COUNT to more precise TD_PAGE_COUNT - changed TD_PAGE_COUNT to 5 based on the previous patch - fixed patch description to address the tds buffers not the td itself drivers/usb/chipidea/ci.h | 1 + drivers/usb/chipidea/udc.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 7dcd390..4ca3373 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -21,6 +21,7 @@ /** * DEFINE */ +#define TD_PAGE_COUNT 5 #define CI13XXX_PAGE_SIZE 4096ul /* page size for TD's */ #define ENDPT_MAX 32 #define RX0 /* similar to USB_DIR_OUT but can be used as an index */ diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index e779870..4294c2c 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -445,7 +445,7 @@ _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq, mReq-ptr-token |= TD_IOC; } mReq-ptr-page[0] = mReq-req.dma; - for (i = 1; i 5; i++) + for (i = 1; i TD_PAGE_COUNT; i++) mReq-ptr-page[i] = (mReq-req.dma + i * CI13XXX_PAGE_SIZE) ~TD_RESERVED_MASK; @@ -697,8 +697,8 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request *req, goto done; } - if (req-length 4 * CI13XXX_PAGE_SIZE) { - req-length = 4 * CI13XXX_PAGE_SIZE; + if (req-length (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE) { + req-length = (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE; this is a bug. You shouldn't change req-length, you might completely screw up a gadget driver. Don't change anything what the gadget driver passes to you, ever. Change some private structure instead. (yeah, I know it's not part of $subject) Beside that bug get fixed by the patch usb: chipidea: udc: add multiple td support to hardware_{en,de}queue in that series. *ALL BUG FIXES DESERVE THEIR OWN PATCH*. Burn this into non-volatile memory. When writing patches, whenever you find a bug fix, you create a separate branch on top of newest -rc (or Greg's usb-linus or my 'fixes' branch depending on which driver you're fixing) fix the bug with as few lines as possible and send the patch so it can be applied during the -rc, without creating dependencies with patches going for next merge window. If bug was introduced prior to last merge window (e.g. you figure bug was introduced in v3.6, instead of v3.8) then you add Cc sta...@vger.kernel.org to make sure folks using older kernels also have the fix. I would suggest Alex doesn't take this series until it's properly split into a bugfixes series and another series of cleanups/new features for next. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: Fix compile error by selecting USB_OTG_UTILS
On Tue, Mar 26, 2013 at 01:37:41PM -0700, Greg KH wrote: On Tue, Mar 26, 2013 at 09:25:35PM +0100, Roland Stigge wrote: On 03/26/2013 08:24 PM, Felipe Balbi wrote: commit ? 1c2088812f095df77f4b3224b65db79d7111a300 Showed up in 3.9-rc3. Felipe, I can take this directly if you don't have any other USB patches for me at the moment. please go ahead Greg: Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 09/12] usb: chipidea: udc: add the define TD_PAGE_COUNT and fix all users
Felipe Balbi ba...@ti.com writes: HI, On Tue, Mar 26, 2013 at 09:24:30PM +0100, Michael Grzeschik wrote: On Tue, Mar 26, 2013 at 07:55:46PM +0200, Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 05:58:45PM +0100, Michael Grzeschik wrote: A static count of transfer descriptors was used everywhere in the driver with the fixed number 5. This patch adds a define, named TD_PAGE_COUNT, and replaces all users of this value. This way its possible to have only one parameter to change and limit the amount of buffer pointers per TD. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- Changes since v1: - renamed TD_COUNT to more precise TD_PAGE_COUNT - changed TD_PAGE_COUNT to 5 based on the previous patch - fixed patch description to address the tds buffers not the td itself drivers/usb/chipidea/ci.h | 1 + drivers/usb/chipidea/udc.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index 7dcd390..4ca3373 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -21,6 +21,7 @@ /** * DEFINE */ +#define TD_PAGE_COUNT 5 #define CI13XXX_PAGE_SIZE 4096ul /* page size for TD's */ #define ENDPT_MAX 32 #define RX0 /* similar to USB_DIR_OUT but can be used as an index */ diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index e779870..4294c2c 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -445,7 +445,7 @@ _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq, mReq-ptr-token |= TD_IOC; } mReq-ptr-page[0] = mReq-req.dma; -for (i = 1; i 5; i++) +for (i = 1; i TD_PAGE_COUNT; i++) mReq-ptr-page[i] = (mReq-req.dma + i * CI13XXX_PAGE_SIZE) ~TD_RESERVED_MASK; @@ -697,8 +697,8 @@ static int _ep_queue(struct usb_ep *ep, struct usb_request *req, goto done; } -if (req-length 4 * CI13XXX_PAGE_SIZE) { -req-length = 4 * CI13XXX_PAGE_SIZE; +if (req-length (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE) { +req-length = (TD_PAGE_COUNT - 1) * CI13XXX_PAGE_SIZE; this is a bug. You shouldn't change req-length, you might completely screw up a gadget driver. Don't change anything what the gadget driver passes to you, ever. Change some private structure instead. (yeah, I know it's not part of $subject) Beside that bug get fixed by the patch usb: chipidea: udc: add multiple td support to hardware_{en,de}queue in that series. *ALL BUG FIXES DESERVE THEIR OWN PATCH*. Burn this into non-volatile memory. When writing patches, whenever you find a bug fix, you create a separate branch on top of newest -rc (or Greg's usb-linus or my 'fixes' branch depending on which driver you're fixing) fix the bug with as few lines as possible and send the patch so it can be applied during the -rc, without creating dependencies with patches going for next merge window. If bug was introduced prior to last merge window (e.g. you figure bug was introduced in v3.6, instead of v3.8) then you add Cc sta...@vger.kernel.org to make sure folks using older kernels also have the fix. I would suggest Alex doesn't take this series until it's properly split into a bugfixes series and another series of cleanups/new features for next. I hear ya Regards, -- Alex -- 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 v4 00/12] usb: chipidea: udc: fixes, iso ep and multi td support
On Tue, Mar 26, 2013 at 03:50:47PM -0700, Greg KH wrote: On Tue, Mar 26, 2013 at 11:57:48PM +0200, Alexander Shishkin wrote: Also, when you resend this patchset, *please* make sure all the patches in it have the same version number, which is greater than *any* of these patches had before. Right now your 0/12 is v4, and the patches contained therein are anything between v2 and v5 and 1/12 doesn't have a version. This sends my head spinning. Welcome to my world :) Ok, i messed it up this time, the series should have been without any version number, like the other series before. But anyway, i will rework the whole thing and split into bugfix and feature patches to repost. For that i will set the series number beginning with the oldest patch version number. Thanks for the review, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
Hi, Understand, UDC driver will call driver-setup(), and if the return value is negative, UDC driver has to set dev-protocol_stall = 1 and maybe call usb_ep_set_halt(). However, the hardware won't be able to send out zero length response. Don't be silly; of course it can. Nobody would be foolish enough to design a piece of USB hardware that couldn't send a zero-length DATA packet. I think the purpose of zero length response is to get an ACK from the host. The purpose of the zero-length response is to complete the Status stage of a control-OUT transfer. Thanks, the halt feature is ok now. The SCSI_READ_10 command has a problem. The reply value from do_read is -5, which means -EIO. The for(;;) loop in do_read() was probably broken at if (amount_left == 0). Is this if-statement valid? Here is the gadget log when receiving SCSI_READ_10 from Linux host. g_file_storage gadget: bulk-out, length 31: : 55 53 42 43 0f 00 00 00 00 10 00 00 80 00 0a 28 0010: 00 00 00 00 00 00 00 08 00 00 00 00 e0 f8 02 SCSI CDB: 28 00 00 00 00 00 00 00 08 00 g_file_storage gadget: SCSI command: READ(10); Dc=10, Di=4096; Hc=10, Hi=4096 driver read from SD card . g_file_storage gadget-lun0: file read 4096 @ 0 - 4096 READ_10 reply -5 *** printk added by me*** g_file_storage gadget: bulk-in, length 13: : 55 53 42 53 0f 00 00 00 00 00 00 00 00 Thanks, victor -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: EHCI: DT support for generic bus glue
Hi folks, I don't think we are consistent in any way. PowerPC sets up a 32 bit DMA mask for all devices during DT probe from arch code, while the common code sets up coherent_dma_mask but not dma_mask, except for AMBA devices, which also get the 32 bit mask. The MIPS Octeon and PowerPC PS3 EHCI backends set up the dma mask because platform code doesn't do it for them, but both drivers are not using DT. The Xilinx and PPC-OF EHCI back-end do not set it up, because on microblaze and powerpc it does come from the platform code. I think it's a horrible mess and if anyone has an idea of what the right solution is, we should probably implement that, but from what I see here, setting a 32-bit dma mask unless there is already one is a reasonable choice. I also ran into this issue with loading the dwc2 driver from OF recently, and stumbled upon this (unfinished) patch that sets up the dma_mask using a dma-mask property in the DT, which looks like the proper way to do this to me: https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-March/013172.html https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-March/013179.html https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-March/013293.html http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/135991.html It's probably not up to you guys to implement this, but perhaps it helps to get some perspective? Gr. Matthijs -- 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 06/12] usb: chipidea: udc: configure iso endpoints
On Tue, Mar 26, 2013 at 07:49:07PM +0200, Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 05:58:42PM +0100, Michael Grzeschik wrote: The implementation is derived from the fsl_udc_core code in fsl_ep_enable and makes basic iso handling possible. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de --- Changes since v4: - rebased on the new alignment patch - changed indention to tabs - removed spare brakets - added special handling for iso in ep_queue and ep_halt - changed TODO list entry in core.c Changes since v3: - added QH_ISO_TRANS macro - removed unused operations mentioned by Peter Changes since v2: - fixed usage of variable max - reworked on writel/readl patches Changes since v1: - fixed coding style issues mentioned by Sergei drivers/usb/chipidea/core.c | 2 +- drivers/usb/chipidea/udc.c | 15 --- drivers/usb/chipidea/udc.h | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 47b8da2..82de38b 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -43,7 +43,7 @@ * * TODO List * - OTG - * - Isochronous Interrupt Traffic + * - Interrupt Traffic * - Handle requests which spawns into several TDs * - GET_STATUS(device) - always reports 0 * - Gadget API (majority of optional features) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 567ce89..cab349b 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1010,6 +1010,8 @@ static int ep_enable(struct usb_ep *ep, unsigned long flags; u32 val = 0; + unsigned short max; + if (ep == NULL || desc == NULL) return -EINVAL; @@ -1026,19 +1028,19 @@ static int ep_enable(struct usb_ep *ep, mEp-num = usb_endpoint_num(desc); mEp-type = usb_endpoint_type(desc); - mEp-ep.maxpacket = usb_endpoint_maxp(desc); + max = mEp-ep.maxpacket = usb_endpoint_maxp(desc); trace_ci_ep_enable(mEp, 0); if (mEp-type == USB_ENDPOINT_XFER_CONTROL) val |= QH_IOS; else if (mEp-type == USB_ENDPOINT_XFER_ISOC) - val = ~QH_MULT; + val |= QH_ISO_TRANS(max) __ffs(QH_MULT); if (mEp-num) val |= QH_ZLT; - val |= (mEp-ep.maxpacket __ffs(QH_MAX_PKT)) QH_MAX_PKT; + val |= (max __ffs(QH_MAX_PKT)) QH_MAX_PKT; mEp-qh.ptr-cap = val; mEp-qh.ptr-td.next |= TD_TERMINATE; /* needed? */ @@ -1182,6 +1184,10 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, } } + if (usb_endpoint_xfer_isoc(mEp-ep.desc) +mReq-req.length mEp-ep.maxpacket) + return -EMSGSIZE; this HW really can't handle requests greater max packet size for isochronous ? please clarify what you're doing here and, possibly, add a comment. Other than that: Reviewed-by: Felipe Balbi ba...@ti.com -- balbi -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place
Hi folks, while going through the dwc2 driver, I've found that there is a lot of places that need info from the hwcfg registers. Currently, each of these places do their own bitshifting and masking, but it occurs to me that the code would become cleaner if all this happened in single place, so all the other spots can just refer to a proper variable instead. I've made a start at implementing this, but before I finish up this patch, I'd like to know what other people think. Is this a good idea, or would you prefer to keep it like this? The advantage of this patch would be that the code is more clear. The disadvantage is a couple of dozen bytes of extra memory used (though this could be reduced to just a few to no bytes by using bitfields to store the values instead, if preferred). Another disadvantage could be that it would be a pretty big, but simple, patch to review. I've put my WIP patch below, let me know what you think. Gr. Matthijs commit 5f67447a06581bf9d9d5f154e635a45a1406994c Author: Matthijs Kooijman matth...@stdin.nl Date: Tue Mar 26 17:35:34 2013 +0100 staging: dwc2: Interpret all hwcfg and related register at init time Before, the hwcfg4 registers were read at device init time, but interpreted at various parts in the code. This commit unpacks the hwcfg register values into a struct with properly labeled variables, which makes all the other code using these values more consise and easier to read. This commit mostly moves code, but also attempts to simplify some expressions, from (val shift) (mask shift) to (val mask) shift. Note: This patch is unfinished, it does not convert all hwcfg accesses yet. diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c index 71a6547..3a9a9ee 100644 --- a/drivers/staging/dwc2/core.c +++ b/drivers/staging/dwc2/core.c @@ -90,12 +90,10 @@ static void dwc2_enable_common_interrupts(struct dwc2_hsotg *hsotg) */ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg) { - u32 hs_phy_type = hsotg-hwcfg2 GHWCFG2_HS_PHY_TYPE_MASK; - u32 fs_phy_type = hsotg-hwcfg2 GHWCFG2_FS_PHY_TYPE_MASK; u32 hcfg, val; - if ((hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI -fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED + if ((hsotg-hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI +hsotg-hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED hsotg-core_params-ulpi_fs_ls 0) || hsotg-core_params-phy_type == DWC2_PHY_TYPE_PARAM_FS) { /* Full speed PHY */ @@ -245,7 +243,7 @@ static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) { - u32 usbcfg, hs_phy_type, fs_phy_type; + u32 usbcfg; if (hsotg-core_params-speed == DWC2_SPEED_PARAM_FULL hsotg-core_params-phy_type == DWC2_PHY_TYPE_PARAM_FS) { @@ -256,11 +254,8 @@ static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) dwc2_hs_phy_init(hsotg, select_phy); } - hs_phy_type = hsotg-hwcfg2 GHWCFG2_HS_PHY_TYPE_MASK; - fs_phy_type = hsotg-hwcfg2 GHWCFG2_FS_PHY_TYPE_MASK; - - if (hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI - fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED + if (hsotg-hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI + hsotg-hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED hsotg-core_params-ulpi_fs_ls 0) { dev_dbg(hsotg-dev, Setting ULPI FSLS\n); usbcfg = readl(hsotg-regs + GUSBCFG); @@ -279,7 +274,7 @@ static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg) { u32 ahbcfg = 0; - switch (hsotg-hwcfg2 GHWCFG2_ARCHITECTURE_MASK) { + switch (hsotg-hw_params.arch) { case GHWCFG2_EXT_DMA_ARCH: dev_err(hsotg-dev, External DMA Mode not supported\n); return -EINVAL; @@ -331,7 +326,7 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) usbcfg = readl(hsotg-regs + GUSBCFG); usbcfg = ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); - switch (hsotg-hwcfg2 GHWCFG2_OP_MODE_MASK) { + switch (hsotg-hw_params.op_mode) { case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: if (hsotg-core_params-otg_cap == DWC2_CAP_PARAM_HNP_SRP_CAPABLE) @@ -392,12 +387,9 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy) dwc2_core_reset(hsotg); dev_dbg(hsotg-dev, num_dev_perio_in_ep=%d\n, - hsotg-hwcfg4 GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT - GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK - GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT); + hsotg-hw_params.num_dev_perio_in_ep); - hsotg-total_fifo_size = hsotg-hwcfg3 GHWCFG3_DFIFO_DEPTH_SHIFT - GHWCFG3_DFIFO_DEPTH_MASK
usb video capture issue due to uvc_complete callback spends more time
Hi I am observing issue while streaming video from usb camera connected to host controller based on mentor graphics. The issue is root caused that there are huge SOF gaps seen between the two isochronous IN token issued by host controller. This is due to fact, significant amount of time is spent in uvc callback function handler. Due to this next request programmed is delayed leads to this failure of video streaming. I have measured time taken by uvc callback function is in range minimum of 11 microsecond to maximum of 3318 microsecond. Looks like callback handler doing some processing and takes more time to return rather than giveback immediately. Need your help to understand why uvc callback handler takes much time, if it does any processing can it move to different task context and return immediately, this will reduce the latency. Appreciate your help. Regards Ravi B -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: PHY: Palmas USB Transceiver Driver
On 26/03/13 20:23, Stephen Warren wrote: On 03/26/2013 10:57 AM, Graeme Gregory wrote: On 26/03/13 16:22, Stephen Warren wrote: On 03/26/2013 03:27 AM, Graeme Gregory wrote: ... If we are tightly coupling as above then using platform_irq is an extra inefficiency. You both have to populate this then parse it afterwards. Why not just use the regmap helper? Ill admit this code is like this as there was a period where platform irqs in DT just was not working right! We should really agree now if we are going for loose or tight coupling now rather than keep switching? Yes, this is something that I think needs to be fully resolved before any more Palmas patches are discussed. So you can have the MFD components fully coupled or completely standalone. We should fully pick one or the other, not some mish-mash of the two. In practical terms, this means that: a) Tightly coupled The top-level MFD device knows exactly which child devices are present. It has an internal table to defined the set of child devices, and instantiate them. It provides them with IRQs, I2C addresses and register base addresses (or regmaps), etc. etc., using purely Palmas-internal APIs. If using device tree, the DT won't include any representation of which child devices are present, nor their I2C addresses, nor their register addresses, nor their IRQs, etc. That's all inside the driver. b) Completely decoupled. The top-level MFD device knows nothing about its children. It simply provides a bus into which they can be instantiated, and a generic IRQ controller into which they can hook. Platform data or device tree is solely used to define which children exist, which of the top-level MFD's I2C addresses is used for each child, the base register address for each child device, the IRQs used by each child device, etc. Which of those two models are different people expecting? (b) appears to be the most flexible, and since you have defined the DT bindings to contain a separate node for each MFD child device, each with its own compatible value, seems to be what you're aiming for. In this scenario, there should be no private APIs between the top-level MFD device and the children though; everything should be using standard bus APIs. I was aiming towards (b) which would allow drivers for IP blocks that I know are shared between multiple devices such as RTC which is shared by twl6030, twl6032, tps80032, tps65910, tps65911, tps65912, tps80035, tps80036 and probably many more. Finishing (b) implementation is currently beyond the time I have available I think. If we choose to ignore path (b) and ignore the code duplication of half a dozen RTC drivers for the same IP than the path to (a) is much quicker and easier. Can just rip out a lot of the DT stuff, use mfd_add_devices. Makes the binding much simpler. Means we don't have to use platform resources for IRQs. Makes palmas and palmas-charger just 2 black boxes which is in line with other MFDs. So I think I have come around to just do it the easy way and select (a) I shall work on the main palmas series to implement (a). This will obviously invalidate some comments on this patch and the main series. Well, my question was more directed at which way we want to model the HW in the device tree, rather than how we want to implement the driver. The driver implementation style might end up being derived from the DT structure, but it shouldn't be the other way around. I think if people think (b) is the right way to go, we should just do it, and ignore any time issues; if it takes a while to get it right upstream, what is the issue with that? I'm going to take a timeout now, I have to travel on an emergency tonight and not sure when I will be back. Graeme -- 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 v4 0/6] support other fsl SoCs with usbmisc + small fixes
On Tue, Mar 26, 2013 at 02:08:05PM +0200, Alexander Shishkin wrote: Michael Grzeschik m...@pengutronix.de writes: Hi Alexander, Fabio, Greg, [...] 618bfec Revert USB: chipidea: add vbus detect for udc This one, though, I will pick. I miss that patch in your posted series. -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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 4/4] usb: gadget: mv_u3d: drop ARCH dependency
Hi, On Fri, Mar 22, 2013 at 09:30:17AM +0200, Felipe Balbi wrote: this driver compiles fine everywhere which means we can use linux-next to compile it for us frequently. By dropping the arch dependency, we also ensure driver writers don't add virtual arch-depdencies to the driver by e.g. using the wrong headers. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/gadget/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index af5cc35..261b1e3 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -324,7 +324,6 @@ config USB_MV_UDC config USB_MV_U3D tristate MARVELL PXA2128 USB 3.0 controller - depends on CPU_MMP3 there's one piece missing here. Marvell's PHY dependency is bogus. Currently PHY depends on UDC, which is nonsense. This means that by dropping ARCH dependency on this driver, which builds fine everywhere, we can could try to build PHY in Arches which don't provide writel_relaxed and readl_relaxed. I fixed the original commit since it wasn't in my next branch yet, see below: commit 60630c2eabd40fb119a1b88af364003d2915b370 Author: Felipe Balbi ba...@ti.com Date: Fri Mar 22 09:15:45 2013 +0200 usb: gadget: mv_u3d: drop ARCH dependency this driver compiles fine everywhere which means we can use linux-next to compile it for us frequently. By dropping the arch dependency, we also ensure driver writers don't add virtual arch-depdencies to the driver by e.g. using the wrong headers. While at that, fix Marvell's USB3 PHY dependency, that's the driver which depends on CPU_MM3, not mv_u3d_core. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index af5cc35..261b1e3 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -324,7 +324,6 @@ config USB_MV_UDC config USB_MV_U3D tristate MARVELL PXA2128 USB 3.0 controller - depends on CPU_MMP3 help MARVELL PXA2128 Processor series include a super speed USB3.0 device controller, which support super speed USB peripheral. diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 7e8fe0f..372db48 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -45,7 +45,7 @@ config ISP1301_OMAP config MV_U3D_PHY bool Marvell USB 3.0 PHY controller Driver - depends on USB_MV_U3D + depends on CPU_MMP3 help Enable this to support Marvell USB 3.0 phy controller for Marvell SoC. -- balbi signature.asc Description: Digital signature
Re: [PATCH RESEND v2 1/1] usb: musb: implement (un)map_urb_for_dma hooks
Hi, On Thu, Mar 14, 2013 at 08:12:09PM +0200, Ruslan Bilovol wrote: MUSB controller cannot work in DMA mode with misaligned buffers, switching in PIO mode. HCD core has hooks that allow to override the default DMA mapping and unmapping routines for host controllers that have special DMA requirements, such as alignment contraints. It is observed that work in PIO mode is slow and it's better to align buffers properly before passing them to MUSB This increased throughput 80-120 MBits/s over musb@omap4 with USB Gigabit ethernet adapter attached. Some ideas taken from ehci-tegra.c Signed-off-by: Ruslan Bilovol ruslan.bilo...@ti.com --- drivers/usb/musb/musb_core.c | 14 ++ drivers/usb/musb/musb_host.c | 102 +- drivers/usb/musb/musb_host.h |2 +- 3 files changed, 116 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 60b41cc..91ac166 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1431,6 +1431,20 @@ static int musb_core_init(u16 musb_type, struct musb *musb) /* log release info */ musb-hwvers = musb_read_hwvers(mbase); + +#ifndef CONFIG_MUSB_PIO_ONLY + /* + * The DMA engine in RTL1.8 and above cannot handle + * DMA addresses that are not aligned to a 4 byte boundary. + * For such engine implemented (un)map_urb_for_dma hooks. + * Do not use these hooks for RTL1.8 + */ + if (musb-hwvers MUSB_HWVERS_1800) { if you move this check to map/unmap and always return error if this is true, you can avoid removing 'const' from our struct hc_driver. Would that work ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling
Hi, On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote: @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct platform_device *pdev) static struct samsung_usbphy_drvdata usb3phy_exynos5 = { .cpu_type = TYPE_EXYNOS5250, .devphy_en_mask = EXYNOS_USBPHY_ENABLE, + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12, why isn't this just a clk_get_rate() ??? -- balbi signature.asc Description: Digital signature
Re: xhci page fault panic on Ubuntu kernel with HP desktop hardware
Le 26/03/2013 17:30, Sarah Sharp a écrit : On Tue, Mar 26, 2013 at 12:11:13PM +0100, Yann Sionneau wrote: Le 25/03/2013 19:13, Sarah Sharp a écrit : On Mon, Mar 25, 2013 at 05:43:40PM +0100, Yann Sionneau wrote: Please compile with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on. If you can't reproduce your bug on 3.8.4, turn them off, recompile, and see if you can reproduce it again. We just reproduced it with a 3.8.4 kernel with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on. I am sending you the picture as attached file. Thanks. I need more information though. Can you follow these directions for setting up netconsole, and send me the full log file from boot to when the machine hangs? Make sure the kernel still has CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on. http://kernelnewbies.org/KernelDebug That will help me figure out what command is causing the null pointer dereference. Sarah Sharp Hi Sarah, I've got some xhci debug traces: ysionneau@yann-HP:~/ssd/olivier$ nc -l -u | tee ~/ssd/olivier/dmesg-`date +%Y-%m-%d-%H-%M`.txt [ 1322.011656] xhci_hcd :00:14.0: Poll event ring: 256448 [ 1322.011671] xhci_hcd :00:14.0: op reg status = 0x0 [ 1322.011677] xhci_hcd :00:14.0: ir_set 0 pending = 0x2 [ 1322.011681] xhci_hcd :00:14.0: HC error bitmask = 0x4 [ 1322.011685] xhci_hcd :00:14.0: Event ring: [ 1322.011691] xhci_hcd :00:14.0: @3057e400 29ce4040 0100 19028000 [ 1322.011696] xhci_hcd :00:14.0: @3057e410 29ce4050 0100 19028000 [ 1322.011701] xhci_hcd :00:14.0: @3057e420 29ce4060 0100 19028000 [ 1322.011706] xhci_hcd :00:14.0: @3057e430 29ce4070 0100 19028000 [ 1322.011711] xhci_hcd :00:14.0: @3057e440 29ce4080 0100 19028000 [ 1322.011716] xhci_hcd :00:14.0: @3057e450 29ce4090 0100 19028000 [ 1322.011722] xhci_hcd :00:14.0: @3057e460 29ce40a0 0100 19028000 [ 1322.011726] xhci_hcd :00:14.0: @3057e470 29ce40b0 0100 19028000 [ 1322.011731] xhci_hcd :00:14.0: @3057e480 29ce40c0 0100 19028000 [ 1322.011736] xhci_hcd :00:14.0: @3057e490 29ce40d0 0100 19028000 [ 1322.011741] xhci_hcd :00:14.0: @3057e4a0 29ce40e0 0100 19028000 [ 1322.011745] xhci_hcd :00:14.0: @3057e4b0 29ce40f0 0100 19028000 [ 1322.011751] xhci_hcd :00:14.0: @3057e4c0 29ce4100 0100 19028000 [ 1322.011756] xhci_hcd :00:14.0: @3057e4d0 29ce4110 0100 19028000 [ 1322.011761] xhci_hcd :00:14.0: @3057e4e0 29ce4120 0100 19028000 [ 1322.011766] xhci_hcd :00:14.0: @3057e4f0 29ce4130 0100 19028000 [ 1322.011771] xhci_hcd :00:14.0: @3057e500 29ce4140 0100 19028000 [ 1322.011776] xhci_hcd :00:14.0: @3057e510 29ce4150 0100 19028000 [ 1322.011781] xhci_hcd :00:14.0: @3057e520 29ce4160 0100 19028000 [ 1322.011785] xhci_hcd :00:14.0: @3057e530 29ce4170 0100 19028000 [ 1322.011790] xhci_hcd :00:14.0: @3057e540 29ce4180 0100 19028000 [ 1322.011795] xhci_hcd :00:14.0: @3057e550 29ce4190 0100 19028000 [ 1322.011800] xhci_hcd :00:14.0: @3057e560 29ce41a0 0100 19028000 [ 1322.011805] xhci_hcd :00:14.0: @3057e570 29ce41b0 0100 19028000 [ 1322.011809] xhci_hcd :00:14.0: @3057e580 29ce41c0 0100 19028000 [ 1322.011814] xhci_hcd :00:14.0: @3057e590 29ce41d0 0100 19028000 [ 1322.011819] xhci_hcd :00:14.0: @3057e5a0 29ce41e0 0100 19028000 [ 1322.011823] xhci_hcd :00:14.0: @3057e5b0 29ce41f0 0100 19028000 [ 1322.011829] xhci_hcd :00:14.0: @3057e5c0 29ce4200 0100 19028000 [ 1322.011834] xhci_hcd :00:14.0: @3057e5d0 29ce4210 0100 19028000 [ 1322.011838] xhci_hcd :00:14.0: @3057e5e0 29ce4220 0100 19028000 [ 1322.011843] xhci_hcd :00:14.0: @3057e5f0 29ce4230 0100 19028000 [ 1322.011848] xhci_hcd :00:14.0: @3057e600 29ce4240 0100 19028000 [ 1322.011853] xhci_hcd :00:14.0: @3057e610 29ce4250 0100 19028000 [ 1322.011858] xhci_hcd :00:14.0: @3057e620 29ce4260 0100 19028000 [ 1322.011863] xhci_hcd :00:14.0: @3057e630 29ce4270 0100 19028000 [ 1322.011868] xhci_hcd :00:14.0: @3057e640 29ce4280 0100 19028000 [ 1322.011873] xhci_hcd :00:14.0: @3057e650 29ce4290 0100 19028000 [ 1322.011878] xhci_hcd :00:14.0: @3057e660 29ce42a0 0100
Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling
Hi Felipe, On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote: @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct platform_device *pdev) static struct samsung_usbphy_drvdata usb3phy_exynos5 = { .cpu_type = TYPE_EXYNOS5250, .devphy_en_mask = EXYNOS_USBPHY_ENABLE, + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12, why isn't this just a clk_get_rate() ??? As the name suggests, this is a function to get appropriate CLKSEL value to write into PHYCLK register for reference clock rate on particular platform (clk_get_rate is used inside to get the rate). Best regards, -- Tomasz Figa Samsung Poland RD Center SW Solution Development, Kernel and System Framework -- 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: phy: samsung: Consolidate reference clock rate handling
Hi, On Wed, Mar 27, 2013 at 02:26:08PM +0100, Tomasz Figa wrote: Hi Felipe, On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote: @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct platform_device *pdev) static struct samsung_usbphy_drvdata usb3phy_exynos5 = { .cpu_type = TYPE_EXYNOS5250, .devphy_en_mask = EXYNOS_USBPHY_ENABLE, + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12, why isn't this just a clk_get_rate() ??? As the name suggests, this is a function to get appropriate CLKSEL value to write into PHYCLK register for reference clock rate on particular platform (clk_get_rate is used inside to get the rate). alright, then you don't need this function pointer at all, look at both your rate_to_clksel functions (quoted below): | +int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy, | + unsigned long rate) | +{ | + unsigned int clksel; | + | + switch (rate) { | + case 12 * MHZ: | + clksel = PHYCLK_CLKSEL_12M; | + break; | + case 24 * MHZ: | + clksel = PHYCLK_CLKSEL_24M; | + break; | + case 48 * MHZ: | + clksel = PHYCLK_CLKSEL_48M; | + break; | + default: | + dev_err(sphy-dev, | + Invalid reference clock frequency: %lu\n, rate); | + return -EINVAL; | + } | + | + return clksel; | +} | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx); | + | +int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy, | + unsigned long rate) | +{ | + unsigned int clksel; | + | + switch (rate) { | + case 9600 * KHZ: | + clksel = FSEL_CLKSEL_9600K; | + break; | + case 10 * MHZ: | + clksel = FSEL_CLKSEL_10M; | + break; | + case 12 * MHZ: | + clksel = FSEL_CLKSEL_12M; | + break; | + case 19200 * KHZ: | + clksel = FSEL_CLKSEL_19200K; | + break; | + case 20 * MHZ: | + clksel = FSEL_CLKSEL_20M; | + break; | + case 24 * MHZ: | + clksel = FSEL_CLKSEL_24M; | + break; | + case 50 * MHZ: | + clksel = FSEL_CLKSEL_50M; | + break; | + default: | + dev_err(sphy-dev, | + Invalid reference clock frequency: %lu\n, rate); | + return -EINVAL; | + } | + | + return clksel; | +} | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_4x12); They both return the same thing and test the same thing. You clearly don't need this function pointer. The only thing you need to be careful is that different platforms will have different clock rates, but that can just as easily be turned into a generic check. I don't see the need for $SUBJECT. -- balbi signature.asc Description: Digital signature
Re: [PATCH V4 0/2] UVC webcam gadget related changes
Hi, On Thu, Mar 21, 2013 at 01:56:07PM +0530, Bhupesh Sharma wrote: This patchset tries to enhance the UVC webcam gadget driver and is based on Laurent's git tree available here (head uvc-gadget): git://linuxtv.org/pinchartl/uvcvideo.git Note that to ease review and integration of these patches, I have rebased them on Laurent's repo and all the relevant patches after review can be pushed in Felipe's repo in one go. The patches 1/2 and 2/2 in this patchset try to handle all the review comments received on the following UVC gadget related patches: [PATCH V3 3/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework [PATCH V3 5/5] usb: gadget/uvc: Add support for 'get_unmapped_area' for MMUless architectures which can be viewed here: [1] http://comments.gmane.org/gmane.linux.usb.general/2 [2] http://www.spinics.net/lists/linux-usb/msg77400.html The rest of the patches in the V3 patchset have already been accepted into Laurent's git tree. I have tested this patchset on a super-speed compliant USB device controller (DWC3), with the VIVI capture device acting as a dummy source of video data and I also have modified the 'uvc-gadget' application written by Laurent (original application available here: http://git.ideasonboard.org/uvc-gadget.git) for testing the complete flow from V4L2 to UVC domain and vice versa. I will send a patch for the modified 'uvc-gadget' application in a separate patch. Changes since V3: - Addresses the review comments received on V3 of this patchset from Laurent and Michael Grzeschik. - The changes suggested by Laurent and Alan to remove WARN_ON messages from the UDC controller drivers in case the gadget tries to dequeue a USB request which was never queued to the UDC controller, will be floated as a separate patch. Laurent, are you giving your Ack here ? It has been almost a week since these were sent to linux-usb. -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling
On Wednesday 27 of March 2013 15:31:49 Felipe Balbi wrote: Hi, On Wed, Mar 27, 2013 at 02:26:08PM +0100, Tomasz Figa wrote: Hi Felipe, On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote: @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct platform_device *pdev) static struct samsung_usbphy_drvdata usb3phy_exynos5 = { .cpu_type = TYPE_EXYNOS5250, .devphy_en_mask = EXYNOS_USBPHY_ENABLE, + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12, why isn't this just a clk_get_rate() ??? As the name suggests, this is a function to get appropriate CLKSEL value to write into PHYCLK register for reference clock rate on particular platform (clk_get_rate is used inside to get the rate). alright, then you don't need this function pointer at all, look at both your rate_to_clksel functions (quoted below): | +int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy, | + unsigned long | rate) | +{ | + unsigned int clksel; | + | + switch (rate) { | + case 12 * MHZ: | + clksel = PHYCLK_CLKSEL_12M; Please note the PHYCLK_CLKSEL_ prefix here... | + break; | + case 24 * MHZ: | + clksel = PHYCLK_CLKSEL_24M; | + break; | + case 48 * MHZ: | + clksel = PHYCLK_CLKSEL_48M; | + break; | + default: | + dev_err(sphy-dev, | + Invalid reference clock frequency: %lu\n, rate); | + return -EINVAL; | + } | + | + return clksel; | +} | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx); | + | +int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy, | + unsigned long | rate) | +{ | + unsigned int clksel; | + | + switch (rate) { | + case 9600 * KHZ: | + clksel = FSEL_CLKSEL_9600K; | + break; | + case 10 * MHZ: | + clksel = FSEL_CLKSEL_10M; | + break; | + case 12 * MHZ: | + clksel = FSEL_CLKSEL_12M; ..and then FSEL_CLKSEL_ here. They have different values. (Their names are a bit unfortunate, though...) Best regards, -- Tomasz Figa Samsung Poland RD Center SW Solution Development, Kernel and System Framework | + break; | + case 19200 * KHZ: | + clksel = FSEL_CLKSEL_19200K; | + break; | + case 20 * MHZ: | + clksel = FSEL_CLKSEL_20M; | + break; | + case 24 * MHZ: | + clksel = FSEL_CLKSEL_24M; | + break; | + case 50 * MHZ: | + clksel = FSEL_CLKSEL_50M; | + break; | + default: | + dev_err(sphy-dev, | + Invalid reference clock frequency: %lu\n, rate); | + return -EINVAL; | + } | + | + return clksel; | +} | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_4x12); They both return the same thing and test the same thing. You clearly don't need this function pointer. The only thing you need to be careful is that different platforms will have different clock rates, but that can just as easily be turned into a generic check. I don't see the need for $SUBJECT. -- 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
Renesas sparse errors
Hi Kuninori, Renesas USB driver constantly gives me sparse errors: linux/drivers/usb/renesas_usbhs/common.c:313:17: error: incompatible types in conditional expression (different base types) linux/drivers/usb/renesas_usbhs/common.c:322:17: error: incompatible types in conditional expression (different base types) linux/drivers/usb/renesas_usbhs/common.c:384:17: error: incompatible types in conditional expression (different base types) linux/drivers/usb/renesas_usbhs/common.c:524:9: error: incompatible types in conditional expression (different base types) linux/drivers/usb/renesas_usbhs/common.c:545:9: error: incompatible types in conditional expression (different base types) linux/drivers/usb/renesas_usbhs/common.c:574:9: error: incompatible types in conditional expression (different base types) linux/drivers/usb/renesas_usbhs/common.c:606:9: error: incompatible types in conditional expression (different base types) Could you look into fixing them for v3.10 or v3.11 ? That would be great as it would make my build-testing scripts a lot happier :-p -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/6] usb: phy: samsung: Consolidate reference clock rate handling
Hi, On Wed, Mar 27, 2013 at 02:39:08PM +0100, Tomasz Figa wrote: On Wednesday 27 of March 2013 15:31:49 Felipe Balbi wrote: Hi, On Wed, Mar 27, 2013 at 02:26:08PM +0100, Tomasz Figa wrote: Hi Felipe, On Wednesday 27 of March 2013 15:19:58 Felipe Balbi wrote: Hi, On Tue, Mar 26, 2013 at 03:53:12PM +0100, Tomasz Figa wrote: @@ -307,6 +310,7 @@ static int samsung_usb3phy_remove(struct platform_device *pdev) static struct samsung_usbphy_drvdata usb3phy_exynos5 = { .cpu_type = TYPE_EXYNOS5250, .devphy_en_mask = EXYNOS_USBPHY_ENABLE, + .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12, why isn't this just a clk_get_rate() ??? As the name suggests, this is a function to get appropriate CLKSEL value to write into PHYCLK register for reference clock rate on particular platform (clk_get_rate is used inside to get the rate). alright, then you don't need this function pointer at all, look at both your rate_to_clksel functions (quoted below): | +int samsung_usbphy_rate_to_clksel_64xx(struct samsung_usbphy *sphy, | + unsigned long | rate) | +{ | + unsigned int clksel; | + | + switch (rate) { | + case 12 * MHZ: | + clksel = PHYCLK_CLKSEL_12M; Please note the PHYCLK_CLKSEL_ prefix here... | + break; | + case 24 * MHZ: | + clksel = PHYCLK_CLKSEL_24M; | + break; | + case 48 * MHZ: | + clksel = PHYCLK_CLKSEL_48M; | + break; | + default: | + dev_err(sphy-dev, | + Invalid reference clock frequency: %lu\n, rate); | + return -EINVAL; | + } | + | + return clksel; | +} | +EXPORT_SYMBOL_GPL(samsung_usbphy_rate_to_clksel_64xx); | + | +int samsung_usbphy_rate_to_clksel_4x12(struct samsung_usbphy *sphy, | + unsigned long | rate) | +{ | + unsigned int clksel; | + | + switch (rate) { | + case 9600 * KHZ: | + clksel = FSEL_CLKSEL_9600K; | + break; | + case 10 * MHZ: | + clksel = FSEL_CLKSEL_10M; | + break; | + case 12 * MHZ: | + clksel = FSEL_CLKSEL_12M; ..and then FSEL_CLKSEL_ here. They have different values. (Their names are a bit unfortunate, though...) indeed, my eyes failed there. So I agree with the patch :-) sorry for the noise. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 0/6] support other fsl SoCs with usbmisc + small fixes
Michael Grzeschik m...@pengutronix.de writes: On Tue, Mar 26, 2013 at 02:08:05PM +0200, Alexander Shishkin wrote: Michael Grzeschik m...@pengutronix.de writes: Hi Alexander, Fabio, Greg, [...] 618bfec Revert USB: chipidea: add vbus detect for udc This one, though, I will pick. I miss that patch in your posted series. I haven't decided yet if it is better to apply a revert or to fix the existing patch for the time being. If you have any opinions on this, please share. Regards, -- Alex -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
On Wed, 27 Mar 2013, victor yeo wrote: Thanks, the halt feature is ok now. The SCSI_READ_10 command has a problem. The reply value from do_read is -5, which means -EIO. The for(;;) loop in do_read() was probably broken at if (amount_left == 0). Is this if-statement valid? Yes, it is. do_read() is supposed to return -EIO. This tells the code at the end of do_scsi_command() that the final reply buffer has already been set up. The final buffer is sent to the host by the finish_reply() routine. Here is the gadget log when receiving SCSI_READ_10 from Linux host. g_file_storage gadget: bulk-out, length 31: : 55 53 42 43 0f 00 00 00 00 10 00 00 80 00 0a 28 0010: 00 00 00 00 00 00 00 08 00 00 00 00 e0 f8 02 SCSI CDB: 28 00 00 00 00 00 00 00 08 00 g_file_storage gadget: SCSI command: READ(10); Dc=10, Di=4096; Hc=10, Hi=4096 driver read from SD card . g_file_storage gadget-lun0: file read 4096 @ 0 - 4096 READ_10 reply -5 *** printk added by me*** g_file_storage gadget: bulk-in, length 13: : 55 53 42 53 0f 00 00 00 00 00 00 00 00 This all looks right. 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 V4 0/2] UVC webcam gadget related changes
Hi Felipe, On Wednesday 27 March 2013 15:33:15 Felipe Balbi wrote: On Thu, Mar 21, 2013 at 01:56:07PM +0530, Bhupesh Sharma wrote: This patchset tries to enhance the UVC webcam gadget driver and is based on Laurent's git tree available here (head uvc-gadget): git://linuxtv.org/pinchartl/uvcvideo.git Note that to ease review and integration of these patches, I have rebased them on Laurent's repo and all the relevant patches after review can be pushed in Felipe's repo in one go. The patches 1/2 and 2/2 in this patchset try to handle all the review comments received on the following UVC gadget related patches: [PATCH V3 3/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework [PATCH V3 5/5] usb: gadget/uvc: Add support for 'get_unmapped_area' for MMUless architectures which can be viewed here: [1] http://comments.gmane.org/gmane.linux.usb.general/2 [2] http://www.spinics.net/lists/linux-usb/msg77400.html The rest of the patches in the V3 patchset have already been accepted into Laurent's git tree. I have tested this patchset on a super-speed compliant USB device controller (DWC3), with the VIVI capture device acting as a dummy source of video data and I also have modified the 'uvc-gadget' application written by Laurent (original application available here: http://git.ideasonboard.org/uvc-gadget.git) for testing the complete flow from V4L2 to UVC domain and vice versa. I will send a patch for the modified 'uvc-gadget' application in a separate patch. Changes since V3: - Addresses the review comments received on V3 of this patchset from Laurent and Michael Grzeschik. - The changes suggested by Laurent and Alan to remove WARN_ON messages from the UDC controller drivers in case the gadget tries to dequeue a USB request which was never queued to the UDC controller, will be floated as a separate patch. Laurent, are you giving your Ack here ? It has been almost a week since these were sent to linux-usb. I'm reviewing the patches. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place
On Wed, Mar 27, 2013 at 10:48:17AM +0100, Matthijs Kooijman wrote: Hi folks, while going through the dwc2 driver, I've found that there is a lot of places that need info from the hwcfg registers. Currently, each of these places do their own bitshifting and masking, but it occurs to me that the code would become cleaner if all this happened in single place, so all the other spots can just refer to a proper variable instead. I've made a start at implementing this, but before I finish up this patch, I'd like to know what other people think. Is this a good idea, or would you prefer to keep it like this? The advantage of this patch would be that the code is more clear. The disadvantage is a couple of dozen bytes of extra memory used (though this could be reduced to just a few to no bytes by using bitfields to store the values instead, if preferred). Another disadvantage could be that it would be a pretty big, but simple, patch to review. Then do it one bit at a time (sorry, bad pun...) But yes, it does seem to be a good idea, as long as these options can't change over time. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place
Hi Greg, But yes, it does seem to be a good idea, Ok, thanks. as long as these options can't change over time. It's only read-only registers, or registers whose power-on values determine their maximum possible value, so they indeed won't change over time. Gr. Matthijs -- 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 v6 3/6] usb: chipidea: udc: move ZLT flag change to ep_enable
Its not necessary and also not specified in the datasheet to change the ZLT flag before every ep_prime. This patch moves this to the ep_enable and applies it only for non configuration endpoints. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 4bbbe62..a8528ad 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -489,7 +489,6 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) /* QH configuration */ mEp-qh.ptr-td.next = mReq-dma;/* TERMINATE = 0 */ mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE); - mEp-qh.ptr-cap |= QH_ZLT; wmb(); /* synchronize before ep prime */ @@ -1052,6 +1051,8 @@ static int ep_enable(struct usb_ep *ep, mEp-qh.ptr-cap = ~QH_MULT; else mEp-qh.ptr-cap = ~QH_ZLT; + if (mEp-num) + mEp-qh.ptr-cap |= QH_ZLT; mEp-qh.ptr-cap |= (mEp-ep.maxpacket ffs_nr(QH_MAX_PKT)) QH_MAX_PKT; -- 1.8.2.rc2 -- 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 v6 0/6] usb: chipidea: udc: bugfixes
Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4 Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function drivers/usb/chipidea/udc.c | 133 ++--- drivers/usb/chipidea/udc.h | 4 +- 2 files changed, 80 insertions(+), 57 deletions(-) -- 1.8.2.rc2 -- 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 v6 5/6] usb: chipidea: udc: don't truncate requests to single tds
It is not safe to truncate requests to the maximum possible size the controller can handle with one td and to keep working. That patch fixes that with proper error handling instead. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index d4db3ac..e7c84ab 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1207,9 +1207,9 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, } if (req-length 4 * CI13XXX_PAGE_SIZE) { - req-length = 4 * CI13XXX_PAGE_SIZE; retval = -EMSGSIZE; - dev_warn(mEp-ci-dev, request length truncated\n); + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; } dbg_queue(_usb_addr(mEp), req, retval); -- 1.8.2.rc2 -- 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 v6 4/6] usb: chipidea: udc: read status of td only once in hardware_dequeue
This patch changes the read of the td status to one atomic operation to analyse coherent bits. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index a8528ad..d4db3ac 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -507,10 +507,12 @@ done: */ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) { + u32 tmptoken = mReq-ptr-token; + if (mReq-req.status != -EALREADY) return -EINVAL; - if ((TD_STATUS_ACTIVE mReq-ptr-token) != 0) + if ((TD_STATUS_ACTIVE tmptoken) != 0) return -EBUSY; if (mReq-zptr) { @@ -524,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) usb_gadget_unmap_request(mEp-ci-gadget, mReq-req, mEp-dir); - mReq-req.status = mReq-ptr-token TD_STATUS; + mReq-req.status = tmptoken TD_STATUS; if ((TD_STATUS_HALTED mReq-req.status) != 0) mReq-req.status = -1; else if ((TD_STATUS_DT_ERR mReq-req.status) != 0) @@ -532,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) else if ((TD_STATUS_TR_ERR mReq-req.status) != 0) mReq-req.status = -1; - mReq-req.actual = mReq-ptr-token TD_TOTAL_BYTES; + mReq-req.actual = tmptoken TD_TOTAL_BYTES; mReq-req.actual = ffs_nr(TD_TOTAL_BYTES); mReq-req.actual = mReq-req.length - mReq-req.actual; mReq-req.actual = mReq-req.status ? 0 : mReq-req.actual; -- 1.8.2.rc2 -- 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 v6 2/6] usb: chipidea: udc: only clear active and halted bits in qhead
The datasheet of the synopsys core describes only to overwrite the active and halted bits in the qhead before priming any endpoint. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 77fb66f..4bbbe62 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -488,7 +488,7 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) /* QH configuration */ mEp-qh.ptr-td.next = mReq-dma;/* TERMINATE = 0 */ - mEp-qh.ptr-td.token = ~TD_STATUS; /* clear status */ + mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE); mEp-qh.ptr-cap |= QH_ZLT; wmb(); /* synchronize before ep prime */ -- 1.8.2.rc2 -- 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 v6 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs
The udc uses an shared dma memory space between hard and software. This memory layout is described in ci13xxx_qh and ci13xxx_td which are marked with the attribute ((packed)). The compiler currently does not know about the alignment of the memory layout, and will create strb and ldrb operations. The Datasheet of the synopsys core describes, that some operations on the mapped memory need to be atomic double word operations. I.e. the next pointer addressing in the qhead, as otherwise the hardware could read wrong data and totally stuck. This patch adds the attribute ((aligned(4))) to the structures to tell the compiler to use 32bit operations. It also adds an wmb() for the prepared TD data before it gets enqueued into the qhead. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 2 ++ drivers/usb/chipidea/udc.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index f64fbea..77fb66f 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -461,6 +461,8 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) mReq-ptr-page[i] = (mReq-req.dma + i * CI13XXX_PAGE_SIZE) ~TD_RESERVED_MASK; + wmb(); + if (!list_empty(mEp-qh.queue)) { struct ci13xxx_req *mReqPrev; int n = hw_ep_bit(mEp-num, mEp-dir); diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h index 4ff2384d..d12e8b5 100644 --- a/drivers/usb/chipidea/udc.h +++ b/drivers/usb/chipidea/udc.h @@ -40,7 +40,7 @@ struct ci13xxx_td { #define TD_CURR_OFFSET(0x0FFFUL 0) #define TD_FRAME_NUM (0x07FFUL 0) #define TD_RESERVED_MASK (0x0FFFUL 0) -} __attribute__ ((packed)); +} __attribute__ ((packed, aligned(4))); /* DMA layout of queue heads */ struct ci13xxx_qh { @@ -57,7 +57,7 @@ struct ci13xxx_qh { /* 9 */ u32 RESERVED; struct usb_ctrlrequest setup; -} __attribute__ ((packed)); +} __attribute__ ((packed, aligned(4))); /** * struct ci13xxx_req - usb request representation -- 1.8.2.rc2 -- 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 v6 6/6] usb: chipidea: udc: move _ep_queue into an unlocked function
There is no need to call ep_queue unlocked inside the own driver. We move its functionionality into an unlocked version. This patch removes potential unlocked timeslots inside isr_setup_status_phase and isr_get_status_response, in which the lock got released just before acquired again inside usb_ep_queue. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de --- drivers/usb/chipidea/udc.c | 118 ++--- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index e7c84ab..61afd71 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -671,6 +671,71 @@ static void isr_get_status_complete(struct usb_ep *ep, struct usb_request *req) } /** + * _ep_queue: queues (submits) an I/O request to an endpoint + * + * Caller must hold lock + */ +static int _ep_queue(struct usb_ep *ep, struct usb_request *req, + gfp_t __maybe_unused gfp_flags) +{ + struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); + struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); + struct ci13xxx *ci = mEp-ci; + int retval = 0; + + if (ep == NULL || req == NULL || mEp-ep.desc == NULL) + return -EINVAL; + + if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { + if (req-length) + mEp = (ci-ep0_dir == RX) ? + ci-ep0out : ci-ep0in; + if (!list_empty(mEp-qh.queue)) { + _ep_nuke(mEp); + retval = -EOVERFLOW; + dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, +_usb_addr(mEp)); + } + } + + if (usb_endpoint_xfer_isoc(mEp-ep.desc)) { + if (mReq-req.length mEp-ep.maxpacket) + return -EMSGSIZE; + } + + /* first nuke then test link, e.g. previous status has not sent */ + if (!list_empty(mReq-queue)) { + retval = -EBUSY; + dev_err(mEp-ci-dev, request already in queue\n); + goto done; + } + + if (req-length 4 * CI13XXX_PAGE_SIZE) { + retval = -EMSGSIZE; + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; + } + + dbg_queue(_usb_addr(mEp), req, retval); + + /* push request */ + mReq-req.status = -EINPROGRESS; + mReq-req.actual = 0; + + retval = _hardware_enqueue(mEp, mReq); + + if (retval == -EALREADY) { + dbg_event(_usb_addr(mEp), QUEUE, retval); + retval = 0; + } + if (!retval) + list_add_tail(mReq-queue, mEp-qh.queue); + + done: + return retval; +} + +/** * isr_get_status_response: get_status request response * @ci: ci struct * @setup: setup request packet @@ -717,9 +782,7 @@ __acquires(mEp-lock) } /* else do nothing; reserved for future use */ - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, req, gfp_flags); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, req, gfp_flags); if (retval) goto err_free_buf; @@ -766,8 +829,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct usb_request *req) * This function returns an error code */ static int isr_setup_status_phase(struct ci13xxx *ci) -__releases(mEp-lock) -__acquires(mEp-lock) { int retval; struct ci13xxx_ep *mEp; @@ -776,9 +837,7 @@ __acquires(mEp-lock) ci-status-context = ci; ci-status-complete = isr_setup_status_complete; - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC); return retval; } @@ -1177,8 +1236,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t __maybe_unused gfp_flags) { struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); - struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); - struct ci13xxx *ci = mEp-ci; int retval = 0; unsigned long flags; @@ -1187,47 +1244,8 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, spin_lock_irqsave(mEp-lock, flags); - if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { - if (req-length) - mEp = (ci-ep0_dir == RX) ? - ci-ep0out : ci-ep0in; - if (!list_empty(mEp-qh.queue)) { - _ep_nuke(mEp); - retval = -EOVERFLOW; - dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, -_usb_addr(mEp)); - } - } - -
Re: [PATCH v6 0/6] usb: chipidea: udc: bugfixes
Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4 Didn't you forget Felipe's reviewed-bys? Regards, -- Alex -- 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 v6 0/6] usb: chipidea: udc: bugfixes
On Wed, Mar 27, 2013 at 06:06:51PM +0200, Alexander Shishkin wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4 Didn't you forget Felipe's reviewed-bys? Yes, i will repost v7 in a minute. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/6] usb: chipidea: udc: move ZLT flag change to ep_enable
Its not necessary and also not specified in the datasheet to change the ZLT flag before every ep_prime. This patch moves this to the ep_enable and applies it only for non configuration endpoints. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/usb/chipidea/udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 4bbbe62..a8528ad 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -489,7 +489,6 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) /* QH configuration */ mEp-qh.ptr-td.next = mReq-dma;/* TERMINATE = 0 */ mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE); - mEp-qh.ptr-cap |= QH_ZLT; wmb(); /* synchronize before ep prime */ @@ -1052,6 +1051,8 @@ static int ep_enable(struct usb_ep *ep, mEp-qh.ptr-cap = ~QH_MULT; else mEp-qh.ptr-cap = ~QH_ZLT; + if (mEp-num) + mEp-qh.ptr-cap |= QH_ZLT; mEp-qh.ptr-cap |= (mEp-ep.maxpacket ffs_nr(QH_MAX_PKT)) QH_MAX_PKT; -- 1.8.2.rc2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 6/6] usb: chipidea: udc: move _ep_queue into an unlocked function
There is no need to call ep_queue unlocked inside the own driver. We move its functionionality into an unlocked version. This patch removes potential unlocked timeslots inside isr_setup_status_phase and isr_get_status_response, in which the lock got released just before acquired again inside usb_ep_queue. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de Reviewed-by: Peter Chen peter.c...@freescale.com --- drivers/usb/chipidea/udc.c | 118 ++--- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index e7c84ab..61afd71 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -671,6 +671,71 @@ static void isr_get_status_complete(struct usb_ep *ep, struct usb_request *req) } /** + * _ep_queue: queues (submits) an I/O request to an endpoint + * + * Caller must hold lock + */ +static int _ep_queue(struct usb_ep *ep, struct usb_request *req, + gfp_t __maybe_unused gfp_flags) +{ + struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); + struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); + struct ci13xxx *ci = mEp-ci; + int retval = 0; + + if (ep == NULL || req == NULL || mEp-ep.desc == NULL) + return -EINVAL; + + if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { + if (req-length) + mEp = (ci-ep0_dir == RX) ? + ci-ep0out : ci-ep0in; + if (!list_empty(mEp-qh.queue)) { + _ep_nuke(mEp); + retval = -EOVERFLOW; + dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, +_usb_addr(mEp)); + } + } + + if (usb_endpoint_xfer_isoc(mEp-ep.desc)) { + if (mReq-req.length mEp-ep.maxpacket) + return -EMSGSIZE; + } + + /* first nuke then test link, e.g. previous status has not sent */ + if (!list_empty(mReq-queue)) { + retval = -EBUSY; + dev_err(mEp-ci-dev, request already in queue\n); + goto done; + } + + if (req-length 4 * CI13XXX_PAGE_SIZE) { + retval = -EMSGSIZE; + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; + } + + dbg_queue(_usb_addr(mEp), req, retval); + + /* push request */ + mReq-req.status = -EINPROGRESS; + mReq-req.actual = 0; + + retval = _hardware_enqueue(mEp, mReq); + + if (retval == -EALREADY) { + dbg_event(_usb_addr(mEp), QUEUE, retval); + retval = 0; + } + if (!retval) + list_add_tail(mReq-queue, mEp-qh.queue); + + done: + return retval; +} + +/** * isr_get_status_response: get_status request response * @ci: ci struct * @setup: setup request packet @@ -717,9 +782,7 @@ __acquires(mEp-lock) } /* else do nothing; reserved for future use */ - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, req, gfp_flags); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, req, gfp_flags); if (retval) goto err_free_buf; @@ -766,8 +829,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct usb_request *req) * This function returns an error code */ static int isr_setup_status_phase(struct ci13xxx *ci) -__releases(mEp-lock) -__acquires(mEp-lock) { int retval; struct ci13xxx_ep *mEp; @@ -776,9 +837,7 @@ __acquires(mEp-lock) ci-status-context = ci; ci-status-complete = isr_setup_status_complete; - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC); return retval; } @@ -1177,8 +1236,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t __maybe_unused gfp_flags) { struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); - struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); - struct ci13xxx *ci = mEp-ci; int retval = 0; unsigned long flags; @@ -1187,47 +1244,8 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, spin_lock_irqsave(mEp-lock, flags); - if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { - if (req-length) - mEp = (ci-ep0_dir == RX) ? - ci-ep0out : ci-ep0in; - if (!list_empty(mEp-qh.queue)) { - _ep_nuke(mEp); - retval = -EOVERFLOW; - dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, -
[PATCH v7 5/6] usb: chipidea: udc: don't truncate requests to single tds
It is not safe to truncate requests to the maximum possible size the controller can handle with one td and to keep working. That patch fixes that with proper error handling instead. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index d4db3ac..e7c84ab 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1207,9 +1207,9 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, } if (req-length 4 * CI13XXX_PAGE_SIZE) { - req-length = 4 * CI13XXX_PAGE_SIZE; retval = -EMSGSIZE; - dev_warn(mEp-ci-dev, request length truncated\n); + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; } dbg_queue(_usb_addr(mEp), req, retval); -- 1.8.2.rc2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/6] usb: chipidea: udc: bugfixes
Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function drivers/usb/chipidea/udc.c | 133 ++--- drivers/usb/chipidea/udc.h | 4 +- 2 files changed, 80 insertions(+), 57 deletions(-) -- 1.8.2.rc2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/6] usb: chipidea: udc: only clear active and halted bits in qhead
The datasheet of the synopsys core describes only to overwrite the active and halted bits in the qhead before priming any endpoint. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/usb/chipidea/udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 77fb66f..4bbbe62 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -488,7 +488,7 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) /* QH configuration */ mEp-qh.ptr-td.next = mReq-dma;/* TERMINATE = 0 */ - mEp-qh.ptr-td.token = ~TD_STATUS; /* clear status */ + mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE); mEp-qh.ptr-cap |= QH_ZLT; wmb(); /* synchronize before ep prime */ -- 1.8.2.rc2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs
The udc uses an shared dma memory space between hard and software. This memory layout is described in ci13xxx_qh and ci13xxx_td which are marked with the attribute ((packed)). The compiler currently does not know about the alignment of the memory layout, and will create strb and ldrb operations. The Datasheet of the synopsys core describes, that some operations on the mapped memory need to be atomic double word operations. I.e. the next pointer addressing in the qhead, as otherwise the hardware could read wrong data and totally stuck. This patch adds the attribute ((aligned(4))) to the structures to tell the compiler to use 32bit operations. It also adds an wmb() for the prepared TD data before it gets enqueued into the qhead. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/usb/chipidea/udc.c | 2 ++ drivers/usb/chipidea/udc.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index f64fbea..77fb66f 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -461,6 +461,8 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) mReq-ptr-page[i] = (mReq-req.dma + i * CI13XXX_PAGE_SIZE) ~TD_RESERVED_MASK; + wmb(); + if (!list_empty(mEp-qh.queue)) { struct ci13xxx_req *mReqPrev; int n = hw_ep_bit(mEp-num, mEp-dir); diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h index 4ff2384d..d12e8b5 100644 --- a/drivers/usb/chipidea/udc.h +++ b/drivers/usb/chipidea/udc.h @@ -40,7 +40,7 @@ struct ci13xxx_td { #define TD_CURR_OFFSET(0x0FFFUL 0) #define TD_FRAME_NUM (0x07FFUL 0) #define TD_RESERVED_MASK (0x0FFFUL 0) -} __attribute__ ((packed)); +} __attribute__ ((packed, aligned(4))); /* DMA layout of queue heads */ struct ci13xxx_qh { @@ -57,7 +57,7 @@ struct ci13xxx_qh { /* 9 */ u32 RESERVED; struct usb_ctrlrequest setup; -} __attribute__ ((packed)); +} __attribute__ ((packed, aligned(4))); /** * struct ci13xxx_req - usb request representation -- 1.8.2.rc2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/6] usb: chipidea: udc: read status of td only once in hardware_dequeue
This patch changes the read of the td status to one atomic operation to analyse coherent bits. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index a8528ad..d4db3ac 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -507,10 +507,12 @@ done: */ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) { + u32 tmptoken = mReq-ptr-token; + if (mReq-req.status != -EALREADY) return -EINVAL; - if ((TD_STATUS_ACTIVE mReq-ptr-token) != 0) + if ((TD_STATUS_ACTIVE tmptoken) != 0) return -EBUSY; if (mReq-zptr) { @@ -524,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) usb_gadget_unmap_request(mEp-ci-gadget, mReq-req, mEp-dir); - mReq-req.status = mReq-ptr-token TD_STATUS; + mReq-req.status = tmptoken TD_STATUS; if ((TD_STATUS_HALTED mReq-req.status) != 0) mReq-req.status = -1; else if ((TD_STATUS_DT_ERR mReq-req.status) != 0) @@ -532,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) else if ((TD_STATUS_TR_ERR mReq-req.status) != 0) mReq-req.status = -1; - mReq-req.actual = mReq-ptr-token TD_TOTAL_BYTES; + mReq-req.actual = tmptoken TD_TOTAL_BYTES; mReq-req.actual = ffs_nr(TD_TOTAL_BYTES); mReq-req.actual = mReq-req.length - mReq-req.actual; mReq-req.actual = mReq-req.status ? 0 : mReq-req.actual; -- 1.8.2.rc2 -- 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: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)
On 26 March 2013 18:50, Alan Stern st...@rowland.harvard.edu wrote: On Tue, 26 Mar 2013, David Linares wrote: The device I am working on is an embedded device, therefore the wireless adapter has to be connected to the internal hub. This hub and the whole device are powered via an usb cable. Oh. Then maybe the USB cable doesn't provide enough power to run both the embedded device and the wireless adapter. There are 2 ports, so does the hub provide 500mA per port or in total? If that's 500mA for each port, that's fine. You seem to be missing the point. You said above that your entire device is powered via a USB cable. That means the entire device gets only 500 mA, total. If the hub sends 500 mA out one of its ports then there will be nothing left for the other port and nothing left for the rest of your device. Thanks Alan. I understand, but the USB cable is plugged into an AC/DC adaptor which can provide an output of 1A. After breaking out an USB cable, I measured that the consumption of my board is around 130mA (nothing connected to the hub). Then I measured with the dongle plugged in. The real consumption of the wireless adapter is around 210mA. The whole thing draws around 340mA Of course, the way it works in reality is that the device uses a significant fraction of the total current, leaving a lot less than 500 mA available for the hub. For example, the hub may get only 200 mA. There's no way it can take that input and magically convert it to 500 mA for each of two ports. I have done some progress on my side. I have noticed that in a failing case, the 2-port hub is actually detected as a standalone hub but when it works fine, it is detected as a compound device. Also printed the hubstatus that I get from here http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1513 and it is really weird: - when it works, the value returned is 1, and therefore, I fall into the Self-powered external hub case - when it doesn't work, the value seems quite random (ex: 0xe6c0) and I fall into the 100mA limit = my Ralink wireless adapter gets a rejected config. That's to be expected. When there isn't enough power, you start getting random data and things stop working. Today, I did some further tests *without* any wireless adapter. So, I have got the device plugged in into a main power socket via USB (delivering up to 1A), the root hub, and the 2-port hub and nothing else USB-wise. The problem still happens from time to time. The 2-port hub sometimes says it will give 500mA to its children and sometimes it gives only 100mA (because of a corrupted hubstatus mentioned earlier). Now that there is nothing connected on the hub, I am just wondering how come the hub is not always detected in the same way at each boot. Also, should it be detected as self powered ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1543 ) or not ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1538 )? I forgot to mention that this issue where the hub is not detected as self-powered external hub is quite hard to reproduce. It happens 1 out of 20 tries. Thanks a lot Alan and everybody else. I really appreciate the support and understanding you are providing. 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 v6 6/6] usb: chipidea: udc: move _ep_queue into an unlocked function
Michael Grzeschik m.grzesc...@pengutronix.de writes: There is no need to call ep_queue unlocked inside the own driver. We move its functionionality into an unlocked version. This patch removes potential unlocked timeslots inside isr_setup_status_phase and isr_get_status_response, in which the lock got released just before acquired again inside usb_ep_queue. Cc: stable sta...@vger.kernel.org Other patches in this set are less questionable, but this is certainly not stable material. See Documentation/stable_kernel_rules.txt Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de --- drivers/usb/chipidea/udc.c | 118 ++--- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index e7c84ab..61afd71 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -671,6 +671,71 @@ static void isr_get_status_complete(struct usb_ep *ep, struct usb_request *req) } /** + * _ep_queue: queues (submits) an I/O request to an endpoint + * + * Caller must hold lock + */ +static int _ep_queue(struct usb_ep *ep, struct usb_request *req, + gfp_t __maybe_unused gfp_flags) +{ + struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); + struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); + struct ci13xxx *ci = mEp-ci; + int retval = 0; + + if (ep == NULL || req == NULL || mEp-ep.desc == NULL) + return -EINVAL; + + if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { + if (req-length) + mEp = (ci-ep0_dir == RX) ? +ci-ep0out : ci-ep0in; + if (!list_empty(mEp-qh.queue)) { + _ep_nuke(mEp); + retval = -EOVERFLOW; + dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, + _usb_addr(mEp)); + } + } + + if (usb_endpoint_xfer_isoc(mEp-ep.desc)) { + if (mReq-req.length mEp-ep.maxpacket) + return -EMSGSIZE; + } + + /* first nuke then test link, e.g. previous status has not sent */ + if (!list_empty(mReq-queue)) { + retval = -EBUSY; + dev_err(mEp-ci-dev, request already in queue\n); + goto done; + } + + if (req-length 4 * CI13XXX_PAGE_SIZE) { + retval = -EMSGSIZE; + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; + } + + dbg_queue(_usb_addr(mEp), req, retval); + + /* push request */ + mReq-req.status = -EINPROGRESS; + mReq-req.actual = 0; + + retval = _hardware_enqueue(mEp, mReq); + + if (retval == -EALREADY) { + dbg_event(_usb_addr(mEp), QUEUE, retval); + retval = 0; + } + if (!retval) + list_add_tail(mReq-queue, mEp-qh.queue); + + done: + return retval; +} + +/** * isr_get_status_response: get_status request response * @ci: ci struct * @setup: setup request packet @@ -717,9 +782,7 @@ __acquires(mEp-lock) } /* else do nothing; reserved for future use */ - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, req, gfp_flags); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, req, gfp_flags); if (retval) goto err_free_buf; @@ -766,8 +829,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct usb_request *req) * This function returns an error code */ static int isr_setup_status_phase(struct ci13xxx *ci) -__releases(mEp-lock) -__acquires(mEp-lock) { int retval; struct ci13xxx_ep *mEp; @@ -776,9 +837,7 @@ __acquires(mEp-lock) ci-status-context = ci; ci-status-complete = isr_setup_status_complete; - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC); return retval; } @@ -1177,8 +1236,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t __maybe_unused gfp_flags) { struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); - struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); - struct ci13xxx *ci = mEp-ci; int retval = 0; unsigned long flags; @@ -1187,47 +1244,8 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, spin_lock_irqsave(mEp-lock, flags); - if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { - if (req-length) - mEp = (ci-ep0_dir == RX) ? -ci-ep0out : ci-ep0in; - if (!list_empty(mEp-qh.queue)) { - _ep_nuke(mEp); -
Re: [PATCH v7 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs
On Wed, Mar 27, 2013 at 05:21:13PM +0100, Michael Grzeschik wrote: The udc uses an shared dma memory space between hard and software. This memory layout is described in ci13xxx_qh and ci13xxx_td which are marked with the attribute ((packed)). The compiler currently does not know about the alignment of the memory layout, and will create strb and ldrb operations. The Datasheet of the synopsys core describes, that some operations on the mapped memory need to be atomic double word operations. I.e. the next pointer addressing in the qhead, as otherwise the hardware could read wrong data and totally stuck. This patch adds the attribute ((aligned(4))) to the structures to tell the compiler to use 32bit operations. It also adds an wmb() for the prepared TD data before it gets enqueued into the qhead. Cc: stable sta...@vger.kernel.org How does this (and the other patches in this series) meet the stable_kernel_rules.txt requirements? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes
Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function Actually, do any of these patches fix tangible bugs, that can be reproduced? If so, you should certainly mention that. But somehow I doubt that, because otherwise the driver will be totally unusable. What I see is patches that bring some parts of udc code in accordance with the spec, which is a good thing to do, but doesn't fix any real bugs that bother people. Or does it? Regards, -- Alex -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes
On 03/27/2013 05:35 PM, Alexander Shishkin wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function Actually, do any of these patches fix tangible bugs, that can be reproduced? If so, you should certainly mention that. But somehow I doubt that, because otherwise the driver will be totally unusable. What I see is patches that bring some parts of udc code in accordance with the spec, which is a good thing to do, but doesn't fix any real bugs that bother people. Or does it? Patch 1/7 is essential to make the udc stable on armv5, should be added to the patch description, though. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes
On 03/27/2013 05:37 PM, Marc Kleine-Budde wrote: On 03/27/2013 05:35 PM, Alexander Shishkin wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function Actually, do any of these patches fix tangible bugs, that can be reproduced? If so, you should certainly mention that. But somehow I doubt that, because otherwise the driver will be totally unusable. What I see is patches that bring some parts of udc code in accordance with the spec, which is a good thing to do, but doesn't fix any real bugs that bother people. Or does it? Patch 1/7 is essential to make the udc stable on armv5, should be added to the patch description, though. I mean 1/6. usb: chipidea: udc: add attribute aligned(4) to shared memory structs Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes
On Wed, Mar 27, 2013 at 06:35:30PM +0200, Alexander Shishkin wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function Actually, do any of these patches fix tangible bugs, that can be reproduced? If so, you should certainly mention that. But somehow I doubt that, because otherwise the driver will be totally unusable. What I see is patches that bring some parts of udc code in accordance with the spec, which is a good thing to do, but doesn't fix any real bugs that bother people. Or does it? Obviously we were hunting a real issue while writing that patches. The most essential one is Patch 1/6, which definitely is a stable patch. I will clearify that in the patch description. The others though should go into v3.10, agreed? Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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/4] usb: remote_wakeup code clean up and usb port related feature
PATCH 1 is for code clean up. PATCH 2~3 is to introduce usb port power off new feature PATCH 4 is to fix usb port doesn't register to any bus_type usb: add usb_enable/disable_remote_wakeup() usb: introduce usb force power off mechanism usb: Add usb port system pm support usb: register usb port to usb_bus_type -- 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/4] usb: add usb_enable/disable_remote_wakeup()
usb2.0 and usb3.0 devices have different ways to enalbe/disable remote wakeup. This patch is to put both their operations into the seperate functions. Otherwise, usb_control_msg() has some long arguments and are usually nested some indentations. So encapsulating it into seperate functions would be convienient to use and more readable. Signed-off-by: Lan Tianyu tianyu@intel.com --- drivers/usb/core/hub.c | 136 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 7815462..6b7f5b9 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2826,22 +2826,72 @@ void usb_enable_ltm(struct usb_device *udev) EXPORT_SYMBOL_GPL(usb_enable_ltm); #ifdef CONFIG_USB_SUSPEND -/* - * usb_disable_function_remotewakeup - disable usb3.0 - * device's function remote wakeup - * @udev: target device - * - * Assume there's only one function on the USB 3.0 - * device and disable remote wake for the first - * interface. FIXME if the interface association - * descriptor shows there's more than one function. - */ -static int usb_disable_function_remotewakeup(struct usb_device *udev) +static int usb_disable_remote_wakeup(struct usb_device *udev) { - return usb_control_msg(udev, usb_sndctrlpipe(udev, 0), - USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE, - USB_INTRF_FUNC_SUSPEND, 0, NULL, 0, + int status = 0; + u16 devstatus; + + if (udev-speed != USB_SPEED_SUPER) { + status = usb_get_status(udev, USB_RECIP_DEVICE, 0, devstatus); + le16_to_cpus(devstatus); + if (!status devstatus (1 USB_DEVICE_REMOTE_WAKEUP)) + status = usb_control_msg(udev, + usb_sndctrlpipe(udev, 0), + USB_REQ_CLEAR_FEATURE, + USB_RECIP_DEVICE, + USB_DEVICE_REMOTE_WAKEUP, 0, + NULL, 0, + USB_CTRL_SET_TIMEOUT); + } else { + /* +* Assume there's only one function on the USB 3.0 +* device and disable remote wake for the first +* interface. FIXME if the interface association +* descriptor shows there's more than one function. +*/ + status = usb_get_status(udev, USB_RECIP_INTERFACE, 0, + devstatus); + le16_to_cpus(devstatus); + if (!status devstatus + (USB_INTRF_STAT_FUNC_RW_CAP | USB_INTRF_STAT_FUNC_RW)) + status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), + USB_REQ_CLEAR_FEATURE, + USB_RECIP_INTERFACE, + USB_INTRF_FUNC_SUSPEND, 0, + NULL, 0, + USB_CTRL_SET_TIMEOUT); + } + + return status; +} + +static int usb_enable_remote_wakeup(struct usb_device *udev) +{ + int status; + + if (udev-speed != USB_SPEED_SUPER) { + status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), + USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, + USB_DEVICE_REMOTE_WAKEUP, 0, + NULL, 0, + USB_CTRL_SET_TIMEOUT); + } else { + /* Assume there's only one function on the USB 3.0 +* device and enable remote wake for the first +* interface. FIXME if the interface association +* descriptor shows there's more than one function. +*/ + status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), + USB_REQ_SET_FEATURE, + USB_RECIP_INTERFACE, + USB_INTRF_FUNC_SUSPEND, + USB_INTRF_FUNC_SUSPEND_RW | + USB_INTRF_FUNC_SUSPEND_LP, + NULL, 0, USB_CTRL_SET_TIMEOUT); + } + + return status; } /* @@ -2905,27 +2955,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) * we don't explicitly enable it here. */ if (udev-do_remote_wakeup) { - if (!hub_is_superspeed(hub-hdev)) { - status = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), - USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, - USB_DEVICE_REMOTE_WAKEUP, 0, - NULL, 0, -
[PATCH 3/4] usb: Add usb port system pm support
This patch is to add usb port system pm support. Add usb port's system suspend/resume callbacks and call usb_port_runtime_resume/suspend() to power off these ports whose pm qos NO_POWER_OFF flag is not set, system wakeup is disabled and persist is enalbed. During system pm, usb port should be powered off after dev being suspended and powered on before dev being resumed. Since usb ports and devs enable async suspend, call device_pm_wait_for_dev() in the usb_port_system_suspend() and usb_port_resume() to keeping the order. If usb port was already powered off by runtime pm with port_dev-power_is_on being false, usb_port_system_suspend() returns directly. If usb port was not powered off during system suspend with port_dev-power_is_on being true, usb_port_system_resume() returns directly. Signed-off-by: Lan Tianyu tianyu@intel.com --- drivers/usb/core/hub.c |4 drivers/usb/core/port.c | 49 --- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c228e69..8c2562e 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3166,6 +3166,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) int status; u16 portchange, portstatus; + /* Wait for usb port system resume finishing */ + if (!PMSG_IS_AUTO(msg)) + device_pm_wait_for_dev(udev-dev, port_dev-dev); + if (port_dev-did_runtime_put) { status = pm_runtime_get_sync(port_dev-dev); port_dev-did_runtime_put = false; diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 797f9d5..f14fc72 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -71,7 +71,7 @@ static void usb_port_device_release(struct device *dev) kfree(port_dev); } -#ifdef CONFIG_USB_SUSPEND +#ifdef CONFIG_PM static int usb_port_runtime_resume(struct device *dev) { struct usb_port *port_dev = to_usb_port(dev); @@ -131,25 +131,68 @@ static int usb_port_runtime_suspend(struct device *dev) set_bit(port1, hub-busy_bits); retval = usb_hub_set_port_power(hdev, port1, false); usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION); - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); + usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); clear_bit(port1, hub-busy_bits); usb_autopm_put_interface(intf); return retval; } -#endif + +static int usb_port_system_suspend(struct device *dev) +{ + struct usb_port *port_dev = to_usb_port(dev); + + if (!port_dev-power_is_on) + return 0; + + if (port_dev-child) { + struct usb_device *udev = port_dev-child; + + /* +* usb port can't be powered off when dev's system +* wakeup is enabled or persist is disabled. +*/ + if (device_may_wakeup(udev-dev) + || !udev-persist_enabled) + return 0; + + /* +* usb port should be powered off after usb dev +* being suspended. +*/ + device_pm_wait_for_dev(dev, port_dev-child-dev); + } + + usb_port_runtime_suspend(dev); + return 0; +} + +static int usb_port_system_resume(struct device *dev) +{ + struct usb_port *port_dev = to_usb_port(dev); + + if (port_dev-power_is_on) + return 0; + + return usb_port_runtime_resume(dev); +} static const struct dev_pm_ops usb_port_pm_ops = { + .suspend = usb_port_system_suspend, + .resume = usb_port_system_resume, #ifdef CONFIG_USB_SUSPEND .runtime_suspend = usb_port_runtime_suspend, .runtime_resume = usb_port_runtime_resume, .runtime_idle = pm_generic_runtime_idle, #endif }; +#endif struct device_type usb_port_device_type = { .name = usb_port, .release = usb_port_device_release, +#if CONFIG_PM .pm = usb_port_pm_ops, +#endif }; int usb_hub_create_port_device(struct usb_hub *hub, int port1) -- 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/4] usb: register usb port to usb_bus_type
Usb port isn't assigned to any bus_type. This seems not good from Greg's comments. http://marc.info/?l=linux-usbm=136200364929942w=2 This patch is to register usb port to usb_bus_type. The usb port's original name is portX. This will cause name confilct after adding usb port to usb_bus_type since the usb ports with same port num under different hub have the same name. So change the usb port's name format to port + (hub dev name) + '.' + (port num) for non-root hub and port + (usb bus num) + '-' + (port num) for root hub. ls /sys/bus/usb/devices 1-0:1.02-0:1.0 port1-1 port1-1.3 port2-1.2 port2-2 port4-3 1-12-1 port1-1.1port1-1.4 port2-1.3 port3-1 port4-4 1-1.1 2-1:1.0 port1-1.2port1-1.5 port2-1.4 port3-2 usb1 1-1:1.03-0:1.0 port1-1.2.1 port1-1.6 port2-1.5 port3-3 usb2 1-1.1:1.0 3-1 port1-1.2.2 port1-2port2-1.6 port3-4 usb3 1-1.2 3-1:1.0 port1-1.2.3 port2-1port2-1.7 port4-1 usb4 1-1.2:1.0 4-0:1.0 port1-1.2.4 port2-1.1 port2-1.8 port4-2 Signed-off-by: Lan Tianyu tianyu@intel.com --- Documentation/ABI/testing/sysfs-bus-usb |6 +++--- drivers/usb/core/port.c | 10 +- drivers/usb/core/usb-acpi.c |7 +-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index c8baaf5..842e8b8 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -221,14 +221,14 @@ Description: The file will be present for all speeds of USB devices, and will always read no for USB 1.1 and USB 2.0 devices. -What: /sys/bus/usb/devices/.../(hub interface)/portX +What: /sys/bus/usb/devices/.../(hub interface)/portX-X Date: August 2012 Contact: Lan Tianyu tianyu@intel.com Description: - The /sys/bus/usb/devices/.../(hub interface)/portX + The /sys/bus/usb/devices/.../(hub interface)/portX-X is usb port device's sysfs directory. -What: /sys/bus/usb/devices/.../(hub interface)/portX/connect_type +What: /sys/bus/usb/devices/.../(hub interface)/portX-X/connect_type Date: January 2013 Contact: Lan Tianyu tianyu@intel.com Description: diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index f14fc72..2c086dc 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -198,6 +198,7 @@ struct device_type usb_port_device_type = { int usb_hub_create_port_device(struct usb_hub *hub, int port1) { struct usb_port *port_dev = NULL; + struct usb_device *hdev = hub-hdev; int retval; port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL); @@ -212,7 +213,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) port_dev-dev.parent = hub-intfdev; port_dev-dev.groups = port_dev_group; port_dev-dev.type = usb_port_device_type; - dev_set_name(port_dev-dev, port%d, port1); + port_dev-dev.bus = usb_bus_type; + + if (!hdev-parent) + dev_set_name(port_dev-dev, port%d-%d, + hdev-bus-busnum, port1); + else + dev_set_name(port_dev-dev, port%s.%d, + dev_name(hdev-dev), port1); retval = device_register(port_dev-dev); if (retval) diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c index b6f4bad..1cc55c9 100644 --- a/drivers/usb/core/usb-acpi.c +++ b/drivers/usb/core/usb-acpi.c @@ -17,7 +17,7 @@ #include linux/pci.h #include acpi/acpi_bus.h -#include usb.h +#include hub.h /** * usb_acpi_power_manageable - check whether usb port has @@ -178,7 +178,10 @@ static int usb_acpi_find_device(struct device *dev, acpi_handle *handle) return -ENODEV; return 0; } else if (is_usb_port(dev)) { - sscanf(dev_name(dev), port%d, port_num); + struct usb_port *port_dev = to_usb_port(dev); + + port_num = port_dev-portnum; + /* Get the struct usb_device point of port's hub */ udev = to_usb_device(dev-parent-parent); -- 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/4] usb: introduce usb force power off mechanism
Some devices' firmware will be broken at some points. Power down and power on device can help device to rework in this case. This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node to repower usb device. First, call hub_port_logical_disconnect() to disconnect device. Second, Power down and up usb port. This patch is also helpful fo some QAs who want to do hcd's memleak test(Plug and unplug device thousand times.) Signed-off-by: Lan Tianyu tianyu@intel.com --- drivers/usb/core/devio.c | 13 + drivers/usb/core/hub.c| 16 drivers/usb/core/usb.h|2 ++ include/uapi/linux/usbdevice_fs.h |1 + 4 files changed, 32 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 8823e98..a76b326 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1102,6 +1102,14 @@ static int proc_resetdevice(struct dev_state *ps) return usb_reset_device(ps-dev); } +static int proc_resetdevicepower(struct dev_state *ps) +{ + struct usb_device *udev = ps-dev; + struct usb_device *hdev = udev-parent; + + return usb_hub_port_power_reset(hdev, udev-portnum); +} + static int proc_setintf(struct dev_state *ps, void __user *arg) { struct usbdevfs_setinterface setintf; @@ -2011,6 +2019,11 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, ret = proc_resetdevice(ps); break; + case USBDEVFS_POWER_RESET: + snoop(dev-dev, %s: RESET POWER\n, __func__); + ret = proc_resetdevicepower(ps); + break; + case USBDEVFS_CLEAR_HALT: snoop(dev-dev, %s: CLEAR_HALT\n, __func__); ret = proc_clearhalt(ps, p); diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 6b7f5b9..c228e69 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -114,6 +114,7 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem); #define HUB_DEBOUNCE_STABLE 100 static int usb_reset_and_verify_device(struct usb_device *udev); +static void hub_port_logical_disconnect(struct usb_hub *hub, int port1); static inline char *portspeed(struct usb_hub *hub, int portstatus) { @@ -741,6 +742,21 @@ int usb_hub_set_port_power(struct usb_device *hdev, int port1, return ret; } +int usb_hub_port_power_reset(struct usb_device *hdev, int port1) +{ + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_interface *intf = to_usb_interface(hub-intfdev); + int ret; + + usb_autopm_get_interface(intf); + hub_port_logical_disconnect(hub, port1); + ret = usb_hub_set_port_power(hdev, port1, false); + ret |= usb_hub_set_port_power(hdev, port1, true); + usb_autopm_put_interface(intf); + + return ret; +} + /** * usb_hub_clear_tt_buffer - clear control/bulk TT state in high speed hub * @urb: an URB associated with the failed or incomplete split transaction diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index a7f20bd..4e7785c 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -185,6 +185,8 @@ extern void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1, enum usb_port_connect_type type); extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev, struct usb_hub_descriptor *desc); +extern int usb_hub_port_power_reset(struct usb_device *hdev, + int port1); #ifdef CONFIG_ACPI extern int usb_acpi_register(void); diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index 0c65e4b..b6e0d17 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -176,5 +176,6 @@ struct usbdevfs_disconnect_claim { #define USBDEVFS_RELEASE_PORT _IOR('U', 25, unsigned int) #define USBDEVFS_GET_CAPABILITIES _IOR('U', 26, __u32) #define USBDEVFS_DISCONNECT_CLAIM _IOR('U', 27, struct usbdevfs_disconnect_claim) +#define USBDEVFS_POWER_RESET _IO('U', 28) #endif /* _UAPI_LINUX_USBDEVICE_FS_H */ -- 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 v7 1/6] usb: chipidea: udc: add attribute aligned(4) to shared memory structs
On Wed, Mar 27, 2013 at 09:31:33AM -0700, Greg KH wrote: On Wed, Mar 27, 2013 at 05:21:13PM +0100, Michael Grzeschik wrote: The udc uses an shared dma memory space between hard and software. This memory layout is described in ci13xxx_qh and ci13xxx_td which are marked with the attribute ((packed)). The compiler currently does not know about the alignment of the memory layout, and will create strb and ldrb operations. The Datasheet of the synopsys core describes, that some operations on the mapped memory need to be atomic double word operations. I.e. the next pointer addressing in the qhead, as otherwise the hardware could this should be will ---^ read wrong data and totally stuck. This patch adds the attribute ((aligned(4))) to the structures to tell the compiler to use 32bit operations. It also adds an wmb() for the prepared TD data before it gets enqueued into the qhead. Cc: stable sta...@vger.kernel.org How does this (and the other patches in this series) meet the stable_kernel_rules.txt requirements? Beside the other patches, this one meets the requirements. I will reword that patch, leave the Cc: stable in it, remove it from the others and repost the series. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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/4] usb: introduce usb force power off mechanism
A small tool to test this patch. #include stdio.h #include unistd.h #include fcntl.h #include errno.h #include sys/ioctl.h #include linux/usbdevice_fs.h #define USBDEVFS_POWER_RESET _IO('U', 28) int main(int argc, char **argv) { const char *filename; int fd; int rc; if (argc != 2) { fprintf(stderr, Usage: usbreset device-filename\n); return 1; } filename = argv[1]; fd = open(filename, O_WRONLY); if (fd 0) { perror(Error opening output file); return 1; } printf(Repowering USB device %s\n, filename); rc = ioctl(fd, USBDEVFS_POWER_RESET, 0); if (rc 0) { perror(Error in ioctl); return 1; } printf(Repower successful\n); close(fd); return 0; } Best Regards Tianyu Lan -Original Message- From: Lan, Tianyu Sent: Thursday, March 28, 2013 1:11 AM To: gre...@linuxfoundation.org; sarah.a.sh...@linux.intel.com; st...@rowland.harvard.edu Cc: linux-usb@vger.kernel.org; Lan, Tianyu Subject: [PATCH 2/4] usb: introduce usb force power off mechanism Some devices' firmware will be broken at some points. Power down and power on device can help device to rework in this case. This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node to repower usb device. First, call hub_port_logical_disconnect() to disconnect device. Second, Power down and up usb port. This patch is also helpful fo some QAs who want to do hcd's memleak test(Plug and unplug device thousand times.) Signed-off-by: Lan Tianyu tianyu@intel.com --- drivers/usb/core/devio.c | 13 + drivers/usb/core/hub.c| 16 drivers/usb/core/usb.h|2 ++ include/uapi/linux/usbdevice_fs.h |1 + 4 files changed, 32 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 8823e98..a76b326 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1102,6 +1102,14 @@ static int proc_resetdevice(struct dev_state *ps) return usb_reset_device(ps-dev); } +static int proc_resetdevicepower(struct dev_state *ps) { + struct usb_device *udev = ps-dev; + struct usb_device *hdev = udev-parent; + + return usb_hub_port_power_reset(hdev, udev-portnum); } + static int proc_setintf(struct dev_state *ps, void __user *arg) { struct usbdevfs_setinterface setintf; @@ -2011,6 +2019,11 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, ret = proc_resetdevice(ps); break; + case USBDEVFS_POWER_RESET: + snoop(dev-dev, %s: RESET POWER\n, __func__); + ret = proc_resetdevicepower(ps); + break; + case USBDEVFS_CLEAR_HALT: snoop(dev-dev, %s: CLEAR_HALT\n, __func__); ret = proc_clearhalt(ps, p); diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 6b7f5b9..c228e69 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -114,6 +114,7 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem); #define HUB_DEBOUNCE_STABLE 100 static int usb_reset_and_verify_device(struct usb_device *udev); +static void hub_port_logical_disconnect(struct usb_hub *hub, int +port1); static inline char *portspeed(struct usb_hub *hub, int portstatus) { @@ -741,6 +742,21 @@ int usb_hub_set_port_power(struct usb_device *hdev, int port1, return ret; } +int usb_hub_port_power_reset(struct usb_device *hdev, int port1) { + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_interface *intf = to_usb_interface(hub-intfdev); + int ret; + + usb_autopm_get_interface(intf); + hub_port_logical_disconnect(hub, port1); + ret = usb_hub_set_port_power(hdev, port1, false); + ret |= usb_hub_set_port_power(hdev, port1, true); + usb_autopm_put_interface(intf); + + return ret; +} + /** * usb_hub_clear_tt_buffer - clear control/bulk TT state in high speed hub * @urb: an URB associated with the failed or incomplete split transaction diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index a7f20bd..4e7785c 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -185,6 +185,8 @@ extern void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1, enum usb_port_connect_type type); extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev, struct usb_hub_descriptor *desc); +extern int usb_hub_port_power_reset(struct usb_device *hdev, + int port1); #ifdef CONFIG_ACPI extern int usb_acpi_register(void); diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index 0c65e4b..b6e0d17 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -176,5 +176,6 @@
Re: [PATCH V4 1/2] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework
Hi Bhupesh, Thanks for the patch. Please see below for a couple of small comments. On Thursday 21 March 2013 13:56:08 Bhupesh Sharma wrote: This patch reworks the videobuffer management logic present in the UVC webcam gadget and ports it to use the more apt videobuf2 framework for video buffer management. To support routing video data captured from a real V4L2 video capture device with a zero copy operation on videobuffers (as they pass from the V4L2 domain to UVC domain via a user-space application), we need to support USER_PTR IO method at the UVC gadget side. So the V4L2 capture device driver can still continue to use MMAP IO method and now the user-space application can just pass a pointer to the video buffers being dequeued from the V4L2 device side while queueing them at the UVC gadget end. This ensures that we have a zero-copy design as the videobuffers pass from the V4L2 capture device to the UVC gadget. Note that there will still be a need to apply UVC specific payload headers on top of each UVC payload data, which will still require a copy operation to be performed in the 'encode' routines of the UVC gadget. This patch also addresses one issue found out while porting the UVC gadget to videobuf2 framework: - In case the usb requests queued by the gadget get completed with a status of -ESHUTDOWN (disconnected from host), the queue of videobuf2 should be cancelled to ensure that the application space daemon is not left in a state waiting for a vb2 to be successfully absorbed at the USB side. Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/uvc_queue.c | 533 - drivers/usb/gadget/uvc_queue.h | 25 +-- drivers/usb/gadget/uvc_v4l2.c | 37 +-- drivers/usb/gadget/uvc_video.c | 17 +- 5 files changed, 188 insertions(+), 425 deletions(-) [snip] diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c index 104ae9c..3f5eeae 100644 --- a/drivers/usb/gadget/uvc_queue.c +++ b/drivers/usb/gadget/uvc_queue.c [snip] +static int uvc_queue_init(struct uvc_video_queue *queue, + enum v4l2_buf_type type) { + int ret; + queue-queue.type = type; + queue-queue.io_modes = VB2_MMAP | VB2_USERPTR; + queue-queue.drv_priv = queue; + queue-queue.buf_struct_size = sizeof(struct uvc_buffer); + queue-queue.ops = uvc_queue_qops; + queue-queue.mem_ops = vb2_vmalloc_memops; + ret = vb2_queue_init(queue-queue); + if (ret) + return ret; + mutex_init(queue-mutex); + spin_lock_init(queue-irqlock); + INIT_LIST_HEAD(queue-irqqueue); I think you should initialize queue-flags to 0 here. + return 0; } [snip] +static int uvc_queue_buffer(struct uvc_video_queue *queue, + struct v4l2_buffer *buf) { + int ret; + + mutex_lock(queue-mutex); + ret = vb2_qbuf(queue-queue, buf); + ret = (queue-flags UVC_QUEUE_PAUSED) != 0; + queue-flags = ~UVC_QUEUE_PAUSED; Access to the queue flags need to be protected by the irqlock spinlock here (in addition to the queue mutex). + mutex_unlock(queue-mutex); + return ret; } [snip] @@ -516,26 +283,32 @@ static void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect) */ static int uvc_queue_enable(struct uvc_video_queue *queue, int enable) { - unsigned int i; + unsigned long flags; int ret = 0; mutex_lock(queue-mutex); if (enable) { - if (uvc_queue_streaming(queue)) { - ret = -EBUSY; + ret = vb2_streamon(queue-queue, queue-queue.type); + if (ret 0) goto done; - } - queue-sequence = 0; Shouldn't you keep this line ? - queue-flags |= UVC_QUEUE_STREAMING; + queue-buf_used = 0; } else { - uvc_queue_cancel(queue, 0); - INIT_LIST_HEAD(queue-mainqueue); + ret = vb2_streamoff(queue-queue, queue-queue.type); + if (ret 0) + goto done; - for (i = 0; i queue-count; ++i) - queue-buffer[i].state = UVC_BUF_STATE_IDLE; + spin_lock_irqsave(queue-irqlock, flags); + INIT_LIST_HEAD(queue-irqqueue); - queue-flags = ~UVC_QUEUE_STREAMING; + /* + * FIXME: We need to clear the DISCONNECTED flag to ensure that + * applications will be able to queue buffers for the next + * streaming run. However, clearing it here doesn't guarantee + * that the device will be reconnected in the meantime. + */ + queue-flags = ~UVC_QUEUE_DISCONNECTED; + spin_unlock_irqrestore(queue-irqlock, flags); }
Re: [PATCH V4 2/2] usb: gadget/uvc: Add support for 'get_unmapped_area' for MMUless architectures
Hi Bhupesh, Thanks for the patch. On Thursday 21 March 2013 13:56:09 Bhupesh Sharma wrote: This patch adds the support for 'get_unmapped_area' in UVC gadget which is called when the 'mmap' system call is executed on MMUless architectures. Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/usb/gadget/uvc_queue.c | 18 ++ drivers/usb/gadget/uvc_v4l2.c | 15 +++ 2 files changed, 33 insertions(+), 0 deletions(-) diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c index 3f5eeae..d0b8e38 100644 --- a/drivers/usb/gadget/uvc_queue.c +++ b/drivers/usb/gadget/uvc_queue.c @@ -228,6 +228,24 @@ static int uvc_queue_mmap(struct uvc_video_queue *queue, return ret; } +#ifndef CONFIG_MMU +/* + * Get unmapped area. + * + * NO-MMU arch need this function to make mmap() work correctly. + */ +static unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue, + unsigned long pgoff) +{ + unsigned long ret; + + mutex_lock(queue-mutex); + ret = vb2_get_unmapped_area(queue-queue, 0, 0, pgoff, 0); + mutex_unlock(queue-mutex); + return ret; +} +#endif + /* * Cancel the video buffers queue. * diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index 7d91740..b83d0ad 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -340,6 +340,18 @@ uvc_v4l2_poll(struct file *file, poll_table *wait) return uvc_queue_poll(uvc-video.queue, file, wait); } +#ifndef CONFIG_MMU +static unsigned long uvc_v4l2_get_unmapped_area(struct file *file, + unsigned long addr, unsigned long len, unsigned long pgoff, + unsigned long flags) +{ + struct video_device *vdev = video_devdata(file); + struct uvc_device *uvc = video_get_drvdata(vdev); + + return uvc_queue_get_unmapped_area(uvc-video.queue, pgoff); +} +#endif + static struct v4l2_file_operations uvc_v4l2_fops = { .owner = THIS_MODULE, .open = uvc_v4l2_open, @@ -347,5 +359,8 @@ static struct v4l2_file_operations uvc_v4l2_fops = { .ioctl = uvc_v4l2_ioctl, .mmap = uvc_v4l2_mmap, .poll = uvc_v4l2_poll, +#ifndef CONFIG_MMU + .get_unmapped_area = uvc_v4l2_get_unmapped_area, +#endif }; -- Regards, Laurent Pinchart -- 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 video capture issue due to uvc_complete callback spends more time
Hi Ravi, On Wednesday 27 March 2013 10:43:24 B, Ravi wrote: Hi I am observing issue while streaming video from usb camera connected to host controller based on mentor graphics. The issue is root caused that there are huge SOF gaps seen between the two isochronous IN token issued by host controller. This is due to fact, significant amount of time is spent in uvc callback function handler. Due to this next request programmed is delayed leads to this failure of video streaming. I have measured time taken by uvc callback function is in range minimum of 11 microsecond to maximum of 3318 microsecond. Looks like callback handler doing some processing and takes more time to return rather than giveback immediately. Need your help to understand why uvc callback handler takes much time, if it does any processing can it move to different task context and return immediately, this will reduce the latency. I'm surprised by that large delay. A quick look at the UVC URB completion handler (I assume you're talking about the host driver, not the gadget driver) doesn't show any significant source of delay. You will likely need to profile the code to find out where the problem comes from. You should also make sure that no other IRQ handler on the system keeps IRQs disabled for a long time. -- Regards, Laurent Pinchart -- 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: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)
On Wed, 27 Mar 2013, David Linares wrote: Thanks Alan. I understand, but the USB cable is plugged into an AC/DC adaptor which can provide an output of 1A. After breaking out an USB cable, I measured that the consumption of my board is around 130mA (nothing connected to the hub). Then I measured with the dongle plugged in. The real consumption of the wireless adapter is around 210mA. The whole thing draws around 340mA Ah, okay. Then maybe this isn't a power issue. Today, I did some further tests *without* any wireless adapter. So, I have got the device plugged in into a main power socket via USB (delivering up to 1A), the root hub, and the 2-port hub and nothing else USB-wise. The problem still happens from time to time. The 2-port hub sometimes says it will give 500mA to its children and sometimes it gives only 100mA (because of a corrupted hubstatus mentioned earlier). Now that there is nothing connected on the hub, I am just wondering how come the hub is not always detected in the same way at each boot. Also, should it be detected as self powered ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1543 ) or not ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1538 )? It sounds like the hub is rather flaky. Also, it should always describe itself as bus-powered, never as self-powered. Maybe you should try using a different kind of hub. I forgot to mention that this issue where the hub is not detected as self-powered external hub is quite hard to reproduce. It happens 1 out of 20 tries. This could even be a loose electrical connection, silly as that seems. You never posted any usbmon results. If you could see the actual data sent by the hub when it's not working, you might get a better idea of where the problem is. Thanks a lot Alan and everybody else. I really appreciate the support and understanding you are providing. You're welcome. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: add usb_enable/disable_remote_wakeup()
On Thu, 28 Mar 2013, Lan Tianyu wrote: usb2.0 and usb3.0 devices have different ways to enalbe/disable remote wakeup. This patch is to put both their operations into the seperate functions. Otherwise, usb_control_msg() has some long arguments and are usually nested some indentations. So encapsulating it into seperate functions would be convienient to use and more readable. Signed-off-by: Lan Tianyu tianyu@intel.com 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 2/4] usb: introduce usb force power off mechanism
On Thu, 28 Mar 2013, Lan Tianyu wrote: Some devices' firmware will be broken at some points. Power down and power on device can help device to rework in this case. This patch is to add ioctl cmd USBDEVFS_POWER_RESET for usbfs node to repower usb device. First, call hub_port_logical_disconnect() to disconnect device. Second, Power down and up usb port. This patch is also helpful fo some QAs who want to do hcd's memleak test(Plug and unplug device thousand times.) This is not a bad idea, but it needs a little more work... --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -114,6 +114,7 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem); #define HUB_DEBOUNCE_STABLE 100 static int usb_reset_and_verify_device(struct usb_device *udev); +static void hub_port_logical_disconnect(struct usb_hub *hub, int port1); static inline char *portspeed(struct usb_hub *hub, int portstatus) { @@ -741,6 +742,21 @@ int usb_hub_set_port_power(struct usb_device *hdev, int port1, return ret; } +int usb_hub_port_power_reset(struct usb_device *hdev, int port1) +{ + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_interface *intf = to_usb_interface(hub-intfdev); + int ret; + + usb_autopm_get_interface(intf); What happens if hdev is NULL? + hub_port_logical_disconnect(hub, port1); + ret = usb_hub_set_port_power(hdev, port1, false); How long do you think the power should remain turned off? This code will leave it off for only a few milliseconds at most. That may not even be long enough for the voltage to drop all the way to 0. The delay probably should be at least 100 ms. Maybe more, I don't know. + ret |= usb_hub_set_port_power(hdev, port1, true); Don't use |=. Skip the second call if the first one fails. + usb_autopm_put_interface(intf); + + return ret; +} If you placed this function later on in the source file, you wouldn't need the forward declaration of hub_port_logical_disconnect(). 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 3/4] usb: Add usb port system pm support
On Thu, 28 Mar 2013, Lan Tianyu wrote: This patch is to add usb port system pm support. Add usb port's system suspend/resume callbacks and call usb_port_runtime_resume/suspend() to power off these ports whose pm qos NO_POWER_OFF flag is not set, system wakeup is disabled and persist is enalbed. During system pm, usb port should be powered off after dev being suspended and powered on before dev being resumed. Since usb ports and devs enable async suspend, call device_pm_wait_for_dev() in the usb_port_system_suspend() and usb_port_resume() to keeping the order. If usb port was already powered off by runtime pm with port_dev-power_is_on being false, usb_port_system_suspend() returns directly. If usb port was not powered off during system suspend with port_dev-power_is_on being true, usb_port_system_resume() returns directly. Signed-off-by: Lan Tianyu tianyu@intel.com ... +static int usb_port_system_suspend(struct device *dev) +{ + struct usb_port *port_dev = to_usb_port(dev); + + if (!port_dev-power_is_on) + return 0; + + if (port_dev-child) { + struct usb_device *udev = port_dev-child; + + /* + * usb port can't be powered off when dev's system + * wakeup is enabled or persist is disabled. + */ + if (device_may_wakeup(udev-dev) + || !udev-persist_enabled) + return 0; + + /* + * usb port should be powered off after usb dev + * being suspended. + */ + device_pm_wait_for_dev(dev, port_dev-child-dev); + } + + usb_port_runtime_suspend(dev); + return 0; +} What happens if there's no device plugged in to the port, but the hub is enabled for remote wakeup? How will the hub be able to detect a plug-in event if the port isn't powered? 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 4/4] usb: register usb port to usb_bus_type
On Thu, 28 Mar 2013, Lan Tianyu wrote: Usb port isn't assigned to any bus_type. This seems not good from Greg's comments. http://marc.info/?l=linux-usbm=136200364929942w=2 This patch is to register usb port to usb_bus_type. The usb port's original name is portX. This will cause name confilct after adding usb port to usb_bus_type since the usb ports with same port num under different hub have the same name. So change the usb port's name format to port + (hub dev name) + '.' + (port num) for non-root hub and port + (usb bus num) + '-' + (port num) for root hub. ls /sys/bus/usb/devices 1-0:1.02-0:1.0 port1-1 port1-1.3 port2-1.2 port2-2 port4-3 1-12-1 port1-1.1port1-1.4 port2-1.3 port3-1 port4-4 1-1.1 2-1:1.0 port1-1.2port1-1.5 port2-1.4 port3-2 usb1 1-1:1.03-0:1.0 port1-1.2.1 port1-1.6 port2-1.5 port3-3 usb2 1-1.1:1.0 3-1 port1-1.2.2 port1-2port2-1.6 port3-4 usb3 1-1.2 3-1:1.0 port1-1.2.3 port2-1port2-1.7 port4-1 usb4 1-1.2:1.0 4-0:1.0 port1-1.2.4 port2-1.1 port2-1.8 port4-2 Signed-off-by: Lan Tianyu tianyu@intel.com This ends up looking like a mess, but I guess there's no way around it. 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 2/4] usb: introduce usb force power off mechanism
On Wed, 27 Mar 2013, Lan, Tianyu wrote: A small tool to test this patch. #include stdio.h #include unistd.h #include fcntl.h #include errno.h #include sys/ioctl.h #include linux/usbdevice_fs.h #define USBDEVFS_POWER_RESET _IO('U', 28) int main(int argc, char **argv) { const char *filename; int fd; int rc; if (argc != 2) { fprintf(stderr, Usage: usbreset device-filename\n); The name should be usbrepower (or usb-repower, which is more readable). Not usbreset -- there's already a different program using that name. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place
From: Matthijs Kooijman [mailto:matth...@stdin.nl] Sent: Wednesday, March 27, 2013 8:42 AM To: Greg KH Hi Greg, But yes, it does seem to be a good idea, Ok, thanks. as long as these options can't change over time. It's only read-only registers, or registers whose power-on values determine their maximum possible value, so they indeed won't change over time. Overall this seems to be an improvement, so I have no objections. There is a pretty high likelihood of something getting broken in the translation, however. For example, this: + hw-host_nperio_tx_fifo_size = (readl(hsotg-regs + GNPTXFSIZ) GNPTXFSIZ_NP_TXF_DEP_MASK) GNPTXFSIZ_NP_TXF_DEP_SHIFT; looks like a bug (shifted the wrong direction). But since the driver is still in staging and has no real users yet, that's not a big deal I guess. You probably should use u32 instead of int for the members of struct dwc2_hw_params, otherwise you might introduce a sign bug if any of the values get shifted into the high bit and become unexpectedly negative. And I would use either u8, bool, or bitfields for the true/false values, to save a little space. -- Paul -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: EHCI: DT support for generic bus glue
Hi again, the patch works fine on my WM8850 netbook. I didn't get any unused-function warning on build, most likely because I have CONFIG_PM set in kernel config. Regards, Peter -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)
On 27 March 2013 18:35, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 27 Mar 2013, David Linares wrote: Thanks Alan. I understand, but the USB cable is plugged into an AC/DC adaptor which can provide an output of 1A. After breaking out an USB cable, I measured that the consumption of my board is around 130mA (nothing connected to the hub). Then I measured with the dongle plugged in. The real consumption of the wireless adapter is around 210mA. The whole thing draws around 340mA Ah, okay. Then maybe this isn't a power issue. Today, I did some further tests *without* any wireless adapter. So, I have got the device plugged in into a main power socket via USB (delivering up to 1A), the root hub, and the 2-port hub and nothing else USB-wise. The problem still happens from time to time. The 2-port hub sometimes says it will give 500mA to its children and sometimes it gives only 100mA (because of a corrupted hubstatus mentioned earlier). Now that there is nothing connected on the hub, I am just wondering how come the hub is not always detected in the same way at each boot. Also, should it be detected as self powered ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1543 ) or not ( http://lxr.free-electrons.com/source/drivers/usb/core/hub.c#L1538 )? It sounds like the hub is rather flaky. Also, it should always describe itself as bus-powered, never as self-powered. Maybe you should try using a different kind of hub. Please correct me if I am wrong. On my iMX25 board, I have got a 1-port root_hub which will provide 500mA max to its unique child. Now that's where I am getting confused. Its child, a 2-port hub, will then be bus-powered. But if it is, it won't provide these 500mA shared between the 2 ports. It will only provide 100mA per port, right? But for me to be able to plug my wireless dongle, in the code, I guess I will need to have hub-mA_per_port = 500; and therefore have the USB_DEVICE_SELF_POWERED bit set in the hubstatus. Is that correct? Because otherwise, my dongle will always pass the test if (c-desc.bMaxPower * 2 udev-bus_mA) if this udev-bus_mA is set to 100mA and always end up in insufficient power. I forgot to mention that this issue where the hub is not detected as self-powered external hub is quite hard to reproduce. It happens 1 out of 20 tries. This could even be a loose electrical connection, silly as that seems. Yes, that could very well be. I will inspect that tomorrow You never posted any usbmon results. If you could see the actual data sent by the hub when it's not working, you might get a better idea of where the problem is. True. Didn't get a chance to do this experiment yet. As you can read, I am still having some difficulties understanding how it should work in practice and who gives which power :-) Will do that tomorrow too. Thanks again. Thanks a lot Alan and everybody else. I really appreciate the support and understanding you are providing. You're welcome. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: EHCI: DT support for generic bus glue
This lets us use the ehci-generic driver on platforms without special requirements for their ehci controllers. In particular, this is true for the vt8500/wm8x50 platforms, which currently have a separate driver that causes problems with multiplatform configurations. Tested-by: Tony Prisk li...@prisktech.co.nz Tested-by: Peter Vasil peterva...@gmail.com Acked-by: Alan Stern st...@rowland.harvard.edu Signed-off-by: Arnd Bergmann a...@arndb.de --- v3: always set dma_mask and coherent_dma_mask when not already set, rather than overriding them for any device without platform data v2: reset the platform data pointer on unload drivers/usb/host/ehci-hcd.c | 5 -- drivers/usb/host/ehci-platform.c | 28 ++-- drivers/usb/host/ehci-vt8500.c | 150 - 3 files changed, 22 insertions(+), 161 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 0c3314c..960e7cf 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1286,11 +1286,6 @@ MODULE_LICENSE (GPL); #define PLATFORM_DRIVERehci_octeon_driver #endif -#ifdef CONFIG_ARCH_VT8500 -#include ehci-vt8500.c -#definePLATFORM_DRIVER vt8500_ehci_driver -#endif - #ifdef CONFIG_PLAT_SPEAR #include ehci-spear.c #define PLATFORM_DRIVERspear_ehci_hcd_driver diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index ca75063..cda0fa9 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -18,11 +18,13 @@ * * Licensed under the GNU/GPL. See COPYING for details. */ +#include linux/dma-mapping.h #include linux/err.h #include linux/kernel.h #include linux/hrtimer.h #include linux/io.h #include linux/module.h +#include linux/of.h #include linux/platform_device.h #include linux/usb.h #include linux/usb/hcd.h @@ -62,22 +64,32 @@ static const struct ehci_driver_overrides platform_overrides __initdata = { .reset =ehci_platform_reset, }; +static struct usb_ehci_pdata ehci_platform_defaults; + static int ehci_platform_probe(struct platform_device *dev) { struct usb_hcd *hcd; struct resource *res_mem; - struct usb_ehci_pdata *pdata = dev-dev.platform_data; + struct usb_ehci_pdata *pdata; int irq; int err = -ENOMEM; - if (!pdata) { - WARN_ON(1); - return -ENODEV; - } - if (usb_disabled()) return -ENODEV; + /* +* use reasonable defaults so platforms don't have to provide these. +* with DT probing on ARM, none of these are set. +*/ + if (!dev-dev.platform_data) + dev-dev.platform_data = ehci_platform_defaults; + if (!dev-dev.dma_mask) + dev-dev.dma_mask = dev-dev.coherent_dma_mask; + if (!dev-dev.coherent_dma_mask) + dev-dev.coherent_dma_mask = DMA_BIT_MASK(32); + + pdata = dev-dev.platform_data; + irq = platform_get_irq(dev, 0); if (irq 0) { dev_err(dev-dev, no irq provided); @@ -139,6 +151,9 @@ static int ehci_platform_remove(struct platform_device *dev) if (pdata-power_off) pdata-power_off(dev); + if (pdata == ehci_platform_defaults) + dev-dev.platform_data = NULL; + return 0; } @@ -183,6 +198,12 @@ static int ehci_platform_resume(struct device *dev) #define ehci_platform_resume NULL #endif /* CONFIG_PM */ +static const struct of_device_id vt8500_ehci_ids[] = { + { .compatible = via,vt8500-ehci, }, + { .compatible = wm,prizm-ehci, }, + {} +}; + static const struct platform_device_id ehci_platform_table[] = { { ehci-platform, 0 }, { } @@ -203,6 +224,7 @@ static struct platform_driver ehci_platform_driver = { .owner = THIS_MODULE, .name = ehci-platform, .pm = ehci_platform_pm_ops, + .of_match_table = of_match_ptr(vt8500_ehci_ids), } }; diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c deleted file mode 100644 index 7ecf709..000 -- 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
Piggy-backing new hardware using old usb-serial
Good evening, We are developing a custom piece of hardware that provides the equivalent of two serial console interfaces. They do not speak the AT command set. One port provides an interactive console to the user for configuration purposes (via minicom/etc). The other speaks a proprietary protocol used by purpose-built userspace tools. Neither port has an inherent baud rate limit; both can fully utilize USB2 bandwidth. The closest standard USB-IF class I could find is the cdc-acm device class. However, this seems to be very much targeted at modems with ATx commands. The linux cdc-acm drivers would put the hardware at /dev/ttyACM and software seems to treat these like modems. The closer matching usb-serial dongles all appear to have unstandardized drivers. We would like our hardware to work out of the box on released Linux versions (and to a lesser extent on windows). Our current prototypes borrow the Sierra VID to trick the kernel into loading the sierra driver. This works well for the interactive console. However, I assume that distributing the device like this will have legal consequences. I could write a custom driver, but then end users need to compile +install it. Our device will never be widely available and thus our driver will never be included in linux, ie: even future users will have to compile+install for themselves. Because the USB logic is inside an FPGA, I can readily change the hardware to suite any existing driver. What driver should I target? Is there a way to AUTOload an older driver despite the new VID:PID? Should I give up and require a custom driver? Thank you for any and all feedback/advice. -- 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: Help with USB issues at boot-up on i.MX25 (linux 3.5.4)
On Wed, 27 Mar 2013, David Linares wrote: Please correct me if I am wrong. On my iMX25 board, I have got a 1-port root_hub which will provide 500mA max to its unique child. You should check that against the iMX25's specs. It's possible that the root hub has a current limit lower than 500 mA. Now that's where I am getting confused. Its child, a 2-port hub, will then be bus-powered. But if it is, it won't provide these 500mA shared between the 2 ports. It will only provide 100mA per port, right? Actually, I don't know. What I said earlier was under the assumption that the 2-port hub receives all its power from the USB bus. But in theory, it's possible for the hub to receive power directly from the iMX25 board's power supply. So the answer depends on how the hardware is wired up. If the 2-port hub doesn't have an additional power source then you are right -- it is bus powered and should provide only 100 mA to each port. But if it does have an external (to the USB bus) supply then it could very well try to provide 500 mA to each port. Even if the external supply isn't strong enough to permit this. But for me to be able to plug my wireless dongle, in the code, I guess I will need to have hub-mA_per_port = 500; and therefore have the USB_DEVICE_SELF_POWERED bit set in the hubstatus. Is that correct? Because otherwise, my dongle will always pass the test if (c-desc.bMaxPower * 2 udev-bus_mA) if this udev-bus_mA is set to 100mA and always end up in insufficient power. Sort of. Your emphasis is backward; this is a hardware issue, and whether or not a software test is passed won't affect the underlying problem. That test is there so that the system will avoid trying to use a device when there isn't enough power for it. Unfortunately the test doesn't always work, because many devices lie about their power sources. Also, hubs don't always limit the amount of current to their downstream ports the way they should. A bus-powered hub is supposed to get an over-current condition and turn off port power if the device attached to the port tries to draw more than 100 mA. But not all of them do this. In any case, you said there were occasional problems even without the dongle attached. In that situation, power should not be an issue. You never posted any usbmon results. If you could see the actual data sent by the hub when it's not working, you might get a better idea of where the problem is. True. Didn't get a chance to do this experiment yet. As you can read, I am still having some difficulties understanding how it should work in practice and who gives which power :-) Will do that tomorrow too. Okay. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Piggy-backing new hardware using old usb-serial
On Wed, Mar 27, 2013 at 08:45:38PM +0100, Wesley W. Terpstra wrote: Good evening, We are developing a custom piece of hardware that provides the equivalent of two serial console interfaces. They do not speak the AT command set. One port provides an interactive console to the user for configuration purposes (via minicom/etc). The other speaks a proprietary protocol used by purpose-built userspace tools. Neither port has an inherent baud rate limit; both can fully utilize USB2 bandwidth. The closest standard USB-IF class I could find is the cdc-acm device class. However, this seems to be very much targeted at modems with ATx commands. The linux cdc-acm drivers would put the hardware at /dev/ttyACM and software seems to treat these like modems. The closer matching usb-serial dongles all appear to have unstandardized drivers. We would like our hardware to work out of the box on released Linux versions (and to a lesser extent on windows). Our current prototypes borrow the Sierra VID to trick the kernel into loading the sierra driver. This works well for the interactive console. However, I assume that distributing the device like this will have legal consequences. Yes, Sierra will get mad at you :) And the USB-IF might revoke your vendor id, if they find you shipping a device with a different vendor id than the one you have been assigned. I could write a custom driver, but then end users need to compile +install it. Our device will never be widely available and thus our driver will never be included in linux, ie: even future users will have to compile+install for themselves. Because the USB logic is inside an FPGA, I can readily change the hardware to suite any existing driver. Why not just add your device id to an existing driver, sending in a patch to do so. All distros will pick that up and then your device will just work on all Linux distros. What driver should I target? What chip do you use for your device? If you just want raw data, then do something like the drivers/usb/serial/zio.c driver, tiny, simple, and trivial. Hope this helps, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: EHCI: DT support for generic bus glue
On Wed, 27 Mar 2013, Arnd Bergmann wrote: This lets us use the ehci-generic driver on platforms without special requirements for their ehci controllers. Well, actually the driver's name is ehci-platform. And the patch lets us use it on platforms where the DMA mask hasn't been set up, as often happens with DT. In particular, this is true for the vt8500/wm8x50 platforms, which currently have a separate driver that causes problems with multiplatform configurations. Tested-by: Tony Prisk li...@prisktech.co.nz Tested-by: Peter Vasil peterva...@gmail.com Acked-by: Alan Stern st...@rowland.harvard.edu Signed-off-by: Arnd Bergmann a...@arndb.de --- v3: always set dma_mask and coherent_dma_mask when not already set, rather than overriding them for any device without platform data v2: reset the platform data pointer on unload The patch all looks good to me. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: avoid error messages when a device is disconnected
This patch (as1673) reduces the amount of log spew from the hub driver by removing a bunch of error messages in the case where the device in question is already known to have been disconnected. Since the disconnect event itself appears in the log, there's no need for other error messages. Signed-off-by: Alan Stern st...@rowland.harvard.edu Tested-by: Jenya Y jy.gerstma...@gmail.com --- drivers/usb/core/generic.c |2 - drivers/usb/core/hub.c | 65 - 2 files changed, 37 insertions(+), 30 deletions(-) Index: usb-3.9/drivers/usb/core/hub.c === --- usb-3.9.orig/drivers/usb/core/hub.c +++ usb-3.9/drivers/usb/core/hub.c @@ -555,8 +555,9 @@ static int hub_port_status(struct usb_hu mutex_lock(hub-status_mutex); ret = get_port_status(hub-hdev, port1, hub-status-port); if (ret 4) { - dev_err(hub-intfdev, - %s failed (err = %d)\n, __func__, ret); + if (ret != -ENODEV) + dev_err(hub-intfdev, + %s failed (err = %d)\n, __func__, ret); if (ret = 0) ret = -EIO; } else { @@ -699,7 +700,7 @@ static void hub_tt_work(struct work_stru /* drop lock so HCD can concurrently report other TT errors */ spin_unlock_irqrestore (hub-tt.lock, flags); status = hub_clear_tt_buffer (hdev, clear-devinfo, clear-tt); - if (status) + if (status status != -ENODEV) dev_err (hdev-dev, clear tt %d (%04x) error %d\n, clear-tt, clear-devinfo, status); @@ -837,10 +838,11 @@ static int hub_hub_status(struct usb_hub mutex_lock(hub-status_mutex); ret = get_hub_status(hub-hdev, hub-status-hub); - if (ret 0) - dev_err (hub-intfdev, - %s failed (err = %d)\n, __func__, ret); - else { + if (ret 0) { + if (ret != -ENODEV) + dev_err(hub-intfdev, + %s failed (err = %d)\n, __func__, ret); + } else { *status = le16_to_cpu(hub-status-hub.wHubStatus); *change = le16_to_cpu(hub-status-hub.wHubChange); ret = 0; @@ -877,11 +879,8 @@ static int hub_usb3_port_disable(struct return -EINVAL; ret = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_SS_DISABLED); - if (ret) { - dev_err(hub-intfdev, cannot disable port %d (err = %d)\n, - port1, ret); + if (ret) return ret; - } /* Wait for the link to enter the disabled state. */ for (total_time = 0; ; total_time += HUB_DEBOUNCE_STEP) { @@ -918,7 +917,7 @@ static int hub_port_disable(struct usb_h ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_ENABLE); } - if (ret) + if (ret ret != -ENODEV) dev_err(hub-intfdev, cannot disable port %d (err = %d)\n, port1, ret); return ret; @@ -2192,8 +2191,9 @@ static int usb_enumerate_device(struct u if (udev-config == NULL) { err = usb_get_configuration(udev); if (err 0) { - dev_err(udev-dev, can't read configurations, error %d\n, - err); + if (err != -ENODEV) + dev_err(udev-dev, can't read configurations, error %d\n, + err); return err; } } @@ -2640,14 +2640,16 @@ static int hub_port_reset(struct usb_hub status = set_port_feature(hub-hdev, port1, (warm ? USB_PORT_FEAT_BH_PORT_RESET : USB_PORT_FEAT_RESET)); - if (status) { + if (status == -ENODEV) { + ; /* The hub is gone */ + } else if (status) { dev_err(hub-intfdev, cannot %sreset port %d (err = %d)\n, warm ? warm : , port1, status); } else { status = hub_port_wait_reset(hub, port1, udev, delay, warm); - if (status status != -ENOTCONN) + if (status status != -ENOTCONN status != -ENODEV) dev_dbg(hub-intfdev, port_wait_reset: err = %d\n, status); @@
[PATCH 1/2] USB: use global suspend for system sleep on USB-2 buses
This patch (as1674) speeds up system sleep transitions by not suspending each individual device on a USB-1.1 or USB-2 bus. The devices will automatically go into suspend when their root hubs are suspended (i.e., stop sending out Start-Of-Frame packets) -- this is what the USB spec calls global suspend. Since this is what we do already when CONFIG_USB_SUSPEND isn't enabled, it shouldn't cause any problems. Signed-off-by: Alan Stern st...@rowland.harvard.edu CC: Peter Chen peter.c...@freescale.com --- drivers/usb/core/hub.c | 30 +++--- 1 file changed, 23 insertions(+), 7 deletions(-) Index: usb-3.9/drivers/usb/core/hub.c === --- usb-3.9.orig/drivers/usb/core/hub.c +++ usb-3.9/drivers/usb/core/hub.c @@ -2882,9 +2882,11 @@ static int usb_disable_function_remotewa * Linux (2.6) currently has NO mechanisms to initiate that: no khubd * timer, no SRP, no requests through sysfs. * - * If CONFIG_USB_SUSPEND isn't enabled, devices only really suspend when - * the root hub for their bus goes into global suspend ... so we don't - * (falsely) update the device power state to say it suspended. + * If CONFIG_USB_SUSPEND isn't enabled, non-SuperSpeed devices really get + * suspended only when their bus goes into global suspend (i.e., the root + * hub is suspended). Nevertheless, we change @udev-state to + * USB_STATE_SUSPENDED as this is the device's logical state. The actual + * upstream port setting is stored in @udev-port_is_suspended. * * Returns 0 on success, else negative errno. */ @@ -2895,6 +2897,7 @@ int usb_port_suspend(struct usb_device * enum pm_qos_flags_status pm_qos_stat; int port1 = udev-portnum; int status; + boolreally_suspend = true; /* enable remote wakeup when appropriate; this lets the device * wake up the upstream hub (including maybe the root hub). @@ -2951,9 +2954,19 @@ int usb_port_suspend(struct usb_device * /* see 7.1.7.6 */ if (hub_is_superspeed(hub-hdev)) status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3); - else + else if (PMSG_IS_AUTO(msg)) status = set_port_feature(hub-hdev, port1, USB_PORT_FEAT_SUSPEND); + /* +* For system suspend, we do not need to enable the suspend feature +* on individual USB-2 ports. The devices will automatically go +* into suspend a few ms after the root hub stops sending packets. +* The USB 2.0 spec calls this global suspend. +*/ + else { + really_suspend = false; + status = 0; + } if (status) { dev_dbg(hub-intfdev, can't suspend port %d, status %d\n, port1, status); @@ -2989,8 +3002,10 @@ int usb_port_suspend(struct usb_device * (PMSG_IS_AUTO(msg) ? auto- : ), udev-do_remote_wakeup); usb_set_device_state(udev, USB_STATE_SUSPENDED); - udev-port_is_suspended = 1; - msleep(10); + if (really_suspend) { + udev-port_is_suspended = 1; + msleep(10); + } } /* -- 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/2] USB: remove CONFIG_USB_SUSPEND option
This patch (as1675) removes the CONFIG_USB_SUSPEND option, essentially replacing it everywhere with CONFIG_PM_RUNTIME (except for one place in hub.c, where it is replaced with CONFIG_PM because the code needs to be used in both runtime and system PM). The net result is code shrinkage and simplification. There's very little point in keeping CONFIG_USB_SUSPEND because almost everybody enables it. The few that don't will find that the usbcore module has gotten somewhat bigger and they will have to take active measures if they want to prevent hubs from being runtime suspended. Signed-off-by: Alan Stern st...@rowland.harvard.edu CC: Peter Chen peter.c...@freescale.com --- drivers/usb/core/Kconfig | 16 drivers/usb/core/driver.c|4 ++-- drivers/usb/core/hcd.c | 10 +- drivers/usb/core/hub.c | 42 +++--- drivers/usb/core/port.c |4 ++-- drivers/usb/core/sysfs.c |4 ++-- drivers/usb/core/usb.c |4 ++-- drivers/usb/core/usb.h |2 +- drivers/usb/host/ehci-pci.c | 12 +--- drivers/usb/host/ohci-hub.c |6 -- drivers/usb/host/sl811-hcd.c |2 +- drivers/usb/host/u132-hcd.c |9 + drivers/usb/host/xhci-hub.c |2 +- drivers/usb/host/xhci.c |4 ++-- include/linux/usb.h |2 +- include/linux/usb/hcd.h |6 +++--- 16 files changed, 35 insertions(+), 94 deletions(-) Index: usb-3.9/drivers/usb/core/Kconfig === --- usb-3.9.orig/drivers/usb/core/Kconfig +++ usb-3.9/drivers/usb/core/Kconfig @@ -38,22 +38,6 @@ config USB_DYNAMIC_MINORS If you are unsure about this, say N here. -config USB_SUSPEND - bool USB runtime power management (autosuspend) and wakeup - depends on USB PM_RUNTIME - help - If you say Y here, you can use driver calls or the sysfs - power/control file to enable or disable autosuspend for - individual USB peripherals (see - Documentation/usb/power-management.txt for more details). - - Also, USB remote wakeup signaling is supported, whereby some - USB devices (like keyboards and network adapters) can wake up - their parent hub. That wakeup cascades up the USB tree, and - could wake the system from states like suspend-to-RAM. - - If you are unsure about this, say N here. - config USB_OTG bool OTG support depends on USB Index: usb-3.9/drivers/usb/core/driver.c === --- usb-3.9.orig/drivers/usb/core/driver.c +++ usb-3.9/drivers/usb/core/driver.c @@ -1407,7 +1407,7 @@ int usb_resume(struct device *dev, pm_me #endif /* CONFIG_PM */ -#ifdef CONFIG_USB_SUSPEND +#ifdef CONFIG_PM_RUNTIME /** * usb_enable_autosuspend - allow a USB device to be autosuspended @@ -1775,7 +1775,7 @@ int usb_set_usb2_hardware_lpm(struct usb return ret; } -#endif /* CONFIG_USB_SUSPEND */ +#endif /* CONFIG_PM_RUNTIME */ struct bus_type usb_bus_type = { .name = usb, Index: usb-3.9/drivers/usb/core/hcd.c === --- usb-3.9.orig/drivers/usb/core/hcd.c +++ usb-3.9/drivers/usb/core/hcd.c @@ -2125,7 +2125,7 @@ int hcd_bus_resume(struct usb_device *rh #endif /* CONFIG_PM */ -#ifdef CONFIG_USB_SUSPEND +#ifdef CONFIG_PM_RUNTIME /* Workqueue routine for root-hub remote wakeup */ static void hcd_resume_work(struct work_struct *work) @@ -2160,7 +2160,7 @@ void usb_hcd_resume_root_hub (struct usb } EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub); -#endif /* CONFIG_USB_SUSPEND */ +#endif /* CONFIG_PM_RUNTIME */ /*-*/ @@ -2336,7 +2336,7 @@ struct usb_hcd *usb_create_shared_hcd(co init_timer(hcd-rh_timer); hcd-rh_timer.function = rh_timer_func; hcd-rh_timer.data = (unsigned long) hcd; -#ifdef CONFIG_USB_SUSPEND +#ifdef CONFIG_PM_RUNTIME INIT_WORK(hcd-wakeup_work, hcd_resume_work); #endif @@ -2582,7 +2582,7 @@ error_create_attr_group: hcd-rh_registered = 0; spin_unlock_irq(hcd_root_hub_lock); -#ifdef CONFIG_USB_SUSPEND +#ifdef CONFIG_PM_RUNTIME cancel_work_sync(hcd-wakeup_work); #endif mutex_lock(usb_bus_list_lock); @@ -2637,7 +2637,7 @@ void usb_remove_hcd(struct usb_hcd *hcd) hcd-rh_registered = 0; spin_unlock_irq (hcd_root_hub_lock); -#ifdef CONFIG_USB_SUSPEND +#ifdef CONFIG_PM_RUNTIME cancel_work_sync(hcd-wakeup_work); #endif Index: usb-3.9/drivers/usb/core/hub.c === --- usb-3.9.orig/drivers/usb/core/hub.c +++ usb-3.9/drivers/usb/core/hub.c @@ -2823,7 +2823,7 @@ void usb_enable_ltm(struct usb_device *u } EXPORT_SYMBOL_GPL(usb_enable_ltm); -#ifdef CONFIG_USB_SUSPEND
Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes
Michael Grzeschik m...@pengutronix.de writes: On Wed, Mar 27, 2013 at 06:35:30PM +0200, Alexander Shishkin wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function Actually, do any of these patches fix tangible bugs, that can be reproduced? If so, you should certainly mention that. But somehow I doubt that, because otherwise the driver will be totally unusable. What I see is patches that bring some parts of udc code in accordance with the spec, which is a good thing to do, but doesn't fix any real bugs that bother people. Or does it? Obviously we were hunting a real issue while writing that patches. The most essential one is Patch 1/6, which definitely is a stable patch. I will clearify that in the patch description. Does it then fall in never worked before category, since it looks like it has never worked before on armv5 or did it work with older compilers? At least the xscale version seems to be fine. The others though should go into v3.10, agreed? Agreed. What about the multi-td patch and the rest? Are you going to resend them still? Regards, -- Alex -- 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: Piggy-backing new hardware using old usb-serial
On Wed, 2013-03-27 at 13:06 -0700, Greg KH wrote: Our current prototypes borrow the Sierra VID And the USB-IF might revoke your vendor id, if they find you shipping a device with a different vendor id than the one you have been assigned. One of the reasons we borrowed that VID is that we do not currently have a VID assigned. We are still deciding whether it is worth joining the USB-IF just to get a vendor ID for a few thousands of devices. I am of the opinion that it is, but I cannot sign the membership forms on behalf of GSI. We probably will end up buying a VID soon. Why not just add your device id to an existing driver, sending in a patch to do so. All distros will pick that up and then your device will just work on all Linux distros. I was under the impression that drivers in the linux mainline had to be for hardware that was widely available. I take it then, that just adding support to an existing driver is acceptable? That wouldn't address people with older linux distributions, but would definitely be a good long term solution. It's really a shame there is no USB-IF standard for usb-serial... then things would even potentially work out of the box on windows. What driver should I target? What chip do you use for your device? The device I am concerned about right now has an Altera arria2 connected to a cy7c68013a-56baxc (fx2lp). We have several form factor variations. A few have FTDI chips where I don't need to care, but can also do less. If you just want raw data, then do something like the drivers/usb/serial/zio.c driver, tiny, simple, and trivial. Yes, if I make source-level changes to the kernel I have many options. PS. Thank you for the very prompt reply! -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: EHCI: DT support for generic bus glue
On Wed, Mar 27, 2013 at 9:09 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 27 Mar 2013, Arnd Bergmann wrote: This lets us use the ehci-generic driver on platforms without special requirements for their ehci controllers. Well, actually the driver's name is ehci-platform. And the patch lets us use it on platforms where the DMA mask hasn't been set up, as often happens with DT. In particular, this is true for the vt8500/wm8x50 platforms, which currently have a separate driver that causes problems with multiplatform configurations. Tested-by: Tony Prisk li...@prisktech.co.nz Tested-by: Peter Vasil peterva...@gmail.com Acked-by: Alan Stern st...@rowland.harvard.edu Signed-off-by: Arnd Bergmann a...@arndb.de --- v3: always set dma_mask and coherent_dma_mask when not already set, rather than overriding them for any device without platform data v2: reset the platform data pointer on unload The patch all looks good to me. Alan Stern -- You received this message because you are subscribed to the Google Groups VT8500/WM8505 Linux Kernel group. To unsubscribe from this group and stop receiving emails from it, send an email to vt8500-wm8505-linux-kernel+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out. Hi, unfortunately I still can't apply it on 3.9-rc4 :-( $ git apply --check ehci-dt-bus-glue.patch error: removal patch leaves file contents error: drivers/usb/host/ehci-vt8500.c: patch does not apply Looks git really doesn't like ehci-vt8500.c removal. If I delete the file manually, git diff actually outputs full file content prefixed with -. Regards, Peter -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes
On Wed, Mar 27, 2013 at 10:24:04PM +0200, Alexander Shishkin wrote: Michael Grzeschik m...@pengutronix.de writes: On Wed, Mar 27, 2013 at 06:35:30PM +0200, Alexander Shishkin wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Hi, this series solve some issues with the chipidea udc. The series is based on v3.9-rc4. Michael Grzeschik (6): usb: chipidea: udc: add attribute aligned(4) to shared memory structs usb: chipidea: udc: only clear active and halted bits in qhead usb: chipidea: udc: move ZLT flag change to ep_enable usb: chipidea: udc: read status of td only once in hardware_dequeue usb: chipidea: udc: don't truncate requests to single tds usb: chipidea: udc: move _ep_queue into an unlocked function Actually, do any of these patches fix tangible bugs, that can be reproduced? If so, you should certainly mention that. But somehow I doubt that, because otherwise the driver will be totally unusable. What I see is patches that bring some parts of udc code in accordance with the spec, which is a good thing to do, but doesn't fix any real bugs that bother people. Or does it? Obviously we were hunting a real issue while writing that patches. The most essential one is Patch 1/6, which definitely is a stable patch. I will clearify that in the patch description. Does it then fall in never worked before category, since it looks like it has never worked before on armv5 or did it work with older compilers? We used gcc-4.6.2 and gcc-4.7.2. In both cases we tested on the mx28 and the system got stuck after some transfers when using the following test: user@target$ getty /dev/ttyGS0 user@host$ microcom -p /dev/ttyACM0 Within the shell we started the following command: $(while true; do find /; done) At least the xscale version seems to be fine. Could you test the above with the xscale? I will refer to another patch which is pretty similar and adressing in detail the same problamatic situation: http://www.spinics.net/lists/linux-usb/msg60768.html The only reason the fsl_udc_core driver does not have run into that, is that its shared structures are not marked with the attribute ((packed)). The others though should go into v3.10, agreed? Agreed. What about the multi-td patch and the rest? Are you going to resend them still? I will clean up this series first and add the following patch to it: usb: chipidea: udc: prepare qhead with dma_alloc_coherent The rest are cleanup and feature patches which will come in seperate series, as Felipe suggested. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: EHCI: DT support for generic bus glue
On Wednesday 27 March 2013, Peter Vasil wrote: Hi, unfortunately I still can't apply it on 3.9-rc4 :-( $ git apply --check ehci-dt-bus-glue.patch error: removal patch leaves file contents error: drivers/usb/host/ehci-vt8500.c: patch does not apply Looks git really doesn't like ehci-vt8500.c removal. If I delete the file manually, git diff actually outputs full file content prefixed with -. Are you using an old git version? I normally use 'git format-patch -M -D' to produce patches in the reduced format that handles moves and deletions nicer but is incompatible with older versions of 'patch'. I'll fix up the changelog to mention the correct driver and resend without -D. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] USB: EHCI: DT support for generic bus glue
This lets us use the ehci-platform driver on platforms without special requirements for their ehci controllers. In particular, this is true for the vt8500/wm8x50 platforms, which currently have a separate driver that causes problems with multiplatform configurations. Tested-by: Tony Prisk li...@prisktech.co.nz Tested-by: Peter Vasil peterva...@gmail.com Acked-by: Alan Stern st...@rowland.harvard.edu Signed-off-by: Arnd Bergmann a...@arndb.de --- v4: fix the name of the driver in the changelog submit the patch with the full file deletion. v3: always set dma_mask and coherent_dma_mask when not already set, rather than overriding them for any device without platform data v2: reset the platform data pointer on unload drivers/usb/host/ehci-hcd.c | 5 -- drivers/usb/host/ehci-platform.c | 34 +++-- drivers/usb/host/ehci-vt8500.c | 150 --- 3 files changed, 28 insertions(+), 161 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 0c3314c..960e7cf 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1286,11 +1286,6 @@ MODULE_LICENSE (GPL); #define PLATFORM_DRIVERehci_octeon_driver #endif -#ifdef CONFIG_ARCH_VT8500 -#include ehci-vt8500.c -#definePLATFORM_DRIVER vt8500_ehci_driver -#endif - #ifdef CONFIG_PLAT_SPEAR #include ehci-spear.c #define PLATFORM_DRIVERspear_ehci_hcd_driver diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index ca75063..cda0fa9 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -18,11 +18,13 @@ * * Licensed under the GNU/GPL. See COPYING for details. */ +#include linux/dma-mapping.h #include linux/err.h #include linux/kernel.h #include linux/hrtimer.h #include linux/io.h #include linux/module.h +#include linux/of.h #include linux/platform_device.h #include linux/usb.h #include linux/usb/hcd.h @@ -62,22 +64,32 @@ static const struct ehci_driver_overrides platform_overrides __initdata = { .reset =ehci_platform_reset, }; +static struct usb_ehci_pdata ehci_platform_defaults; + static int ehci_platform_probe(struct platform_device *dev) { struct usb_hcd *hcd; struct resource *res_mem; - struct usb_ehci_pdata *pdata = dev-dev.platform_data; + struct usb_ehci_pdata *pdata; int irq; int err = -ENOMEM; - if (!pdata) { - WARN_ON(1); - return -ENODEV; - } - if (usb_disabled()) return -ENODEV; + /* +* use reasonable defaults so platforms don't have to provide these. +* with DT probing on ARM, none of these are set. +*/ + if (!dev-dev.platform_data) + dev-dev.platform_data = ehci_platform_defaults; + if (!dev-dev.dma_mask) + dev-dev.dma_mask = dev-dev.coherent_dma_mask; + if (!dev-dev.coherent_dma_mask) + dev-dev.coherent_dma_mask = DMA_BIT_MASK(32); + + pdata = dev-dev.platform_data; + irq = platform_get_irq(dev, 0); if (irq 0) { dev_err(dev-dev, no irq provided); @@ -139,6 +151,9 @@ static int ehci_platform_remove(struct platform_device *dev) if (pdata-power_off) pdata-power_off(dev); + if (pdata == ehci_platform_defaults) + dev-dev.platform_data = NULL; + return 0; } @@ -183,6 +198,12 @@ static int ehci_platform_resume(struct device *dev) #define ehci_platform_resume NULL #endif /* CONFIG_PM */ +static const struct of_device_id vt8500_ehci_ids[] = { + { .compatible = via,vt8500-ehci, }, + { .compatible = wm,prizm-ehci, }, + {} +}; + static const struct platform_device_id ehci_platform_table[] = { { ehci-platform, 0 }, { } @@ -203,6 +224,7 @@ static struct platform_driver ehci_platform_driver = { .owner = THIS_MODULE, .name = ehci-platform, .pm = ehci_platform_pm_ops, + .of_match_table = of_match_ptr(vt8500_ehci_ids), } }; diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c deleted file mode 100644 index 7ecf709..000 --- a/drivers/usb/host/ehci-vt8500.c +++ /dev/null @@ -1,150 +0,0 @@ -/* - * drivers/usb/host/ehci-vt8500.c - * - * Copyright (C) 2010 Alexey Charkov alch...@gmail.com - * - * Based on ehci-au1xxx.c - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - */ -
Re: 3.8.4: ohci question
[+cc linux-usb] On Wed, Mar 27, 2013 at 11:37 AM, Udo van den Heuvel udo...@xs4all.nl wrote: Hello, When my dmesg gives me a growing number of lines like the one below, what is going on? ohci_hcd :00:12.0: urb 88023025c500 path 2 ep1in 6c16 cc 6 -- status -71 Please let me know! Kind regards, Udo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: EHCI: DT support for generic bus glue
On Wed, Mar 27, 2013 at 10:41 PM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 27 March 2013, Peter Vasil wrote: Hi, unfortunately I still can't apply it on 3.9-rc4 :-( $ git apply --check ehci-dt-bus-glue.patch error: removal patch leaves file contents error: drivers/usb/host/ehci-vt8500.c: patch does not apply Looks git really doesn't like ehci-vt8500.c removal. If I delete the file manually, git diff actually outputs full file content prefixed with -. Are you using an old git version? I normally use 'git format-patch -M -D' to produce patches in the reduced format that handles moves and deletions nicer but is incompatible with older versions of 'patch'. I'll fix up the changelog to mention the correct driver and resend without -D. Arnd Hm, no idea what the latest versions are :-) I use archlinux, with latest updates installed. Here are my git and patch versions: $ git --version git version 1.8.2 $ patch --version GNU patch 2.7.1 Btw. my git understands format-patch -D, but its help looks a bit suspicous: -D, --irreversible-delete Omit the preimage for deletes, i.e. print only the header but not the diff between the preimage and /dev/null. The resulting patch is not meant to be applied with patch nor git apply; this is solely for people who want to just concentrate on reviewing the text after the change. In addition, the output obviously lack enough information to apply such a patch in reverse, even manually, hence the name of the option. When used together with -B, omit also the preimage in the deletion part of a delete/create pair. Perhaps there is a newer version that can also apply such a reduced patch? Regards, Peter -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: EHCI: DT support for generic bus glue
On Wednesday 27 March 2013, Peter Vasil wrote: Hm, no idea what the latest versions are :-) I use archlinux, with latest updates installed. Here are my git and patch versions: $ git --version git version 1.8.2 $ patch --version GNU patch 2.7.1 Btw. my git understands format-patch -D, but its help looks a bit suspicous: -D, --irreversible-delete Omit the preimage for deletes, i.e. print only the header but not the diff between the preimage and /dev/null. The resulting patch is not meant to be applied with patch nor git apply; this is solely for people who want to just concentrate on reviewing the text after the change. In addition, the output obviously lack enough information to apply such a patch in reverse, even manually, hence the name of the option. When used together with -B, omit also the preimage in the deletion part of a delete/create pair. Perhaps there is a newer version that can also apply such a reduced patch? Hmm, my version is slightly older than yours. I also find the same thing here: $ git format-patch -M -D HEAD^ 0001-USB-EHCI-DT-support-for-generic-bus-glue.patch $ git reset --hard HEAD^ HEAD is now at 417c765 USB: EHCI: fix up incorrect merge resolution $ git am 0001-USB-EHCI-DT-support-for-generic-bus-glue.patch Applying: USB: EHCI: DT support for generic bus glue error: removal patch leaves file contents error: drivers/usb/host/ehci-vt8500.c: patch does not apply Patch failed at 0001 USB: EHCI: DT support for generic bus glue The copy of the patch that failed is found in: /home/arnd/arm-soc/.git/rebase-apply/patch When you have resolved this problem, run git am --resolved. If you prefer to skip this patch, run git am --skip instead. To restore the original branch and stop patching, run git am --abort. I guess I won't be using the -D switch again then. Thanks for pointing this out! Arnd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci page fault panic on Ubuntu kernel with HP desktop hardware
On Wed, Mar 27, 2013 at 02:24:24PM +0100, Yann Sionneau wrote: Le 26/03/2013 17:30, Sarah Sharp a écrit : On Tue, Mar 26, 2013 at 12:11:13PM +0100, Yann Sionneau wrote: Le 25/03/2013 19:13, Sarah Sharp a écrit : On Mon, Mar 25, 2013 at 05:43:40PM +0100, Yann Sionneau wrote: Please compile with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on. If you can't reproduce your bug on 3.8.4, turn them off, recompile, and see if you can reproduce it again. We just reproduced it with a 3.8.4 kernel with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on. I am sending you the picture as attached file. Thanks. I need more information though. Can you follow these directions for setting up netconsole, and send me the full log file from boot to when the machine hangs? Make sure the kernel still has CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING turned on. http://kernelnewbies.org/KernelDebug That will help me figure out what command is causing the null pointer dereference. Sarah Sharp Hi Sarah, I've got some xhci debug traces: ysionneau@yann-HP:~/ssd/olivier$ nc -l -u | tee ~/ssd/olivier/dmesg-`date +%Y-%m-%d-%H-%M`.txt That was helpful, thanks! However, next time will you send the log file as an attachment? Your mail client is wrapping lines, and it makes it hard for me to read the logs. [ 1375.761963] xhci_hcd :00:14.0: Timeout while waiting for address device command [ 1375.761974] xhci_hcd :00:14.0: Abort command ring [ 1375.762029] BUG: unable to handle kernel NULL pointer dereference at 0018 [ 1375.762102] IP: [c14854b0] xhci_stream_id_to_ring+0x30/0x40 Ok, so it may be to be related to xHCI command cancellation. That's an area that I've been working on, and I've run across a couple bugs. I'm not sure yet if they're the particular bug you're running into though. Could you please test this branch, and let me know if it solves your issue? If not please use netconsole to capture the output again. git clone git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git -b ful-command-queue-fixes-simple Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 4/8] usb: chipidea: udc: read status of td only once in hardware_dequeue
This patch changes the read of the td status to one atomic operation to analyse coherent bits. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index a8528ad..d4db3ac 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -507,10 +507,12 @@ done: */ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) { + u32 tmptoken = mReq-ptr-token; + if (mReq-req.status != -EALREADY) return -EINVAL; - if ((TD_STATUS_ACTIVE mReq-ptr-token) != 0) + if ((TD_STATUS_ACTIVE tmptoken) != 0) return -EBUSY; if (mReq-zptr) { @@ -524,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) usb_gadget_unmap_request(mEp-ci-gadget, mReq-req, mEp-dir); - mReq-req.status = mReq-ptr-token TD_STATUS; + mReq-req.status = tmptoken TD_STATUS; if ((TD_STATUS_HALTED mReq-req.status) != 0) mReq-req.status = -1; else if ((TD_STATUS_DT_ERR mReq-req.status) != 0) @@ -532,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) else if ((TD_STATUS_TR_ERR mReq-req.status) != 0) mReq-req.status = -1; - mReq-req.actual = mReq-ptr-token TD_TOTAL_BYTES; + mReq-req.actual = tmptoken TD_TOTAL_BYTES; mReq-req.actual = ffs_nr(TD_TOTAL_BYTES); mReq-req.actual = mReq-req.length - mReq-req.actual; mReq-req.actual = mReq-req.status ? 0 : mReq-req.actual; -- 1.8.2.rc2 -- 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 v8 8/8] usb: chipidea: udc: prepare qhead with dma_alloc_coherent
The prepared memory for the qhead needs to be contiguos and 2K aligned. We change the code from allocating extra buffer for every ep qhead to one big area. This patch lowers the amount of code to prepare the memory. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/ci.h | 9 +++-- drivers/usb/chipidea/udc.c | 31 ++- drivers/usb/chipidea/udc.h | 2 +- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index e25d126..12eca83 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -110,7 +110,7 @@ struct hw_bank { * @is_otg: if the device is otg-capable * @work: work for role changing * @wq: workqueue thread - * @qh_pool: allocation pool for queue heads + * @qh: allocation struct for queue heads * @td_pool: allocation pool for transfer descriptors * @gadget: device side representation for peripheral controller * @driver: gadget driver @@ -142,7 +142,12 @@ struct ci13xxx { struct work_struct vbus_work; struct workqueue_struct *wq; - struct dma_pool *qh_pool; + struct { + struct ci13xxx_qh *ptr; + dma_addr_t dma; + size_t size; + } qh; + struct dma_pool *td_pool; struct usb_gadget gadget; diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index a0a0a64..c9f0a1a 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -23,6 +23,7 @@ #include linux/irq.h #include linux/kernel.h #include linux/slab.h +#include linux/sizes.h #include linux/pm_runtime.h #include linux/usb/ch9.h #include linux/usb/gadget.h @@ -1417,7 +1418,7 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active) pm_runtime_get_sync(_gadget-dev); hw_device_reset(ci, USBMODE_CM_DC); hw_enable_vbus_intr(ci); - hw_device_state(ci, ci-ep0out-qh.dma); + hw_device_state(ci, ci-qh.dma); } else { hw_device_state(ci, 0); if (ci-platdata-notify_event) @@ -1520,8 +1521,8 @@ static int init_eps(struct ci13xxx *ci) mEp-ep.maxpacket = (unsigned short)~0; INIT_LIST_HEAD(mEp-qh.queue); - mEp-qh.ptr = dma_pool_alloc(ci-qh_pool, GFP_KERNEL, -mEp-qh.dma); + mEp-qh.ptr = ci-qh.ptr[i * 2 + j]; + if (mEp-qh.ptr == NULL) retval = -ENOMEM; else @@ -1549,13 +1550,8 @@ static int init_eps(struct ci13xxx *ci) static void destroy_eps(struct ci13xxx *ci) { - int i; - - for (i = 0; i ci-hw_ep_max; i++) { - struct ci13xxx_ep *mEp = ci-ci13xxx_ep[i]; - - dma_pool_free(ci-qh_pool, mEp-qh.ptr, mEp-qh.dma); - } + dma_free_coherent(ci-dev, ci-qh.size, + ci-qh.ptr, ci-qh.dma); } /** @@ -1601,7 +1597,7 @@ static int ci13xxx_start(struct usb_gadget *gadget, } } - retval = hw_device_state(ci, ci-ep0out-qh.dma); + retval = hw_device_state(ci, ci-qh.dma); if (retval) pm_runtime_put_sync(ci-gadget.dev); @@ -1747,11 +1743,14 @@ static int udc_start(struct ci13xxx *ci) ci-gadget.dev.parent = dev; ci-gadget.dev.release = udc_release; + ci-qh.size = ci-hw_ep_max * sizeof(struct ci13xxx_qh); + ci-qh.size = ALIGN(ci-qh.size, SZ_2K); + /* alloc resources */ - ci-qh_pool = dma_pool_create(ci13xxx_qh, dev, - sizeof(struct ci13xxx_qh), - 64, CI13XXX_PAGE_SIZE); - if (ci-qh_pool == NULL) + ci-qh.ptr = dma_alloc_coherent(dev, ci-qh.size, + ci-qh.dma, GFP_ATOMIC); + + if (ci-qh.ptr == NULL) return -ENOMEM; ci-td_pool = dma_pool_create(ci13xxx_td, dev, @@ -1831,7 +1830,6 @@ destroy_eps: free_pools: dma_pool_destroy(ci-td_pool); free_qh_pool: - dma_pool_destroy(ci-qh_pool); return retval; } @@ -1853,7 +1851,6 @@ static void udc_stop(struct ci13xxx *ci) destroy_eps(ci); dma_pool_destroy(ci-td_pool); - dma_pool_destroy(ci-qh_pool); if (!IS_ERR_OR_NULL(ci-transceiver)) { otg_set_peripheral(ci-transceiver-otg, NULL); diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h index d12e8b5..59c091b 100644 --- a/drivers/usb/chipidea/udc.h +++ b/drivers/usb/chipidea/udc.h @@ -57,7 +57,7 @@ struct ci13xxx_qh { /* 9 */ u32
[PATCH v8 2/8] usb: chipidea: udc: only clear active and halted bits in qhead
The datasheet of the synopsys core describes only to overwrite the active and halted bits in the qhead before priming any endpoint. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/usb/chipidea/udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 77fb66f..4bbbe62 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -488,7 +488,7 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) /* QH configuration */ mEp-qh.ptr-td.next = mReq-dma;/* TERMINATE = 0 */ - mEp-qh.ptr-td.token = ~TD_STATUS; /* clear status */ + mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE); mEp-qh.ptr-cap |= QH_ZLT; wmb(); /* synchronize before ep prime */ -- 1.8.2.rc2 -- 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 v8 7/8] usb: chipidea: udc: fix possible memory leak in _ep_nuke
In hardware_enqueue code adds one extra td with dma_pool_alloc if mReq-req.zero is true. When _ep_nuke will be called for that endpoint, dma_pool_free will not be called to free that memory again. That patch fixes this. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index b40c259..a0a0a64 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -564,6 +564,12 @@ __acquires(mEp-lock) struct ci13xxx_req *mReq = \ list_entry(mEp-qh.queue.next, struct ci13xxx_req, queue); + + if (mReq-zptr) { + dma_pool_free(mEp-td_pool, mReq-zptr, mReq-zdma); + mReq-zptr = NULL; + } + list_del_init(mReq-queue); mReq-req.status = -ESHUTDOWN; -- 1.8.2.rc2 -- 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 v8 6/8] usb: chipidea: udc: move _ep_queue into an unlocked function
There is no need to call ep_queue unlocked inside the own driver. We move its functionionality into an unlocked version. This patch removes potential unlocked timeslots inside isr_setup_status_phase and isr_get_status_response, in which the lock got released just before acquired again inside usb_ep_queue. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de --- drivers/usb/chipidea/udc.c | 113 + 1 file changed, 63 insertions(+), 50 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index e7c84ab..b40c259 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -671,6 +671,66 @@ static void isr_get_status_complete(struct usb_ep *ep, struct usb_request *req) } /** + * _ep_queue: queues (submits) an I/O request to an endpoint + * + * Caller must hold lock + */ +static int _ep_queue(struct usb_ep *ep, struct usb_request *req, + gfp_t __maybe_unused gfp_flags) +{ + struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); + struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); + struct ci13xxx *ci = mEp-ci; + int retval = 0; + + if (ep == NULL || req == NULL || mEp-ep.desc == NULL) + return -EINVAL; + + if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { + if (req-length) + mEp = (ci-ep0_dir == RX) ? + ci-ep0out : ci-ep0in; + if (!list_empty(mEp-qh.queue)) { + _ep_nuke(mEp); + retval = -EOVERFLOW; + dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, +_usb_addr(mEp)); + } + } + + /* first nuke then test link, e.g. previous status has not sent */ + if (!list_empty(mReq-queue)) { + retval = -EBUSY; + dev_err(mEp-ci-dev, request already in queue\n); + goto done; + } + + if (req-length 4 * CI13XXX_PAGE_SIZE) { + retval = -EMSGSIZE; + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; + } + + dbg_queue(_usb_addr(mEp), req, retval); + + /* push request */ + mReq-req.status = -EINPROGRESS; + mReq-req.actual = 0; + + retval = _hardware_enqueue(mEp, mReq); + + if (retval == -EALREADY) { + dbg_event(_usb_addr(mEp), QUEUE, retval); + retval = 0; + } + if (!retval) + list_add_tail(mReq-queue, mEp-qh.queue); + + done: + return retval; +} + +/** * isr_get_status_response: get_status request response * @ci: ci struct * @setup: setup request packet @@ -717,9 +777,7 @@ __acquires(mEp-lock) } /* else do nothing; reserved for future use */ - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, req, gfp_flags); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, req, gfp_flags); if (retval) goto err_free_buf; @@ -766,8 +824,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct usb_request *req) * This function returns an error code */ static int isr_setup_status_phase(struct ci13xxx *ci) -__releases(mEp-lock) -__acquires(mEp-lock) { int retval; struct ci13xxx_ep *mEp; @@ -776,9 +832,7 @@ __acquires(mEp-lock) ci-status-context = ci; ci-status-complete = isr_setup_status_complete; - spin_unlock(mEp-lock); - retval = usb_ep_queue(mEp-ep, ci-status, GFP_ATOMIC); - spin_lock(mEp-lock); + retval = _ep_queue(mEp-ep, ci-status, GFP_ATOMIC); return retval; } @@ -1177,8 +1231,6 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t __maybe_unused gfp_flags) { struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep); - struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req); - struct ci13xxx *ci = mEp-ci; int retval = 0; unsigned long flags; @@ -1187,47 +1239,8 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, spin_lock_irqsave(mEp-lock, flags); - if (mEp-type == USB_ENDPOINT_XFER_CONTROL) { - if (req-length) - mEp = (ci-ep0_dir == RX) ? - ci-ep0out : ci-ep0in; - if (!list_empty(mEp-qh.queue)) { - _ep_nuke(mEp); - retval = -EOVERFLOW; - dev_warn(mEp-ci-dev, endpoint ctrl %X nuked\n, -_usb_addr(mEp)); - } - } + retval = _ep_queue(ep, req, gfp_flags); - /* first nuke then test link, e.g. previous status has not sent */ - if (!list_empty(mReq-queue)) { - retval = -EBUSY; -
[PATCH v8 1/8] usb: chipidea: udc: add attribute aligned(4) to shared memory structs
The udc uses an shared dma memory space between hard and software. This memory layout is described in ci13xxx_qh and ci13xxx_td which are marked with the attribute ((packed)). The compiler currently does not know about the alignment of the memory layout, and will create strb and ldrb operations. The Datasheet of the synopsys core describes, that some operations on the mapped memory need to be atomic double word operations. I.e. the next pointer addressing in the qhead, as otherwise the hardware will read wrong data and totally stuck. This is also possible while working with the current active td queue, and preparing the td-ptr.next in software while the hardware is still working with the current active td which is supposed to be changed: writeb(0xde, td-ptr.next + 0x0); /* strb */ writeb(0xad, td-ptr.next + 0x1); /* strb */ - hardware reads value of td-ptr.next and get stuck! writeb(0xbe, td-ptr.next + 0x2); /* strb */ writeb(0xef, td-ptr.next + 0x3); /* strb */ This appeares on armv5 machines where the hardware does not support unaligned 32bit operations. This patch adds the attribute ((aligned(4))) to the structures to tell the compiler to use 32bit operations. It also adds an wmb() for the prepared TD data before it gets enqueued into the qhead. Cc: stable sta...@vger.kernel.org Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/usb/chipidea/udc.c | 2 ++ drivers/usb/chipidea/udc.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index f64fbea..77fb66f 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -461,6 +461,8 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) mReq-ptr-page[i] = (mReq-req.dma + i * CI13XXX_PAGE_SIZE) ~TD_RESERVED_MASK; + wmb(); + if (!list_empty(mEp-qh.queue)) { struct ci13xxx_req *mReqPrev; int n = hw_ep_bit(mEp-num, mEp-dir); diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h index 4ff2384d..d12e8b5 100644 --- a/drivers/usb/chipidea/udc.h +++ b/drivers/usb/chipidea/udc.h @@ -40,7 +40,7 @@ struct ci13xxx_td { #define TD_CURR_OFFSET(0x0FFFUL 0) #define TD_FRAME_NUM (0x07FFUL 0) #define TD_RESERVED_MASK (0x0FFFUL 0) -} __attribute__ ((packed)); +} __attribute__ ((packed, aligned(4))); /* DMA layout of queue heads */ struct ci13xxx_qh { @@ -57,7 +57,7 @@ struct ci13xxx_qh { /* 9 */ u32 RESERVED; struct usb_ctrlrequest setup; -} __attribute__ ((packed)); +} __attribute__ ((packed, aligned(4))); /** * struct ci13xxx_req - usb request representation -- 1.8.2.rc2 -- 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 v8 5/8] usb: chipidea: udc: don't truncate requests to single tds
It is not safe to truncate requests to the maximum possible size the controller can handle with one td and to keep working. That patch fixes that with proper error handling instead. Reported-by: Felipe Balbi ba...@ti.com Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de --- drivers/usb/chipidea/udc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index d4db3ac..e7c84ab 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1207,9 +1207,9 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req, } if (req-length 4 * CI13XXX_PAGE_SIZE) { - req-length = 4 * CI13XXX_PAGE_SIZE; retval = -EMSGSIZE; - dev_warn(mEp-ci-dev, request length truncated\n); + dev_err(mEp-ci-dev, request bigger than one td\n); + goto done; } dbg_queue(_usb_addr(mEp), req, retval); -- 1.8.2.rc2 -- 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 v8 3/8] usb: chipidea: udc: move ZLT flag change to ep_enable
Its not necessary and also not specified in the datasheet to change the ZLT flag before every ep_prime. This patch moves this to the ep_enable and applies it only for non configuration endpoints. Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/usb/chipidea/udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 4bbbe62..a8528ad 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -489,7 +489,6 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq) /* QH configuration */ mEp-qh.ptr-td.next = mReq-dma;/* TERMINATE = 0 */ mEp-qh.ptr-td.token = ~(TD_STATUS_HALTED|TD_STATUS_ACTIVE); - mEp-qh.ptr-cap |= QH_ZLT; wmb(); /* synchronize before ep prime */ @@ -1052,6 +1051,8 @@ static int ep_enable(struct usb_ep *ep, mEp-qh.ptr-cap = ~QH_MULT; else mEp-qh.ptr-cap = ~QH_ZLT; + if (mEp-num) + mEp-qh.ptr-cap |= QH_ZLT; mEp-qh.ptr-cap |= (mEp-ep.maxpacket ffs_nr(QH_MAX_PKT)) QH_MAX_PKT; -- 1.8.2.rc2 -- 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: Piggy-backing new hardware using old usb-serial
On Wed, Mar 27, 2013 at 09:28:11PM +0100, Wesley W. Terpstra wrote: On Wed, 2013-03-27 at 13:06 -0700, Greg KH wrote: Our current prototypes borrow the Sierra VID And the USB-IF might revoke your vendor id, if they find you shipping a device with a different vendor id than the one you have been assigned. One of the reasons we borrowed that VID is that we do not currently have a VID assigned. We are still deciding whether it is worth joining the USB-IF just to get a vendor ID for a few thousands of devices. I am of the opinion that it is, but I cannot sign the membership forms on behalf of GSI. We probably will end up buying a VID soon. Why not just add your device id to an existing driver, sending in a patch to do so. All distros will pick that up and then your device will just work on all Linux distros. I was under the impression that drivers in the linux mainline had to be for hardware that was widely available. Not true at all, we take drivers for _anything_ :) I take it then, that just adding support to an existing driver is acceptable? That's also ok, if you are using the same chip that the driver supports. That wouldn't address people with older linux distributions, but would definitely be a good long term solution. It's really a shame there is no USB-IF standard for usb-serial... then things would even potentially work out of the box on windows. You can use the CDC-ACM driver, as you have pointed out, which should allow your device to work on any OS with no driver needed, so maybe just use that instead. There's no need to support modem commands in your device if you use it. What driver should I target? What chip do you use for your device? The device I am concerned about right now has an Altera arria2 connected to a cy7c68013a-56baxc (fx2lp). We have several form factor variations. A few have FTDI chips where I don't need to care, but can also do less. If you use the ftdi chip, then use the ftdi driver, and add the device id that ftdi gives you to it. Hope this helps, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: Add usb port system pm support
On 2013/3/28 2:47, Alan Stern wrote: On Thu, 28 Mar 2013, Lan Tianyu wrote What happens if there's no device plugged in to the port, but the hub is enabled for remote wakeup? How will the hub be able to detect a plug-in event if the port isn't powered? Alan Stern Hi Alan: Great thanks for your review. The hub will not detect the new devices. From my opinion, this depends on the user space since the port only will be powered off when pm qos NO_POWER_OFF flag is unset(it is default to be set). If unset the flag, losing plug-in event should have been took into account. -- Best Regards Tianyu Lan linux kernel enabling team -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 0/6] remove the clock name from pdata
On Wed, Mar 27, 2013 at 9:14 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Mon, Mar 25, 2013 at 03:06:51AM -0400, Chao Xie wrote: The clock is defined by device, so the driver knows how many clocks needed by the device. The orignal way that passing the clock name by pdata is not correct. The following patches fix it. V2-V1: typo fix rebased on latest usb-next Chao Xie (6): usb: gadget: mv_udc_core: remove unused clock usb: otg: mv_otg: remove unused clock usb: ehci: mv_ehci: remove unused clock arm: mmp: remove clock from usb pdata for aspenite arm: mmp: remove clock name from usb pdata for ttc usb: mv_usb: remove clock name from pdata how should this series be handled ? If you want me to carry all patches, I need Acks from other maintainers for ehci and arch/arm/ parts. Until then, I'm dropping this from my TODO list. -- balbi hi, Alan Can you help me to review the EHCI parts? It changes the way of getting clock. The orginal way uses the pdata to get the clock name, and it is not correct, and the patch fix it. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/6] usb: ehci: mv_ehci: remove unused clock
hi, Alan This is the patch for EHCI clock fix. Can you help to review and ack it? Thanks. On Mon, Mar 25, 2013 at 3:06 PM, Chao Xie chao@marvell.com wrote: The origianl understanding of clock is wrong. The EHCI controller only have one clock input. Passing clock name by pdata is wrong. The clock is defined by device iteself. Signed-off-by: Chao Xie chao@marvell.com --- drivers/usb/host/ehci-mv.c | 35 ++- 1 files changed, 10 insertions(+), 25 deletions(-) diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c index 3804820..6bad41a 100644 --- a/drivers/usb/host/ehci-mv.c +++ b/drivers/usb/host/ehci-mv.c @@ -33,25 +33,17 @@ struct ehci_hcd_mv { struct mv_usb_platform_data *pdata; - /* clock source and total clock number */ - unsigned int clknum; - struct clk *clk[0]; + struct clk *clk; }; static void ehci_clock_enable(struct ehci_hcd_mv *ehci_mv) { - unsigned int i; - - for (i = 0; i ehci_mv-clknum; i++) - clk_prepare_enable(ehci_mv-clk[i]); + clk_prepare_enable(ehci_mv-clk); } static void ehci_clock_disable(struct ehci_hcd_mv *ehci_mv) { - unsigned int i; - - for (i = 0; i ehci_mv-clknum; i++) - clk_disable_unprepare(ehci_mv-clk[i]); + clk_disable_unprepare(ehci_mv-clk); } static int mv_ehci_enable(struct ehci_hcd_mv *ehci_mv) @@ -144,9 +136,8 @@ static int mv_ehci_probe(struct platform_device *pdev) struct ehci_hcd *ehci; struct ehci_hcd_mv *ehci_mv; struct resource *r; - int clk_i, retval = -ENODEV; + int retval = -ENODEV; u32 offset; - size_t size; if (!pdata) { dev_err(pdev-dev, missing platform_data\n); @@ -160,8 +151,7 @@ static int mv_ehci_probe(struct platform_device *pdev) if (!hcd) return -ENOMEM; - size = sizeof(*ehci_mv) + sizeof(struct clk *) * pdata-clknum; - ehci_mv = devm_kzalloc(pdev-dev, size, GFP_KERNEL); + ehci_mv = devm_kzalloc(pdev-dev, sizeof(*ehci_mv), GFP_KERNEL); if (ehci_mv == NULL) { dev_err(pdev-dev, cannot allocate ehci_hcd_mv\n); retval = -ENOMEM; @@ -172,16 +162,11 @@ static int mv_ehci_probe(struct platform_device *pdev) ehci_mv-pdata = pdata; ehci_mv-hcd = hcd; - ehci_mv-clknum = pdata-clknum; - for (clk_i = 0; clk_i ehci_mv-clknum; clk_i++) { - ehci_mv-clk[clk_i] = - devm_clk_get(pdev-dev, pdata-clkname[clk_i]); - if (IS_ERR(ehci_mv-clk[clk_i])) { - dev_err(pdev-dev, error get clck \%s\\n, - pdata-clkname[clk_i]); - retval = PTR_ERR(ehci_mv-clk[clk_i]); - goto err_clear_drvdata; - } + ehci_mv-clk = devm_clk_get(pdev-dev, NULL); + if (IS_ERR(ehci_mv-clk)) { + dev_err(pdev-dev, error getting clock\n); + retval = PTR_ERR(ehci_mv-clk); + goto err_clear_drvdata; } r = platform_get_resource_byname(pdev, IORESOURCE_MEM, phyregs); -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 4/6] arm: mmp: remove clock from usb pdata for aspenite
hi, Haojian The patches correct the wrong usage of clock. It finally do not need pass clock by pdata. Can you help to review the patches relates to arch-mmp, and ack it?Thanks. On Mon, Mar 25, 2013 at 3:06 PM, Chao Xie chao@marvell.com wrote: The clock name will directly get by driver. Removing the name from pdata. Signed-off-by: Chao Xie chao@marvell.com --- arch/arm/mach-mmp/aspenite.c |6 -- 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-mmp/aspenite.c b/arch/arm/mach-mmp/aspenite.c index 9f64d56..76901f4 100644 --- a/arch/arm/mach-mmp/aspenite.c +++ b/arch/arm/mach-mmp/aspenite.c @@ -223,13 +223,7 @@ static struct pxa27x_keypad_platform_data aspenite_keypad_info __initdata = { }; #if defined(CONFIG_USB_EHCI_MV) -static char *pxa168_sph_clock_name[] = { - [0] = PXA168-USBCLK, -}; - static struct mv_usb_platform_data pxa168_sph_pdata = { - .clknum = 1, - .clkname= pxa168_sph_clock_name, .mode = MV_USB_MODE_HOST, .phy_init = pxa_usb_phy_init, .phy_deinit = pxa_usb_phy_deinit, -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html