[PATCH v5 0/1] add support for the R-Car H2 and M2 xHCI controllers
This patch adds the USB xHCI support for the R-Car H2 and M2 SoCs. These SoCs use an xHCI but still need specific initialization, mainly to setup the firmware downloading and the specific registers. This patch set is based on the usb-next branch of Greg usb.git tree. (commit id = 4a95b1fce97756d0333f8232eb7ed6974e93b054) Changes since v4: - Rebase the latest usb-next branch Changes since v3: - Remove a general phy driver calling in xhci-plat.c - Change xhci_rcar_init_quirk() calling into xhci_plat_setup(). Because of usb_add_hcd() will call a generic phy driver calling. - Modify xhci_rcar_init_quirk() Changes since v2: - Fix goto direction in patch 1. - Remove some unnecessary checks in patch 1. - Add MODULE_FIRMWARE() in patch 2. - Add a comment about xhci_rcar_init_quirk() in patch 2. Changes since v1 (about patch 2 only because patch 1 and 3 ware applied): - Fix some typos in patch 2. - Remove duplicated #define in patch 2. - Remove usb_phy framework calling in patch 2. - Change prototype of xhci_rcar_start() in patch 2. - Change compatible strings from "renesas,r8a779[01]-xhci" to "renesas,xhci-r8a779[01]" in patch 2. Because of Renesas SoC maintainer's comment: http://marc.info/?l=linux-sh&m=140109049002958&w=2 - Rewrite the custom get_unaligned_le32() and add comment about it in patch 2. Yoshihiro Shimoda (1): usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers drivers/usb/host/Kconfig |8 +++ drivers/usb/host/Makefile|3 + drivers/usb/host/xhci-plat.c | 19 ++ drivers/usb/host/xhci-rcar.c | 148 ++ drivers/usb/host/xhci-rcar.h | 27 5 files changed, 205 insertions(+) create mode 100644 drivers/usb/host/xhci-rcar.c create mode 100644 drivers/usb/host/xhci-rcar.h -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/1] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
The R-Car H2 and M2 SoCs come with an xHCI controller that requires
some specific initializations related to the firmware downloading and
some specific registers. This patch adds the support for this special
configuration as an xHCI quirk executed during probe and start.
Signed-off-by: Yoshihiro Shimoda
---
drivers/usb/host/Kconfig |8 +++
drivers/usb/host/Makefile|3 +
drivers/usb/host/xhci-plat.c | 19 ++
drivers/usb/host/xhci-rcar.c | 148 ++
drivers/usb/host/xhci-rcar.h | 27
5 files changed, 205 insertions(+)
create mode 100644 drivers/usb/host/xhci-rcar.c
create mode 100644 drivers/usb/host/xhci-rcar.h
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 61b7817..537d9e1 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -37,6 +37,14 @@ config USB_XHCI_MVEBU
Say 'Y' to enable the support for the xHCI host controller
found in Marvell Armada 375/38x ARM SOCs.
+config USB_XHCI_RCAR
+ tristate "xHCI support for Renesas R-Car SoCs"
+ select USB_XHCI_PLATFORM
+ depends on ARCH_SHMOBILE || COMPILE_TEST
+ ---help---
+ Say 'Y' to enable the support for the xHCI host controller
+ found in Renesas R-Car ARM SoCs.
+
endif # USB_XHCI_HCD
config USB_EHCI_HCD
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index af89a90..144c038 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -22,6 +22,9 @@ ifneq ($(CONFIG_USB_XHCI_PLATFORM), )
ifneq ($(CONFIG_USB_XHCI_MVEBU), )
xhci-hcd-y += xhci-mvebu.o
endif
+ifneq ($(CONFIG_USB_XHCI_RCAR), )
+ xhci-hcd-y += xhci-rcar.o
+endif
endif
obj-$(CONFIG_USB_WHCI_HCD) += whci/
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 29d8adb..b6f2b6b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -20,6 +20,7 @@
#include "xhci.h"
#include "xhci-mvebu.h"
+#include "xhci-rcar.h"
static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
{
@@ -34,11 +35,27 @@ static void xhci_plat_quirks(struct device *dev, struct
xhci_hcd *xhci)
/* called during probe() after chip reset completes */
static int xhci_plat_setup(struct usb_hcd *hcd)
{
+ struct device_node *of_node = hcd->self.controller->of_node;
+ int ret;
+
+ if (of_device_is_compatible(of_node, "renesas,xhci-r8a7790") ||
+ of_device_is_compatible(of_node, "renesas,xhci-r8a7791")) {
+ ret = xhci_rcar_init_quirk(hcd);
+ if (ret)
+ return ret;
+ }
+
return xhci_gen_setup(hcd, xhci_plat_quirks);
}
static int xhci_plat_start(struct usb_hcd *hcd)
{
+ struct device_node *of_node = hcd->self.controller->of_node;
+
+ if (of_device_is_compatible(of_node, "renesas,xhci-r8a7790") ||
+ of_device_is_compatible(of_node, "renesas,xhci-r8a7791"))
+ xhci_rcar_start(hcd);
+
return xhci_run(hcd);
}
@@ -270,6 +287,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
{ .compatible = "xhci-platform" },
{ .compatible = "marvell,armada-375-xhci"},
{ .compatible = "marvell,armada-380-xhci"},
+ { .compatible = "renesas,xhci-r8a7790"},
+ { .compatible = "renesas,xhci-r8a7791"},
{ },
};
MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
new file mode 100644
index 000..d94cb72
--- /dev/null
+++ b/drivers/usb/host/xhci-rcar.c
@@ -0,0 +1,148 @@
+/*
+ * xHCI host controller driver for R-Car SoCs
+ *
+ * Copyright (C) 2014 Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "xhci.h"
+#include "xhci-rcar.h"
+
+#define FIRMWARE_NAME "r8a779x_usb3_v1.dlmem"
+MODULE_FIRMWARE(FIRMWARE_NAME);
+
+/*** Register Offset ***/
+#define RCAR_USB3_INT_ENA 0x224 /* Interrupt Enable */
+#define RCAR_USB3_DL_CTRL 0x250 /* FW Download Control & Status */
+#define RCAR_USB3_FW_DATA0 0x258 /* FW Data0 */
+
+#define RCAR_USB3_LCLK 0xa44 /* LCLK Select */
+#define RCAR_USB3_CONF10xa48 /* USB3.0 Configuration1 */
+#define RCAR_USB3_CONF20xa5c /* USB3.0 Configuration2 */
+#define RCAR_USB3_CONF30xaa8 /* USB3.0 Configuration3 */
+#define RCAR_USB3_RX_POL 0xab0 /* USB3.0 RX Polarity */
+#define RCAR_USB3_TX_POL 0xab8 /* USB3.0 TX Polarity */
+
+/*** Register Settings ***/
+/* Interrupt Enable */
+#define RCAR_USB3_INT_XHC_ENA 0x0001
+#define RCAR_USB3_INT_PME_ENA 0x0002
+#define RCAR_USB3_INT_HSE_ENA 0x0004
+#define RCAR_USB3_INT_ENA_VA
[PATCH v2] usb: dwc3: Keeping 'resource' related code together
Putting together the code related to getting the 'IORESOURCE_MEM'
and assigning the same to dwc->xhci_resources, for increasing
the readability.
Signed-off-by: Vivek Gautam
---
Changes from v1:
- Fixed issue with 'res->start' as pointed out by Paul Zimmerman
by decrementing it back to its original value.
drivers/usb/dwc3/core.c | 44 +---
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 3f59c12..8b1acad 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -656,6 +656,31 @@ static int dwc3_probe(struct platform_device *pdev)
return -ENODEV;
}
+ dwc->xhci_resources[0].start = res->start;
+ dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
+ DWC3_XHCI_REGS_END;
+ dwc->xhci_resources[0].flags = res->flags;
+ dwc->xhci_resources[0].name = res->name;
+
+ res->start += DWC3_GLOBALS_REGS_START;
+
+ /*
+* Request memory region but exclude xHCI regs,
+* since it will be requested by the xhci-plat driver.
+*/
+ regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
+ dwc->regs = regs;
+ dwc->regs_size = resource_size(res);
+ /*
+* restore res->start back to its original value so that,
+* in case the probe is deferred, we don't end up getting error in
+* request the memory region the next time probe is called.
+*/
+ res->start -= DWC3_GLOBALS_REGS_START;
+
if (node) {
dwc->maximum_speed = of_usb_get_maximum_speed(node);
@@ -676,28 +701,9 @@ static int dwc3_probe(struct platform_device *pdev)
if (ret)
return ret;
- dwc->xhci_resources[0].start = res->start;
- dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
- DWC3_XHCI_REGS_END;
- dwc->xhci_resources[0].flags = res->flags;
- dwc->xhci_resources[0].name = res->name;
-
- res->start += DWC3_GLOBALS_REGS_START;
-
- /*
-* Request memory region but exclude xHCI regs,
-* since it will be requested by the xhci-plat driver.
-*/
- regs = devm_ioremap_resource(dev, res);
- if (IS_ERR(regs))
- return PTR_ERR(regs);
-
spin_lock_init(&dwc->lock);
platform_set_drvdata(pdev, dwc);
- dwc->regs = regs;
- dwc->regs_size = resource_size(res);
-
dev->dma_mask = dev->parent->dma_mask;
dev->dma_parms = dev->parent->dma_parms;
dma_set_coherent_mask(dev, dev->parent->coherent_dma_mask);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
Hi,
On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
>
> Signed-off-by: Vivek Gautam
Although this patch and rest in the series are merged.
But i have got a doubt, so making this thread alive.
> ---
> drivers/usb/host/ohci-exynos.c |7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 9cf80cb..dec691d 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -120,10 +120,9 @@ skip_phy:
>
> hcd->rsrc_start = res->start;
> hcd->rsrc_len = resource_size(res);
> - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> - if (!hcd->regs) {
> - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> - err = -ENOMEM;
> + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
which internally
requests the memory region and then does a "devm_ioremap()" or
"devm_ioremap_nocache()"
based on the check for IORESOURCE_CACHEABLE flag.
But this flag is not set for the resource of this device.
So should we be explicitly setting the flag in driver ?
The query goes for other patches too in this series, wherein
devm_ioremap() call is replaced with devm_ioremap_resource().
[snip]
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] u_ether: move hardware transmit to RX NAPI
W dniu 30.05.2014 15:34, Weinn Jheng pisze: In order to reduce the interrupt times in the embedded system, a receiving workqueue is introduced. This modification also enhanced the overall throughput as the benefits of reducing interrupt occurrence. Sorry, still problem: [ 29.934435] BUG: scheduling while atomic: swapper/0/0x00010003 [ 29.938784] Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether libcomposite [ 29.946235] Preemption disabled at:[< (null)>] (null) [ 29.951521] [ 29.953003] CPU: 0 PID: 0 Comm: swapper Not tainted 3.15.0-rc4+ #461 [ 29.959361] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 29.967047] [] (show_stack) from [] (dump_stack+0x20/0x28) [ 29.974244] [] (dump_stack) from [] (__schedule_bug+0x98/0xbc) [ 29.981775] [] (__schedule_bug) from [] (__schedule+0x42c/0x4ec) [ 29.989485] [] (__schedule) from [] (schedule+0x40/0x90) [ 29.996504] [] (schedule) from [] (schedule_timeout+0x108/0x1dc) [ 30.004218] [] (schedule_timeout) from [] (schedule_timeout_uninterruptible+0x30/0x34) [ 30.013845] [] (schedule_timeout_uninterruptible) from [] (msleep+0x24/0x30) This looks suspicious: [ 30.022604] [] (msleep) from [] (gether_disconnect+0x48/0x2b4 [u_ether]) [ 30.031010] [] (gether_disconnect [u_ether]) from [] (ecm_disable+0x48/0x9c [usb_f_ecm]) [ 30.040817] [] (ecm_disable [usb_f_ecm]) from [] (reset_config+0x54/0xa0 [libcomposite]) [ 30.050600] [] (reset_config [libcomposite]) from [] (composite_disconnect+0x44/0x6c [libcomposite]) [ 30.061425] [] (composite_disconnect [libcomposite]) from [] (s3c_hsotg_disconnect+0x98/0xa4) [ 30.071638] [] (s3c_hsotg_disconnect) from [] (s3c_hsotg_complete_setup+0x180/0x508) [ 30.081090] [] (s3c_hsotg_complete_setup) from [] (s3c_hsotg_complete_request+0x98/0x14c) [ 30.090963] [] (s3c_hsotg_complete_request) from [] (s3c_hsotg_handle_outdone+0xc8/0x16c) [ 30.100839] [] (s3c_hsotg_handle_outdone) from [] (s3c_hsotg_irq+0x6b4/0x950) [ 30.109684] [] (s3c_hsotg_irq) from [] (handle_irq_event_percpu+0x6c/0x28c) [ 30.118347] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x4c/0x6c) [ 30.127186] [] (handle_irq_event) from [] (handle_level_irq+0xac/0x148) [ 30.135504] [] (handle_level_irq) from [] (generic_handle_irq+0x34/0x48) [ 30.143910] [] (generic_handle_irq) from [] (handle_IRQ+0x54/0xc0) [ 30.151795] [] (handle_IRQ) from [] (vic_handle_irq+0x6c/0xac) [ 30.159332] [] (vic_handle_irq) from [] (__irq_svc+0x44/0x78) [ 30.166779] Exception stack(0xc0587f28 to 0xc0587f70) [ 30.171810] 7f20: c058e83c 1198 c058e83c c0586000 c05d2540 [ 30.179955] 7f40: c0586000 c05cd3b0 c058e0d0 c05cd3b0 c0586000 c0587f7c c0587f70 c0587f70 [ 30.188098] 7f60: c00108a0 c00108a4 6013 When the gadget is physically disconnected we end up in interrupt context. gether_disconnect() calls napi_disable() which, in certain circumstances, calls msleep(). But please see include/linux/usb/gadget.h, struct usb_gadget_driver, description of the disconnect() method: "Invoked after all transfers have been stopped, when the host is disconnected. May be called in_interrupt; this may not sleep." This means that napi_disable() cannot be called where you call it. If you need to call it, you have to do it in a context which can sleep. AP -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage
Hi Alan, W dniu 03.06.2014 16:48, Alan Stern pisze: On Tue, 3 Jun 2014 wei.y...@windriver.com wrote: From: Yang Wei While loading g_mass_storage module, the following warning is triggered. In fact, it is more easy to reproduce it with RT kernel. WARNING: at drivers/usb/gadget/composite.c: usb_composite_setup_continue: Unexpected call Modules linked in: fat vfat minix nls_cp437 nls_iso8859_1 g_mass_storage [<800179cc>] (unwind_backtrace+0x0/0x104) from [<80619608>] (dump_stack+0x20/0x24) [<80619608>] (dump_stack+0x20/0x24) from [<80025100>] (warn_slowpath_common+0x64/0x74) [<80025100>] (warn_slowpath_common+0x64/0x74) from [<800251cc>] (warn_slowpath_fmt+0x40/0x48) [<800251cc>] (warn_slowpath_fmt+0x40/0x48) from [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) from [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) from [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) from [<8004bc90>] (kthread+0x98/0x9c) [<8004bc90>] (kthread+0x98/0x9c) from [<8000faec>] (kernel_thread_exit+0x0/0x8) The root cause just likes the following scenario. irq thread composite_disconnect() | |->fsg_disable() fsg->common->new_fsg = NULL and then wake fsg_main_thread with seting common->state to FSG_STATE_CONFIG_CHANGE. fsg_main_thread | |->do_set_interface() irq thread set_config() | |->fsg_set_alt() fsg->common->new_fsg = new_fsg and then also wake up fsg_main_thread with setting common->state to FSG_STATE_CONFIG_CHANGE. |-> if(common->new_fsg) usb_composite_setup_continue() In this case, fsg_main_thread would invoke usb_composite_setup_continue twice, so the second call would trigger the above call trace, as we also save common->new_fsg while changing the common->state. Michal and Andrzej: I haven't paid much attention to these matters, because you handled the conversion from g_file_storage to f_mass_storage using the composite framework. But this patch seemed odd, so I took a closer look. Actually when I started dealing with usb gadgets the f_mass_storage had already been there. My involvement started with some cleanup, then moving to the new function registration interface (usb_get/put_function_instance(), usb_get/put_function()) and adding configfs support. That said, it is not impossible for me to have spoilt something :O In f_mass_storage.c, struct fsg_common is shared among all the function instances. This structure includes things like cmnd and nluns, which will in general be different for different functions. That's okay if each function is in a separate config, but what happens when there are multiple functions in the same config, using different interfaces? What if the host sends concurrent commands to two of these functions? When Sebastian introduced the function registration interface I didn't specially like the naming: struct usb_function_instance is something different than an instance of struct usb_function. The purpose of fsg_alloc_inst() is to create a usb_function_instance whose container_of is struct fsg_opts. In fact it is struct fsg_opts which is actually allocated; one of its members is struct fsg_common which is also allocated - individually for each struct usb_function_instance. Among traditional gadgets there is no gadget which uses mass storage function more than once. On the other hand, when gadgets are created with configfs it is forbidden to link a given function more than once into a given config, that is a struct usb_function_instance can be referenced by more than one config, but can be referenced only once in a given config; each symbolic link corresponds to an instance of struct usb_function (don't confuse with struct usb_function_instance). So yes, an fsg_common can be shared among instances of struct usb_function, but neither with traditional gadgets as they are now nor with configfs is it possible to have the same fsg_common referenced more than once in a given config. AP -- 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
Reply
I, Liliane authenticate this email, you can read about me on: I write to you because I intend to give to you a portion of my Net-worth which I have been banking. I want to cede it out as gift hoping it would be of help to you and others too. Respond for confirmation. (Email:lilianebettencour...@outlook.com) With love, Liliane Bettencourt -- 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 04/12] trivial: drivers/usb/phy/phy.c: remove a leading space
Signed-off-by: Antonio Ospite Cc: Felipe Balbi Cc: linux-usb@vger.kernel.org --- drivers/usb/phy/phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index 36b6bce..6d0f608 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -147,7 +147,7 @@ err0: } EXPORT_SYMBOL_GPL(usb_get_phy); - /** +/** * devm_usb_get_phy_by_phandle - find the USB PHY by phandle * @dev - device that requests this phy * @phandle - name of the property holding the phy phandle value -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> Hi,
>
>
> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam
> wrote:
> > Using devm_ioremap_resource() API should actually be preferred over
> > devm_ioremap(), since the former request the mem region first and then
> > gives back the ioremap'ed memory pointer.
> > devm_ioremap_resource() calls request_mem_region(), therby preventing
> > other drivers to make any overlapping call to the same region.
> >
> > Signed-off-by: Vivek Gautam
>
> Although this patch and rest in the series are merged.
> But i have got a doubt, so making this thread alive.
>
> > ---
> > drivers/usb/host/ohci-exynos.c |7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > index 9cf80cb..dec691d 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -120,10 +120,9 @@ skip_phy:
> >
> > hcd->rsrc_start = res->start;
> > hcd->rsrc_len = resource_size(res);
> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> > - if (!hcd->regs) {
> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> > - err = -ENOMEM;
> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>
> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> which internally requests the memory region
I guess this could lead to problems if drivers haven't been written to
cleanly split the register ranges that they access, since now two
overlapping regions may be requested and cause the drivers to fail.
> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> the check for IORESOURCE_CACHEABLE flag.
>
> But this flag is not set for the resource of this device.
> So should we be explicitly setting the flag in driver ?
I don't think it makes much sense to map these registers cached anyway.
Drivers will likely expect writes to this region to take effect without
needing any kind of flushing.
Thierry
pgphdTmeFKZ9N.pgp
Description: PGP signature
Re: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage
On Wed, 4 Jun 2014, Yang,Wei wrote: > On 06/04/2014 09:45 AM, Peter Chen wrote: > > > >> commit d18f7116a5ddb8263fe62b05ad63e5ceb5875791 > >> Author: Robert Baldyga > >> Date: Thu Nov 21 13:49:18 2013 +0100 > >> > >> usb: gadget: s3c-hsotg: fix disconnect handling > >> > >> This patch moves s3c_hsotg_disconnect function call from USBSusp > >> interrupt > >> handler to SET_ADDRESS request handler. > >> > > It is a little strange we call gadget's disconnect at SET_ADDRESS? > > How the udc calls gadget driver the disconnection has happened when > > the usb cable is disconnected from the host? > > > > Usually, we call gadget's disconnect at two situations > > > > - udc's reset handler if udc's speed is not UNKNOWN, it is usually happened > > when the host sends reset after enumeration. > > - udc's disconnect handler, it is usually happened when the usb cable > > is disconnected from host. > > Hmm, usually the two situations, but according to the commit log, s3c > hsotg does not support Disconnected interrupt for device mode, > so the second situation does not happen for s3c hsotg, therefore, he has > to disconnect the connection before it is connected again. Why does he need to do that? Why not just skip the disconnect notification if the hardware can't detect a disconnect? It makes no sense at all to call a disconnect handler from within the SET_ADDRESS routine. 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: ftdi_sio BUG: NULL pointer dereference
On Tue, Jun 03, 2014 at 06:17:33AM -0400, Mike Remski wrote:
> On 06/02/2014 01:46 PM, Johan Hovold wrote:
> > On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
> >> On 06/02/2014 12:49 PM, Johan Hovold wrote:
> >>> On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
> On 06/02/2014 12:20 PM, Johan Hovold wrote:
> > On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
> >> On 06/02/2014 11:40 AM, Johan Hovold wrote:
> >>> [ Please avoid top-posting. ]
> >>>
> >>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
> > The third interface lacks endpoints and crashes the ftdi_sio driver.
> > This shouldn't happen (even if you're forcing the wrong driver to bind),
> > so I'll fix it up if still broken in v3.15-rc.
> >
> Johan,
> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
> they've programmed in their own ids. They supply a Windows driver for
> it, but that doesn't do me any good. :)
> >>> Not just their own ID's it seems.
> >>>
> >>> Have you tried just using the cdc-acm driver? The ports should up as
> >>> /dev/ttyACMx instead of ttyUSBx.
> >>>
> >> Not yet, next on the list.
> > You really should try this before anything else. :)
How did it go with cdc-acm?
You probably need the following commit (depending how well-supported your
old kernel is):
5b239f0aebd4d ("USB: cdc-acm: Add pseudo modem without AT
command capabilities")
in order for the device to bind.
> >> I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
> >> stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
> >> later on, I'm hitting the NULL dereference.
> > Yeah, the code is obviously broken (also in v3.15-rc). It should
> > probably work to just return from ftdi_set_max_packet_size if
> > num_endpoints is 0 if you want to try that (or you can use your ?:
> > construct), but I should be able to fix this up properly on Wednesday.
> >
> > Thanks,
> > Johan
> Johan,
> I had a chance to play around with code over in ftdi_sio.c; adding this:
>
> if (!num_endpoints) {
> return;
> }
> after the "Number of endpoints" message gets rid of the crash,
> everything looks to be working correctly.
Good. Care to try the patch below so I can add a formal Tested-by tag as
well?
Thanks,
Johan
>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
Fix NULL-pointer dereference when probing an interface with no
endpoints.
These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.
Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.
Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")
Reported-by: Mike Remski
Cc: # 2.3.61
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc =
&interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;
unsigned num_endpoints;
- int i;
+ unsigned i;
num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
--
1.8.5.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: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
Hello Michal,
> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Michal Nazarewicz
> Sent: Tuesday, February 25, 2014 6:02 PM
> To: Manu Gautam
> Cc: ba...@ti.com; ja...@codeaurora.org; pheat...@codeaurora.org;
> linux-usb@vger.kernel.org; linux-arm-...@vger.kernel.org;
> ben...@android.com; Andrzej Pietrasiewicz;
> gre...@linuxfoundation.org; Manu Gautam
> Subject: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors
> block
>
> This reworks the way SuperSpeed descriptors are added and instead
> of
> having a magick after full and high speed descriptors, it reworks
> the
> whole descriptors block to include a flags field which lists which
> descriptors are present and makes future extensions possible.
>
> Signed-off-by: Michal Nazarewicz
> ---
> drivers/usb/gadget/f_fs.c | 136 +++-
>
> drivers/usb/gadget/u_fs.h | 12 ++--
> include/uapi/linux/usb/functionfs.h | 49 -
> 3 files changed, 94 insertions(+), 103 deletions(-)
>
> All right, with some fidling with my fan and limiting CPU frequency
> to
> the mininimum, I've managed to compile this patch and fixed all the
> typos.
>
> Manu, if you could include it in your series, adjust your user
> space
> client and test it, it would be wonderful. :]
>
(...) snip
> diff --git a/include/uapi/linux/usb/functionfs.h
> b/include/uapi/linux/usb/functionfs.h
> index 0f8f7be..ee6fcbc 100644
> --- a/include/uapi/linux/usb/functionfs.h
> +++ b/include/uapi/linux/usb/functionfs.h
> @@ -10,7 +10,14 @@
>
> enum {
> FUNCTIONFS_DESCRIPTORS_MAGIC = 1,
> - FUNCTIONFS_STRINGS_MAGIC = 2
> + FUNCTIONFS_STRINGS_MAGIC = 2,
> + FUNCTIONFS_DESCRIPTORS_MAGIC_V2 = 3,
> +};
> +
> +enum functionfs_flags {
> + FUNCTIONFS_HAS_FS_DESC = 1,
> + FUNCTIONFS_HAS_HS_DESC = 2,
> + FUNCTIONFS_HAS_SS_DESC = 4,
> };
>
> #define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C
> @@ -30,33 +37,39 @@ struct usb_endpoint_descriptor_no_audio {
>
>
> /*
> - * All numbers must be in little endian order.
> - */
> -
> -struct usb_functionfs_descs_head {
> - __le32 magic;
> - __le32 length;
> - __le32 fs_count;
> - __le32 hs_count;
> -} __attribute__((packed));
> -
> -/*
I have tried to compile FFS examples with v3.15-rc8 but I have faced an
issue that they are unable to build due to missing definition of this
structure. The same is with adb, sdb and all userspace apps which use
legacy API. What is the reason of removing it? Was your intentions?
Maybe would be a good idea to leave this structure untouched as legacy
userspace API?
There is also no structure definition for new API, maybe suitable
structure should be defined (struct usb_functionfs_descs_head2 for
example) to make userspace life easier?
--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opas...@samsung.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 10:19 AM, Johan Hovold wrote:
On Tue, Jun 03, 2014 at 06:17:33AM -0400, Mike Remski wrote:
On 06/02/2014 01:46 PM, Johan Hovold wrote:
On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
On 06/02/2014 12:49 PM, Johan Hovold wrote:
On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
On 06/02/2014 12:20 PM, Johan Hovold wrote:
On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
On 06/02/2014 11:40 AM, Johan Hovold wrote:
[ Please avoid top-posting. ]
On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
The third interface lacks endpoints and crashes the ftdi_sio driver.
This shouldn't happen (even if you're forcing the wrong driver to bind),
so I'll fix it up if still broken in v3.15-rc.
Johan,
Thanks again. Yes, the device does indeed have an FTDI embedded in it;
they've programmed in their own ids. They supply a Windows driver for
it, but that doesn't do me any good. :)
Not just their own ID's it seems.
Have you tried just using the cdc-acm driver? The ports should up as
/dev/ttyACMx instead of ttyUSBx.
Not yet, next on the list.
You really should try this before anything else. :)
How did it go with cdc-acm?
You probably need the following commit (depending how well-supported your
old kernel is):
5b239f0aebd4d ("USB: cdc-acm: Add pseudo modem without AT
command capabilities")
in order for the device to bind.
I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
later on, I'm hitting the NULL dereference.
Yeah, the code is obviously broken (also in v3.15-rc). It should
probably work to just return from ftdi_set_max_packet_size if
num_endpoints is 0 if you want to try that (or you can use your ?:
construct), but I should be able to fix this up properly on Wednesday.
Thanks,
Johan
Johan,
I had a chance to play around with code over in ftdi_sio.c; adding this:
if (!num_endpoints) {
return;
}
after the "Number of endpoints" message gets rid of the crash,
everything looks to be working correctly.
Good. Care to try the patch below so I can add a formal Tested-by tag as
well?
Thanks,
Johan
>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
Fix NULL-pointer dereference when probing an interface with no
endpoints.
These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.
Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.
Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")
Reported-by: Mike Remski
Cc: # 2.3.61
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc =
&interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;
unsigned num_endpoints;
- int i;
+ unsigned i;
num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
Thanks Johan. I tried to get the cdc_acm working; did not have much
luck/time (typical overcommit on workload) I will retry with the commit
mentioned.
I will try the patch today and get back to you. Nice on the ep_desc:
looking at the code priv->max_packet_size is attached to the port, your
change would use the last thing off of cur_altsetting->endpoint[], but
I'm wondering if we should actually be setting priv->max_packet_size to
whatever the max is of all endpoint[].desc->wMaxPacketSize?
Thoughts?
m
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
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: ftdi_sio BUG: NULL pointer dereference
On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
> On 06/04/2014 10:19 AM, Johan Hovold wrote:
> > >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
> > From: Johan Hovold
> > Date: Wed, 4 Jun 2014 14:09:43 +0200
> > Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
> >
> > Fix NULL-pointer dereference when probing an interface with no
> > endpoints.
> >
> > These devices have two bulk endpoints per interface, but this avoids
> > crashing the kernel if a user forces a non-FTDI device to be probed.
> >
> > Note that the iterator variable was made unsigned in order to avoid
> > a maybe-uninitialized compiler warning for ep_desc after the loop.
> >
> > Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
> > calculation")
> >
> > Reported-by: Mike Remski
> > Cc: # 2.3.61
> > Signed-off-by: Johan Hovold
> > ---
> > drivers/usb/serial/ftdi_sio.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> > index 7c6e1dedeb06..3019141397eb 100644
> > --- a/drivers/usb/serial/ftdi_sio.c
> > +++ b/drivers/usb/serial/ftdi_sio.c
> > @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
> > usb_serial_port *port)
> > struct usb_device *udev = serial->dev;
> >
> > struct usb_interface *interface = serial->interface;
> > - struct usb_endpoint_descriptor *ep_desc =
> > &interface->cur_altsetting->endpoint[1].desc;
> > + struct usb_endpoint_descriptor *ep_desc;
> >
> > unsigned num_endpoints;
> > - int i;
> > + unsigned i;
> >
> > num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
> > dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
> >
> > + if (!num_endpoints)
> > + return;
> > +
> > /* NOTE: some customers have programmed FT232R/FT245R devices
> > * with an endpoint size of 0 - not good. In this case, we
> > * want to override the endpoint descriptor setting and use a
>
> Thanks Johan. I tried to get the cdc_acm working; did not have much
> luck/time (typical overcommit on workload) I will retry with the commit
> mentioned.
> I will try the patch today and get back to you. Nice on the ep_desc:
> looking at the code priv->max_packet_size is attached to the port, your
> change would use the last thing off of cur_altsetting->endpoint[], but
> I'm wondering if we should actually be setting priv->max_packet_size to
> whatever the max is of all endpoint[].desc->wMaxPacketSize?
>
> Thoughts?
This is the exact same behaviour as the old code (minus the NULL-deref).
These device have two bulk endpoints per interface and they are supposed
to be using the same max packet size (64 or 512 depending on device and
host).
This value is also used during depacketisation of incoming data (and
packetisation of outgoing data for legacy devices). I'm pretty convinced
you're using the wrong driver, something which would lead to corruption
of incoming data when the (non-existing) status bytes are stripped from
the stream.
You really should try cdc-acm.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 10:52 AM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
On 06/04/2014 10:19 AM, Johan Hovold wrote:
>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
Fix NULL-pointer dereference when probing an interface with no
endpoints.
These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.
Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.
Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")
Reported-by: Mike Remski
Cc: # 2.3.61
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc =
&interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;
unsigned num_endpoints;
- int i;
+ unsigned i;
num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
Thanks Johan. I tried to get the cdc_acm working; did not have much
luck/time (typical overcommit on workload) I will retry with the commit
mentioned.
I will try the patch today and get back to you. Nice on the ep_desc:
looking at the code priv->max_packet_size is attached to the port, your
change would use the last thing off of cur_altsetting->endpoint[], but
I'm wondering if we should actually be setting priv->max_packet_size to
whatever the max is of all endpoint[].desc->wMaxPacketSize?
Thoughts?
This is the exact same behaviour as the old code (minus the NULL-deref).
These device have two bulk endpoints per interface and they are supposed
to be using the same max packet size (64 or 512 depending on device and
host).
This value is also used during depacketisation of incoming data (and
packetisation of outgoing data for legacy devices). I'm pretty convinced
you're using the wrong driver, something which would lead to corruption
of incoming data when the (non-existing) status bytes are stripped from
the stream.
You really should try cdc-acm.
Johan
Thanks for the explaination. Yes, I am working on trying cdc-acm.
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
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: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 10:52 AM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
On 06/04/2014 10:19 AM, Johan Hovold wrote:
>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
Fix NULL-pointer dereference when probing an interface with no
endpoints.
These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.
Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.
Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")
Reported-by: Mike Remski
Cc: # 2.3.61
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc =
&interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;
unsigned num_endpoints;
- int i;
+ unsigned i;
num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
Thanks Johan. I tried to get the cdc_acm working; did not have much
luck/time (typical overcommit on workload) I will retry with the commit
mentioned.
I will try the patch today and get back to you. Nice on the ep_desc:
looking at the code priv->max_packet_size is attached to the port, your
change would use the last thing off of cur_altsetting->endpoint[], but
I'm wondering if we should actually be setting priv->max_packet_size to
whatever the max is of all endpoint[].desc->wMaxPacketSize?
Thoughts?
This is the exact same behaviour as the old code (minus the NULL-deref).
These device have two bulk endpoints per interface and they are supposed
to be using the same max packet size (64 or 512 depending on device and
host).
This value is also used during depacketisation of incoming data (and
packetisation of outgoing data for legacy devices). I'm pretty convinced
you're using the wrong driver, something which would lead to corruption
of incoming data when the (non-existing) status bytes are stripped from
the stream.
You really should try cdc-acm.
Johan
Sorry, forgot to add:
Tested patch and it works as desired.
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
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: ftdi_sio BUG: NULL pointer dereference
On Wed, Jun 04, 2014 at 10:55:28AM -0400, Mike Remski wrote:
> On 06/04/2014 10:52 AM, Johan Hovold wrote:
> > On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
> >> On 06/04/2014 10:19 AM, Johan Hovold wrote:
> >>> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
> >>> From: Johan Hovold
> >>> Date: Wed, 4 Jun 2014 14:09:43 +0200
> >>> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
> >>>
> >>> Fix NULL-pointer dereference when probing an interface with no
> >>> endpoints.
> >>>
> >>> These devices have two bulk endpoints per interface, but this avoids
> >>> crashing the kernel if a user forces a non-FTDI device to be probed.
> >>>
> >>> Note that the iterator variable was made unsigned in order to avoid
> >>> a maybe-uninitialized compiler warning for ep_desc after the loop.
> >>>
> >>> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
> >>> calculation")
> >>>
> >>> Reported-by: Mike Remski
> >>> Cc: # 2.3.61
> >>> Signed-off-by: Johan Hovold
> >>> ---
> >>>drivers/usb/serial/ftdi_sio.c | 7 +--
> >>>1 file changed, 5 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> >>> index 7c6e1dedeb06..3019141397eb 100644
> >>> --- a/drivers/usb/serial/ftdi_sio.c
> >>> +++ b/drivers/usb/serial/ftdi_sio.c
> >>> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
> >>> usb_serial_port *port)
> >>> struct usb_device *udev = serial->dev;
> >>>
> >>> struct usb_interface *interface = serial->interface;
> >>> - struct usb_endpoint_descriptor *ep_desc =
> >>> &interface->cur_altsetting->endpoint[1].desc;
> >>> + struct usb_endpoint_descriptor *ep_desc;
> >>>
> >>> unsigned num_endpoints;
> >>> - int i;
> >>> + unsigned i;
> >>>
> >>> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
> >>> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
> >>>
> >>> + if (!num_endpoints)
> >>> + return;
> >>> +
> >>> /* NOTE: some customers have programmed FT232R/FT245R devices
> >>>* with an endpoint size of 0 - not good. In this case, we
> >>>* want to override the endpoint descriptor setting and use a
> >> Thanks Johan. I tried to get the cdc_acm working; did not have much
> >> luck/time (typical overcommit on workload) I will retry with the commit
> >> mentioned.
> >> I will try the patch today and get back to you. Nice on the ep_desc:
> >> looking at the code priv->max_packet_size is attached to the port, your
> >> change would use the last thing off of cur_altsetting->endpoint[], but
> >> I'm wondering if we should actually be setting priv->max_packet_size to
> >> whatever the max is of all endpoint[].desc->wMaxPacketSize?
> >>
> >> Thoughts?
> > This is the exact same behaviour as the old code (minus the NULL-deref).
> >
> > These device have two bulk endpoints per interface and they are supposed
> > to be using the same max packet size (64 or 512 depending on device and
> > host).
> >
> > This value is also used during depacketisation of incoming data (and
> > packetisation of outgoing data for legacy devices). I'm pretty convinced
> > you're using the wrong driver, something which would lead to corruption
> > of incoming data when the (non-existing) status bytes are stripped from
> > the stream.
> >
> > You really should try cdc-acm.
> >
> > Johan
> Sorry, forgot to add:
> Tested patch and it works as desired.
Thanks, I'll add a Tested-by tag then.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 11:09 AM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 10:55:28AM -0400, Mike Remski wrote:
On 06/04/2014 10:52 AM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
On 06/04/2014 10:19 AM, Johan Hovold wrote:
>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
Fix NULL-pointer dereference when probing an interface with no
endpoints.
These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.
Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.
Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")
Reported-by: Mike Remski
Cc: # 2.3.61
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc =
&interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;
unsigned num_endpoints;
- int i;
+ unsigned i;
num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
Thanks Johan. I tried to get the cdc_acm working; did not have much
luck/time (typical overcommit on workload) I will retry with the commit
mentioned.
I will try the patch today and get back to you. Nice on the ep_desc:
looking at the code priv->max_packet_size is attached to the port, your
change would use the last thing off of cur_altsetting->endpoint[], but
I'm wondering if we should actually be setting priv->max_packet_size to
whatever the max is of all endpoint[].desc->wMaxPacketSize?
Thoughts?
This is the exact same behaviour as the old code (minus the NULL-deref).
These device have two bulk endpoints per interface and they are supposed
to be using the same max packet size (64 or 512 depending on device and
host).
This value is also used during depacketisation of incoming data (and
packetisation of outgoing data for legacy devices). I'm pretty convinced
you're using the wrong driver, something which would lead to corruption
of incoming data when the (non-existing) status bytes are stripped from
the stream.
You really should try cdc-acm.
Johan
Sorry, forgot to add:
Tested patch and it works as desired.
Thanks, I'll add a Tested-by tag then.
Johan
Thanks for quick response. I'll let you know the results of cdc-acm
(had to pull in the commit you mentioned).
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage
On Wed, 4 Jun 2014, Andrzej Pietrasiewicz wrote: > When Sebastian introduced the function registration interface I didn't > specially like the naming: struct usb_function_instance is something > different than an instance of struct usb_function. What is the difference in purpose between usb_function and usb_function_instance? I can't tell just by reading the header file. Does one of them get created dynamically when the host sets the appropriate config? It's quite noticeable that composite.h does not contain nearly enough documentation. Only four of the structures defined there have any kerneldoc, and none of the functions do. Also, there seems to be some confusion between structures that represent drivers and those that represent devices (or parts of a device). For example, struct usb_function contains instance data as well as driver callbacks. > The purpose of fsg_alloc_inst() is to create a usb_function_instance > whose container_of is struct fsg_opts. In fact it is struct fsg_opts > which is actually allocated; one of its members is struct fsg_common > which is also allocated - individually for each struct usb_function_instance. > > Among traditional gadgets there is no gadget which uses mass storage function > more than once. On the other hand, when gadgets are created with configfs > it is forbidden to link a given function more than once into a given > config, What is the reason for this restriction? > that is a struct usb_function_instance can be referenced by more > than one config, but can be referenced only once in a given config; > each symbolic link corresponds to an instance of struct usb_function > (don't confuse with struct usb_function_instance). It's extremely easy to confuse them, since I don't understand the differences between them. It even seems like you confused them in this description: You mentioned "link a given function", "link corresponds to an instance of struct usb_function", and "struct usb_function_instance can be referenced by more than one config". What's the difference between linking a usb_function and referencing a usb_function_instance? Normally "linking" and "referencing" mean more or less the same thing. > So yes, an fsg_common can be shared among instances of struct usb_function, > but neither with traditional gadgets as they are now nor with configfs > is it possible to have the same fsg_common referenced more than once > in a given config. That's a relief. But it still seems like a bad design. If there can be only one struct fsg_dev associated with struct fsg_common, why have separate structures? And if there can be multiple fsg_dev structures associated with struct fsg_common, why does struct fsg_common contain a pointer to an fsg_dev (in fact, two of them)? The issue that started these thoughts was the way fsg_common.new_fsg gets used, as modified by the patch in the thread's original email. It's not clear why new_fsg is needed at all. 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: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 11:09 AM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 10:55:28AM -0400, Mike Remski wrote:
On 06/04/2014 10:52 AM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
On 06/04/2014 10:19 AM, Johan Hovold wrote:
>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
Fix NULL-pointer dereference when probing an interface with no
endpoints.
These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.
Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.
Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")
Reported-by: Mike Remski
Cc: # 2.3.61
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc =
&interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;
unsigned num_endpoints;
- int i;
+ unsigned i;
num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
Thanks Johan. I tried to get the cdc_acm working; did not have much
luck/time (typical overcommit on workload) I will retry with the commit
mentioned.
I will try the patch today and get back to you. Nice on the ep_desc:
looking at the code priv->max_packet_size is attached to the port, your
change would use the last thing off of cur_altsetting->endpoint[], but
I'm wondering if we should actually be setting priv->max_packet_size to
whatever the max is of all endpoint[].desc->wMaxPacketSize?
Thoughts?
This is the exact same behaviour as the old code (minus the NULL-deref).
These device have two bulk endpoints per interface and they are supposed
to be using the same max packet size (64 or 512 depending on device and
host).
This value is also used during depacketisation of incoming data (and
packetisation of outgoing data for legacy devices). I'm pretty convinced
you're using the wrong driver, something which would lead to corruption
of incoming data when the (non-existing) status bytes are stripped from
the stream.
You really should try cdc-acm.
Johan
Sorry, forgot to add:
Tested patch and it works as desired.
Thanks, I'll add a Tested-by tag then.
Johan
Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
hitting something similar; I noticed there where a few commits for NULL
pointer dereferences in cdc-acm.c.
I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
you know what I find (or perhaps this looks similar to something already
seen?). This device is the same as the original ftdi crash, 1.2 is the
same interface with bNumEndpoints == 0.
Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
calls on its own. It is not a modem.
Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
pointer dereference at 0004
Jun 4 11:35:20 nicA91A84 kernel: IP: []
acm_probe+0x44e/0x127c [cdc_acm]
Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 354eb001 *pde =
7f003067
Jun 4 11:35:20 nicA91A84 kernel: Oops: [#1] SMP
Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
/sys/devices/pci:00/:00:1d.0/usb5/5-2/product
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
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: ftdi_sio BUG: NULL pointer dereference
On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
> Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
> in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
> hitting something similar; I noticed there where a few commits for NULL
> pointer dereferences in cdc-acm.c.
>
> I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
> you know what I find (or perhaps this looks similar to something already
> seen?). This device is the same as the original ftdi crash, 1.2 is the
> same interface with bNumEndpoints == 0.
>
> Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
> calls on its own. It is not a modem.
> Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
> pointer dereference at 0004
> Jun 4 11:35:20 nicA91A84 kernel: IP: []
> acm_probe+0x44e/0x127c [cdc_acm]
> Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 354eb001 *pde =
> 7f003067
> Jun 4 11:35:20 nicA91A84 kernel: Oops: [#1] SMP
> Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
> /sys/devices/pci:00/:00:1d.0/usb5/5-2/product
Yeah, you really need to update your kernel. You'll definitely need at
least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
dereference").
No more reports until you've tried a recent kernel, ok? ;)
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 12:00 PM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
hitting something similar; I noticed there where a few commits for NULL
pointer dereferences in cdc-acm.c.
I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
you know what I find (or perhaps this looks similar to something already
seen?). This device is the same as the original ftdi crash, 1.2 is the
same interface with bNumEndpoints == 0.
Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
calls on its own. It is not a modem.
Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
pointer dereference at 0004
Jun 4 11:35:20 nicA91A84 kernel: IP: []
acm_probe+0x44e/0x127c [cdc_acm]
Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 354eb001 *pde =
7f003067
Jun 4 11:35:20 nicA91A84 kernel: Oops: [#1] SMP
Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
/sys/devices/pci:00/:00:1d.0/usb5/5-2/product
Yeah, you really need to update your kernel. You'll definitely need at
least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
dereference").
No more reports until you've tried a recent kernel, ok? ;)
Johan
Agreed. But I still have to understand it so I can backport the
appropriate fixes, no?
I'll be quiet now.
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
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: ftdi_sio BUG: NULL pointer dereference
On 06/04/2014 12:00 PM, Johan Hovold wrote:
On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
hitting something similar; I noticed there where a few commits for NULL
pointer dereferences in cdc-acm.c.
I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
you know what I find (or perhaps this looks similar to something already
seen?). This device is the same as the original ftdi crash, 1.2 is the
same interface with bNumEndpoints == 0.
Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
calls on its own. It is not a modem.
Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
pointer dereference at 0004
Jun 4 11:35:20 nicA91A84 kernel: IP: []
acm_probe+0x44e/0x127c [cdc_acm]
Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 354eb001 *pde =
7f003067
Jun 4 11:35:20 nicA91A84 kernel: Oops: [#1] SMP
Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
/sys/devices/pci:00/:00:1d.0/usb5/5-2/product
Yeah, you really need to update your kernel. You'll definitely need at
least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
dereference").
No more reports until you've tried a recent kernel, ok? ;)
Johan
Device in question does not cause kernel crash when using the cdc-acm
driver on a Fedora 19 system running Fedora kernel 3.9.5-301.
Thanks for all the help, I'll take care of my ancient kernels.
--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage
> From: linux-usb-ow...@vger.kernel.org > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern > Sent: Wednesday, June 04, 2014 6:57 AM > > On Wed, 4 Jun 2014, Yang,Wei wrote: > > > On 06/04/2014 09:45 AM, Peter Chen wrote: > > > > > >> commit d18f7116a5ddb8263fe62b05ad63e5ceb5875791 > > >> Author: Robert Baldyga > > >> Date: Thu Nov 21 13:49:18 2013 +0100 > > >> > > >> usb: gadget: s3c-hsotg: fix disconnect handling > > >> > > >> This patch moves s3c_hsotg_disconnect function call from USBSusp > > >> interrupt > > >> handler to SET_ADDRESS request handler. > > >> > > > It is a little strange we call gadget's disconnect at SET_ADDRESS? > > > How the udc calls gadget driver the disconnection has happened when > > > the usb cable is disconnected from the host? > > > > > > Usually, we call gadget's disconnect at two situations > > > > > > - udc's reset handler if udc's speed is not UNKNOWN, it is usually > > > happened > > > when the host sends reset after enumeration. > > > - udc's disconnect handler, it is usually happened when the usb cable > > > is disconnected from host. > > > > Hmm, usually the two situations, but according to the commit log, s3c > > hsotg does not support Disconnected interrupt for device mode, > > so the second situation does not happen for s3c hsotg, therefore, he has > > to disconnect the connection before it is connected again. > > Why does he need to do that? Why not just skip the disconnect > notification if the hardware can't detect a disconnect? > > It makes no sense at all to call a disconnect handler from within the > SET_ADDRESS routine. FWIW, here is the section from the DWC2 databook that describes how a disconnect should be handled for the device: When OTG_MODE is set to 0, 1, or 3, the device disconnect flow is as follows: 1. When the USB cable is unplugged or when the VBUS is switched off by the Host, the Device core triggers GINTSTS.OTGInt [bit 2] interrupt bit. 2. When the device application detects GINTSTS.OTGInt interrupt, it checks that the GOTGINT.SesEndDet (Session End Detected) bit is set to 1'b1. When OTG_MODE is set to 2 or 4, the device disconnect flow is as follows: 1. When the USB cable is unplugged or when the VBUS is switched off by the Host, the Device core triggers GINTSTS.USBRst [bit 12] interrupt bit. 2. When the device application detects GINTSTS.USBRst, the application sets a timeout check for SET ADDRESS Control Xfer from Host. 3. If application does not receive SET ADDRESS Control Xfer from Host before the timeout period, it is treated as a device disconnection. OTG_MODE is a configuration parameter that is set when the core is built. From this discussion, it sounds like the s3c-hsotg core is built for either mode 2 or 4. So SET ADDRESS should be involved, but not in the way the driver is currently doing it. Unfortunately I don't have the s3c-hsotg hardware, so I can't work on this myself. -- 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: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
>> -struct usb_functionfs_descs_head {
>> -__le32 magic;
>> -__le32 length;
>> -__le32 fs_count;
>> -__le32 hs_count;
>> -} __attribute__((packed));
On Wed, Jun 04 2014, Krzysztof Opasiak wrote:
> I have tried to compile FFS examples with v3.15-rc8 but I have faced an
> issue that they are unable to build due to missing definition of this
> structure.
https://lkml.org/lkml/2014/5/21/522
> The same is with adb, sdb and all userspace apps which use legacy
> API. What is the reason of removing it? Was your intentions?
It was a mistake on my part.
> Maybe would be a good idea to leave this structure untouched as legacy
> userspace API?
I don't care either way to be honest. I guess if there's non-negligible
number of usb_functionfs_descs_head structure users out there, I can
prepare a CL adding the structure back.
> There is also no structure definition for new API, maybe suitable
> structure should be defined (struct usb_functionfs_descs_head2 for
> example) to make userspace life easier?
That structure would not have many fields though, since what exactly the
header contains depends on the flags. Whether fs_count, hs_count and
ss_count fields are present depends on which bits in the flags are set.
So the usb_functionfs_descs_head2 structure could only contain:
struct usb_functionfs_descs_head2 {
__le32 magic;
__le32 length;
__le32 flags;
};
I'm not sure if that would be particularly helpful.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
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
ohci-hcd patch for testing
Dennis and Matteo:
I promised to send both of you a patch changing the way ohci-hcd
handles hardware bugs. Well, it's finally ready for testing. There's
only a limited amount I can do on my own machine, so now it's up to you
guys.
The patch was made against an early -rc version of 3.15, but it will
apply okay to 3.14 and maybe even earlier kernels.
Alan Stern
Index: usb-3.15/drivers/usb/host/ohci.h
===
--- usb-3.15.orig/drivers/usb/host/ohci.h
+++ usb-3.15/drivers/usb/host/ohci.h
@@ -47,6 +47,7 @@ struct ed {
struct ed *ed_next; /* on schedule or rm_list */
struct ed *ed_prev; /* for non-interrupt EDs */
struct list_headtd_list;/* "shadow list" of our TDs */
+ struct list_headin_use_list;
/* create --> IDLE --> OPER --> ... --> IDLE --> destroy
* usually: OPER --> UNLINK --> (IDLE | OPER) --> ...
@@ -66,6 +67,13 @@ struct ed {
/* HC may see EDs on rm_list until next frame (frame_no == tick) */
u16 tick;
+
+ /* Detect TDs not added to the done list */
+ unsignedtakeback_wdh_cnt;
+ struct td *pending_td;
+#defineOKAY_TO_TAKEBACK(ohci, ed) \
+ ((int) (ohci->wdh_cnt - ed->takeback_wdh_cnt) >= 0)
+
} __attribute__ ((aligned(16)));
#define ED_MASK((u32)~0x0f)/* strip hw status in low addr
bits */
@@ -380,7 +388,9 @@ struct ohci_hcd {
struct dma_pool *td_cache;
struct dma_pool *ed_cache;
struct td *td_hash [TD_HASH_SIZE];
+ struct td *dl_start, *dl_end; /* the done list */
struct list_headpending;
+ struct list_headeds_in_use; /* all EDs with at least 1 TD */
/*
* driver state
@@ -392,6 +402,8 @@ struct ohci_hcd {
unsigned long next_statechange; /* suspend/resume */
u32 fminterval; /* saved register */
unsignedautostop:1; /* rh auto stopping/stopped */
+ unsignedworking:1;
+ unsignedrestart_work:1;
unsigned long flags; /* for HC bugs */
#defineOHCI_QUIRK_AMD756 0x01/* erratum #4 */
@@ -407,13 +419,12 @@ struct ohci_hcd {
#defineOHCI_QUIRK_AMD_PREFETCH 0x400 /* pre-fetch
for ISO transfer */
// there are also chip quirks/bugs in init logic
- struct work_struct nec_work; /* Worker for NEC quirk */
+ unsignedprev_frame_no;
+ unsignedwdh_cnt, prev_wdh_cnt;
+ u32 prev_donehead;
+ struct timer_list io_watchdog;
- /* Needed for ZF Micro quirk */
- struct timer_list unlink_watchdog;
- unsignededs_scheduled;
- struct ed *ed_to_check;
- unsignedzf_delay;
+ struct work_struct nec_work; /* Worker for NEC quirk */
struct dentry *debug_dir;
struct dentry *debug_async;
Index: usb-3.15/drivers/usb/host/ohci-hcd.c
===
--- usb-3.15.orig/drivers/usb/host/ohci-hcd.c
+++ usb-3.15/drivers/usb/host/ohci-hcd.c
@@ -72,12 +72,14 @@
static const char hcd_name [] = "ohci_hcd";
#defineSTATECHANGE_DELAY msecs_to_jiffies(300)
+#defineIO_WATCHDOG_DELAY msecs_to_jiffies(250)
#include "ohci.h"
#include "pci-quirks.h"
-static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-static void ohci_stop (struct usb_hcd *hcd);
+static void ohci_dump(struct ohci_hcd *ohci);
+static void ohci_stop(struct usb_hcd *hcd);
+static void io_watchdog_func(unsigned long _ohci);
#include "ohci-hub.c"
#include "ohci-dbg.c"
@@ -202,6 +204,16 @@ static int ohci_urb_enqueue (
usb_hcd_unlink_urb_from_ep(hcd, urb);
goto fail;
}
+
+ /* Start up the IO watchdog timer, if it's not running */
+ if (!timer_pending(&ohci->io_watchdog) &&
+ list_empty(&ohci->eds_in_use)) {
+ ohci->prev_frame_no = ohci_frame_no(ohci);
+ mod_timer(&ohci->io_watchdog,
+ jiffies + IO_WATCHDOG_DELAY);
+ }
+ list_add(&ed->in_use_list, &ohci->eds_in_use);
+
if (ed->type == PIPE_ISOCHRONOUS) {
u16 frame = ohci_frame_no(ohci);
@@ -277,30 +289,24 @@ static int ohci_urb_dequeue(struct usb_h
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
unsigned long flags;
[PATCH] staging: usbip: stub_main.c: Cleaning up missing null-terminate after strncpy call
Added a guaranteed null-terminate after call to strncpy.
This was partly found using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist
---
drivers/staging/usbip/stub_main.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/usbip/stub_main.c
b/drivers/staging/usbip/stub_main.c
index 9c5832a..f1eb6a2 100644
--- a/drivers/staging/usbip/stub_main.c
+++ b/drivers/staging/usbip/stub_main.c
@@ -166,6 +166,7 @@ static ssize_t store_match_busid(struct device_driver *dev,
const char *buf,
return -EINVAL;
strncpy(busid, buf + 4, BUSID_SIZE);
+ busid[BUSID_SIZE - 1] = '\0';
if (!strncmp(buf, "add ", 4)) {
if (add_match_busid(busid) < 0)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage
> On Wed, 4 Jun 2014, Yang,Wei wrote: > > > On 06/04/2014 09:45 AM, Peter Chen wrote: > > > > > >> commit d18f7116a5ddb8263fe62b05ad63e5ceb5875791 > > >> Author: Robert Baldyga > > >> Date: Thu Nov 21 13:49:18 2013 +0100 > > >> > > >> usb: gadget: s3c-hsotg: fix disconnect handling > > >> > > >> This patch moves s3c_hsotg_disconnect function call from > > >> USBSusp interrupt > > >> handler to SET_ADDRESS request handler. > > >> > > > It is a little strange we call gadget's disconnect at SET_ADDRESS? > > > How the udc calls gadget driver the disconnection has happened when > > > the usb cable is disconnected from the host? > > > > > > Usually, we call gadget's disconnect at two situations > > > > > > - udc's reset handler if udc's speed is not UNKNOWN, it is usually > > > happened when the host sends reset after enumeration. > > > - udc's disconnect handler, it is usually happened when the usb > > > cable is disconnected from host. > > > > Hmm, usually the two situations, but according to the commit log, s3c > > hsotg does not support Disconnected interrupt for device mode, so the > > second situation does not happen for s3c hsotg, therefore, he has to > > disconnect the connection before it is connected again. > > Why does he need to do that? Why not just skip the disconnect > notification if the hardware can't detect a disconnect? > We must call gadget driver's disconnect, otherwise, there at least has a warning when unload module, please refer to __composite_unbind at drivers/usb/gadget/composite.c Peter > It makes no sense at all to call a disconnect handler from within the > SET_ADDRESS routine. > > 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
