[PATCH] usb: remove intel_mid_otg.h
It's not used anymore. Signed-off-by: Heikki Krogerus --- include/linux/usb/intel_mid_otg.h | 180 -- 1 file changed, 180 deletions(-) delete mode 100644 include/linux/usb/intel_mid_otg.h diff --git a/include/linux/usb/intel_mid_otg.h b/include/linux/usb/intel_mid_otg.h deleted file mode 100644 index 756cf55..000 --- a/include/linux/usb/intel_mid_otg.h +++ /dev/null @@ -1,180 +0,0 @@ -/* - * Intel MID (Langwell/Penwell) USB OTG Transceiver driver - * Copyright (C) 2008 - 2010, Intel Corporation. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms and conditions of the GNU General Public License, - * version 2, as published by the Free Software Foundation. - * - * This program is distributed in the hope 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. - * - * You should have received a copy of the GNU General Public License along with - * this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA. - * - */ - -#ifndef __INTEL_MID_OTG_H -#define __INTEL_MID_OTG_H - -#include -#include -#include - -struct intel_mid_otg_xceiv; - -/* This is a common data structure for Intel MID platform to - * save values of the OTG state machine */ -struct otg_hsm { - /* Input */ - int a_bus_resume; - int a_bus_suspend; - int a_conn; - int a_sess_vld; - int a_srp_det; - int a_vbus_vld; - int b_bus_resume; - int b_bus_suspend; - int b_conn; - int b_se0_srp; - int b_ssend_srp; - int b_sess_end; - int b_sess_vld; - int id; -/* id values */ -#define ID_B 0x05 -#define ID_A 0x04 -#define ID_ACA_C 0x03 -#define ID_ACA_B 0x02 -#define ID_ACA_A 0x01 - int power_up; - int adp_change; - int test_device; - - /* Internal variables */ - int a_set_b_hnp_en; - int b_srp_done; - int b_hnp_enable; - int hnp_poll_enable; - - /* Timeout indicator for timers */ - int a_wait_vrise_tmout; - int a_wait_bcon_tmout; - int a_aidl_bdis_tmout; - int a_bidl_adis_tmout; - int a_bidl_adis_tmr; - int a_wait_vfall_tmout; - int b_ase0_brst_tmout; - int b_bus_suspend_tmout; - int b_srp_init_tmout; - int b_srp_fail_tmout; - int b_srp_fail_tmr; - int b_adp_sense_tmout; - - /* Informative variables */ - int a_bus_drop; - int a_bus_req; - int a_clr_err; - int b_bus_req; - int a_suspend_req; - int b_bus_suspend_vld; - - /* Output */ - int drv_vbus; - int loc_conn; - int loc_sof; - - /* Others */ - int vbus_srp_up; -}; - -/* must provide ULPI access function to read/write registers implemented in - * ULPI address space */ -struct iotg_ulpi_access_ops { - int (*read)(struct intel_mid_otg_xceiv *iotg, u8 reg, u8 *val); - int (*write)(struct intel_mid_otg_xceiv *iotg, u8 reg, u8 val); -}; - -#define OTG_A_DEVICE 0x0 -#define OTG_B_DEVICE 0x1 - -/* - * the Intel MID (Langwell/Penwell) otg transceiver driver needs to interact - * with device and host drivers to implement the USB OTG related feature. More - * function members are added based on usb_phy data structure for this - * purpose. - */ -struct intel_mid_otg_xceiv { - struct usb_phy otg; - struct otg_hsm hsm; - - /* base address */ - void __iomem*base; - - /* ops to access ulpi */ - struct iotg_ulpi_access_ops ulpi_ops; - - /* atomic notifier for interrupt context */ - struct atomic_notifier_head iotg_notifier; - - /* start/stop USB Host function */ - int (*start_host)(struct intel_mid_otg_xceiv *iotg); - int (*stop_host)(struct intel_mid_otg_xceiv *iotg); - - /* start/stop USB Peripheral function */ - int (*start_peripheral)(struct intel_mid_otg_xceiv *iotg); - int (*stop_peripheral)(struct intel_mid_otg_xceiv *iotg); - - /* start/stop ADP sense/probe function */ - int (*set_adp_probe)(struct intel_mid_otg_xceiv *iotg, - bool enabled, int dev); - int (*set_adp_sense)(struct intel_mid_otg_xceiv *iotg, - bool enabled); - -#ifdef CONFIG_PM - /* suspend/resume USB host function */ - int (*suspend_host)(struct intel_mid_otg_xceiv *iotg, - pm_message_t message); - int (*resume_host)(struct intel_mid_otg_xceiv *iotg); - - int (*suspend_peripheral)(struct intel_mid_otg_xceiv *iotg, - pm_message_t message); - int
Re: [PATCH v7 09/10] usb: dwc3: omap: manage "usb_otg_ss_refclk960m" clock
Greg, On 10/03/2013 06:41 PM, Greg KH wrote: > On Thu, Oct 03, 2013 at 05:54:14PM +0300, Roger Quadros wrote: >> On 10/03/2013 03:29 PM, Felipe Balbi wrote: >>> Hi, >>> >>> On Wed, Oct 02, 2013 at 04:41:53PM +0300, Roger Quadros wrote: On 10/02/2013 04:11 PM, Felipe Balbi wrote: > On Mon, Sep 23, 2013 at 04:11:30PM +0300, Roger Quadros wrote: >> Hi Felipe, >> >> On 09/18/2013 03:49 PM, Roger Quadros wrote: >>> "usb_otg_ss_refclk960m" is an optional functional clock to the >>> UBS_OTG_SS module. So manage it in the driver. >>> >> >> Just realized that "usb_otg_ss_refclk960m" is in fact functional clock >> to the >> PHY and not USB_OTG_SS module. The name is misleading. >> >> So please ignore patch 9 and 10. > > ignored. All others are fine, right ? > Yes. But I might have to rebase this on top of Phy framework stuff. >>> >>> alright, Greg already took the PHY framework, so maybe we need to just >>> give him my Acked-by and he takes the patches directly as I don't have >>> PYH framework. >>> >> OK. I'll resend this series to Greg in a while. > > It looks like you did, but forgot Felipe's ack :( > I was under the impression that Felipe will explicitly give his Ack there. Should I resend with his Acked-bys? cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe 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 09/10] usb: dwc3: omap: manage "usb_otg_ss_refclk960m" clock
On Fri, Oct 04, 2013 at 01:46:08PM +0300, Roger Quadros wrote: > Greg, > > On 10/03/2013 06:41 PM, Greg KH wrote: > > On Thu, Oct 03, 2013 at 05:54:14PM +0300, Roger Quadros wrote: > >> On 10/03/2013 03:29 PM, Felipe Balbi wrote: > >>> Hi, > >>> > >>> On Wed, Oct 02, 2013 at 04:41:53PM +0300, Roger Quadros wrote: > On 10/02/2013 04:11 PM, Felipe Balbi wrote: > > On Mon, Sep 23, 2013 at 04:11:30PM +0300, Roger Quadros wrote: > >> Hi Felipe, > >> > >> On 09/18/2013 03:49 PM, Roger Quadros wrote: > >>> "usb_otg_ss_refclk960m" is an optional functional clock to the > >>> UBS_OTG_SS module. So manage it in the driver. > >>> > >> > >> Just realized that "usb_otg_ss_refclk960m" is in fact functional clock > >> to the > >> PHY and not USB_OTG_SS module. The name is misleading. > >> > >> So please ignore patch 9 and 10. > > > > ignored. All others are fine, right ? > > > Yes. But I might have to rebase this on top of Phy framework stuff. > >>> > >>> alright, Greg already took the PHY framework, so maybe we need to just > >>> give him my Acked-by and he takes the patches directly as I don't have > >>> PYH framework. > >>> > >> OK. I'll resend this series to Greg in a while. > > > > It looks like you did, but forgot Felipe's ack :( > > > I was under the impression that Felipe will explicitly give his Ack there. > Should I resend with his Acked-bys? No need, I took the patches yesterday, you should have seen the automated emails, right? 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: Usb 3.0 external dd offlined, not ready after error recovery
On Thu, 3 Oct 2013, [ISO-8859-1] Jorge Mu�oz Camadro wrote: > > Although the drive is USB-3 and is capable of communicating at > > SuperSpeed (5 Gb/s), it is running at only high speed (480 Mb/s). > > This suggests there may be something wrong with the cable. Are you > > using a high-quality USB-3 cable? > > I'm using the one who comes with the drive, i've never changed it, and > doesn't show any sign of been broken or something like that. It seems > like a propietary model. Well, it sure looks like something is wrong. Do you have another computer with a USB-3 port you can try plugging the drive into? > > It's possible that this is a link power management problem. There have > > been reports of similar problems in that area, and a patch was posted > > here: > > > > http://marc.info/?l=linux-usb&m=138005811528076&w=2 > > > > Does the drive work with that patch installed? > > > > Guess not, i'm using exactly the same kernel provided by my distro, no > changes. That patch is not yet included in any distro kernels. To test it you will have to build your own kernel. Or else wait until your distro releases it, which will probably take some weeks. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/3] usb: dwc3: msm: Add device tree binding information
On Wed, Aug 21, 2013 at 04:29:44PM +0300, Ivan T. Ivanov wrote: > From: "Ivan T. Ivanov" > > MSM USB3.0 core wrapper consist of USB3.0 IP from Synopsys > (SNPS) and HS, SS PHY's control and configuration registers. > > It could operate in device mode (SS, HS, FS) and host > mode (SS, HS, FS, LS). > > Signed-off-by: Ivan T. Ivanov these patches were sent *ages* and nobody from DT mailing list has reviewed. Unless someone steps up now, I'll be queueing these patches next week. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 07/19] usb/gadget: f_mass_storage: use fsg_common_setup in fsg_common_init
Hi, On Wed, Oct 02, 2013 at 04:15:06PM +0200, Andrzej Pietrasiewicz wrote: > W dniu 01.10.2013 16:53, Felipe Balbi pisze: > >On Thu, Sep 26, 2013 at 02:38:21PM +0200, Andrzej Pietrasiewicz wrote: > >>fsg_common_init is a lengthy function. Now there are helper functions > >>which cover all parts of it. Use them. > >> > >>Signed-off-by: Andrzej Pietrasiewicz > >>Signed-off-by: Kyungmin Park > > > >this patch adds Warnings: > > > >In file included from > >drivers/usb/gadget/mass_storage.c:58:0: > >drivers/usb/gadget/f_mass_storage.c:2646:27: > >warning: 'fsg_common_setup' defined but not used [-Wunused-function] > >In file included from > >linux/drivers/usb/gadget/acm_ms.c:43:0: > >linux/drivers/usb/gadget/f_mass_storage.c:2646:27: > >warning: 'fsg_common_setup' defined but not used [-Wunused-function] > >In file included from > >linux/drivers/usb/gadget/multi.c:44:0: > >linux/drivers/usb/gadget/f_mass_storage.c:2646:27: > >warning: 'fsg_common_setup' defined but not used [-Wunused-function] > > > >I've stopped applying this series. when resending, please rebase on my > >'testing' branch. > > > Hi Felipe, > > I believe these warnings come from the patch #6 in the series; > the patch you are referring to (#7) _is_ actually about _using_ the > fsg_common_setup(). > > I have now distributed the code of the series over the patches in a > different way so that there are no warnings about unused functions. > The new series has an additional benefit of some patches being shorter. > I will be sending the series tomorrow as I would like to do some testing > before I submit it. alright, please rebase on my testing branch (or next) since some of your original patches were already applied. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] USB: gadget: s3c-hsotg: add isochronous transfers support
On Wed, Oct 02, 2013 at 12:34:21PM +0200, Robert Baldyga wrote: > On 10/01/2013 04:45 PM, Felipe Balbi wrote: > Hello, > >Hi, > > > >On Tue, Sep 24, 2013 at 11:47:16AM +0200, Robert Baldyga wrote: > >>Hello, > >> > >>This is update for my proposal for isochronous transfers support in > >>s3c-hsotg > >>driver. I've fixed issuses pointed by Bartlomiej Zolnierkiewicz. For more > >>information, please check the change log at the end of the mail. > > > >this shouldn't be part of commit log. > > > >>This patch adds isochronous transfer support. It adds few modifications: > >>- Modify s3c_hsotg_write_fifo() function. It actually calculates transfer > >> size, taking into account Multi Count value, which indicates number of > >> transactions per microframe. > >>- Fix s3c_hsotg_start_req() function by setting number of packets to Multi > >> Count field in DIEPTSIZ register for isochronous endpoints. > >>- Fix s3c_hsotg_set_ep_maxpacket() function. Field wMaxPacketSize of > >>endpoint > >> descriptor is now splitted into maximum packet size value and number of > >> additional transaction per microframe. > >>- Modify s3c_hsotg_epint() function. Some interrupts are ignored for > >> isochronous endpoints, (e.g. INTknTXFEmpMsk) becouse isochronous request > >> is > >> always transfered in single transaction, which ends with XferCompl > >> interrupt. > >> Add Odd/Even microframe toggle to allow data transfering in each > >> microframe. > >>- Fix s3c_hsotg_ep_enable() function by supporting isochronous endpoint > >>type. > > > >you're doing way too many things in a single patch. > > All these changes concerns to isochronous transfers support, and I > think they should not be splitted into number of patches. This entire > patch adds one compact functionality and it has no sense to put small > parts of this functionality to separated patches. you have at least two bug fixes here which should be splitted into their own patches. I'm not taking this patch the way it is. -- balbi signature.asc Description: Digital signature
[PATCH] xhci: use usb_ss_max_streams in xhci_check_streams_endpoint
The ss_ep_comp bmAttributes filed can contain more info then just the streams, use usb_ss_max_streams to properly get max streams. Signed-off-by: Hans de Goede --- drivers/usb/host/xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index e06ee5d..17add9c 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2964,7 +2964,7 @@ static int xhci_check_streams_endpoint(struct xhci_hcd *xhci, ret = xhci_check_args(xhci_to_hcd(xhci), udev, ep, 1, true, __func__); if (ret <= 0) return -EINVAL; - if (ep->ss_ep_comp.bmAttributes == 0) { + if (usb_ss_max_streams(&ep->ss_ep_comp) == 0) { xhci_warn(xhci, "WARN: SuperSpeed Endpoint Companion" " descriptor for ep 0x%x does not support streams\n", ep->desc.bEndpointAddress); -- 1.8.3.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
[PATCH 8/9] usbfs: Add ep_to_host_endpoint helper function
Signed-off-by: Hans de Goede --- drivers/usb/core/devio.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 102dbd2..107e107 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -753,6 +753,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, return ret; } +static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, +unsigned char ep) +{ + if (ep & USB_ENDPOINT_DIR_MASK) + return dev->ep_in[ep & USB_ENDPOINT_NUMBER_MASK]; + else + return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1198,15 +1207,10 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (ret) return ret; } - if ((uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0) { - is_in = 1; - ep = ps->dev->ep_in[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } else { - is_in = 0; - ep = ps->dev->ep_out[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } + ep = ep_to_host_endpoint(ps->dev, uurb->endpoint); if (!ep) return -ENOENT; + is_in = (uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0; u = 0; switch(uurb->type) { -- 1.8.3.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
[PATCH 6/9] usbfs: proc_do_submiturb use a local variable for number_of_packets
This is a preparation patch for adding support for bulk streams. Signed-off-by: Hans de Goede --- drivers/usb/core/devio.c | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 7dfaa87..3ea1551 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1176,6 +1176,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, struct usb_ctrlrequest *dr = NULL; unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; + int number_of_packets = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1229,7 +1230,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, le16_to_cpup(&dr->wIndex)); if (ret) goto error; - uurb->number_of_packets = 0; uurb->buffer_length = le16_to_cpup(&dr->wLength); uurb->buffer += 8; if ((dr->bRequestType & USB_DIR_IN) && uurb->buffer_length) { @@ -1257,9 +1257,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, case USB_ENDPOINT_XFER_INT: /* allow single-shot interrupt transfers */ uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; - goto interrupt_urb; + break; } - uurb->number_of_packets = 0; num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; @@ -1268,8 +1267,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, case USBDEVFS_URB_TYPE_INTERRUPT: if (!usb_endpoint_xfer_int(&ep->desc)) return -EINVAL; - interrupt_urb: - uurb->number_of_packets = 0; break; case USBDEVFS_URB_TYPE_ISO: @@ -1279,15 +1276,16 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, return -EINVAL; if (!usb_endpoint_xfer_isoc(&ep->desc)) return -EINVAL; + number_of_packets = uurb->number_of_packets; isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) * - uurb->number_of_packets; + number_of_packets; if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL))) return -ENOMEM; if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) { ret = -EFAULT; goto error; } - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { /* * arbitrary limit need for USB 3.0 * bMaxBurst (0~15 allowed, 1~16 packets) @@ -1318,7 +1316,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ret = -EFAULT; goto error; } - as = alloc_async(uurb->number_of_packets); + as = alloc_async(number_of_packets); if (!as) { ret = -ENOMEM; goto error; @@ -1412,7 +1410,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->setup_packet = (unsigned char *)dr; dr = NULL; as->urb->start_frame = uurb->start_frame; - as->urb->number_of_packets = uurb->number_of_packets; + as->urb->number_of_packets = number_of_packets; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); @@ -1420,7 +1418,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->interval = ep->desc.bInterval; as->urb->context = as; as->urb->complete = async_completed; - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { as->urb->iso_frame_desc[u].offset = totlen; as->urb->iso_frame_desc[u].length = isopkt[u].length; totlen += isopkt[u].length; -- 1.8.3.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
[PATCH 3/9] usb-core: Track if an endpoint has streams
This is a preparation patch for adding support for bulk streams to usbfs. Signed-off-by: Hans de Goede --- drivers/usb/core/hcd.c | 40 +++- include/linux/usb.h| 1 + 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index bd5acdd..be6f5d1 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2048,7 +2048,7 @@ int usb_alloc_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); @@ -2057,13 +2057,24 @@ int usb_alloc_streams(struct usb_interface *interface, if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ - for (i = 0; i < num_eps; i++) + for (i = 0; i < num_eps; i++) { + /* Streams only apply to bulk endpoints. */ if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) return -EINVAL; + /* Re-alloc is not allowed */ + if (eps[i]->has_streams) + return -EINVAL; + } - return hcd->driver->alloc_streams(hcd, dev, eps, num_eps, + ret = hcd->driver->alloc_streams(hcd, dev, eps, num_eps, num_streams, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->has_streams = 1; + + return ret; } EXPORT_SYMBOL_GPL(usb_alloc_streams); @@ -2085,19 +2096,30 @@ int usb_free_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ - for (i = 0; i < num_eps; i++) - if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc)) + for (i = 0; i < num_eps; i++) { + /* Streams only apply to bulk endpoints. */ + if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) + return -EINVAL; + /* Double-free is not allowed */ + if (!eps[i]->has_streams) return -EINVAL; + } + + ret = hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->has_streams = 0; - return hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + return ret; } EXPORT_SYMBOL_GPL(usb_free_streams); diff --git a/include/linux/usb.h b/include/linux/usb.h index 449ca54..124cbed 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -71,6 +71,7 @@ struct usb_host_endpoint { unsigned char *extra; /* Extra descriptors */ int extralen; int enabled; + int has_streams; }; /* host-side wrapper for one interface setting's parsed descriptors */ -- 1.8.3.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
[PATCH 9/9] usbfs: Add support for allocating / freeing streams
Signed-off-by: Hans de Goede --- drivers/usb/core/devio.c | 113 ++ include/uapi/linux/usbdevice_fs.h | 7 +++ 2 files changed, 120 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 107e107..d00a28e 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -762,6 +762,74 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; } +static int parse_usbdevfs_streams(struct usb_device *dev, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 1024 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 1024)) + return -EINVAL; + + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); + if (!eps) + return -ENOMEM; + + for (i = 0; i < num_eps; i++) { + if (get_user(ep, &streams->eps[i])) { + ret = -EFAULT; + goto error; + } + eps[i] = ep_to_host_endpoint(dev, ep); + if (!eps[i]) { + ret = -EINVAL; + goto error; + } + + /* usb_alloc/free_streams operate on an usb_interface */ + ret = findintfep(dev, ep); + if (ret < 0) + goto error; + + if (i == 0) { + intf = usb_ifnum_to_if(dev, ret); + } else { + /* Verify all eps belong to the same interface */ + if (ret != intf->altsetting->desc.bInterfaceNumber) { + ret = -EINVAL; + goto error; + } + } + } + + if (num_streams_ret) + *num_streams_ret = num_streams; + *num_eps_ret = num_eps; + *eps_ret = eps; + *intf_ret = intf; + + return 0; + +error: + kfree(eps); + return ret; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1976,6 +2044,45 @@ static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) return claimintf(ps, dc.interface); } +static int proc_alloc_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps->dev, arg, &num_streams, &num_eps, + &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_alloc_streams(intf, eps, num_eps, num_streams, GFP_KERNEL); + kfree(eps); + return r; +} + +static int proc_free_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps->dev, arg, NULL, &num_eps, &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_free_streams(intf, eps, num_eps, GFP_KERNEL); + kfree(eps); + return r; +} + /* * NOTE: All requests here that have interface numbers as parameters * are assuming that somehow the configuration has been prevented from @@ -2152,6 +2259,12 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, case USBDEVFS_DISCONNECT_CLAIM: ret = proc_disconnect_claim(ps, p); break; + case USBDEVFS_ALLOC_STREAMS: + ret = proc_alloc_streams(ps, p); + break; + case USBDEVFS_FREE_STREAMS: + ret = proc_free_streams(ps, p); + break; } usb_unlock_device(dev); if (ret >= 0) diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index cbf122d..abe5f4b 100644 --- a/include/uapi/linux/usbdevice_fs.h
[PATCH 7/9] usbfs: Add support for bulk stream ids
This patch makes it possible to specify a bulk stream id when submitting an urb using the async usbfs API. It overloads the number_of_packets usbdevfs_urb field for this. This is not pretty, but given other constraints it is the best we can do. The reasoning leading to this goes as follows: 1) We want to support bulk streams in the usbfs API 2) We do not want to extend the usbdevfs_urb struct with a new member, as that would mean defining new ioctl numbers for all async API ioctls + adding compat versions for the old ones (times 2 for 32 bit support) 3) 1 + 2 means we need to re-use an existing field 4) number_of_packets is only used for isoc urbs, and streams are bulk only so it is the best (and only) candidate for re-using Note that: 1) This patch only uses number_of_packets as stream_id if the app has actually allocated streams on the ep, so that old apps which may have garbage in there (as it was unused until now in the bulk case), will not break 2) This patch does not add support for allocating / freeing bulk-streams, that is done in a follow up patch Signed-off-by: Hans de Goede --- drivers/usb/core/devio.c | 4 include/uapi/linux/usbdevice_fs.h | 5 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 3ea1551..102dbd2 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1177,6 +1177,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; int number_of_packets = 0; + unsigned int stream_id = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1262,6 +1263,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; + if (ep->has_streams) + stream_id = uurb->stream_id; break; case USBDEVFS_URB_TYPE_INTERRUPT: @@ -1411,6 +1414,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, dr = NULL; as->urb->start_frame = uurb->start_frame; as->urb->number_of_packets = number_of_packets; + as->urb->stream_id = stream_id; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index 0c65e4b..cbf122d 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -102,7 +102,10 @@ struct usbdevfs_urb { int buffer_length; int actual_length; int start_frame; - int number_of_packets; + union { + int number_of_packets; /* Only used for isoc urbs */ + unsigned int stream_id; /* Only used with bulk streams */ + }; int error_count; unsigned int signr; /* signal to be sent on completion, or 0 if none should be sent. */ -- 1.8.3.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
[PATCH 1/9] usb-core: Fix usb_free_streams return value documentation
Signed-off-by: Hans de Goede --- drivers/usb/core/hcd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 9795a21..bd5acdd 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2077,8 +2077,7 @@ EXPORT_SYMBOL_GPL(usb_alloc_streams); * Reverts a group of bulk endpoints back to not using stream IDs. * Can fail if we are given bad arguments, or HCD is broken. * - * Return: On success, the number of allocated streams. On failure, a negative - * error code. + * Return: 0 on success. On failure, a negative error code. */ int usb_free_streams(struct usb_interface *interface, struct usb_host_endpoint **eps, unsigned int num_eps, -- 1.8.3.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
[PATCH 0/9] usbfs: Add support for usb3 bulk streams
Hi All, I'm very happy to present the (hopefully) final version of my patch set for adding bulk stream support to usbfs. I've tested this using an uas device redirected to a qemu vm (which uses usbfs to access the device), and with the xhci fixes I send yesterday this works well. This patch set has the following changes compared to the RFC I posted a while back: -add a patch to kill urbs on interface on set_interface. This is somewhat unrelated but the alloc / free streams function do the same, so this is here for completeness -propagate free_streams error return in proc_free_streams -kill pending urbs on interface before alloc / free streams -added a patch to properly release streams on interface release -error check / limit num_streams and num_eps on stream alloc / free -make stream_id unsigned Greg, assuming there are no objections, can you add this to the usb-next tree please ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe 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/9] usb-core: Free bulk streams on interface release
Documentation/usb/bulk-streams.txt says: All stream IDs will be deallocated when the driver releases the interface, to ensure that drivers that don't support streams will be able to use the endpoint This commit actually implements this. Signed-off-by: Hans de Goede --- drivers/usb/core/driver.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index f7841d4..a44ff12 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -369,8 +369,9 @@ static int usb_unbind_interface(struct device *dev) { struct usb_driver *driver = to_usb_driver(dev->driver); struct usb_interface *intf = to_usb_interface(dev); + struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS]; struct usb_device *udev; - int error, r, lpm_disable_error; + int i, j, error, r, lpm_disable_error; intf->condition = USB_INTERFACE_UNBINDING; @@ -394,6 +395,15 @@ static int usb_unbind_interface(struct device *dev) driver->disconnect(intf); usb_cancel_queued_reset(intf); + /* Free streams */ + for (i = 0, j = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) { + ep = &intf->cur_altsetting->endpoint[i]; + if (usb_endpoint_xfer_bulk(&ep->desc) && ep->has_streams) + eps[j++] = ep; + } + if (j) + usb_free_streams(intf, eps, j, GFP_KERNEL); + /* Reset other interface state. * We cannot do a Set-Interface if the device is suspended or * if it is prepared for a system sleep (since installing a new -- 1.8.3.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
[PATCH 2/9] usb-core: Move USB_MAXENDPOINTS definitions to usb.h
So that it can be used in other places too. Signed-off-by: Hans de Goede --- drivers/usb/core/config.c | 1 - include/linux/usb.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index a6b2cab..e4f970c 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -11,7 +11,6 @@ #define USB_MAXALTSETTING 128 /* Hard limit */ -#define USB_MAXENDPOINTS 30 /* Hard limit */ #define USB_MAXCONFIG 8 /* Arbitrary limit */ diff --git a/include/linux/usb.h b/include/linux/usb.h index f726c39..449ca54 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -202,6 +202,8 @@ static inline void usb_set_intfdata(struct usb_interface *intf, void *data) struct usb_interface *usb_get_intf(struct usb_interface *intf); void usb_put_intf(struct usb_interface *intf); +/* Hard limit */ +#define USB_MAXENDPOINTS 30 /* this maximum is arbitrary */ #define USB_MAXINTERFACES 32 #define USB_MAXIADS(USB_MAXINTERFACES/2) -- 1.8.3.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
[PATCH 5/9] usbfs: Kill urbs on interface before doing a set_interface
The usb_set_interface documentation says: * Also, drivers must not change altsettings while urbs are scheduled for * endpoints in that interface; all such urbs must first be completed * (perhaps forced by unlinking). For in kernel drivers we trust the drivers to get this right, but we cannot trust userspace to get this right, so enforce it by killing any urbs still pending on the interface. Signed-off-by: Hans de Goede --- drivers/usb/core/devio.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index f4f2300..7dfaa87 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -,6 +,9 @@ static int proc_setintf(struct dev_state *ps, void __user *arg) return -EFAULT; if ((ret = checkintf(ps, setintf.interface))) return ret; + + destroy_async_on_interface(ps, setintf.interface); + return usb_set_interface(ps->dev, setintf.interface, setintf.altsetting); } -- 1.8.3.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 3/9] usb-core: Track if an endpoint has streams
On Fri, 4 Oct 2013, Hans de Goede wrote: > This is a preparation patch for adding support for bulk streams to usbfs. > + for (i = 0; i < num_eps; i++) > + eps[i]->has_streams = 1; > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -71,6 +71,7 @@ struct usb_host_endpoint { > unsigned char *extra; /* Extra descriptors */ > int extralen; > int enabled; > + int has_streams; Instead of storing a 0/1 value, wouldn't it be more useful to store the number of streams the endpoint has? 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/9] usb-core: Free bulk streams on interface release
On Fri, 4 Oct 2013, Hans de Goede wrote: > Documentation/usb/bulk-streams.txt says: > > All stream IDs will be deallocated when the driver releases the interface, to > ensure that drivers that don't support streams will be able to use the > endpoint > > This commit actually implements this. > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c > @@ -369,8 +369,9 @@ static int usb_unbind_interface(struct device *dev) > { > struct usb_driver *driver = to_usb_driver(dev->driver); > struct usb_interface *intf = to_usb_interface(dev); > + struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS]; That's a big array to put on the stack: 30 entries each containing 8 bytes (on a 64-bit arch). 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: wusbcore: add support for isoc out transfers
This patch adds support for isochronous out transfers to the HWA. The primary changes are: 1. Add a isoc_pack_desc_urb field to struct wa_seg. This urb is used to send the isochronous packet info message to the HWA which describes the isoc data segment(s) that will be sent as the payload of the transfer request. 2. Use the URB iso_frame_desc field to populate the isochronous packet info message and data segments sent to the HWA. 3. After the data is sent and transfer result is returned from the HWA, read the isoc packet status message from the HWA. The contents of the isoc packet status message are used to set the iso_frame_desc status and actual_length fields in the original isoc URB. This feature required the addition of a some state tracking variables in struct wahc so the dti_urb knows what type of packet it expects to receive next. Signed-off-by: Thomas Pugliese --- drivers/usb/wusbcore/wa-hc.h | 15 ++ drivers/usb/wusbcore/wa-xfer.c | 493 +++- 2 files changed, 406 insertions(+), 102 deletions(-) diff --git a/drivers/usb/wusbcore/wa-hc.h b/drivers/usb/wusbcore/wa-hc.h index ab39934..b44aca3 100644 --- a/drivers/usb/wusbcore/wa-hc.h +++ b/drivers/usb/wusbcore/wa-hc.h @@ -122,6 +122,11 @@ struct wa_rpipe { }; +enum wa_dti_state { + WA_DTI_TRANSFER_RESULT_PENDING, + WA_DTI_ISOC_PACKET_STATUS_PENDING +}; + /** * Instance of a HWA Host Controller * @@ -181,6 +186,15 @@ struct wahc { spinlock_t rpipe_bm_lock; /* protect rpipe_bm */ struct mutex rpipe_mutex; /* assigning resources to endpoints */ + /* +* dti_state is used to track the state of the dti_urb. When dti_state +* is WA_DTI_ISOC_PACKET_STATUS_PENDING, dti_isoc_xfer_in_progress and +* dti_isoc_xfer_seg identify which xfer the incoming isoc packet status +* refers to. +*/ + enum wa_dti_state dti_state; + u32 dti_isoc_xfer_in_progress; + u8 dti_isoc_xfer_seg; struct urb *dti_urb;/* URB for reading xfer results */ struct urb *buf_in_urb; /* URB for reading data in */ struct edc dti_edc; /* DTI error density counter */ @@ -247,6 +261,7 @@ static inline void wa_init(struct wahc *wa) { edc_init(&wa->nep_edc); atomic_set(&wa->notifs_queued, 0); + wa->dti_state = WA_DTI_TRANSFER_RESULT_PENDING; wa_rpipe_init(wa); edc_init(&wa->dti_edc); INIT_LIST_HEAD(&wa->xfer_list); diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c index 7a78240..a5543e9 100644 --- a/drivers/usb/wusbcore/wa-xfer.c +++ b/drivers/usb/wusbcore/wa-xfer.c @@ -115,6 +115,7 @@ static void wa_xfer_delayed_run(struct wa_rpipe *); */ struct wa_seg { struct urb tr_urb; /* transfer request urb. */ + struct urb *isoc_pack_desc_urb; /* for isoc packet descriptor. */ struct urb *dto_urb;/* for data output. */ struct list_head list_node; /* for rpipe->req_list */ struct wa_xfer *xfer; /* out xfer */ @@ -122,7 +123,6 @@ struct wa_seg { enum wa_seg_status status; ssize_t result; /* bytes xfered or error */ struct wa_xfer_hdr xfer_hdr; - u8 xfer_extra[];/* xtra space for xfer_hdr_ctl */ }; static inline void wa_seg_init(struct wa_seg *seg) @@ -169,7 +169,7 @@ static inline void wa_xfer_init(struct wa_xfer *xfer) /* * Destroy a transfer structure * - * Note that freeing xfer->seg[cnt]->urb will free the containing + * Note that freeing xfer->seg[cnt]->tr_urb will free the containing * xfer->seg[cnt] memory that was allocated by __wa_xfer_setup_segs. */ static void wa_xfer_destroy(struct kref *_xfer) @@ -178,12 +178,14 @@ static void wa_xfer_destroy(struct kref *_xfer) if (xfer->seg) { unsigned cnt; for (cnt = 0; cnt < xfer->segs; cnt++) { - if (xfer->seg[cnt]) { - if (xfer->seg[cnt]->dto_urb) { - kfree(xfer->seg[cnt]->dto_urb->sg); - usb_free_urb(xfer->seg[cnt]->dto_urb); + struct wa_seg *seg = xfer->seg[cnt]; + if (seg) { + usb_free_urb(seg->isoc_pack_desc_urb); + if (seg->dto_urb) { + kfree(seg->dto_urb->sg); + usb_free_urb(seg->dto_urb); } - usb_free_urb(&xfer->seg[cnt]->tr_urb); + usb_free_urb(&seg->tr_urb); } } kfree(xfer->seg); @@ -291,7 +293,8 @@ static unsigned __wa_xfer_is_done(struct wa_xfer *xfer) goto out;
Re: [PATCH 9/9] usbfs: Add support for allocating / freeing streams
On Fri, 4 Oct 2013, Hans de Goede wrote: > +static int parse_usbdevfs_streams(struct usb_device *dev, > + struct usbdevfs_streams __user *streams, > + unsigned int *num_streams_ret, > + unsigned int *num_eps_ret, > + struct usb_host_endpoint ***eps_ret, > + struct usb_interface **intf_ret) > +{ > + unsigned int i, num_streams, num_eps; > + struct usb_host_endpoint **eps; > + struct usb_interface *intf = NULL; > + unsigned char ep; > + int ret; > + > + if (get_user(num_streams, &streams->num_streams) || > + get_user(num_eps, &streams->num_eps)) > + return -EFAULT; > + > + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) > + return -EINVAL; > + > + /* The XHCI controller allows max 1024 streams */ > + if (num_streams_ret && (num_streams < 2 || num_streams > 1024)) > + return -EINVAL; > + > + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); > + if (!eps) > + return -ENOMEM; > + > + for (i = 0; i < num_eps; i++) { > + if (get_user(ep, &streams->eps[i])) { > + ret = -EFAULT; > + goto error; > + } > + eps[i] = ep_to_host_endpoint(dev, ep); > + if (!eps[i]) { > + ret = -EINVAL; > + goto error; > + } > + > + /* usb_alloc/free_streams operate on an usb_interface */ > + ret = findintfep(dev, ep); > + if (ret < 0) > + goto error; > + > + if (i == 0) { > + intf = usb_ifnum_to_if(dev, ret); > + } else { > + /* Verify all eps belong to the same interface */ > + if (ret != intf->altsetting->desc.bInterfaceNumber) { > + ret = -EINVAL; > + goto error; > + } > + } > + } > + > + if (num_streams_ret) > + *num_streams_ret = num_streams; > + *num_eps_ret = num_eps; > + *eps_ret = eps; > + *intf_ret = intf; > + > + return 0; > + > +error: > + kfree(eps); > + return ret; > +} Somewhere in here you should check that the caller has claimed the interface containing these endpoints. 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] memory mapping for usbfs (v0.4)
On Mon, Sep 30, 2013 at 5:12 PM, Markus Rechberger wrote: > On Mon, Sep 30, 2013 at 4:59 PM, Alan Stern wrote: >> On Mon, 30 Sep 2013, Markus Rechberger wrote: >> >>> to explain why Isochronous makes such a difference, the kernel driver >>> doesn't do the memset anymore for each urb packet. >>> However that patch addresses multiple issues >>> * Isochronous improvement by removing memset for each packet >>> * Pre-Allocation to prevent allocation failures during runtime. >>> * Allow to restore the original behavior for >15k buffers (I'd >> >> Why do you use 15 KB as the boundary line? Wouldn't it make more sense >> to use 16 KB (that is, four pages)? >> > > it is 16k yes. > >>> recommend to make further decisions about forcing SG transfers over >>> Bulk transfers once manufacturers deliver low-end devices which >>> include both possibilities) here we have seen the latency issue with >>> certain chipsets when buffers are too small I don't know the >>> behavior/latency of SG transfers unfortunately and there's no way to >>> test it on the particular target system - other systems do not trigger >>> that problem. >> >> In general, an SG transfer has slightly higher time and memory >> requirements than a non-SG transfer. It's not a big difference. >> >> But with non-SG, the memory is allocated as a single, big buffer, >> whereas with SG the memory is allocated as a bunch of smaller buffers. >> Therefore, when memory is fragmented (which will happen on a system >> with a small amount of memory), SG transfers are more likely to succeed >> than non-SG. >> >> For example, you are more likely to be able to allocate eight 16-KB >> memory regions (SG) than a single 128-KB region (non-SG). >> > > I'm just worried about that single case where timing of the USB frames > seem to matter, it worked > with 50k buffers. > The 16k transfer size buffer seems to stop the transfer randomly after > some time needing to toggle > a register to re-enable the data again (the transfer length is > reported to be 0 the packets themself still arrive on the host). > I'm not against the SG transfers here but I'd rather like to keep both > possibilities (the old one which was available with Linux 3.5 where > there was no 16k limitation) and the SG one, I'd certainly like to > test both of them. > To make SG transfers foolproof I recommend anyway to use pre-allocated > buffers, you saw the bugreports that permanently re-allocating 16k > buffers over some time is not reliable. > >>> As far as I can see there's still some space for improvements. (eg. >>> pre-allocate large buffers (15k) for USB 3.x otherwise it will end up >>> with allocation failures during runtime on low-end systems - as seen >>> by our customers with USB 2.0 and 15k buffers on low-end systems). >>> With SG and USB 3.0 this problem should happen even quicker than with USB >>> 2.0. >> >> Why would low-end systems be using USB-3? >> > > Routers for sharing Harddisks/NAS Systems. There are already some > available on the market. > is this going to be fixed? http://support.sundtek.com/index.php/topic,350.0.html http://support.sundtek.com/index.php/topic,1097.msg7780.html#msg7780 http://support.sundtek.com/index.php/topic,411.msg2153.html#msg2153 http://support.sundtek.com/index.php/topic,531.msg2956.html#msg2956 http://support.sundtek.com/index.php/topic,364.msg1829.html#msg1829 or do you have another solution for that issue on systems with a lower performance? https://developer.apple.com/library/mac/documentation/IOKit/Reference/IOUSBInterfaceInterface192_reference/translated_content/IOUSBInterfaceInterface192.html#//apple_ref/doc/uid/TP40011544-CLSCHIOUSBInterfaceInterface192-DontLinkElementID_2 Apple obviously did it the same way, though that doesn't mean that Linux has to do it that way of course. Markus > Markus > >> 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] USB: host: tegra: CONFIG_USB_EHCI_TEGRA requires ULPI and ULPI viewport support
On 10/04/2013 12:02 AM, Paul Walmsley wrote: > > Selecting CONFIG_USB_EHCI_TEGRA requires CONFIG_USB_ULPI_VIEWPORT. > Otherwise the build can break with: > > drivers/usb/phy/phy-tegra-usb.c: In function 'ulpi_open': > drivers/usb/phy/phy-tegra-usb.c:689:31: error: > 'ulpi_viewport_access_ops' undeclared (first use in this function) > drivers/usb/phy/phy-tegra-usb.c:689:31: note: each undeclared identifier > is reported only once for each function it appears in > > if CONFIG_USB_ULPI_VIEWPORT is not manually selected. > > Fix by forcing CONFIG_USB_ULPI_VIEWPORT to be selected when > CONFIG_USB_EHCI_TEGRA is selected. Then, since CONFIG_USB_ULPI_VIEWPORT > requires CONFIG_USB_ULPI to be selected, add that too. > > N.B.: ULPI is deprecated on this controller for T114, so it might make > sense to split the ULPI support code into a separate file, compiled only > if a ULPI PHY is selected. This was fixed at least in 3.13 (perhaps 3.12 too?) by: config ARCH_TEGRA ... select USB_ARCH_HAS_EHCI if USB_SUPPORT select USB_ULPI if USB_PHY select USB_ULPI_VIEWPORT if USB_PHY I think it'd be better to back-port that patch to stable, for consistency. -- To unsubscribe from this list: send the line "unsubscribe 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/9] usb-core: Track if an endpoint has streams
Hi, On 10/04/2013 05:35 PM, Alan Stern wrote: On Fri, 4 Oct 2013, Hans de Goede wrote: This is a preparation patch for adding support for bulk streams to usbfs. + for (i = 0; i < num_eps; i++) + eps[i]->has_streams = 1; --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -71,6 +71,7 @@ struct usb_host_endpoint { unsigned char *extra; /* Extra descriptors */ int extralen; int enabled; + int has_streams; Instead of storing a 0/1 value, wouldn't it be more useful to store the number of streams the endpoint has? That might be useful in the future, yes, I'll upate this in my next version of this patch set. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe 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/9] usb-core: Free bulk streams on interface release
Hi, On 10/04/2013 05:37 PM, Alan Stern wrote: On Fri, 4 Oct 2013, Hans de Goede wrote: Documentation/usb/bulk-streams.txt says: All stream IDs will be deallocated when the driver releases the interface, to ensure that drivers that don't support streams will be able to use the endpoint This commit actually implements this. --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -369,8 +369,9 @@ static int usb_unbind_interface(struct device *dev) { struct usb_driver *driver = to_usb_driver(dev->driver); struct usb_interface *intf = to_usb_interface(dev); + struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS]; That's a big array to put on the stack: 30 entries each containing 8 bytes (on a 64-bit arch). I was wondering about this myself when I wrote this. What do you suggest as an alternative, kmalloc an array with intf->cur_altsetting->desc.bNumEndpoints pointers ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe 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 9/9] usbfs: Add support for allocating / freeing streams
Hi, On 10/04/2013 05:44 PM, Alan Stern wrote: On Fri, 4 Oct 2013, Hans de Goede wrote: +static int parse_usbdevfs_streams(struct usb_device *dev, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 1024 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 1024)) + return -EINVAL; + + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); + if (!eps) + return -ENOMEM; + + for (i = 0; i < num_eps; i++) { + if (get_user(ep, &streams->eps[i])) { + ret = -EFAULT; + goto error; + } + eps[i] = ep_to_host_endpoint(dev, ep); + if (!eps[i]) { + ret = -EINVAL; + goto error; + } + + /* usb_alloc/free_streams operate on an usb_interface */ + ret = findintfep(dev, ep); + if (ret < 0) + goto error; + + if (i == 0) { + intf = usb_ifnum_to_if(dev, ret); + } else { + /* Verify all eps belong to the same interface */ + if (ret != intf->altsetting->desc.bInterfaceNumber) { + ret = -EINVAL; + goto error; + } + } + } + + if (num_streams_ret) + *num_streams_ret = num_streams; + *num_eps_ret = num_eps; + *eps_ret = eps; + *intf_ret = intf; + + return 0; + +error: + kfree(eps); + return ret; +} Somewhere in here you should check that the caller has claimed the interface containing these endpoints. Yes, good point. Will fix in next revision. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe 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] usbnet: smsc95xx: Add device tree input for MAC address
If the smsc95xx does not have a valid MAC address stored within the eeprom then a random number is generated. The MAC can also be set by uBoot but the smsc95xx does not have a way to read this. Create the binding for the smsc95xx so that uBoot can set the MAC and the code can retrieve the MAC from the modified DTB file. Signed-off-by: Dan Murphy --- Documentation/devicetree/bindings/net/smsc95xx.txt | 17 ++ drivers/net/usb/smsc95xx.c | 24 2 files changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/smsc95xx.txt diff --git a/Documentation/devicetree/bindings/net/smsc95xx.txt b/Documentation/devicetree/bindings/net/smsc95xx.txt new file mode 100644 index 000..4c37280 --- /dev/null +++ b/Documentation/devicetree/bindings/net/smsc95xx.txt @@ -0,0 +1,17 @@ +* Smart Mixed-Signal Connectivity (SMSC) 95xx Controller + +Required properties: +None + +Optional properties: +- mac-address - Read the mac address that was stored by uBoot +- local-address - Read the mac address that was stored by uBoot +- address - Read the mac address that was stored by uBoot + +Examples: + +smsc0: smsc95xx@0 { + /* Filled in by U-Boot */ + mac-address = [ 00 00 00 00 00 00 ]; +}; + diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 3f38ba8..baee0bd 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -31,6 +31,9 @@ #include #include #include +#include +#include + #include "smsc95xx.h" #define SMSC_CHIPNAME "smsc95xx" @@ -62,6 +65,8 @@ #define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \ SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3) +#define SMSC95XX_OF_NAME "/smsc95xx@" + struct smsc95xx_priv { u32 mac_cr; u32 hash_hi; @@ -767,6 +772,25 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd) static void smsc95xx_init_mac_address(struct usbnet *dev) { + +#ifdef CONFIG_OF + struct device_node *ap; + const char *mac = NULL; + char *of_name = SMSC95XX_OF_NAME; + + sprintf(of_name, "%s%i", SMSC95XX_OF_NAME, dev->udev->dev.id); + ap = of_find_node_by_path(of_name); + if (ap) { + mac = of_get_mac_address(ap); + if (is_valid_ether_addr(mac)) { + /* Device tree has a mac for this so use that */ + memcpy(dev->net->dev_addr, mac, ETH_ALEN); + netif_dbg(dev, ifup, dev->net, "MAC address read from DTB\n"); + return; + } + } +#endif + /* try reading mac address from EEPROM */ if (smsc95xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN, dev->net->dev_addr) == 0) { -- 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 4/9] usb-core: Free bulk streams on interface release
On Fri, 4 Oct 2013, Hans de Goede wrote: > >> + struct usb_host_endpoint *ep, *eps[USB_MAXENDPOINTS]; > > > > That's a big array to put on the stack: 30 entries each containing 8 > > bytes (on a 64-bit arch). > > I was wondering about this myself when I wrote this. What do you suggest > as an alternative, kmalloc an array with > intf->cur_altsetting->desc.bNumEndpoints > pointers ? Or simply do 30 pointers every time. 240 bytes isn't all that much for kmalloc. 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] memory mapping for usbfs (v0.4)
On Sun, 29 Sep 2013, Markus Rechberger wrote: > This patch adds memory mapping support to USBFS for isochronous and bulk > data transfers, it allows to pre-allocate usb transfer buffers. > > The CPU usage decreases 1-2% on my 1.3ghz U7300 notebook > The CPU usage decreases 6-8% on an Intel Atom n270 when > transferring 20mbyte/sec (isochronous), it should be more interesting to > see those > statistics on embedded systems where copying data is more expensive. > > Usage from userspace: > allocation: >rv = ioctl(priv->usbfd, USBDEVFS_ALLOC_MEMORY, &mem); > if (rv == 0) > buffer = mmap(NULL, size, PROT_READ|PROT_WRITE, > MAP_SHARED, priv->usbfd, mem.offset); > use the mapped buffer with urb->buffer. > release: > rv = munmap(buffer, size); > memset(&mem, 0x0, sizeof(struct usbdevfs_memory)); > mem.buffer = buffer; > rv = ioctl(priv->usbfd, USBDEVFS_RELEASE_MEMORY, &mem); > > non freed memory buffers are collected and will be released when closing > the node. > > I tested this patch with Bulk and Isochronous, with and without memory > mapping (applications which don't use mmap will just fall back to the > legacy mechanism). On the whole this seems reasonable. There are a few stylistic things that could be cleaned up (missing blank lines after variable declarations, for example, and other checkpatch issues), but they are minor. Why do you constantly compute (PAGE_SIZEsize directly? (And why is the variable sometimes called usbm and sometimes called usbmem?) The biggest problem is that your proc_alloc_memory() routine doesn't call usbfs_increase_memory_usage(). Without that, there's nothing to prevent a user from allocating all the available kernel memory. > Version 0.3: > * Removed comment > * Renamed USBDEVFS_FREE_MEMORY to USBDEVFS_RELEASE_MEMORY > * Clearing allocated memory > > Version 0.4: > * overriding SG transfers once memory mapped buffers are allocated for > BULK > * adding pgprot_noncached to ensure that the IO memory is not cached > (thanks to Ming Lei for mentioning that) I don't understand this. Why shouldn't the buffer memory be cached? Won't uncached buffers be a lot slower? As I understand it, you wrote this in order to solve problems where memory couldn't be allocated for USB transfers. If the system is running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also fail? 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] memory mapping for usbfs (v0.4)
On Fri, Oct 4, 2013 at 8:41 PM, Alan Stern wrote: > On the whole this seems reasonable. There are a few stylistic things > that could be cleaned up (missing blank lines after variable > declarations, for example, and other checkpatch issues), but they are > minor. > > Why do you constantly compute (PAGE_SIZEinstead of just using usbm->size directly? (And why is the variable > sometimes called usbm and sometimes called usbmem?) > ok. > The biggest problem is that your proc_alloc_memory() routine doesn't > call usbfs_increase_memory_usage(). Without that, there's nothing to > prevent a user from allocating all the available kernel memory. > only root is supposed to have raw USB access, which limit would you suggest? root could also delete the entire usb harddisk with appropriate commands. >> Version 0.3: >> * Removed comment >> * Renamed USBDEVFS_FREE_MEMORY to USBDEVFS_RELEASE_MEMORY >> * Clearing allocated memory >> >> Version 0.4: >> * overriding SG transfers once memory mapped buffers are allocated for >> BULK >> * adding pgprot_noncached to ensure that the IO memory is not cached >> (thanks to Ming Lei for mentioning that) > > I don't understand this. Why shouldn't the buffer memory be cached? > Won't uncached buffers be a lot slower? > I was only testing reading the data so I didn't see any caching effects since I don't have a device or driver which I can send a lot data out. As far as I understand pgprot_noncached would only be required when sending data (writing data to the DMA'able buffer). This question is still open. > As I understand it, you wrote this in order to solve problems where > memory couldn't be allocated for USB transfers. If the system is > running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also > fail? > No, the buffers will be kept in a list and re-used (aside of that they are also kept in a list in devio.c and re-used). When the buffers are allocated initially there's no chance to fail this process during run-time. please note the transfer buffer are allocated and freed permanently during run-time with the old mechanism, this mechanism usually fails during run-time on low end systems. Markus -- To unsubscribe from this list: send the line "unsubscribe 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: host: tegra: CONFIG_USB_EHCI_TEGRA requires ULPI and ULPI viewport support
On 10/04/2013 09:04 AM, Stephen Warren wrote: On 10/04/2013 12:02 AM, Paul Walmsley wrote: Selecting CONFIG_USB_EHCI_TEGRA requires CONFIG_USB_ULPI_VIEWPORT. Otherwise the build can break with: drivers/usb/phy/phy-tegra-usb.c: In function 'ulpi_open': drivers/usb/phy/phy-tegra-usb.c:689:31: error: 'ulpi_viewport_access_ops' undeclared (first use in this function) drivers/usb/phy/phy-tegra-usb.c:689:31: note: each undeclared identifier is reported only once for each function it appears in if CONFIG_USB_ULPI_VIEWPORT is not manually selected. Fix by forcing CONFIG_USB_ULPI_VIEWPORT to be selected when CONFIG_USB_EHCI_TEGRA is selected. Then, since CONFIG_USB_ULPI_VIEWPORT requires CONFIG_USB_ULPI to be selected, add that too. N.B.: ULPI is deprecated on this controller for T114, so it might make sense to split the ULPI support code into a separate file, compiled only if a ULPI PHY is selected. This was fixed at least in 3.13 (perhaps 3.12 too?) by: config ARCH_TEGRA ... select USB_ARCH_HAS_EHCI if USB_SUPPORT select USB_ULPI if USB_PHY select USB_ULPI_VIEWPORT if USB_PHY I think it'd be better to back-port that patch to stable, for consistency. Yep agreed. Want to cc that patch to linux-stable instead? The v3.13.x -stable builds are breaking here for T114-only Kconfigs... - 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] memory mapping for usbfs (v0.4)
On Fri, 4 Oct 2013, Markus Rechberger wrote: > > The biggest problem is that your proc_alloc_memory() routine doesn't > > call usbfs_increase_memory_usage(). Without that, there's nothing to > > prevent a user from allocating all the available kernel memory. > > > > only root is supposed to have raw USB access, Not true at all. Any user may be allowed to access a USB device, depending on the permissions for the usbfs device file. > which limit would you suggest? The limit is already in place. All you have to do is call the routine to make sure that you don't exceed the limit. Check the other places where usbfs_increase_memory_usage() is used and you'll see how it works. > >> Version 0.4: > >> * overriding SG transfers once memory mapped buffers are allocated for > >> BULK > >> * adding pgprot_noncached to ensure that the IO memory is not cached > >> (thanks to Ming Lei for mentioning that) > > > > I don't understand this. Why shouldn't the buffer memory be cached? > > Won't uncached buffers be a lot slower? > > > > I was only testing reading the data so I didn't see any caching > effects since I don't have a device or driver which I can send a lot > data out. > As far as I understand pgprot_noncached would only be required when > sending data (writing data to the DMA'able buffer). > > This question is still open. The buffer should be cached. The userspace program will have to make sure that it doesn't try to access the buffer while DMA is in progress. As long as that restriction is obeyed, the USB core will take care of mapping the buffer for DMA (which flushes the cache). > > As I understand it, you wrote this in order to solve problems where > > memory couldn't be allocated for USB transfers. If the system is > > running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also > > fail? > > > > No, the buffers will be kept in a list and re-used (aside of that they > are also kept in a list in devio.c and re-used). When the buffers are > allocated initially there's no chance to fail this process during > run-time. Why not? What if the memory is already almost full? The call to alloc_pages_exact() could fail. I agree this is less likely than a failure with the current system, but it is still possible. > please note the transfer buffer are allocated and freed permanently > during run-time with the old mechanism, this mechanism usually fails > during run-time on low end systems. It's true that the existing system allocates and deallocates the buffer for each transfer. With your patch, the buffer is allocated only once. In addition, the same buffer is used in userspace and in the kernel, removing the need to allocate two buffers and copy everything over. This will be a significant saving of both time and space. By the way, have you considered that the user program might want the I/O transfer to start at some position other than the beginning of the buffer? 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: g_ffs: fix compilation warning
Due to lack of "u_ether.h" header file, g_ffs.c compiles with following warning: drivers/usb/gadget/g_ffs.c:81:1: warning: data definition has no type or storage class [enabled by default] drivers/usb/gadget/g_ffs.c:81:1: warning: type defaults to ‘int’ in declaration of ‘USB_ETHERNET_MODULE_PARAMETERS’ [-Wimplicit-int] drivers/usb/gadget/g_ffs.c:81:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes] This patch adds the missing header. Signed-off-by: David Cohen --- drivers/usb/gadget/g_ffs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 5327c82..ed3d9ab 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -13,6 +13,9 @@ #define pr_fmt(fmt) "g_ffs: " fmt #include + +#include "u_ether.h" + /* * kbuild is not very cooperative with respect to linking separately * compiled library objects into one module. So for now we won't use -- 1.8.4.rc3 -- To unsubscribe from this list: send the line "unsubscribe 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: g_ffs: fix compilation warning
On Fri, Oct 04, 2013 at 02:48:46PM -0700, David Cohen wrote: > Due to lack of "u_ether.h" header file, g_ffs.c compiles with following > warning: It does? In what tree/branch/release is this happening? 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] usb: g_ffs: fix compilation warning
On 10/04/2013 02:51 PM, Greg KH wrote: On Fri, Oct 04, 2013 at 02:48:46PM -0700, David Cohen wrote: Due to lack of "u_ether.h" header file, g_ffs.c compiles with following warning: It does? In what tree/branch/release is this happening? This is intended for Linus' 3.12-rc3. Haven't checked any other kernel. In my compilation I disabled USB Host support and enabled DWC3 (gadget only) with ePCI + NOP_USB_XCEIV. Br, David 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] usb: g_ffs: fix compilation warning
On 10/04/2013 03:03 PM, David Cohen wrote: On 10/04/2013 02:51 PM, Greg KH wrote: On Fri, Oct 04, 2013 at 02:48:46PM -0700, David Cohen wrote: Due to lack of "u_ether.h" header file, g_ffs.c compiles with following warning: It does? In what tree/branch/release is this happening? This is intended for Linus' 3.12-rc3. Haven't checked any other kernel. In my compilation I disabled USB Host support and enabled DWC3 (gadget only) with ePCI + NOP_USB_XCEIV. After checking again this patch is not fixing properly. u_ether.h is conditionally included. I'll solve it in another way. Please do not consider this patch anymore. BR, David Br, David 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] memory mapping for usbfs (v0.4)
On Fri, Oct 4, 2013 at 10:34 PM, Alan Stern wrote: > On Fri, 4 Oct 2013, Markus Rechberger wrote: > >> > The biggest problem is that your proc_alloc_memory() routine doesn't >> > call usbfs_increase_memory_usage(). Without that, there's nothing to >> > prevent a user from allocating all the available kernel memory. >> > >> >> only root is supposed to have raw USB access, > > Not true at all. Any user may be allowed to access a USB device, > depending on the permissions for the usbfs device file. > well I mean by default if you connect an unknown device only root will usually have access to it. I know there are udev rules for some special devices. >> which limit would you suggest? > > The limit is already in place. All you have to do is call the routine > to make sure that you don't exceed the limit. Check the other places > where usbfs_increase_memory_usage() is used and you'll see how it > works. > ok >> >> Version 0.4: >> >> * overriding SG transfers once memory mapped buffers are allocated for >> >> BULK >> >> * adding pgprot_noncached to ensure that the IO memory is not cached >> >> (thanks to Ming Lei for mentioning that) >> > >> > I don't understand this. Why shouldn't the buffer memory be cached? >> > Won't uncached buffers be a lot slower? >> > >> >> I was only testing reading the data so I didn't see any caching >> effects since I don't have a device or driver which I can send a lot >> data out. >> As far as I understand pgprot_noncached would only be required when >> sending data (writing data to the DMA'able buffer). >> >> This question is still open. > > The buffer should be cached. The userspace program will have to make > sure that it doesn't try to access the buffer while DMA is in progress. > As long as that restriction is obeyed, the USB core will take care of > mapping the buffer for DMA (which flushes the cache). > ok so it can be removed. >> > As I understand it, you wrote this in order to solve problems where >> > memory couldn't be allocated for USB transfers. If the system is >> > running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also >> > fail? >> > >> >> No, the buffers will be kept in a list and re-used (aside of that they >> are also kept in a list in devio.c and re-used). When the buffers are >> allocated initially there's no chance to fail this process during >> run-time. > > Why not? What if the memory is already almost full? The call to > alloc_pages_exact() could fail. > > I agree this is less likely than a failure with the current system, but > it is still possible. > if it happens the allocation ioctl returns an error, and the application knows from the beginning on that this is not going to work. But once it's allocated it should be solid. Our driver starts up early as well there has never been a failure at driver startup. I mentioned within another email we also have another kernel stub driver which does the pre-allocation within the module, no issues have been reported when the pre-allocation module was in place. There are discussions on several mailinglists about pre-allocating DMA'able memory when kernel drivers are loaded, that way is not so uncommon.. >> please note the transfer buffer are allocated and freed permanently >> during run-time with the old mechanism, this mechanism usually fails >> during run-time on low end systems. > > It's true that the existing system allocates and deallocates the buffer > for each transfer. With your patch, the buffer is allocated only once. > In addition, the same buffer is used in userspace and in the kernel, > removing the need to allocate two buffers and copy everything over. > This will be a significant saving of both time and space. > > By the way, have you considered that the user program might want the > I/O transfer to start at some position other than the beginning of the > buffer? > I thought about that but I couldn't find any useful usecase for that. I guess it would be very much device dependent. The URB buffers themself aren't that big either. Just when comparing the MacOSX API it wouldn't support that either. If someone needs it he could still add it. Markus > 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 1/2] USB/host: Use existing macros instead of hard-coded values in uhci-debug.c
From: Deng-Cheng Zhu Now that UHCI IO registers have been defined in uhci-hcd.h, use them. Reviewed-by: James Hogan Signed-off-by: Deng-Cheng Zhu --- drivers/usb/host/uhci-debug.c | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c index 4557375..8e239cd 100644 --- a/drivers/usb/host/uhci-debug.c +++ b/drivers/usb/host/uhci-debug.c @@ -310,14 +310,14 @@ static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len) unsigned short portsc1, portsc2; - usbcmd= uhci_readw(uhci, 0); - usbstat = uhci_readw(uhci, 2); - usbint= uhci_readw(uhci, 4); - usbfrnum = uhci_readw(uhci, 6); - flbaseadd = uhci_readl(uhci, 8); - sof = uhci_readb(uhci, 12); - portsc1 = uhci_readw(uhci, 16); - portsc2 = uhci_readw(uhci, 18); + usbcmd= uhci_readw(uhci, USBCMD); + usbstat = uhci_readw(uhci, USBSTS); + usbint= uhci_readw(uhci, USBINTR); + usbfrnum = uhci_readw(uhci, USBFRNUM); + flbaseadd = uhci_readl(uhci, USBFLBASEADD); + sof = uhci_readb(uhci, USBSOF); + portsc1 = uhci_readw(uhci, USBPORTSC1); + portsc2 = uhci_readw(uhci, USBPORTSC2); out += sprintf(out, " usbcmd= %04x %s%s%s%s%s%s%s%s\n", usbcmd, -- 1.7.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
[PATCH 0/2] USB/host: Cleanups for UHCI
This series contains hardcoding-to-macro changes in uhci-debug.c and removal of the redundant OK() macro in uhci-hub.c. Deng-Cheng Zhu (2): USB/host: Use existing macros instead of hard-coded values in uhci-debug.c USB/host: Cleaning up the OK macro in uhci-hub.c drivers/usb/host/uhci-debug.c | 16 drivers/usb/host/uhci-hub.c | 30 ++ 2 files changed, 22 insertions(+), 24 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe 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/host: Cleaning up the OK macro in uhci-hub.c
From: Deng-Cheng Zhu The logic "len = (x)" in OK(x) is dead. Clean it up. Reviewed-by: James Hogan Signed-off-by: Deng-Cheng Zhu --- drivers/usb/host/uhci-hub.c | 30 ++ 1 files changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c index 9189bc9..e72973b 100644 --- a/drivers/usb/host/uhci-hub.c +++ b/drivers/usb/host/uhci-hub.c @@ -75,8 +75,6 @@ static inline int get_hub_status_data(struct uhci_hcd *uhci, char *buf) return !!*buf; } -#define OK(x) len = (x); break - #define CLR_RH_PORTSTAT(x) \ status = uhci_readw(uhci, port_addr); \ status &= ~(RWC_BITS|WZ_BITS); \ @@ -258,7 +256,7 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, case GetHubStatus: *(__le32 *)buf = cpu_to_le32(0); - OK(4); /* hub power */ + break; /* hub power */ case GetPortStatus: if (port >= uhci->rh_numports) goto err; @@ -311,13 +309,13 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, *(__le16 *)buf = cpu_to_le16(wPortStatus); *(__le16 *)(buf + 2) = cpu_to_le16(wPortChange); - OK(4); + break; case SetHubFeature: /* We don't implement these */ case ClearHubFeature: switch (wValue) { case C_HUB_OVER_CURRENT: case C_HUB_LOCAL_POWER: - OK(0); + break; default: goto err; } @@ -329,7 +327,7 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, switch (wValue) { case USB_PORT_FEAT_SUSPEND: SET_RH_PORTSTAT(USBPORTSC_SUSP); - OK(0); + break; case USB_PORT_FEAT_RESET: SET_RH_PORTSTAT(USBPORTSC_PR); @@ -338,10 +336,10 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, /* USB v2.0 7.1.7.5 */ uhci->ports_timeout = jiffies + msecs_to_jiffies(50); - OK(0); + break; case USB_PORT_FEAT_POWER: /* UHCI has no power switching */ - OK(0); + break; default: goto err; } @@ -356,10 +354,10 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, /* Disable terminates Resume signalling */ uhci_finish_suspend(uhci, port, port_addr); - OK(0); + break; case USB_PORT_FEAT_C_ENABLE: CLR_RH_PORTSTAT(USBPORTSC_PEC); - OK(0); + break; case USB_PORT_FEAT_SUSPEND: if (!(uhci_readw(uhci, port_addr) & USBPORTSC_SUSP)) { @@ -382,22 +380,22 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, uhci->ports_timeout = jiffies + msecs_to_jiffies(20); } - OK(0); + break; case USB_PORT_FEAT_C_SUSPEND: clear_bit(port, &uhci->port_c_suspend); - OK(0); + break; case USB_PORT_FEAT_POWER: /* UHCI has no power switching */ goto err; case USB_PORT_FEAT_C_CONNECTION: CLR_RH_PORTSTAT(USBPORTSC_CSC); - OK(0); + break; case USB_PORT_FEAT_C_OVER_CURRENT: CLR_RH_PORTSTAT(USBPORTSC_OCC); - OK(0); + break; case USB_PORT_FEAT_C_RESET: /* this driver won't report these */ - OK(0); + break; default: goto err; } @@ -407,7 +405,7 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, memcpy(buf, root_hub_hub_des, len); if (len > 2) buf[2] = uhci->rh_numports; - OK(len); + break; default: err: retval = -EPIPE; -- 1.7.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
[PATCH v2] usb: g_ffs: fix compilation warning
If USB_FUNCTIONFS is selected without USB_FUNCTIONFS_ETH and USB_FUNCTIONFS_RNIS, u_ether.h won't be included and then USB_ETHERNET_MODULE_PARAMAETERS macro won't be available causing the following warning compilation: drivers/usb/gadget/g_ffs.c:81:1: warning: data definition has no type or storage class [enabled by default] drivers/usb/gadget/g_ffs.c:81:1: warning: type defaults to ‘int’ in declaration of ‘USB_ETHERNET_MODULE_PARAMETERS’ [-Wimplicit-int] drivers/usb/gadget/g_ffs.c:81:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes] This patch fixes the warning by making USB_ETHERNET_MODULE_PARAMETERS to be used iff u_ether.h is included, otherwise it is not needed. Signed-off-by: David Cohen --- drivers/usb/gadget/g_ffs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 5327c82..2344efe 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -76,7 +76,9 @@ struct gfs_ffs_obj { USB_GADGET_COMPOSITE_OPTIONS(); +#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS USB_ETHERNET_MODULE_PARAMETERS(); +#endif static struct usb_device_descriptor gfs_dev_desc = { .bLength= sizeof gfs_dev_desc, -- 1.8.4.rc3 -- To unsubscribe from this list: send the line "unsubscribe 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: EHCI bus glue driver works for storage, fails for a WiFi module
Hi all, first of all thank you all for your help. I now have some news to report. Using your hint about timing I've inserted a bunch of udelays around the read/write functions that get called from _rtl92c_write_fw and got rid of the "detected XactErr len 0/0 retry" errors. Then I just tried to use the WLAN adapter and it worked. Afterwards, I've removed udelays and tried it again and guess what... everything still worked. I do not really know if I should consider these errors harmless but it looks as if they are. What do you think? I do however have something fishy to report, but this is probably an off-topic for linux-usb mailing list so I'll just mention it here briefly for completeness. Fist, I've compared the speed of the rtl8192cu vs 8192cu in 3.4 kernel. The latter one is the Realtek's driver. I've did many download tests of the same file and it turns out 8192cu is 1.5 times faster. Things haven't improved since then. The 3.12-rc1 rtl8192cu shows the same performance as in 3.4. In addition with 3.12-rc1 I get some strange messages in the dmesg [1]. Best regards, Arokux [1] http://sprunge.us/dide -- To unsubscribe from this list: send the line "unsubscribe 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] usb: g_ffs: fix compilation warning
Hi Greg, On 10/04/2013 03:30 PM, David Cohen wrote: If USB_FUNCTIONFS is selected without USB_FUNCTIONFS_ETH and USB_FUNCTIONFS_RNIS, u_ether.h won't be included and then USB_ETHERNET_MODULE_PARAMAETERS macro won't be available causing the following warning compilation: drivers/usb/gadget/g_ffs.c:81:1: warning: data definition has no type or storage class [enabled by default] drivers/usb/gadget/g_ffs.c:81:1: warning: type defaults to ‘int’ in declaration of ‘USB_ETHERNET_MODULE_PARAMETERS’ [-Wimplicit-int] drivers/usb/gadget/g_ffs.c:81:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes] This patch fixes the warning by making USB_ETHERNET_MODULE_PARAMETERS to be used iff u_ether.h is included, otherwise it is not needed. Signed-off-by: David Cohen --- drivers/usb/gadget/g_ffs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 5327c82..2344efe 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -76,7 +76,9 @@ struct gfs_ffs_obj { USB_GADGET_COMPOSITE_OPTIONS(); +#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS USB_ETHERNET_MODULE_PARAMETERS(); This warning happens on kernel v3.11+ But luckily it has no real effect on kernel despite the warning, so no tests would report it. Br, David Cohen +#endif static struct usb_device_descriptor gfs_dev_desc = { .bLength= sizeof gfs_dev_desc, -- To unsubscribe from this list: send the line "unsubscribe 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 3.0 external dd offlined, not ready after error recovery
On 4 October 2013 10:53, Alan Stern wrote: > On Thu, 3 Oct 2013, [ISO-8859-1] Jorge Muñoz Camadro wrote: > >> >> I'm using the one who comes with the drive, i've never changed it, and >> doesn't show any sign of been broken or something like that. It seems >> like a propietary model. > > Well, it sure looks like something is wrong. Do you have another > computer with a USB-3 port you can try plugging the drive into? > For now i don't have the chance. >> >> Guess not, i'm using exactly the same kernel provided by my distro, no >> changes. > > That patch is not yet included in any distro kernels. To test it you > will have to build your own kernel. Or else wait until your distro > releases it, which will probably take some weeks. > So maybe the best choice is waiting until the patch is released with the kernel of my distro. > 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