RE: [PATCH] USB: chipidea: i.MX: Probe usbmisc early
>
> Probe the usbmisc driver earlier, otherwise the ci_hdrc_imx.c will
> get -EPROBE_DEFER from usbmisc when both are compiled into the kernel
> and thus USB gadget mode won't work.
>
Hi Marek, you know the root cause of it, you suggested using
a better way to fix it.
http://marc.info/?l=linux-usb&m=138060107809076&w=2
Peter
> Signed-off-by: Marek Vasut
> Cc: Alexander Shishkin
> Cc: Peter Chen
> ---
> drivers/usb/chipidea/usbmisc_imx.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> NOTE: I'm not sure if this is a correct approach. The fs_initcall() is
> what we use in the PCI express driver to circumvent similar
> ordering
> issue, but maybe it should be in some arch_initcall() or something?
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c
> b/drivers/usb/chipidea/usbmisc_imx.c
> index 8a1094b..7764101 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -224,7 +224,6 @@ static int usbmisc_imx_remove(struct platform_device
> *pdev)
> }
>
> static struct platform_driver usbmisc_imx_driver = {
> - .probe = usbmisc_imx_probe,
> .remove = usbmisc_imx_remove,
> .driver = {
> .name = "usbmisc_imx",
> @@ -233,7 +232,17 @@ static struct platform_driver usbmisc_imx_driver = {
>},
> };
>
> -module_platform_driver(usbmisc_imx_driver);
> +static int __init usbmisc_imx_init(void)
> +{
> + return platform_driver_probe(&usbmisc_imx_driver,
> usbmisc_imx_probe);
> +}
> +fs_initcall(usbmisc_imx_init);
> +
> +static void __exit usbmisc_imx_exit(void)
> +{
> + platform_driver_unregister(&usbmisc_imx_driver);
> +}
> +module_exit(usbmisc_imx_exit);
>
> MODULE_ALIAS("platform:usbmisc-imx");
> MODULE_LICENSE("GPL v2");
> --
> 1.8.4.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
> > > This gives us some breathing room later to actually do the right
> thing
> > > without additionally performing renames all over the place to make
> > > imx5 -> imx53 (again)/imx51 (new).
> >
> > You may take matt's suggestion, it can reduce the code change now and
> in future.
> > We can only add device_id for imx51 in this patch, split imx53 and
> imx51's ops
> > when their differences are added in future.
>
> OK. Names of registers and bits also keep as is?
>
Yes.
Peter
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
[PATCH] USB: chipidea: i.MX: Probe usbmisc early
Probe the usbmisc driver earlier, otherwise the ci_hdrc_imx.c will
get -EPROBE_DEFER from usbmisc when both are compiled into the kernel
and thus USB gadget mode won't work.
Signed-off-by: Marek Vasut
Cc: Alexander Shishkin
Cc: Peter Chen
---
drivers/usb/chipidea/usbmisc_imx.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
NOTE: I'm not sure if this is a correct approach. The fs_initcall() is
what we use in the PCI express driver to circumvent similar ordering
issue, but maybe it should be in some arch_initcall() or something?
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 8a1094b..7764101 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -224,7 +224,6 @@ static int usbmisc_imx_remove(struct platform_device *pdev)
}
static struct platform_driver usbmisc_imx_driver = {
- .probe = usbmisc_imx_probe,
.remove = usbmisc_imx_remove,
.driver = {
.name = "usbmisc_imx",
@@ -233,7 +232,17 @@ static struct platform_driver usbmisc_imx_driver = {
},
};
-module_platform_driver(usbmisc_imx_driver);
+static int __init usbmisc_imx_init(void)
+{
+ return platform_driver_probe(&usbmisc_imx_driver, usbmisc_imx_probe);
+}
+fs_initcall(usbmisc_imx_init);
+
+static void __exit usbmisc_imx_exit(void)
+{
+ platform_driver_unregister(&usbmisc_imx_driver);
+}
+module_exit(usbmisc_imx_exit);
MODULE_ALIAS("platform:usbmisc-imx");
MODULE_LICENSE("GPL v2");
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: CDC-ACM device issue
I *thought* 3.12 fixed this, but it seems it just gives EOF instead of EIO... It seems Linux is trying an interrupt in and a control out, and getting ENOENT and EPIPE respectively. Windows (where this works), on the other hand, is doing a whole bunch of other things... Here are USB captures of both: http://luke.dashjr.org/tmp/code/technobit-linux-cdc.usbcap.pcapng.xz (376 B) http://luke.dashjr.org/tmp/code/technobit-windows-usbser.usbcap.pcap.xz (880 B Luke -- To unsubscribe from this list: send the line "unsubscribe 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 31/68] uas: Pack iu struct definitions
Hi, On 11/16/2013 07:11 PM, Alan Stern wrote: On Sat, 16 Nov 2013, Hans de Goede wrote: No, Linux relies on the compiler to pack things like this together and not put padding in, with the __packed__ attribute (don't we have a __packed macro instead of the __attribute__() thing?) Yes we've a __packed macro, but the norm in the usb subsys seems to be to use __attribute__ ((__packed__)), see include/[uapi/]linux/usb/*.h That's different. Things under include/uapi are meant for use by userspace programs, so they can't rely on kernel-internal things like the __packed macro. Right, but almost all headers under include/linux/usb/*.h also use the long form. 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 27/31] USB: EHCI: tegra: use reset framework
On Fri, 15 Nov 2013, Stephen Warren wrote: > From: Stephen Warren > > Tegra's clock driver now provides an implementation of the common > reset API (include/linux/reset.h). Use this instead of the old Tegra- > specific API; that will soon be removed. > > Cc: tred...@nvidia.com > Cc: pdeschrij...@nvidia.com > Cc: linux-te...@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: Greg Kroah-Hartman > Cc: Alan Stern > Cc: linux-usb@vger.kernel.org > Signed-off-by: Stephen Warren > --- > This patch is part of a series with strong internal depdendencies. I'm > looking for an ack so that I can take the entire series through the Tegra > and arm-soc trees. The series will be part of a stable branch that can be > merged into other subsystems if needed to avoid/resolve dependencies. Acked-by: 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 31/68] uas: Pack iu struct definitions
On Sat, 16 Nov 2013, Hans de Goede wrote: > > No, Linux relies on the compiler to pack things like this together and > > not put padding in, with the __packed__ attribute (don't we have a > > __packed macro instead of the __attribute__() thing?) > > Yes we've a __packed macro, but the norm in the usb subsys seems to be to use > __attribute__ ((__packed__)), see include/[uapi/]linux/usb/*.h That's different. Things under include/uapi are meant for use by userspace programs, so they can't rely on kernel-internal things like the __packed macro. This is also the reason why those .h files always use __u8 or __le16 rather than u8 or le16. 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 49/70] uas: Verify endpoint descriptors from uas_use_uas_driver()
Signed-off-by: Hans de Goede
---
drivers/usb/storage/uas-detect.h | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index 8de030a..b8a02e1 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -70,13 +70,23 @@ static int uas_find_endpoints(struct usb_host_interface
*alt,
static int uas_use_uas_driver(struct usb_interface *intf,
const struct usb_device_id *id)
{
+ struct usb_host_endpoint *eps[4] = { };
struct usb_device *udev = interface_to_usbdev(intf);
unsigned long flags = id->driver_info;
+ int r, alt;
usb_stor_adjust_quirks(udev, &flags);
if (flags & US_FL_IGNORE_UAS)
return 0;
- return uas_find_uas_alt_setting(intf) >= 0;
+ alt = uas_find_uas_alt_setting(intf);
+ if (alt < 0)
+ return 0;
+
+ r = uas_find_endpoints(&intf->altsetting[alt], eps);
+ if (r < 0)
+ return 0;
+
+ return 1;
}
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 69/70] uas: Add Hans de Goede as uas maintainer
At the kernel-summit Sarah Sharp asked me if I was willing to become the
uas maintainer. I said yes, and here is a patch to make this official.
Also remove Matthew Wilcox and Sarah Sharp as maintainers at their request.
I've also added myself to the module's author tag, so that if people look there
rather then in maintainers they will know they should bug me about uas too.
Signed-off-by: Hans de Goede
---
MAINTAINERS | 3 +--
drivers/usb/storage/uas.c | 4 +++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index c75199c..4189c94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8704,8 +8704,7 @@ S:Maintained
F: drivers/net/wireless/ath/ar5523/
USB ATTACHED SCSI
-M: Matthew Wilcox
-M: Sarah Sharp
+M: Hans de Goede
M: Gerd Hoffmann
L: linux-usb@vger.kernel.org
L: linux-s...@vger.kernel.org
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index daf2f75..44ce353 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -2,6 +2,7 @@
* USB Attached SCSI
* Note that this is not the same as the USB Mass Storage driver
*
+ * Copyright Hans de Goede for Red Hat, Inc. 2013
* Copyright Matthew Wilcox for Intel Corp, 2010
* Copyright Sarah Sharp for Intel Corp, 2010
*
@@ -1264,4 +1265,5 @@ static struct usb_driver uas_driver = {
module_usb_driver(uas_driver);
MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Matthew Wilcox and Sarah Sharp");
+MODULE_AUTHOR(
+ "Hans de Goede , Matthew Wilcox and Sarah Sharp");
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 48/70] uas: Drop fixed endpoint config handling
The fixed endpoint config code was only necessary to deal with an early
uas prototype which has never been released, so lets drop it and enforce
proper uas endpoint descriptors.
Signed-off-by: Hans de Goede
---
drivers/usb/storage/uas.c | 36 +++-
1 file changed, 11 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 49ae714..5397045 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -913,31 +913,17 @@ static void uas_configure_endpoints(struct uas_dev_info
*devinfo)
devinfo->cmnd = NULL;
r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps);
-
- /*
-* Assume that if we didn't find a proper set of descriptors, we're
-* using a device with old firmware that happens to be set up like
-* this.
-*/
- if (r != 0) {
- devinfo->cmd_pipe = usb_sndbulkpipe(udev, 1);
- devinfo->status_pipe = usb_rcvbulkpipe(udev, 1);
- devinfo->data_in_pipe = usb_rcvbulkpipe(udev, 2);
- devinfo->data_out_pipe = usb_sndbulkpipe(udev, 2);
-
- eps[1] = usb_pipe_endpoint(udev, devinfo->status_pipe);
- eps[2] = usb_pipe_endpoint(udev, devinfo->data_in_pipe);
- eps[3] = usb_pipe_endpoint(udev, devinfo->data_out_pipe);
- } else {
- devinfo->cmd_pipe = usb_sndbulkpipe(udev,
-usb_endpoint_num(&eps[0]->desc));
- devinfo->status_pipe = usb_rcvbulkpipe(udev,
-usb_endpoint_num(&eps[1]->desc));
- devinfo->data_in_pipe = usb_rcvbulkpipe(udev,
-usb_endpoint_num(&eps[2]->desc));
- devinfo->data_out_pipe = usb_sndbulkpipe(udev,
-usb_endpoint_num(&eps[3]->desc));
- }
+ if (r)
+ return r;
+
+ devinfo->cmd_pipe = usb_sndbulkpipe(udev,
+ usb_endpoint_num(&eps[0]->desc));
+ devinfo->status_pipe = usb_rcvbulkpipe(udev,
+ usb_endpoint_num(&eps[1]->desc));
+ devinfo->data_in_pipe = usb_rcvbulkpipe(udev,
+ usb_endpoint_num(&eps[2]->desc));
+ devinfo->data_out_pipe = usb_sndbulkpipe(udev,
+ usb_endpoint_num(&eps[3]->desc));
devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256,
GFP_KERNEL);
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 44/70] uas: Honor no-uas quirk set in usb-storage's quirks module parameter
Falling back from uas to usb-storage requires coordination between uas and
usb-storage, so use usb-storage's quirks module parameter, rather then
requiring the user to pass a param to 2 different modules.
Signed-off-by: Hans de Goede
---
drivers/usb/storage/Kconfig | 2 +-
drivers/usb/storage/uas-detect.h | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig
index 8470e1b..5939570 100644
--- a/drivers/usb/storage/Kconfig
+++ b/drivers/usb/storage/Kconfig
@@ -202,7 +202,7 @@ config USB_STORAGE_ENE_UB6250
config USB_UAS
tristate "USB Attached SCSI"
- depends on SCSI && BROKEN
+ depends on SCSI && USB_STORAGE && BROKEN
help
The USB Attached SCSI protocol is supported by some USB
storage devices. It permits higher performance by supporting
diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index 02bf5ec..082bde1 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -1,5 +1,6 @@
#include
#include
+#include "usb.h"
static int uas_is_interface(struct usb_host_interface *intf)
{
@@ -42,8 +43,11 @@ static int uas_find_uas_alt_setting(struct usb_interface
*intf)
static int uas_use_uas_driver(struct usb_interface *intf,
const struct usb_device_id *id)
{
+ struct usb_device *udev = interface_to_usbdev(intf);
unsigned long flags = id->driver_info;
+ usb_stor_adjust_quirks(udev, &flags);
+
if (flags & US_FL_IGNORE_UAS)
return 0;
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 43/70] usb-storage: Modify and export adjust_quirks so that it can be used by uas
Signed-off-by: Hans de Goede
---
drivers/usb/storage/usb.c | 16 ++--
drivers/usb/storage/usb.h | 3 +++
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 2055bc7..a045f52 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -464,14 +464,14 @@ static int associate_dev(struct us_data *us, struct
usb_interface *intf)
#define TOLOWER(x) ((x) | 0x20)
/* Adjust device flags based on the "quirks=" module parameter */
-static void adjust_quirks(struct us_data *us)
+void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags)
{
char *p;
- u16 vid = le16_to_cpu(us->pusb_dev->descriptor.idVendor);
- u16 pid = le16_to_cpu(us->pusb_dev->descriptor.idProduct);
+ u16 vid = le16_to_cpu(udev->descriptor.idVendor);
+ u16 pid = le16_to_cpu(udev->descriptor.idProduct);
unsigned f = 0;
unsigned int mask = (US_FL_SANE_SENSE | US_FL_BAD_SENSE |
- US_FL_FIX_CAPACITY |
+ US_FL_FIX_CAPACITY | US_FL_IGNORE_UAS |
US_FL_CAPACITY_HEURISTICS | US_FL_IGNORE_DEVICE |
US_FL_NOT_LOCKABLE | US_FL_MAX_SECTORS_64 |
US_FL_CAPACITY_OK | US_FL_IGNORE_RESIDUE |
@@ -542,14 +542,18 @@ static void adjust_quirks(struct us_data *us)
case 's':
f |= US_FL_SINGLE_LUN;
break;
+ case 'u':
+ f |= US_FL_IGNORE_UAS;
+ break;
case 'w':
f |= US_FL_NO_WP_DETECT;
break;
/* Ignore unrecognized flag characters */
}
}
- us->fflags = (us->fflags & ~mask) | f;
+ *fflags = (*fflags & ~mask) | f;
}
+EXPORT_SYMBOL_GPL(usb_stor_adjust_quirks);
/* Get the unusual_devs entries and the string descriptors */
static int get_device_info(struct us_data *us, const struct usb_device_id *id,
@@ -569,7 +573,7 @@ static int get_device_info(struct us_data *us, const struct
usb_device_id *id,
idesc->bInterfaceProtocol :
unusual_dev->useTransport;
us->fflags = id->driver_info;
- adjust_quirks(us);
+ usb_stor_adjust_quirks(us->pusb_dev, &us->fflags);
if (us->fflags & US_FL_IGNORE_DEVICE) {
dev_info(pdev, "device ignored\n");
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 75f70f0..307e339 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -201,4 +201,7 @@ extern int usb_stor_probe1(struct us_data **pus,
extern int usb_stor_probe2(struct us_data *us);
extern void usb_stor_disconnect(struct usb_interface *intf);
+extern void usb_stor_adjust_quirks(struct usb_device *dev,
+ unsigned long *fflags);
+
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT pull for 3.14 v2] xhci streams and uas fixes + usbfs streams support
Hi Sarah, Here is v2 of my patch-series / pull-req for you to pull into your xhci tree for 3.14 . This new version has the following changes: "usb-core: Track if an endpoint has streams": Remove unnecessary ep type check from usb_free_streams, the check if the endpoint has streams is sufficient. "uas: Drop fixed endpoint config handling" New patch dropping workaround code for early uas prototype hardware. "uas: Verify endpoint descriptors from uas_use_uas_driver()" Drop fixed endpoint config handling. "uas: Add a usbcore.nouas kernel cmdline option" Dropped. "usb-storage: Modify and export adjust_quirks so that it can be used by uas" "uas: Honor no-uas quirk set in usb-storage's quirks module parameter" 2 new patches replacing "uas: Add a usbcore.nouas kernel cmdline option". "uas: Add Hans de Goede as uas maintainer" Also remove Matthew Wilcox and Sarah Sharp as maintainers at their request. I'm going to only send the changed patches as a follow up to this mail, rather then re-sending the entire series again. The following changes since commit 7d49f0bac41ee9b012af1efe2f725d91a87a8fe9: USB: Maintainers change for usb serial drivers (2013-10-31 08:53:52 -0700) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git usb-next-for-sarah for you to fetch changes up to 843af30c55c1441de2775f3a1ef624ab7605c8a3: uas: Remove comment about registering a uas scsi controller for each usb bus (2013-11-16 12:29:27 +0100) Gerd Hoffmann (6): xhci: fix usb3 streams uas: properly reinitialize in uas_eh_bus_reset_handler uas: make work list per-device uas: add dead request list uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE() uas: remove BROKEN Hans de Goede (63): xhci: Free streams when they are still allocated on a set_interface call xhci: Check size rather then number of streams when allocating stream ctxs xhci: For streams the css flag most be read from the stream-ctx on ep stop xhci: Set SCT field for Set TR dequeue on streams xhci: For streams the dequeue ptr must be read from the stream ctx xhci: use usb_ss_max_streams in xhci_check_streams_endpoint usb-core: Fix usb_free_streams return value documentation usb-core: Move USB_MAXENDPOINTS definitions to usb.h usb-core: Track if an endpoint has streams usb-core: Free bulk streams on interface release usbfs: Kill urbs on interface before doing a set_interface usbfs: proc_do_submiturb use a local variable for number_of_packets usbfs: Add support for bulk stream ids usbfs: Add ep_to_host_endpoint helper function usbfs: Add support for allocating / freeing streams uas: Urbs must be anchored before submitting them uas: Properly set interface to altsetting 0 on probe failure uas: Avoid unnecessary unlock / lock calls around unlink_data_urbs uas: uas_alloc_cmd_urb: drop unused stream_id parameter uas: Fix uas not working when plugged into an ehci port uas: Fix reset locking uas: Fix reset handling for externally triggered reset uas: s/response_ui/response_iu/ uas: Fix response iu struct definition uas: Pack iu struct definitions uas: Use proper packet size when submitting reponse urbs uas: Use all available stream ids uas: Add a uas_find_uas_alt_setting helper function uas: Move uas detect code to uas-detect.h xhci: xhci_mem_cleanup: make sure cmd_ring_reserved_trbs really is 0 xhci: The trb_address_map radix tree expects 1KB segment memory aligment xhci: Handle MaxPSASize == 0 usb: Clear host_endpoint->streams when implicitly freeing streams usb: Reset USB-3 devices on USB-3 link bounce uas: Add the posibilty to blacklist uas devices from using the uas driver usb-storage: Don't bind to uas devices if the uas driver is enabled usb-storage: Modify and export adjust_quirks so that it can be used by uas uas: Honor no-uas quirk set in usb-storage's quirks module parameter uas: Add uas_find_endpoints() helper function uas: Fix bounds check in uas_find_endpoints uas: Move uas_find_endpoints to uas-detect.h uas: Drop fixed endpoint config handling uas: Verify endpoint descriptors from uas_use_uas_driver() uas: Not being able to alloc streams when connected through usb-3 is an error uas: task_mgmt: Kill the sense-urb if we fail to submit the cmd urb uas: Don't allow more then one task to run at the same time uas: Use GFP_NOIO rather then GFP_ATOMIC where possible uas: Add suspend/resume support uas: Reset device on reboot uas: Fix task-management not working when connected over USB-2 uas: uas_alloc_data_urb: Remove unnecessary use_streams check uas: Properly complete inflight commands on bus-reset or disconnect uas: add uas_
[PATCH 11/70] 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
Signed-off-by: Sarah Sharp
---
drivers/usb/core/hcd.c | 34 ++
include/linux/usb.h| 2 ++
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index e60e08d..df0b401 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2049,7 +2049,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);
@@ -2058,13 +2058,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]->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]->streams = ret;
+
+ return ret;
}
EXPORT_SYMBOL_GPL(usb_alloc_streams);
@@ -2086,19 +2097,26 @@ 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. */
+ /* Double-free is not allowed */
for (i = 0; i < num_eps; i++)
- if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc))
+ if (!eps[i] || !eps[i]->streams)
return -EINVAL;
- return hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags);
+ 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]->streams = 0;
+
+ return ret;
}
EXPORT_SYMBOL_GPL(usb_free_streams);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 4c53d54..a4c8406 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -57,6 +57,7 @@ struct ep_device;
* @extra: descriptors following this endpoint in the configuration
* @extralen: how many bytes of "extra" are valid
* @enabled: URBs may be submitted to this endpoint
+ * @streams: number of USB-3 streams allocated on the endpoint
*
* USB requests are always queued to a given endpoint, identified by a
* descriptor within an active interface in a given USB configuration.
@@ -71,6 +72,7 @@ struct usb_host_endpoint {
unsigned char *extra; /* Extra descriptors */
int extralen;
int enabled;
+ int streams;
};
/* host-side wrapper for one interface setting's parsed descriptors */
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
Hello.
> > > Signed-off-by: Alexander Shiyan
> > > ---
> > > drivers/usb/chipidea/usbmisc_imx.c | 40 +-
> >
> > > 1 file changed, 22 insertions(+), 18 deletions(-)
> > >
> > >
> > > -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> > > +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
> >
> > Can we keep usbmisc_imx53_init named this?
> >
> > There is more to do here on i.MX5 for cases where the bootloader did
> > not already try to set up USB (or did it badly) such as the
> > transceiver reference clock rate and also setting the USB sysclk clock
> > source (internal DPLL or from external transceiver in the external
> > ULPI case) and so on, which is related and needs doing on both, but is
> > different on i.MX51 than i.MX53. This is information sort of best
> > passed in the PHY node that goes along with this, but it's set within
> > the usbmisc block of the chips so the usbmisc driver will have a
> > responsibility to go see if it's an external PHY that is feeding it's
> > clock back into the USB block in this way.
> >
> > I am not sure we (Peter etc.) discussed how best to do this, the code
> > to pull the correct information out always seems kind of misplaced no
> > matter where it goes, but the responsibility for tweaking those
> > registers is most certainly this driver.
> >
> > Essentially the layout of usbmisc->base + 0x10 register (USB_CTRL_1)
> > is different when doing the above, and dependent on a board-specific
> > option for the input clock to the transceiver. We could reduce a
> > little churn, later, when usbmisc_imx could be given related usbphy
> > information and actually do the right thing. I have a patch kinda
> > sitting in the wings to do this.. and two *real* pieces of consumer
> > hardware that need it, and some other kicking, to make USB work in the
> > never-touched-before-Linux case.
> >
> > > -static const struct usbmisc_ops imx53_usbmisc_ops = {
> > > - .init = usbmisc_imx53_init,
> > > +static const struct usbmisc_ops imx5_usbmisc_ops = {
> > > + .init = usbmisc_imx5_init,
> > > };
> >
> > And keep imx53_usbmisc_ops named this?
> >
> > > static const struct usbmisc_ops imx6q_usbmisc_ops = {
> > > @@ -204,8 +204,12 @@ static const struct of_device_id
> > usbmisc_imx_dt_ids[] = {
> > > .data = &imx27_usbmisc_ops,
> > > },
> > > {
> > > + .compatible = "fsl,imx51-usbmisc",
> > > + .data = &imx5_usbmisc_ops,
> >
> > And then just use &imx53_usbmisc_ops?
> >
> > This gives us some breathing room later to actually do the right thing
> > without additionally performing renames all over the place to make
> > imx5 -> imx53 (again)/imx51 (new).
>
> You may take matt's suggestion, it can reduce the code change now and in
> future.
> We can only add device_id for imx51 in this patch, split imx53 and imx51's ops
> when their differences are added in future.
OK. Names of registers and bits also keep as is?
---
RE: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
> >
> > Signed-off-by: Alexander Shiyan
> > ---
> > drivers/usb/chipidea/usbmisc_imx.c | 40 +-
>
> > 1 file changed, 22 insertions(+), 18 deletions(-)
> >
> >
> > -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> > +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
>
> Can we keep usbmisc_imx53_init named this?
>
> There is more to do here on i.MX5 for cases where the bootloader did
> not already try to set up USB (or did it badly) such as the
> transceiver reference clock rate and also setting the USB sysclk clock
> source (internal DPLL or from external transceiver in the external
> ULPI case) and so on, which is related and needs doing on both, but is
> different on i.MX51 than i.MX53. This is information sort of best
> passed in the PHY node that goes along with this, but it's set within
> the usbmisc block of the chips so the usbmisc driver will have a
> responsibility to go see if it's an external PHY that is feeding it's
> clock back into the USB block in this way.
>
> I am not sure we (Peter etc.) discussed how best to do this, the code
> to pull the correct information out always seems kind of misplaced no
> matter where it goes, but the responsibility for tweaking those
> registers is most certainly this driver.
>
> Essentially the layout of usbmisc->base + 0x10 register (USB_CTRL_1)
> is different when doing the above, and dependent on a board-specific
> option for the input clock to the transceiver. We could reduce a
> little churn, later, when usbmisc_imx could be given related usbphy
> information and actually do the right thing. I have a patch kinda
> sitting in the wings to do this.. and two *real* pieces of consumer
> hardware that need it, and some other kicking, to make USB work in the
> never-touched-before-Linux case.
>
> > -static const struct usbmisc_ops imx53_usbmisc_ops = {
> > - .init = usbmisc_imx53_init,
> > +static const struct usbmisc_ops imx5_usbmisc_ops = {
> > + .init = usbmisc_imx5_init,
> > };
>
> And keep imx53_usbmisc_ops named this?
>
> > static const struct usbmisc_ops imx6q_usbmisc_ops = {
> > @@ -204,8 +204,12 @@ static const struct of_device_id
> usbmisc_imx_dt_ids[] = {
> > .data = &imx27_usbmisc_ops,
> > },
> > {
> > + .compatible = "fsl,imx51-usbmisc",
> > + .data = &imx5_usbmisc_ops,
>
> And then just use &imx53_usbmisc_ops?
>
> This gives us some breathing room later to actually do the right thing
> without additionally performing renames all over the place to make
> imx5 -> imx53 (again)/imx51 (new).
Hi Alexander,
You may take matt's suggestion, it can reduce the code change now and in future.
We can only add device_id for imx51 in this patch, split imx53 and imx51's ops
when their differences are added in future.
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 42/68] uas: Add a usbcore.nouas kernel cmdline option
Hi, On 11/15/2013 10:07 PM, Greg KH wrote: On Fri, Nov 15, 2013 at 09:21:40PM +0100, Hans de Goede wrote: Hi, On 11/15/2013 08:19 PM, Sarah Sharp wrote: On Fri, Nov 15, 2013 at 04:06:56PM +0100, Hans de Goede wrote: Note this is done through a usbcore module parameter as the option is needed by both the uas and usb-storage drivers. I'm a bit confused as to why you need both this patch and the previous patch. The previous patch adds the ability to add an usual dev entry to the UAS driver to make it not bind to a UAS device (usb-storage would get used instead). This one adds a module parameter to the USB core to disable UAS all together. I believe you can add unusual dev entries via modprobe arguments For the usb-storage driver that is correct, but not for the uas driver, the current modprobe / sysfs options are for setting quirks only on the usb-storage side. uas cannot access these quirk since this info is all inside the usb-storage module. Then export it from the usb-storage module, creating a new one if needed to share the data (or just have uas depend on usb-storage), don't create a new one in the usb core. Ok, I'll make uas depend on usb-storage then, in practice this should not make any difference since plugging in an uas device will cause both drivers to get loaded anyways. As Sarah rightly points out, we don't want to add new module parameters at all if possible, and not for the USB core for something like this, sorry. No problem, I'll go and rework this bit of the patchset to make uas use the quirks from usb-storage, as well as deal with some minor other remarks, and then send a v2 pull-req to Sarah. 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 31/68] uas: Pack iu struct definitions
Hi,
On 11/15/2013 10:09 PM, Greg KH wrote:
On Fri, Nov 15, 2013 at 03:56:31PM -, David Laight wrote:
From: Hans de Goede [mailto:hdego...@redhat.com]
On 11/15/2013 04:29 PM, David Laight wrote:
From: Of Hans de Goede
The iu struct definitions are usb packet definitions, so no alignment should
happen. Notice that assuming 32 bit alignment this does not make any
difference at all.
Signed-off-by: Hans de Goede
---
include/linux/usb/uas.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h
index 772b66b..3fc8e8b 100644
--- a/include/linux/usb/uas.h
+++ b/include/linux/usb/uas.h
@@ -9,7 +9,7 @@ struct iu {
__u8 iu_id;
__u8 rsvd1;
__be16 tag;
-};
+} __attribute__((__packed__));
Won't this cause the compiler to generate multiple byte accesses
for the 'tag' field in systems that don't support misaligned
accesses.
The tag field is aligned to a multiple of its size, so that should not
happen, unless some system wants 32 bits alignment for 16 bit variables.
That isn't going to be true. It might even be against the C standard.
In any case such a system wouldn't be able to define __be16
(unless the compiler implemented it using byte accesses).
...
Without packed struct members will never be mis-aligned in memory as the
compiler will simply add padding. packed is used when the struct represent
data coming from an outside source, ie a file or a socket, or in this case
an usb data packet.
What would be useful is an attribute that errorred if any implicit padding
were added.
If you are worried about padding add a compile-time test on the size of the
structure.
No, Linux relies on the compiler to pack things like this together and
not put padding in, with the __packed__ attribute (don't we have a
__packed macro instead of the __attribute__() thing?)
Yes we've a __packed macro, but the norm in the usb subsys seems to be to use
__attribute__ ((__packed__)), see include/[uapi/]linux/usb/*.h
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 11/68] usb-core: Track if an endpoint has streams
Hi,
On 11/16/2013 08:45 AM, Oliver Neukum wrote:
On Fri, 2013-11-15 at 23:26 +0100, Hans de Goede wrote:
Hi,
On 11/15/2013 11:16 PM, Oliver Neukum wrote:
On Fri, 2013-11-15 at 16:06 +0100, Hans de Goede wrote:
This is a preparation patch for adding support for bulk streams to usbfs.
@@ -2086,19 +2097,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))
Why bother checking for the endpoint's type?
Not sure what you're trying to say here, the check gets re-added a couple of
lines
later in the patch. Are you trying to say the ep type check can be removed
entirely?
Yes, the check makes sense on allocation only.
Now that we check that a stream has endpoints, indeed the type check does not
make
a whole lot of sense, since only bulk eps will have streams. I'll respin the
patch
with the unnecessary check removed.
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
