[PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis
use the values for RNDIS over Ethernet as defined in http://www.usb.org/developers/defined_class (search for RDNIS): - baseclass: 0xef (miscellaneous) - subclass: 0x04 - protocol: 0x01 with this setings the file in Documentation/usb/linux.inf is obsolete. Signed-off-by: Heiko Schocher h...@denx.de --- Cc: Felipe Balbi ba...@ti.com Cc: Greg Kroah-Hartman gre...@suse.de Cc: linux-usb@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: Oliver Neukum oli...@neukum.name Cc: net...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: Andrzej Pietrasiewicz andrze...@samsung.com Cc: Michal Nazarewicz min...@mina86.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Dan Carpenter dan.carpen...@oracle.com Cc: Macpaul Lin macp...@gmail.com Tested with the USB Compliance test suite which runs Windows, see: http://www.usb.org/developers/tools/usb20_tools/#usb20cv drivers/net/usb/cdc_ether.c | 6 +++--- drivers/usb/core/generic.c| 6 +++--- drivers/usb/gadget/function/f_rndis.c | 6 +++--- include/uapi/linux/usb/cdc.h | 3 +++ 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c index 2a32d91..9c216c2 100644 --- a/drivers/net/usb/cdc_ether.c +++ b/drivers/net/usb/cdc_ether.c @@ -35,9 +35,9 @@ static int is_rndis(struct usb_interface_descriptor *desc) { - return (desc-bInterfaceClass == USB_CLASS_COMM - desc-bInterfaceSubClass == 2 - desc-bInterfaceProtocol == 0xff); + return (desc-bInterfaceClass == USB_CLASS_MISC + desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS + desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH); } static int is_activesync(struct usb_interface_descriptor *desc) diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index 358ca8d..a2a4e05 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -28,9 +28,9 @@ static inline const char *plural(int n) static int is_rndis(struct usb_interface_descriptor *desc) { - return desc-bInterfaceClass == USB_CLASS_COMM -desc-bInterfaceSubClass == 2 -desc-bInterfaceProtocol == 0xff; + return desc-bInterfaceClass == USB_CLASS_MISC +desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS +desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH; } static int is_activesync(struct usb_interface_descriptor *desc) diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c index ddb09dc..cc06046 100644 --- a/drivers/usb/gadget/function/f_rndis.c +++ b/drivers/usb/gadget/function/f_rndis.c @@ -117,9 +117,9 @@ static struct usb_interface_descriptor rndis_control_intf = { /* .bInterfaceNumber = DYNAMIC */ /* status endpoint is optional; this could be patched later */ .bNumEndpoints =1, - .bInterfaceClass = USB_CLASS_COMM, - .bInterfaceSubClass = USB_CDC_SUBCLASS_ACM, - .bInterfaceProtocol = USB_CDC_ACM_PROTO_VENDOR, + .bInterfaceClass = USB_CLASS_MISC, + .bInterfaceSubClass = USB_CDC_SUBCLASS_RNDIS, + .bInterfaceProtocol = USB_CDC_RNDIS_PROTO_ETH, /* .iInterface = DYNAMIC */ }; diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h index b6a9cdd..8e8fc85 100644 --- a/include/uapi/linux/usb/cdc.h +++ b/include/uapi/linux/usb/cdc.h @@ -12,6 +12,7 @@ #include linux/types.h #define USB_CDC_SUBCLASS_ACM 0x02 +#define USB_CDC_SUBCLASS_RNDIS 0x04 #define USB_CDC_SUBCLASS_ETHERNET 0x06 #define USB_CDC_SUBCLASS_WHCM 0x08 #define USB_CDC_SUBCLASS_DMM 0x09 @@ -31,6 +32,8 @@ #define USB_CDC_ACM_PROTO_AT_CDMA 6 #define USB_CDC_ACM_PROTO_VENDOR 0xff +#define USB_CDC_RNDIS_PROTO_ETH1 + #define USB_CDC_PROTO_EEM 7 #define USB_CDC_NCM_PROTO_NTB 1 -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Wednesday 24 September 2014 10:27:52 Peter Chen wrote: Antoine is adding a generic chipdea glue layer driver, which like ehci generic platform driver: drivers/usb/host/ehci-platform.c, since other architectures like MIPS (Someone submitted mips chipidea driver before) may not have device tree support, I think non-dt support is also needed. Right. It is a good suggestion for adding DT support for core driver, Since we did not do it at the first, it is a little embarrass at current situation. - For the new chipidea glue drivers, it is ok we can have a child node for core device at glue device node, and some common entries can be there like: phy, vbus, dr_mode, etc. We need to add support for getting these common things for both through device tree and platform data (parent is DT support and parent is non-DT support) at core driver. I don't really want to see the child devices show up in DT, as this is really an implementation detail of the way that the driver currently works, and IMHO we should change that. I know that Felipe disagrees with me on this, but almost every other driver that has soc-specific specializations does not use parent/child nodes, neither in DT nor in Linux. Instead you have a single device node that gets distinguished by compatible string. - For the existing glue drivers, since we can't change existed dts, we can only do it from future SoC support. Then, in this kinds of glue drivers, we need to support for both create core driver by node and by current calling platform_device_add way. We should be able to easily change the ci_hdrc_add_device call into a different exported function that calls a version of the probe() code directly, as we do in all sorts of other drivers (ehci, ohci, dwc2, ..., but not dwc3 and musb as they are maintained by Felipe). We can also gradually move in some of the other glue drivers into the main driver if the differences are small enough. Moving the PCI driver over to this model requires a little more work but is absolutely doable (again, see ehci, ahci, sdhci examples) by moving the platform_device handling out of core.c and changing it just operate on the plain 'struct device', with an exported probe function that gets called on either the platform device or the pci device. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[resend PATCH] usb: dwc3: pci: Add PCI ID for Intel Braswell
From: Alan Cox a...@linux.intel.com The device controller is the same but it has different PCI ID. Add this new ID to the driver's list of supported IDs. Signed-off-by: Alan Cox a...@linux.intel.com Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/dwc3/dwc3-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c index 436fb08..a36cf66 100644 --- a/drivers/usb/dwc3/dwc3-pci.c +++ b/drivers/usb/dwc3/dwc3-pci.c @@ -30,6 +30,7 @@ #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd #define PCI_DEVICE_ID_INTEL_BYT0x0f37 #define PCI_DEVICE_ID_INTEL_MRFLD 0x119e +#define PCI_DEVICE_ID_INTEL_BSW0x22B7 struct dwc3_pci { struct device *dev; @@ -181,6 +182,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = { PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3), }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW), }, { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), }, { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), }, { }/* Terminating Entry */ -- 2.1.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 0/2] USB: core: add add device-qualifier quirk
On Tue, Sep 23, 2014 at 05:27:22PM +0200, Johan Hovold wrote: On Tue, Sep 23, 2014 at 08:21:51AM -0700, Greg Kroah-Hartman wrote: On Tue, Sep 23, 2014 at 04:56:24PM +0200, Johan Hovold wrote: On Mon, Aug 25, 2014 at 05:51:25PM +0200, Johan Hovold wrote: This is quirk is indeed needed to get the Elan Touchscreen found on some Samsung laptops to enumerate reliably. [...] Johan Hovold (2): USB: core: add device-qualifier quirk USB: quirks: enable device-qualifier quirk for Elan Touchscreen Are these patches still in your queue, Greg? Just checking now that the merge window is around the corner. My queue is huge and I hope to tackle it tonight... Yeah, that's the impression I got. Just a heads up: There was another quirk submitted after this one that uses the same quirk mask. Ah, thanks for the warning, I'll watch out for it. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[resend PATCH 2/3] usb: dwc3: core: only setting the dma_mask when needed
If the probe drivers have already set the dma_mask, not replacing the value. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/dwc3/core.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index b0f4d52..d08cac5 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -708,9 +708,11 @@ static int dwc3_probe(struct platform_device *pdev) spin_lock_init(dwc-lock); platform_set_drvdata(pdev, dwc); - dev-dma_mask = dev-parent-dma_mask; - dev-dma_parms = dev-parent-dma_parms; - dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask); + if (!dev-dma_mask) { + dev-dma_mask = dev-parent-dma_mask; + dev-dma_parms = dev-parent-dma_parms; + dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask); + } pm_runtime_enable(dev); pm_runtime_get_sync(dev); -- 2.1.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
[resend PATCH 1/3] ACPI / platform: provide default DMA mask
Most devices are configured for 32-bit DMA addresses. Setting the mask to 32-bit here removes the need for the drivers to do it separately. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: Rafael J. Wysocki r...@rjwysocki.net --- drivers/acpi/acpi_platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 2bf9082..8d099e6 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -16,6 +16,7 @@ #include linux/err.h #include linux/kernel.h #include linux/module.h +#include linux/dma-mapping.h #include linux/platform_device.h #include internal.h @@ -102,6 +103,7 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev) pdevinfo.res = resources; pdevinfo.num_res = count; pdevinfo.acpi_node.companion = adev; + pdevinfo.dma_mask = DMA_BIT_MASK(32); pdev = platform_device_register_full(pdevinfo); if (IS_ERR(pdev)) dev_err(adev-dev, platform device creation failed: %ld\n, -- 2.1.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
[resend PATCH 0/3] usb: dwc3: ACPI support
Hi, The original series included patch that adds Braswell PCI ID, but I send it separately. The DMA mask caused a problem as our acpi platform code does not provide anything for us. Instead of trying to fix it in dwc3 I decided to suggest the first patch in this series where I provide default DMA mask for all the ACPI platform devices. Heikki Krogerus (3): ACPI / platform: provide default DMA mask usb: dwc3: core: only setting the dma_mask when needed usb: dwc3: add ACPI support drivers/acpi/acpi_platform.c | 2 ++ drivers/usb/dwc3/core.c | 18 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) -- 2.1.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
[resend PATCH 3/3] usb: dwc3: add ACPI support
Adds ACPI ID used on newer Intel SoCs. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/dwc3/core.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index d08cac5..c2cf2d8 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -32,6 +32,7 @@ #include linux/delay.h #include linux/dma-mapping.h #include linux/of.h +#include linux/acpi.h #include linux/usb/ch9.h #include linux/usb/gadget.h @@ -960,12 +961,21 @@ static const struct of_device_id of_dwc3_match[] = { MODULE_DEVICE_TABLE(of, of_dwc3_match); #endif +#ifdef CONFIG_ACPI +static const struct acpi_device_id dwc3_acpi_match[] = { + { 808622B7, 0 }, + { }, +}; +MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match); +#endif + static struct platform_driver dwc3_driver = { .probe = dwc3_probe, .remove = dwc3_remove, .driver = { .name = dwc3, .of_match_table = of_match_ptr(of_dwc3_match), + .acpi_match_table = ACPI_PTR(dwc3_acpi_match), .pm = DWC3_PM_OPS, }, }; -- 2.1.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] drivers: usb: fsl: Check IP version 2.4 for mph USB controller
On Thu, Aug 21, 2014 at 12:43:30PM +0530, Nikhil Badola wrote: Check 2.4 IP version for multi port host USB controller and return FSL_USB_VER_2_4 macro Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com --- drivers/usb/host/fsl-mph-dr-of.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c index 9162d1b..e0315de 100644 --- a/drivers/usb/host/fsl-mph-dr-of.c +++ b/drivers/usb/host/fsl-mph-dr-of.c @@ -126,6 +126,7 @@ static int usb_get_ver_info(struct device_node *np) /* * returns 1 for usb controller version 1.6 * returns 2 for usb controller version 2.2 + * returns 3 for usb controller version 2.4 * returns 0 otherwise */ if (of_device_is_compatible(np, fsl-usb2-dr)) { @@ -150,6 +151,8 @@ static int usb_get_ver_info(struct device_node *np) ver = FSL_USB_VER_1_6; else if (of_device_is_compatible(np, fsl-usb2-mph-v2.2)) ver = FSL_USB_VER_2_2; + else if (of_device_is_compatible(np, fsl-usb2-mph-v2.4)) + ver = FSL_USB_VER_2_4; Why do we care? -- 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] drivers: usb :fsl: Add support for USB controller version-2.5
On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote: Add support for USB controller version-2.5 used in T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A. Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com --- - Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124 drivers: usb: fsl: Check IP version 2.4 for mph USB controller That ocmmit isn't in Linus's tree, how can you use the git commit id for it? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers: usb :fsl: Add support for USB controller version-2.5
On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote: Add support for USB controller version-2.5 used in T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A. Signed-off-by: Nikhil Badola nikhil.bad...@freescale.com --- - Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124 drivers: usb: fsl: Check IP version 2.4 for mph USB controller drivers/usb/host/fsl-mph-dr-of.c | 5 + include/linux/fsl_devices.h | 1 + 2 files changed, 6 insertions(+) diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c index e0315de..45b9e36 100644 --- a/drivers/usb/host/fsl-mph-dr-of.c +++ b/drivers/usb/host/fsl-mph-dr-of.c @@ -127,6 +127,7 @@ static int usb_get_ver_info(struct device_node *np) * returns 1 for usb controller version 1.6 * returns 2 for usb controller version 2.2 * returns 3 for usb controller version 2.4 + * returns 4 for usb controller version 2.5 * returns 0 otherwise */ if (of_device_is_compatible(np, fsl-usb2-dr)) { @@ -136,6 +137,8 @@ static int usb_get_ver_info(struct device_node *np) ver = FSL_USB_VER_2_2; else if (of_device_is_compatible(np, fsl-usb2-dr-v2.4)) ver = FSL_USB_VER_2_4; + else if (of_device_is_compatible(np, fsl-usb2-dr-v2.5)) + ver = FSL_USB_VER_2_5; else /* for previous controller versions */ ver = FSL_USB_VER_OLD; @@ -153,6 +156,8 @@ static int usb_get_ver_info(struct device_node *np) ver = FSL_USB_VER_2_2; else if (of_device_is_compatible(np, fsl-usb2-mph-v2.4)) ver = FSL_USB_VER_2_4; + else if (of_device_is_compatible(np, fsl-usb2-mph-v2.5)) + ver = FSL_USB_VER_2_5; else /* for previous controller versions */ ver = FSL_USB_VER_OLD; } diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h index a82296a..2a2f56b 100644 --- a/include/linux/fsl_devices.h +++ b/include/linux/fsl_devices.h @@ -24,6 +24,7 @@ #define FSL_USB_VER_1_6 1 #define FSL_USB_VER_2_2 2 #define FSL_USB_VER_2_4 3 +#define FSL_USB_VER_2_5 4 Why not just keep going and add the rest of the numbers while you are at it? Seriously, what are these two patches doing? You just set the value, never do anything with it, what good is it? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH fix for 3.17 v5] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands
On Mon, Sep 15, 2014 at 04:13:52PM +0200, Hans de Goede wrote: Hi, On 09/15/2014 04:08 PM, Greg Kroah-Hartman wrote: On Mon, Sep 15, 2014 at 04:04:12PM +0200, Hans de Goede wrote: And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one seems to hang upon receiving an ATA_12 or ATA_16 command. https://bugzilla.kernel.org/show_bug.cgi?id=79511 https://bbs.archlinux.org/viewtopic.php?id=183190 While at it also add missing documentation for the u value for usb-storage quirks. Cc: sta...@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede hdego...@redhat.com -- Changes in v2: Add documentation for new t and u usb-storage.quirks flags Changes in v3: Fix typo in documentation Changes in v4: Also apply the quirk to (0bc2:3312) Changes in v5: Rebased on 3.17-rc5, drop u documentation, already upstream So I should wait another day for v6? :) Hehe, no I certainly hope not. Chances are I there will eventually be more seagate models needing the quirk, but I've none in the pipeline atm, and we can always add those with an incremental patch. So if this is not too late for 3.17 and you can pick up v5 that would be great. It's too late for 3.17, but I'll mark it for stable for 3.17 and 3.16. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Added support for Seluxit USB dongle to cp210x driver.
On Mon, Sep 22, 2014 at 10:52:56AM +0200, Johan Hovold wrote: On Mon, Sep 22, 2014 at 09:50:43AM +0200, Andreas Bomholtz wrote: Added the Seluxit ApS USB Serial Dongle to cp210x driver. Signed-off-by: Andreas Bomholtz andr...@seluxit.com --- Thanks for resending. Applied just fine now. Was this in the last pull request you made to me? I don't see it there... thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] usb-core: Remove the local_irq_save/local_irq_restore around complete
On Mon, Sep 15, 2014 at 10:22:49AM -0500, dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com When enabling HCD_BH for the DWC2 HCD, these local_irq_save/local_irq_restore was causing a timeout with a webcam. Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com --- drivers/usb/core/hcd.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 487abcf..463dc44 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1678,9 +1678,7 @@ static void __usb_hcd_giveback_urb(struct urb *urb) * and no one may trigger the above deadlock situation when * running complete() in tasklet. */ - local_irq_save(flags); urb-complete(urb); - local_irq_restore(flags); usb_anchor_resume_wakeups(anchor); atomic_dec(urb-use_count); That really looks like an incorrect patch, are you _sure_ you can do this? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] usb: rename phy to usb_phy in HCD
On Fri, Sep 05, 2014 at 01:42:09AM +0400, Sergei Shtylyov wrote: From: Antoine Tenart antoine.ten...@free-electrons.com The USB PHY member of the HCD structure is renamed to 'usb_phy' and modifications are done in all drivers accessing it. This is in preparation to adding the generic PHY support. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects caused by patch reordering, updated changelog.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Felipe Balbi ba...@ti.com --- Changes in version 5: - imported the patch from Antoine Tenart's series; - added missing 'drivers/usb/misc/lvstest.c' file; - resolved rejects caused by patch reordering; - refreshed patch; - updated changelog. drivers/usb/chipidea/host.c |2 +- drivers/usb/core/hcd.c| 20 ++-- drivers/usb/core/hub.c|8 drivers/usb/host/ehci-fsl.c | 16 drivers/usb/host/ehci-hub.c |2 +- drivers/usb/host/ehci-msm.c |4 ++-- drivers/usb/host/ehci-tegra.c | 16 drivers/usb/host/ohci-omap.c | 20 ++-- drivers/usb/misc/lvstest.c|8 include/linux/usb/hcd.h |2 +- 10 files changed, 49 insertions(+), 49 deletions(-) This doesn't apply to my tree at all anymore, can you refresh it and resend? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Replace a single-value sscanf() call with a call to kstrtou32(), as recommended by checkpatch.pl.
On Sat, Sep 06, 2014 at 11:12:27PM -0400, Lars R. Damerow wrote: Signed-off-by: Lars R. Damerow l...@grandstreet.us No changelog body? Also, the subject should have usb and vhci_sysfs somewhere in there, right? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] storage: Replace magic number with define in usb_stor_euscsi_init()
On Sun, Sep 21, 2014 at 07:59:42PM +0100, Mark wrote: Hi, usb_stor_euscsi_init() calls usb_stor_control_msg() with timeout argument 5000. USB_CTRL_SET_TIMEOUT is defined to be 5000 in usb.h, so would it make sense to use that instead? Patch below if it would. Signed-off-by: Mark Knibbs ma...@clara.co.uk Note, your email client doesn't show your full name in the From: line, can you fix it? I don't like having to hand-edit patches to add it back in... thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] USB: Add device quirk for ASUS T100 Base Station keyboard
On Fri, Sep 19, 2014 at 10:13:50AM +0800, Lu Baolu wrote: This full-speed USB device generates spurious remote wakeup event as soon as USB_DEVICE_REMOTE_WAKEUP feature is set. As the result, Linux can't enter system suspend and S0ix power saving modes once this keyboard is used. This patch tries to introduce USB_QUIRK_IGNORE_REMOTE_WAKEUP quirk. With this quirk set, wakeup capability will be ignored during device configure. This patch could be back-ported to kernels as old as 2.6.39. I had to renumber the quirk as a different patch grabbed that number, but all should be fine now. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/6] usb: hub: convert khubd into workqueue
On Sat, Sep 20, 2014 at 08:41:21PM +, Paul Zimmerman wrote: From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern Sent: Friday, September 19, 2014 12:39 PM On Fri, 19 Sep 2014, Petr Mladek wrote: The 3rd version of the patchset is slightly reordered and refactored as suggested earlier. See below for more details. IMHO, the result is more clean. But feel free to ask me to revert it. I do not want to make the review more complicated. Well, I double checked the diff of patches files between v2 and v3. It is pretty small and only the expected changes are there. Changes in: + v3: + split the optimization of hub-hdev reference counting into separate patch; usable standalone (1st patch) + switch order of the two patches that convert the kthread and that remove the while cycle in hub_event(); it is cleaner (2nd and 3rd patch) + use ordered workqueue by default; it makes it compatible with the original kthread solution (3rd patch) + added optional patch that allows to process events in parallel (6th patch) + fixed the suggested stylistic problems (2nd and 3rd patch) + v2: + solved potential races: + get hub-kref in kick_hub_wq() + call usb_get_dev(hdev) in hub_probe() and usb_put_dev(hdev) in hub_release() + INIT_WORK only once in hub_probe() + do not call cancel_work_sync() in hub_disconnect() to keep it fast + rename kick_khubd() to kick_hub_wq() already in the first patch; IMHO, it is cleaner while adding only very few changes The workqueue API is well defined and tested. It has many options that could be used to tune the scheduling. The code is usually easier and thus more safe. It allows to avoid the extra thread in most cases. It has has clearly defined behavior vrt. system suspend. This patchset converts khubd into the workqueue. It saves one thread, lock, and list. It looks huge but the main change is in the first patch. The rest is simple renaming of functions, comments and documentation. Thanks a lot Alan Stern and Tejun Heo for hints and guidance. The patches can be applied either against Linus' tree or linux-next. Very nice work. I haven't tried it out, but it all looks good. No doubt we will be able to sort out unforeseen troubles if any arise, without much difficulty. Hmm. What are the odds that some old kernel monitoring program, in an ancient Fedora distribution let's say, will be checking for 'khubd' in the kernel process list? Pretty high I would think. I really doubt it, what would that tell them? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: core: downgrade log severity to info when descriptor unavailable
On Tue, Sep 23, 2014 at 07:12:40PM +, Scot Doyle wrote: According to commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0 USB: fix LANGID=0 regression usb devices are not required to report string descriptors. Since they are optional, log an info message instead of an error message. Signed-off-by: Scot Doyle lkm...@scotdoyle.com --- drivers/usb/core/message.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0c8a7fc..da2f1f2 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -780,8 +780,12 @@ static int usb_get_langid(struct usb_device *dev, unsigned char *tbuf) * deal with strings at all. Set string_langid to -1 in order to * prevent any string to be retrieved from the device */ if (err 0) { - dev_err(dev-dev, string descriptor 0 read error: %d\n, - err); + if (err == -EPIPE) + dev_info(dev-dev, + string descriptor 0 read error: -EPIPE\n); + else + dev_err(dev-dev, + string descriptor 0 read error: %d\n, err); What real difference does this patch make? What would a user do with -EPIPE? And USB devices _are_ required to provide a string descriptor if they provide a string id, so your changelog body doesn't make much sense. I suggest rewriting it to say they aren't required to provide a string descriptor for string 0. But even then, I don't understand the goal of this patch. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] usb: core: log more general message on malformed LANGID descriptor
On Tue, Sep 23, 2014 at 10:28:49PM +, Scot Doyle wrote: I'd like to change this error message: [3.325837] usb 1-4: string descriptor 0 malformed (err = -61), defaulting to 0x0409 into an error message followed by a debug message: [3.324726] usb 1-4: malformed string descriptor; unknown language, defaulting to English [3.327514] usb 1-4: string descriptor 0 malformed (err = -61), defaulting to 0x0409 in order to communicate more information from the log itself. Are there any problems with this approach? Would it be better to put all the information on a single line? Something else? How about just one line, why two? And something like device not implementing langid specifier, defaulting to English, but cleaned up to sound a bit better? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Wednesday 24 September 2014 09:44:19 Arnd Bergmann wrote: We can also gradually move in some of the other glue drivers into the main driver if the differences are small enough. FWIW, I've just looked at the other glue drivers that already exist: - zevio can just get merged into the common driver, all that seems to be needed for that is the additional compatible string, and keying off the ci_default_zevio_platdata on the .data field of the of_device_id table. - msm has a custom notifier, which justifies leaving it in a separate driver, but it's also small enough that it wouldn't hurt to have that merged into the main driver too. - imx requires a lot of other things, in particular the dependency on the usbmisc driver means we don't want to have that in the core anyway, so we can't really merge that in. - the proposed ar933x driver again looks almost trivial, so no reason for a separate glue driver for that. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Added support for Seluxit USB dongle to cp210x driver.
On Tue, Sep 23, 2014 at 09:44:48PM -0700, Greg Kroah-Hartman wrote: On Mon, Sep 22, 2014 at 10:52:56AM +0200, Johan Hovold wrote: On Mon, Sep 22, 2014 at 09:50:43AM +0200, Andreas Bomholtz wrote: Added the Seluxit ApS USB Serial Dongle to cp210x driver. Signed-off-by: Andreas Bomholtz andr...@seluxit.com --- Thanks for resending. Applied just fine now. Was this in the last pull request you made to me? I don't see it there... I just sent you a final pull request for 3.17. Just two device ids, including this one. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/4] gpiolib: add irq_not_threaded flag to gpio_chip
On Fri, Sep 19, 2014 at 10:22 PM, Octavian Purdila octavian.purd...@intel.com wrote: Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set operation but do not need a threaded irq handler. Signed-off-by: Octavian Purdila octavian.purd...@intel.com Usually I don't apply patches adding interfaces with no users, but this seems very useful, so patch applied. I guess your driver will appear on v3.19+ so then you can rely on this having been merged for v3.18. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis
On Wed, Sep 24 2014, Heiko Schocher h...@denx.de wrote: use the values for RNDIS over Ethernet as defined in http://www.usb.org/developers/defined_class (search for RDNIS): - baseclass: 0xef (miscellaneous) - subclass: 0x04 - protocol: 0x01 with this setings the file in Documentation/usb/linux.inf is obsolete. Signed-off-by: Heiko Schocher h...@denx.de --- Cc: Felipe Balbi ba...@ti.com Cc: Greg Kroah-Hartman gre...@suse.de Cc: linux-usb@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: Oliver Neukum oli...@neukum.name Cc: net...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: Andrzej Pietrasiewicz andrze...@samsung.com Cc: Michal Nazarewicz min...@mina86.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Dan Carpenter dan.carpen...@oracle.com Cc: Macpaul Lin macp...@gmail.com Tested with the USB Compliance test suite which runs Windows, see: http://www.usb.org/developers/tools/usb20_tools/#usb20cv drivers/net/usb/cdc_ether.c | 6 +++--- drivers/usb/core/generic.c| 6 +++--- drivers/usb/gadget/function/f_rndis.c | 6 +++--- include/uapi/linux/usb/cdc.h | 3 +++ 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c index 2a32d91..9c216c2 100644 --- a/drivers/net/usb/cdc_ether.c +++ b/drivers/net/usb/cdc_ether.c @@ -35,9 +35,9 @@ static int is_rndis(struct usb_interface_descriptor *desc) { - return (desc-bInterfaceClass == USB_CLASS_COMM - desc-bInterfaceSubClass == 2 - desc-bInterfaceProtocol == 0xff); + return (desc-bInterfaceClass == USB_CLASS_MISC + desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS + desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH); } Does that mean that new kernels will stop working with old RNDIs gadgets because they stop recognising them as RNDIS? I feel like this function should accept both, i.e.: return (desc-bInterfaceClass == USB_CLASS_COMM desc-bInterfaceSubClass == 2 desc-bInterfaceProtocol == 0xff) || (desc-bInterfaceClass == USB_CLASS_MISC desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH); static int is_activesync(struct usb_interface_descriptor *desc) diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index 358ca8d..a2a4e05 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -28,9 +28,9 @@ static inline const char *plural(int n) static int is_rndis(struct usb_interface_descriptor *desc) { - return desc-bInterfaceClass == USB_CLASS_COMM - desc-bInterfaceSubClass == 2 - desc-bInterfaceProtocol == 0xff; + return desc-bInterfaceClass == USB_CLASS_MISC + desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS + desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH; } Ditto. static int is_activesync(struct usb_interface_descriptor *desc) diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c index ddb09dc..cc06046 100644 --- a/drivers/usb/gadget/function/f_rndis.c +++ b/drivers/usb/gadget/function/f_rndis.c @@ -117,9 +117,9 @@ static struct usb_interface_descriptor rndis_control_intf = { /* .bInterfaceNumber = DYNAMIC */ /* status endpoint is optional; this could be patched later */ .bNumEndpoints =1, - .bInterfaceClass = USB_CLASS_COMM, - .bInterfaceSubClass = USB_CDC_SUBCLASS_ACM, - .bInterfaceProtocol = USB_CDC_ACM_PROTO_VENDOR, + .bInterfaceClass = USB_CLASS_MISC, + .bInterfaceSubClass = USB_CDC_SUBCLASS_RNDIS, + .bInterfaceProtocol = USB_CDC_RNDIS_PROTO_ETH, /* .iInterface = DYNAMIC */ }; diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h index b6a9cdd..8e8fc85 100644 --- a/include/uapi/linux/usb/cdc.h +++ b/include/uapi/linux/usb/cdc.h @@ -12,6 +12,7 @@ #include linux/types.h #define USB_CDC_SUBCLASS_ACM 0x02 +#define USB_CDC_SUBCLASS_RNDIS 0x04 #define USB_CDC_SUBCLASS_ETHERNET0x06 #define USB_CDC_SUBCLASS_WHCM0x08 #define USB_CDC_SUBCLASS_DMM 0x09 @@ -31,6 +32,8 @@ #define USB_CDC_ACM_PROTO_AT_CDMA6 #define USB_CDC_ACM_PROTO_VENDOR 0xff +#define USB_CDC_RNDIS_PROTO_ETH 1 + #define USB_CDC_PROTO_EEM7 #define USB_CDC_NCM_PROTO_NTB1 -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the
Re: [PATCH 2/6] phy: improved lookup method
Hi, On Tuesday 23 September 2014 05:13 PM, Heikki Krogerus wrote: On Tue, Sep 23, 2014 at 04:33:09PM +0530, Kishon Vijay Abraham I wrote: Hi, On Tuesday 23 September 2014 04:23 PM, Heikki Krogerus wrote: On Mon, Sep 22, 2014 at 05:07:55PM +0530, Kishon Vijay Abraham I wrote: On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote: On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote: On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote: Assume you have 2 phys in your system.. static struct phy_lookup usb_lookup = { .phy_name = phy-usb.0, .dev_id = usb.0, .con_id = usb, }; static struct phy_lookup sata_lookup = { .phy_name = sata-usb.1, .dev_id = sata.0, .con_id = sata, }; First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it creates the PHY. The phy-core will find a free id (now it will be 0) and then name the phy as phy-usb.0. Then with modprobe phy-sata, the phy-core will create phy-sata.1. This is an ideal case where the .phy_name in phy_lookup matches. Consider if the order is flipped and the user does modprobe phy-sata first. The phy_names won't match anymore (the sata phy device name would be sata-usb.0). Actually, I don't think there would be this problem if we used the name of the actual device which is the parent of phy devices, right? hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we might end up with the same problem. I'm not completely sure what you mean? If you are talking about platforms with multiple instances of a single phy, I don't see how there could ever be a scenario where we did not know the order in which they were enumerated. Can you give an example again? If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next), the parent for all the phy devices would be the same. OK, got it. So I guess we need to match to the phy dev and to the phy name. First to the dev and then in case the phy name is defined in the lookup, to that as well. That should cover both cases. So what would be the phy name? I mean it's completely user-defined or it's derived from device name? Isn't making the PHY to be aware of it's user much simpler? Thanks Kishon -- 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 1/2] mfd: viperboard: allocate I/O buffer separately
On Mon, 22 Sep 2014, Octavian Purdila wrote: Currently the I/O buffer is allocated part of the device status structure, potentially sharing the same cache line with other members in this structure. Allocate the buffer separately, to avoid the I/O operations corrupting the device status structure due to cache line sharing. Compiled tested only as I don't have access to hardware. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/viperboard.c | 8 include/linux/mfd/viperboard.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c index e00f534..5f62f4e 100644 --- a/drivers/mfd/viperboard.c +++ b/drivers/mfd/viperboard.c @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface, return -ENOMEM; } + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL); Can you obtain the 'struct device' first then use managed resources (devm_*)? + if (vb-buf == NULL) { + ret = -ENOMEM; + goto error; + } + mutex_init(vb-lock); vb-usb_dev = usb_get_dev(interface_to_usbdev(interface)); @@ -105,6 +111,7 @@ static int vprbrd_probe(struct usb_interface *interface, error: if (vb) { usb_put_dev(vb-usb_dev); + kfree(vb-buf); Then you don't need this. kfree(vb); } @@ -118,6 +125,7 @@ static void vprbrd_disconnect(struct usb_interface *interface) mfd_remove_devices(interface-dev); usb_set_intfdata(interface, NULL); usb_put_dev(vb-usb_dev); + kfree(vb-buf); Or this. kfree(vb); dev_dbg(interface-dev, disconnected\n); diff --git a/include/linux/mfd/viperboard.h b/include/linux/mfd/viperboard.h index 1934528..af928d0 100644 --- a/include/linux/mfd/viperboard.h +++ b/include/linux/mfd/viperboard.h @@ -103,7 +103,7 @@ struct vprbrd_i2c_addr_msg { struct vprbrd { struct usb_device *usb_dev; /* the usb device for this device */ struct mutex lock; - u8 buf[sizeof(struct vprbrd_i2c_write_msg)]; + u8 *buf; struct platform_device pdev; }; -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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 2/2] mfd: viperboard: remove redundant OOM message and NULL pointer check
On Mon, 22 Sep 2014, Octavian Purdila wrote: Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/viperboard.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) Looks fine, but won't apply without patch 1. diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c index 5f62f4e..d27c131 100644 --- a/drivers/mfd/viperboard.c +++ b/drivers/mfd/viperboard.c @@ -59,10 +59,8 @@ static int vprbrd_probe(struct usb_interface *interface, /* allocate memory for our device state and initialize it */ vb = kzalloc(sizeof(*vb), GFP_KERNEL); - if (vb == NULL) { - dev_err(interface-dev, Out of memory\n); + if (vb == NULL) return -ENOMEM; - } vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL); if (vb-buf == NULL) { @@ -109,11 +107,9 @@ static int vprbrd_probe(struct usb_interface *interface, return 0; error: - if (vb) { - usb_put_dev(vb-usb_dev); - kfree(vb-buf); - kfree(vb); - } + usb_put_dev(vb-usb_dev); + kfree(vb-buf); + kfree(vb); return ret; } -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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 1/2] mfd: viperboard: allocate I/O buffer separately
On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote: On Mon, 22 Sep 2014, Octavian Purdila wrote: Currently the I/O buffer is allocated part of the device status structure, potentially sharing the same cache line with other members in this structure. Allocate the buffer separately, to avoid the I/O operations corrupting the device status structure due to cache line sharing. Compiled tested only as I don't have access to hardware. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/viperboard.c | 8 include/linux/mfd/viperboard.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c index e00f534..5f62f4e 100644 --- a/drivers/mfd/viperboard.c +++ b/drivers/mfd/viperboard.c @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface, return -ENOMEM; } + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL); Can you obtain the 'struct device' first then use managed resources (devm_*)? I think any devres conversion should be done in a follow-up patch and not be included in the fix (e.g. in order to facilitate backporting). We also don't want to mix allocation schemes. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices
On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote: This patch implements the USB part of the Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2. Details about the device can be found here: https://www.diolan.com/i2c/i2c_interface.html. Information about the USB protocol can be found in the Programmer's Reference Manual [1], see section 1.7. Because the hardware has a single transmit endpoint and a single receive endpoint the communication between the various DLN2 drivers and the hardware will be muxed/demuxed by this driver. Each DLN2 module will be identified by the handle field within the DLN2 message header. If a DLN2 module issues multiple commands in parallel they will be identified by the echo counter field in the message header. The DLN2 modules can use the dln2_transfer() function to issue a command and wait for its response. They can also register a callback that is going to be called when a specific event id is generated by the device (e.g. GPIO interrupts). The device uses handle 0 for sending events. [1] https://www.diolan.com/downloads/dln-api-manual.pdf Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/Kconfig | 9 + drivers/mfd/Makefile | 1 + drivers/mfd/dln2.c | 758 +++ include/linux/mfd/dln2.h | 67 + 4 files changed, 835 insertions(+) create mode 100644 drivers/mfd/dln2.c create mode 100644 include/linux/mfd/dln2.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index de5abf2..7bcf895 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -183,6 +183,15 @@ config MFD_DA9063 Additional drivers must be enabled in order to use the functionality of the device. +config MFD_DLN2 + tristate Diolan DLN2 support + select MFD_CORE + depends on USB + help + This adds support for Diolan USB-I2C/SPI/GPIO Master Adapter DLN-2. + Additional drivers must be enabled in order to use the functionality + of the device. + config MFD_MC13XXX tristate depends on (SPI_MASTER || I2C) diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index f001487..591988d 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) += as3711.o obj-$(CONFIG_MFD_AS3722) += as3722.o obj-$(CONFIG_MFD_STW481X)+= stw481x.o obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o +obj-$(CONFIG_MFD_DLN2) += dln2.o intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c new file mode 100644 index 000..36c53cd --- /dev/null +++ b/drivers/mfd/dln2.c @@ -0,0 +1,758 @@ +/* + * Driver for the Diolan DLN-2 USB adapter + * + * Copyright (c) 2014 Intel Corporation + * + * Derived from: + * i2c-diolan-u2c.c + * Copyright (c) 2010-2011 Ericsson AB + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2. + */ + +#include linux/kernel.h +#include linux/module.h +#include linux/types.h +#include linux/slab.h +#include linux/usb.h +#include linux/i2c.h +#include linux/mutex.h +#include linux/platform_device.h +#include linux/mfd/core.h +#include linux/mfd/dln2.h +#include linux/rculist.h + +struct dln2_header { + __le16 size; + __le16 id; + __le16 echo; + __le16 handle; +} __packed; + +struct dln2_response { + struct dln2_header hdr; + __le16 result; +} __packed; + +#define DLN2_GENERIC_MODULE_ID 0x00 +#define DLN2_GENERIC_CMD(cmd)DLN2_CMD(cmd, DLN2_GENERIC_MODULE_ID) +#define CMD_GET_DEVICE_VER DLN2_GENERIC_CMD(0x30) +#define CMD_GET_DEVICE_SNDLN2_GENERIC_CMD(0x31) + +#define DLN2_HW_ID 0x200 +#define DLN2_USB_TIMEOUT 200 /* in ms */ +#define DLN2_MAX_RX_SLOTS16 +#define DLN2_MAX_URBS16 +#define DLN2_RX_BUF_SIZE 512 + +#define DLN2_HANDLE_EVENT0 +#define DLN2_HANDLE_CTRL 1 +#define DLN2_HANDLE_GPIO 2 +#define DLN2_HANDLE_I2C 3 +#define DLN2_HANDLES 4 This should be an enum. + + +/* + * Receive context used between the receive demultiplexer and the + * transfer routine. While sending a request the transfer routine + * will look for a free receive context and use it to wait for a + * response and to receive the URB and thus the response data. + */ +struct dln2_rx_context { + /* completion used to wait a response */ wait for + struct completion done; + + /* if non-NULL the URB contains the response */ + struct urb *urb; + + /* if true then
Re: [PATCH v5 2/4] i2c: add support for Diolan DLN-2 USB-I2C adapter
On Fri, Sep 19, 2014 at 11:22:43PM +0300, Octavian Purdila wrote: +struct dln2_i2c { + struct platform_device *pdev; + struct i2c_adapter adapter; + u32 freq; + u32 min_freq; + u32 max_freq; + /* + * Buffer to hold the packet for read or write transfers. One + * is enough since we can't have multiple transfers in + * parallel on the i2c adapter. + */ + union { + struct { + u8 port; + u8 addr; + u8 mem_addr_len; + __le32 mem_addr; + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed tx; + struct { + __le16 buf_len; + u8 buf[DLN2_I2C_MAX_XFER_SIZE]; + } __packed rx; + } *buf; Please declare these structs separately from the dln2_i2c struct so you can use temporary pointers below. Perhaps just allocate a large enough buffer (or page) here and keep you transfer-buffer declarations in the two functions where they are used if you prefer. +}; +static int dln2_i2c_enable(struct dln2_i2c *dln2, bool enable) +{ + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev); + int ret; + u8 port; + u16 cmd; + + port = pdata-port; Store the port number in the dln2_i2c struct rather than access it through the platform data for every I/O operation. + + if (enable) + cmd = DLN2_I2C_ENABLE; + else + cmd = DLN2_I2C_DISABLE; + + ret = dln2_transfer(dln2-pdev, cmd, port, sizeof(port), NULL, NULL); + if (ret 0) + return ret; + + return 0; +} + +static int dln2_i2c_set_frequency(struct dln2_i2c *dln2, u32 freq) +{ + int ret; + struct tx_data { + u8 port; + __le32 freq; + } __packed tx; + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev); + + tx.port = pdata-port; + tx.freq = cpu_to_le32(freq); + + ret = dln2_transfer(dln2-pdev, DLN2_I2C_SET_FREQUENCY, tx, sizeof(tx), + NULL, NULL); + if (ret 0) + return ret; + + dln2-freq = freq; + + return 0; +} + +static int dln2_i2c_get_freq(struct dln2_i2c *dln2, u16 cmd, u32 *res) +{ + int ret; + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev); + u8 port = pdata-port; + __le32 freq; + unsigned len = sizeof(freq); + + ret = dln2_transfer(dln2-pdev, cmd, port, sizeof(port), + freq, len); + if (ret 0) + return ret; + if (len sizeof(freq)) + return -EPROTO; + + *res = le32_to_cpu(freq); + + return 0; +} + +static int dln2_i2c_setup(struct dln2_i2c *dln2) +{ + int ret; + + ret = dln2_i2c_enable(dln2, false); + if (ret 0) + return ret; + + ret = dln2_i2c_get_freq(dln2, DLN2_I2C_GET_MIN_FREQUENCY, + dln2-min_freq); + if (ret 0) + return 0; return ret + + ret = dln2_i2c_get_freq(dln2, DLN2_I2C_GET_MAX_FREQUENCY, + dln2-max_freq); + if (ret 0) + return 0; return ret + + ret = dln2_i2c_set_frequency(dln2, DLN2_I2C_FREQ_STD); + if (ret 0) + return ret; + + ret = dln2_i2c_enable(dln2, true); + + return ret; +} + +static int dln2_i2c_write(struct dln2_i2c *dln2, u8 addr, + u8 *data, u16 data_len) +{ + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev); + int ret; + unsigned len; + + dln2-buf-tx.port = pdata-port; + dln2-buf-tx.addr = addr; + dln2-buf-tx.mem_addr_len = 0; + dln2-buf-tx.mem_addr = 0; + dln2-buf-tx.buf_len = cpu_to_le16(data_len); + memcpy(dln2-buf-tx.buf, data, data_len); + + len = sizeof(dln2-buf-tx) + data_len - DLN2_I2C_MAX_XFER_SIZE; + ret = dln2_transfer(dln2-pdev, DLN2_I2C_WRITE, dln2-buf-tx, len, + NULL, NULL); Use a temporary pointer to the tx struct above for readability. + if (ret 0) + return ret; + + return data_len; +} + +static int dln2_i2c_read(struct dln2_i2c *dln2, u16 addr, u8 *data, + u16 data_len) +{ + struct dln2_platform_data *pdata = dev_get_platdata(dln2-pdev-dev); + int ret; + struct { + u8 port; + u8 addr; + u8 mem_addr_len; + __le32 mem_addr; + __le16 buf_len; + } __packed tx; + unsigned rx_len; + + tx.port = pdata-port; + tx.addr = addr; + tx.mem_addr_len = 0; + tx.mem_addr = 0; + tx.buf_len = cpu_to_le16(data_len); + + rx_len = sizeof(dln2-buf-rx); + + ret = dln2_transfer(dln2-pdev, DLN2_I2C_READ, tx, sizeof(tx), +
Re: [PATCH v5 3/4] gpiolib: add irq_not_threaded flag to gpio_chip
On Wed, Sep 24, 2014 at 10:54:51AM +0200, Linus Walleij wrote: On Fri, Sep 19, 2014 at 10:22 PM, Octavian Purdila octavian.purd...@intel.com wrote: Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set operation but do not need a threaded irq handler. Signed-off-by: Octavian Purdila octavian.purd...@intel.com Usually I don't apply patches adding interfaces with no users, but this seems very useful, so patch applied. I guess your driver will appear on v3.19+ so then you can rely on this having been merged for v3.18. Octavian, please include this one in any future revision of you series so that it is self-contained (at least until v3.18-rc1 is out) nonetheless. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: Remove .owner field for driver
There is no need to init .owner field. Based on the patch from Peter Griffin peter.grif...@linaro.org mmc: remove .owner field for drivers using module_platform_driver This patch removes the superflous .owner field for drivers which use the module_platform_driver API, as this is overriden in platform_driver_register anyway. Signed-off-by: Kiran Padwal kiran.pad...@smartplayin.com --- drivers/usb/dwc3/dwc3-keystone.c |1 - drivers/usb/dwc3/dwc3-qcom.c |1 - drivers/usb/phy/phy-msm-usb.c|1 - 3 files changed, 3 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c index 1fad161..7ec8495 100644 --- a/drivers/usb/dwc3/dwc3-keystone.c +++ b/drivers/usb/dwc3/dwc3-keystone.c @@ -189,7 +189,6 @@ static struct platform_driver kdwc3_driver = { .remove = kdwc3_remove, .driver = { .name = keystone-dwc3, - .owner = THIS_MODULE, .of_match_table = kdwc3_of_match, }, }; diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index 611f8e7..8c2e8ee 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c @@ -118,7 +118,6 @@ static struct platform_driver dwc3_qcom_driver = { .remove = dwc3_qcom_remove, .driver = { .name = qcom-dwc3, - .owner = THIS_MODULE, .of_match_table = of_dwc3_match, }, }; diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 7bb48af..7843ef7 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1841,7 +1841,6 @@ static struct platform_driver msm_otg_driver = { .remove = msm_otg_remove, .driver = { .name = DRIVER_NAME, - .owner = THIS_MODULE, .pm = msm_otg_dev_pm_ops, .of_match_table = msm_otg_dt_match, }, -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Wed, Sep 24, 2014 at 10:30:41AM +0200, Arnd Bergmann wrote: On Wednesday 24 September 2014 09:44:19 Arnd Bergmann wrote: We can also gradually move in some of the other glue drivers into the main driver if the differences are small enough. FWIW, I've just looked at the other glue drivers that already exist: - zevio can just get merged into the common driver, all that seems to be needed for that is the additional compatible string, and keying off the ci_default_zevio_platdata on the .data field of the of_device_id table. - msm has a custom notifier, which justifies leaving it in a separate driver, but it's also small enough that it wouldn't hurt to have that merged into the main driver too. - imx requires a lot of other things, in particular the dependency on the usbmisc driver means we don't want to have that in the core anyway, so we can't really merge that in. - the proposed ar933x driver again looks almost trivial, so no reason for a separate glue driver for that. Arnd Thanks, Arnd. So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver (dwc3, musb, chipidea) you are talking about, right? Except for creating another platform driver as well as related DT node (optional), are there any advantages compared to current IP core driver structure? In this thread, we are talking about creating common platform driver for glue layer, its design purpose (adapt it for as many as platforms) should be the same, no matter the IP core part is a LIB or platform driver, am I missing something? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Poor performance with USB 1.1 drive connected to USB 3.0 port
Hi, I did some benchmarks to check the maximum transfer rate of a USB-to-SCSI converter. The converter is USB 1.1, so limited to the 12Mbps full speed rate. The performance when connected to a USB 3.0 port is significantly worse than when connected to a USB 2.0 port, about 26.5% slower (0.63MB/sec vs 0.86MB/sec). The USB 3.0 port is provided by an ExpressCard which has a Renesas controller. It doesn't seem to be defective because I can get 138MB/sec from a USB 3.0 hard disk. lspci shows 05:00.0 USB Controller: Renesas Technology Corp. Device 0015 (rev 02) I didn't test with an unmodified mainline kernel, but the issue is present with both Ubuntu kernel 3.0 and Debian 3.14 (booted from Kali Linux live DVD). I used sg_rbuf from the sg3-utils package. That issues READ BUFFER commands instead of reading data from disk, so the result should be closer to the maximum achievable. The USB-SCSI converter was a Newer Technology uSCSI connected to an HP 2600fx MO drive. In a USB 2.0 port: # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 242.800241 secs, 0.86 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 242.800230 secs, 0.86 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) In a USB 3.0 port: # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 330.311337 secs, 0.63 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 330.388556 secs, 0.63 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) By comparison, I checked the transfer rate in native Windows (Vista) with the WinSAT program. That reported 0.66MB/sec for both USB 2.0 and 3.0 ports. (WinSAT reads data from disk, probably in smaller chunks than 512KB. There's no sg_rbuf executable in the Windows sg3_utils archive.) Any ideas what the reason for the discrepancy might be? Can anyone else reproduce it? I guess USB 1.1-only devices aren't that common nowadays. Mark -- 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 1/2] mfd: viperboard: allocate I/O buffer separately
On Wed, 24 Sep 2014, Johan Hovold wrote: On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote: On Mon, 22 Sep 2014, Octavian Purdila wrote: Currently the I/O buffer is allocated part of the device status structure, potentially sharing the same cache line with other members in this structure. Allocate the buffer separately, to avoid the I/O operations corrupting the device status structure due to cache line sharing. Compiled tested only as I don't have access to hardware. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/viperboard.c | 8 include/linux/mfd/viperboard.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c index e00f534..5f62f4e 100644 --- a/drivers/mfd/viperboard.c +++ b/drivers/mfd/viperboard.c @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface, return -ENOMEM; } + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL); Can you obtain the 'struct device' first then use managed resources (devm_*)? I think any devres conversion should be done in a follow-up patch and not be included in the fix (e.g. in order to facilitate backporting). We also don't want to mix allocation schemes. I agree, but equally I'm not keen on accepting this patch as I believe it could be done better. Please submit two patches, one converting to shared resources and this being the subsequent one, fixed up to do the right thing. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx
On Wednesday 24 September 2014 19:29:05 Peter Chen wrote: So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver (dwc3, musb, chipidea) you are talking about, right? Except for creating another platform driver as well as related DT node (optional), are there any advantages compared to current IP core driver structure? Having a library module usually requires less code, and is more consistent with other drivers, which helps new developers understand what the driver is doing. The most important aspect though is the DT binding: once the structure with separate kernel drivers leaks out into the DT format, you can't easily change the driver any more, e.g. to make a property that was introduced for one glue driver more general so it can get handled by the common part. Having a single node lets us convert to the library model later, so that would be a strong reason to keep the DT binding simple, without child nodes. Following that logic, it turns into another reason for using the library model to start with, because otherwise the child device does not have any DT properties itself and has to rely on the parent device properties, which somewhat complicates the driver structure. In this thread, we are talking about creating common platform driver for glue layer, its design purpose (adapt it for as many as platforms) should be the same, no matter the IP core part is a LIB or platform driver, am I missing something? No, you are absolutely right with that, introducing the generic glue driver is orthogonal to the structure of the core driver in principle. I mainly brought it up because I noticed how this patch could be done in a simpler way by combining the new generic glue with the move to a library driver model. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][RFC] storage: Simplify condition to reject bad target in usb_stor_control_thread()
Hi, This follows on from something I mentioned in my recent post [PATCH] storage: Don't scan target 7 for SCM USB-SCSI converters. In drivers/usb/storage/usb.c, usb_stor_control_thread(): /* reject if target != 0 or if LUN is higher than * the maximum known LUN */ else if (us-srb-device-id !(us-fflags US_FL_SCM_MULT_TARG)) { usb_stor_dbg(us, Bad target number (%d:%llu)\n, us-srb-device-id, us-srb-device-lun); us-srb-result = DID_BAD_TARGET 16; } else if (us-srb-device-lun us-max_lun) { usb_stor_dbg(us, Bad LUN (%d:%llu)\n, ... The condition (us-srb-device-id !(us-fflags US_FL_SCM_MULT_TARG)) means: - If not SCM_MULT_TARG, reject if us-srb-device-id is non-zero. That works for the usual case. - If SCM_MULT_TARG, never reject. Hmm... Would it be better/clearer to change the condition to simply (us-srb-device-id = host-max_id) Then the command is rejected if us-srb-device-id is too large, and works for the SCM_MULT_TARG case too. (host-max_id is 1 for the non-SCM_MULT_TARG case.) Plus the condition matches nicely with the later LUN check. Untested patch below. Signed-off-by: Mark Knibbs ma...@clara.co.uk diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig linux-3.17-rc6/drivers/usb/storage/usb.c --- linux-3.17-rc6/drivers/usb/storage/usb.c.orig 2014-09-21 23:43:02.0 +0100 +++ linux-3.17-rc6/drivers/usb/storage/usb.c2014-09-24 13:09:55.0 +0100 @@ -342,11 +342,10 @@ static int usb_stor_control_thread(void us-srb-result = DID_ERROR 16; } - /* reject if target != 0 or if LUN is higher than + /* reject if target is invalid or if LUN is higher than * the maximum known LUN */ - else if (us-srb-device-id - !(us-fflags US_FL_SCM_MULT_TARG)) { + else if (us-srb-device-id = host-max_id) { usb_stor_dbg(us, Bad target number (%d:%llu)\n, us-srb-device-id, us-srb-device-lun); -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis
On 2014-09-24 13:48, Heiko Schocher wrote: use the values for RNDIS over Ethernet as defined in http://www.usb.org/developers/defined_class (search for RDNIS): - baseclass: 0xef (miscellaneous) - subclass: 0x04 - protocol: 0x01 That is usb class, it is not the same thing as communication device class. --- a/include/uapi/linux/usb/cdc.h +++ b/include/uapi/linux/usb/cdc.h @@ -12,6 +12,7 @@ #include linux/types.h #define USB_CDC_SUBCLASS_ACM 0x02 +#define USB_CDC_SUBCLASS_RNDIS 0x04 No, no, no. There is no CDC_SUBCLASS_RNDIS and you can not define one over an already used cdc subclass number, 0x04 is Multi-Channel Control Model -- 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 1/2] mfd: viperboard: allocate I/O buffer separately
On Wed, Sep 24, 2014 at 01:00:02PM +0100, Lee Jones wrote: On Wed, 24 Sep 2014, Johan Hovold wrote: On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote: On Mon, 22 Sep 2014, Octavian Purdila wrote: Currently the I/O buffer is allocated part of the device status structure, potentially sharing the same cache line with other members in this structure. Allocate the buffer separately, to avoid the I/O operations corrupting the device status structure due to cache line sharing. Compiled tested only as I don't have access to hardware. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/viperboard.c | 8 include/linux/mfd/viperboard.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c index e00f534..5f62f4e 100644 --- a/drivers/mfd/viperboard.c +++ b/drivers/mfd/viperboard.c @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface, return -ENOMEM; } + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL); Can you obtain the 'struct device' first then use managed resources (devm_*)? I think any devres conversion should be done in a follow-up patch and not be included in the fix (e.g. in order to facilitate backporting). We also don't want to mix allocation schemes. I agree, but equally I'm not keen on accepting this patch as I believe it could be done better. Please submit two patches, one converting to shared resources and this being the subsequent one, fixed up to do the right thing. A buffer-corruption fix is a candidate for stable, whereas a devres conversion (clean up) is not. Hence the former should not depend on the latter. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] mfd: viperboard: allocate I/O buffer separately
On Wed, Sep 24, 2014 at 3:26 PM, Johan Hovold jo...@kernel.org wrote: On Wed, Sep 24, 2014 at 01:00:02PM +0100, Lee Jones wrote: On Wed, 24 Sep 2014, Johan Hovold wrote: On Wed, Sep 24, 2014 at 11:12:06AM +0100, Lee Jones wrote: On Mon, 22 Sep 2014, Octavian Purdila wrote: Currently the I/O buffer is allocated part of the device status structure, potentially sharing the same cache line with other members in this structure. Allocate the buffer separately, to avoid the I/O operations corrupting the device status structure due to cache line sharing. Compiled tested only as I don't have access to hardware. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/mfd/viperboard.c | 8 include/linux/mfd/viperboard.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c index e00f534..5f62f4e 100644 --- a/drivers/mfd/viperboard.c +++ b/drivers/mfd/viperboard.c @@ -64,6 +64,12 @@ static int vprbrd_probe(struct usb_interface *interface, return -ENOMEM; } + vb-buf = kzalloc(sizeof(struct vprbrd_i2c_write_msg), GFP_KERNEL); Can you obtain the 'struct device' first then use managed resources (devm_*)? I think any devres conversion should be done in a follow-up patch and not be included in the fix (e.g. in order to facilitate backporting). We also don't want to mix allocation schemes. I agree, but equally I'm not keen on accepting this patch as I believe it could be done better. Please submit two patches, one converting to shared resources and this being the subsequent one, fixed up to do the right thing. A buffer-corruption fix is a candidate for stable, whereas a devres conversion (clean up) is not. Hence the former should not depend on the latter. I can follow-up with a v3 3 patch series: first for the fix, second for the OOM error path cleanup, third for devm conversion. I think this will address both issues (backport and better final result). -- 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 1/2] mfd: viperboard: allocate I/O buffer separately
On Wed, Sep 24, 2014 at 03:34:08PM +0300, Octavian Purdila wrote: I can follow-up with a v3 3 patch series: first for the fix, second for the OOM error path cleanup, third for devm conversion. I'd include the error-path clean up bit in the devres conversion as that is what it's really all about. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/4] gpio: add support for the Diolan DLN-2 USB GPIO driver
On Fri, Sep 19, 2014 at 11:22:45PM +0300, Octavian Purdila wrote: +struct dln2_gpio { + struct platform_device *pdev; + struct gpio_chip gpio; + + /* + * Cache pin direction to save us one transfer, since the + * hardware has separate commands to read the in and out + * values. Bit set for out, bit clear for in. + */ + DECLARE_BITMAP(pin_dir, DLN2_GPIO_MAX_PINS); Using the more common name output_enabled might be preferred? Then you can even get rid of (part of) the comment. + + DECLARE_BITMAP(irqs_masked, DLN2_GPIO_MAX_PINS); + DECLARE_BITMAP(irqs_enabled, DLN2_GPIO_MAX_PINS); + DECLARE_BITMAP(irqs_pending, DLN2_GPIO_MAX_PINS); + struct dln2_irq_work *irq_work; +}; [...] +#define DLN2_GPIO_DIRECTION_IN 0 +#define DLN2_GPIO_DIRECTION_OUT 1 + +static int dln2_gpio_request(struct gpio_chip *chip, unsigned offset) +{ + struct dln2_gpio *dln2 = container_of(chip, struct dln2_gpio, gpio); + struct dln2_gpio_pin req = { + .pin = cpu_to_le16(offset), + }; + struct dln2_gpio_pin_val rsp; + int len = sizeof(rsp); + int ret; + + ret = dln2_gpio_pin_cmd(dln2, DLN2_GPIO_PIN_ENABLE, offset); + if (ret 0) + return ret; + + /* cache the pin direction */ + ret = dln2_transfer(dln2-pdev, DLN2_GPIO_PIN_GET_DIRECTION, + req, sizeof(req), rsp, len); + if (ret 0) + return ret; + if (len sizeof(rsp) || req.pin != rsp.pin) + return -EPROTO; Disable the pin? + + switch (rsp.value) { + case DLN2_GPIO_DIRECTION_IN: + clear_bit(offset, dln2-pin_dir); + return 0; + case DLN2_GPIO_DIRECTION_OUT: + set_bit(offset, dln2-pin_dir); + return 0; + default: + return -EPROTO; Disable the pin? + } +} [...] +static struct platform_driver dln2_gpio_driver = { + .driver.name= dln2-gpio, + .driver.owner = THIS_MODULE, No need to set the owner field when using the module_platform_driver macro. + .probe = dln2_gpio_probe, + .remove = dln2_gpio_remove, +}; + +module_platform_driver(dln2_gpio_driver); + +MODULE_AUTHOR(Daniel Baluta daniel.bal...@intel.com); +MODULE_DESCRIPTION(Driver for the Diolan DLN2 GPIO interface); +MODULE_LICENSE(GPL v2); +MODULE_ALIAS(platform:dln2-gpio); Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis
Hello Michal, Am 24.09.2014 11:38, schrieb Michal Nazarewicz: On Wed, Sep 24 2014, Heiko Schocherh...@denx.de wrote: use the values for RNDIS over Ethernet as defined in http://www.usb.org/developers/defined_class (search for RDNIS): - baseclass: 0xef (miscellaneous) - subclass: 0x04 - protocol: 0x01 with this setings the file in Documentation/usb/linux.inf is obsolete. Signed-off-by: Heiko Schocherh...@denx.de --- Cc: Felipe Balbiba...@ti.com Cc: Greg Kroah-Hartmangre...@suse.de Cc: linux-usb@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: Oliver Neukumoli...@neukum.name Cc: net...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: Andrzej Pietrasiewiczandrze...@samsung.com Cc: Michal Nazarewiczmin...@mina86.com Cc: Kyungmin Parkkyungmin.p...@samsung.com Cc: Dan Carpenterdan.carpen...@oracle.com Cc: Macpaul Linmacp...@gmail.com Tested with the USB Compliance test suite which runs Windows, see: http://www.usb.org/developers/tools/usb20_tools/#usb20cv drivers/net/usb/cdc_ether.c | 6 +++--- drivers/usb/core/generic.c| 6 +++--- drivers/usb/gadget/function/f_rndis.c | 6 +++--- include/uapi/linux/usb/cdc.h | 3 +++ 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c index 2a32d91..9c216c2 100644 --- a/drivers/net/usb/cdc_ether.c +++ b/drivers/net/usb/cdc_ether.c @@ -35,9 +35,9 @@ static int is_rndis(struct usb_interface_descriptor *desc) { - return (desc-bInterfaceClass == USB_CLASS_COMM - desc-bInterfaceSubClass == 2 - desc-bInterfaceProtocol == 0xff); + return (desc-bInterfaceClass == USB_CLASS_MISC + desc-bInterfaceSubClass == USB_CDC_SUBCLASS_RNDIS + desc-bInterfaceProtocol == USB_CDC_RNDIS_PROTO_ETH); } Does that mean that new kernels will stop working with old RNDIs gadgets because they stop recognising them as RNDIS? I feel like this function should accept both, i.e.: Hmm.. I am not a usb guru ... but I think, yes, you are right. I add this to a v2 (if this patch has a chance to go in mainline). Thanks! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis
Hello Lars, Am 24.09.2014 14:25, schrieb Lars Melin: On 2014-09-24 13:48, Heiko Schocher wrote: use the values for RNDIS over Ethernet as defined in http://www.usb.org/developers/defined_class (search for RDNIS): - baseclass: 0xef (miscellaneous) - subclass: 0x04 - protocol: 0x01 That is usb class, it is not the same thing as communication device class. --- a/include/uapi/linux/usb/cdc.h +++ b/include/uapi/linux/usb/cdc.h @@ -12,6 +12,7 @@ #include linux/types.h #define USB_CDC_SUBCLASS_ACM 0x02 +#define USB_CDC_SUBCLASS_RNDIS 0x04 No, no, no. There is no CDC_SUBCLASS_RNDIS and you can not define one over an already used cdc subclass number, 0x04 is Multi-Channel Control Model Ah, ok, so I have to define this values in a new header file, as there is no current file for the USB_CLASS_MISC defines? Or is there a proper place for them? BTW: where do I find the cdc subclass number, 0x04 is Multi-Channel Control Model define? bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Equivalent of g_webcam with configfs
This series aims at integrating configfs into uvc, the way it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet, mass_storage, FunctionFS, loopback, sourcesink, uac1 and uac2. The preparation series is already on Felipe's next, so this series in fact consists of a small preparation patch and the configfs support proper. Rebased onto Felipe's next. @Felipe: I know you asked for not sending patches until 3.18-rc1 is out. I'm sending this short series anyway, so that e.g. Laurent can have a look at it. @Laurent: Can you have a look at this series? Below is an example listing of gadget's function directory. The attributes whose access permissions are -r--r--r-- are there only for inspection by the user but not for modifications. For color matching, camera terminal, processing unit and output terminal only default descriptors are provided, that is, creating user's own is not supported at this moment. drwxr-xr-x . drwxr-xr-x ./streaming drwxr-xr-x ./streaming/class drwxr-xr-x ./streaming/class/ss drwxr-xr-x ./streaming/class/hs lrwxrwxrwx ./streaming/class/hs/h - \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/header/h drwxr-xr-x ./streaming/class/fs lrwxrwxrwx ./streaming/class/fs/h - \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/header/h drwxr-xr-x ./streaming/color_matching drwxr-xr-x ./streaming/color_matching/default -r--r--r-- ./streaming/color_matching/default/bMatrixCoefficients -r--r--r-- ./streaming/color_matching/default/bTransferCharacteristics -r--r--r-- ./streaming/color_matching/default/bColorPrimaries drwxr-xr-x ./streaming/uncompressed drwxr-xr-x ./streaming/uncompressed/u drwxr-xr-x ./streaming/uncompressed/u/360p -rw-r--r-- ./streaming/uncompressed/u/360p/dwFrameInterval -rw-r--r-- ./streaming/uncompressed/u/360p/dwDefaultFrameInterval -rw-r--r-- ./streaming/uncompressed/u/360p/dwMaxVideoFrameBufferSize -rw-r--r-- ./streaming/uncompressed/u/360p/dwMaxBitRate -rw-r--r-- ./streaming/uncompressed/u/360p/dwMinBitRate -rw-r--r-- ./streaming/uncompressed/u/360p/wHeight -rw-r--r-- ./streaming/uncompressed/u/360p/wWidth -rw-r--r-- ./streaming/uncompressed/u/360p/bmCapabilities -rw-r--r-- ./streaming/uncompressed/u/bmaControls -r--r--r-- ./streaming/uncompressed/u/bmInterfaceFlags -r--r--r-- ./streaming/uncompressed/u/bAspectRatioY -r--r--r-- ./streaming/uncompressed/u/bAspectRatioX -rw-r--r-- ./streaming/uncompressed/u/bDefaultFrameIndex -rw-r--r-- ./streaming/uncompressed/u/bBitsPerPixel -rw-r--r-- ./streaming/uncompressed/u/guidFormat drwxr-xr-x ./streaming/header drwxr-xr-x ./streaming/header/h lrwxrwxrwx ./streaming/header/h/u - \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/uncompressed/u -r--r--r-- ./streaming/header/h/bTriggerUsage -r--r--r-- ./streaming/header/h/bTriggerSupport -r--r--r-- ./streaming/header/h/bStillCaptureMethod -r--r--r-- ./streaming/header/h/bTerminalLink -r--r--r-- ./streaming/header/h/bmInfo drwxr-xr-x ./control drwxr-xr-x ./control/class drwxr-xr-x ./control/class/ss lrwxrwxrwx ./control/class/ss/h - \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/control/header/h drwxr-xr-x ./control/class/fs lrwxrwxrwx ./control/class/fs/h - \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/control/header/h drwxr-xr-x ./control/terminal drwxr-xr-x ./control/terminal/output drwxr-xr-x ./control/terminal/output/default -r--r--r-- ./control/terminal/output/default/iTerminal -r--r--r-- ./control/terminal/output/default/bSourceID -r--r--r-- ./control/terminal/output/default/bAssocTerminal -r--r--r-- ./control/terminal/output/default/wTerminalType -r--r--r-- ./control/terminal/output/default/bTerminalID drwxr-xr-x ./control/terminal/camera drwxr-xr-x ./control/terminal/camera/default -r--r--r-- ./control/terminal/camera/default/bmControls -r--r--r-- ./control/terminal/camera/default/wOcularFocalLength -r--r--r-- ./control/terminal/camera/default/wObjectiveFocalLengthMax -r--r--r-- ./control/terminal/camera/default/wObjectiveFocalLengthMin -r--r--r-- ./control/terminal/camera/default/iTerminal -r--r--r-- ./control/terminal/camera/default/bAssocTerminal -r--r--r-- ./control/terminal/camera/default/wTerminalType -r--r--r-- ./control/terminal/camera/default/bTerminalID drwxr-xr-x ./control/processing drwxr-xr-x ./control/processing/default -r--r--r-- ./control/processing/default/iProcessing -r--r--r-- ./control/processing/default/bmControls -r--r--r-- ./control/processing/default/wMaxMultiplier -r--r--r-- ./control/processing/default/bSourceID -r--r--r-- ./control/processing/default/bUnitID drwxr-xr-x ./control/header drwxr-xr-x ./control/header/h -rw-r--r-- ./control/header/h/dwClockFrequency -rw-r--r-- ./control/header/h/bcdUVC -rw-r--r-- ./streaming_maxburst -rw-r--r-- ./streaming_maxpacket -rw-r--r-- ./streaming_interval BACKWARD COMPATIBILITY
[PATCH 1/2] usb: gadget: f_uvc: rename a macro to avoid conflicts
When configfs is integrated, CONFIGFS_ATTR_STRUCT and CONFIGFS_ATTR_OPS macros should be used, but the latter expects that tere is a to_f_uvc_opts function accepting a config_item, whereas the macro being changed can be applied to a different type of argument. Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com --- drivers/usb/gadget/function/f_uvc.c | 6 +++--- drivers/usb/gadget/function/u_uvc.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index e126439..71e5935 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -571,7 +571,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) INFO(cdev, uvc_function_bind\n); - opts = to_f_uvc_opts(f-fi); + opts = fi_to_f_uvc_opts(f-fi); /* Sanity check the streaming endpoint module parameters. */ opts-streaming_interval = clamp(opts-streaming_interval, 1U, 16U); @@ -732,7 +732,7 @@ error: static void uvc_free_inst(struct usb_function_instance *f) { - struct f_uvc_opts *opts = to_f_uvc_opts(f); + struct f_uvc_opts *opts = fi_to_f_uvc_opts(f); kfree(opts); } @@ -784,7 +784,7 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi) return ERR_PTR(-ENOMEM); uvc-state = UVC_STATE_DISCONNECTED; - opts = to_f_uvc_opts(fi); + opts = fi_to_f_uvc_opts(fi); uvc-desc.fs_control = opts-fs_control; uvc-desc.ss_control = opts-ss_control; diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h index 2a8dfdf..c0706a3 100644 --- a/drivers/usb/gadget/function/u_uvc.h +++ b/drivers/usb/gadget/function/u_uvc.h @@ -18,7 +18,7 @@ #include linux/usb/composite.h -#define to_f_uvc_opts(f) container_of(f, struct f_uvc_opts, func_inst) +#define fi_to_f_uvc_opts(f)container_of(f, struct f_uvc_opts, func_inst) struct f_uvc_opts { struct usb_function_instancefunc_inst; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: gadget: uvc: configfs support in uvc function
Add support for using the uvc function as a component of USB gadgets composed with configfs. Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com --- Documentation/ABI/testing/configfs-usb-gadget-uvc | 264 +++ drivers/usb/gadget/Kconfig| 11 + drivers/usb/gadget/function/Makefile |2 +- drivers/usb/gadget/function/f_uvc.c | 122 +- drivers/usb/gadget/function/u_uvc.h | 23 + drivers/usb/gadget/function/uvc_configfs.c| 2439 + drivers/usb/gadget/function/uvc_configfs.h| 23 + 7 files changed, 2879 insertions(+), 5 deletions(-) create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-uvc create mode 100644 drivers/usb/gadget/function/uvc_configfs.c create mode 100644 drivers/usb/gadget/function/uvc_configfs.h diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc new file mode 100644 index 000..8f2825d --- /dev/null +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -0,0 +1,264 @@ +What: /config/usb-gadget/gadget/functions/uvc.name +Date: Nov 2014 +KernelVersion: 3.18 +Description: UVC function directory + + streaming_maxburst - 0..15 (ss only) + streaming_maxpacket - 1..1023 (fs), 1..3072 (hs/ss) + streaming_interval - 1..16 + +What: /config/usb-gadget/gadget/functions/uvc.name/control +Date: Nov 2014 +KernelVersion: 3.18 +Description: Control descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/class +Date: Nov 2014 +KernelVersion: 3.18 +Description: Class descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/class/ss +Date: Nov 2014 +KernelVersion: 3.18 +Description: Super speed control class descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/class/fs +Date: Nov 2014 +KernelVersion: 3.18 +Description: Full speed control class descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal +Date: Nov 2014 +KernelVersion: 3.18 +Description: Terminal descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/output +Date: Nov 2014 +KernelVersion: 3.18 +Description: Output terminal descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/output/default +Date: Nov 2014 +KernelVersion: 3.18 +Description: Default output terminal descriptors + + All attributes read only: + iTerminal - index of string descriptor + bSourceID - id of the terminal to which this terminal + is connected + bAssocTerminal - id of the input terminal to which this output + terminal is associated + wTerminalType - terminal type + bTerminalID - a non-zero id of this terminal + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/camera +Date: Nov 2014 +KernelVersion: 3.18 +Description: Camera terminal descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/camera/default +Date: Nov 2014 +KernelVersion: 3.18 +Description: Default camera terminal descriptors + + All attributes read only: + bmControls - bitmap specifying which controls are + supported for the video stream + wOcularFocalLength - the value of Locular + wObjectiveFocalLengthMax- the value of Lmin + wObjectiveFocalLengthMin- the value of Lmax + iTerminal - index of string descriptor + bAssocTerminal - id of the output terminal to which + this terminal is connected + wTerminalType - terminal type + bTerminalID - a non-zero id of this terminal + +What: /config/usb-gadget/gadget/functions/uvc.name/control/processing +Date: Nov 2014 +KernelVersion: 3.18 +Description: Processing unit descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/processing/default +Date: Nov 2014 +KernelVersion: 3.18 +Description: Default processing unit descriptors + + All attributes read only: + iProcessing - index of string descriptor + bmControls - bitmap specifying which controls are + supported for the video stream + wMaxMultiplier - maximum digital magnification x100 + bSourceID - id of the terminal to which this unit is + connected +
Re: [resend PATCH 1/3] ACPI / platform: provide default DMA mask
On Wednesday, September 24, 2014 11:00:37 AM Heikki Krogerus wrote: Most devices are configured for 32-bit DMA addresses. Setting the mask to 32-bit here removes the need for the drivers to do it separately. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: Rafael J. Wysocki r...@rjwysocki.net ACK --- drivers/acpi/acpi_platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 2bf9082..8d099e6 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -16,6 +16,7 @@ #include linux/err.h #include linux/kernel.h #include linux/module.h +#include linux/dma-mapping.h #include linux/platform_device.h #include internal.h @@ -102,6 +103,7 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev) pdevinfo.res = resources; pdevinfo.num_res = count; pdevinfo.acpi_node.companion = adev; + pdevinfo.dma_mask = DMA_BIT_MASK(32); pdev = platform_device_register_full(pdevinfo); if (IS_ERR(pdev)) dev_err(adev-dev, platform device creation failed: %ld\n, -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote: On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote: snip + * dln2_dev.mod_rx_slots and then the echo header field to index the + * slots field and find the receive context for a particular + * request. + */ +struct dln2_mod_rx_slots { + /* RX slots bitmap */ + unsigned long bmap; + + /* used to wait for a free RX slot */ + wait_queue_head_t wq; + + /* used to wait for an RX operation to complete */ + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS]; + + /* device has been disconnected */ + bool disconnected; This belongs in the dln2_dev struct. I think you're overcomplicating the disconnect handling by intertwining it with your slots. Add a lock, an active-transfer counter, a disconnected flag, and a wait queue to struct dln2_dev. I agree that disconnected is better suited in dln2_dev. However, I don't think that we need the active-transfer counter and a new wait queue. We can simply use the existing waiting queues and the implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still waiting for I/O. snip + +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd, + const void *obuf, unsigned obuf_len, + void *ibuf, unsigned *ibuf_len) +{ + int ret = 0; + u16 result; + int rx_slot; + unsigned long flags; + struct dln2_response *rsp; + struct dln2_rx_context *rxc; + struct device *dev; + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000; + struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[handle]; + Check the disconnected flag before incrementing the transfer count (while holding the device lock) here. Then decrement counter before returning and wake up an waiters if disconnected below. That is sufficient to implement wait-until-io-has-completed. Anything else you do below and in the helper functions is only to speed things up at disconnect (and can be done without locking the device). + rx_slot = alloc_rx_slot(rxs); + if (rx_slot 0) + return rx_slot; + + dev = dln2-interface-dev; + + ret = dln2_send_wait(dln2, handle, cmd, rx_slot, obuf, obuf_len); + if (ret 0) { + free_rx_slot(dln2, rxs, rx_slot); goto out_free_rx_slot + dev_err(dev, USB write failed: %d, ret); + return ret; + } + + rxc = rxs-slots[rx_slot]; + + ret = wait_for_completion_interruptible_timeout(rxc-done, timeout); + if (ret = 0) { + if (!ret) + ret = -ETIMEDOUT; + goto out_free_rx_slot; + } + + spin_lock_irqsave(rxs-lock, flags); + + if (!rxc-urb) { Just check the disconnected flag directly here. Locking not needed (see below). I think we need the check here for the case when we cancel the completion and no response has been received yet. In that case rx-urb will be NULL (even if we remove the rx-urb = NULL statement in dln2_stop). + ret = -ECONNRESET; -ENODEV OK. + spin_unlock_irqrestore(rxs-lock, flags); + goto out_free_rx_slot; + } + + /* if we got here we know that the response header has been checked */ + rsp = rxc-urb-transfer_buffer; + + spin_unlock_irqrestore(rxs-lock, flags); + + +static void dln2_stop(struct dln2_dev *dln2) +{ + int i, j; + + for (i = 0; i DLN2_HANDLES; i++) { + struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[i]; + unsigned long flags; + + spin_lock_irqsave(rxs-lock, flags); + + /* fail further alloc_rx_slot calls */ + rxs-disconnected = true; + + /* cancel all response waiters */ + for (j = 0; j DLN2_MAX_RX_SLOTS; j++) { + struct dln2_rx_context *rxc = rxs-slots[j]; + + if (rxc-connected) { + rxc-urb = NULL; Don't overload the meaning of urb. Check the disconnected flag were needed. Also what would prevent a racing completion handler from setting rxc-urb again when you release the lock below? That should not be an issue, in that case _dln2_transfer will complete successfully. But you are right, we don't need to set rxc-urb to NULL here. + complete(rxc-done); + } + } + spin_unlock_irqrestore(rxs-lock, flags); + + /* wait for callers to release all rx_slots */ + wait_event(rxs-wq, dln2_all_rx_slots_free(rxs)); + } +} + +static void dln2_disconnect(struct usb_interface *interface) +{ + struct dln2_dev *dln2 = usb_get_intfdata(interface); + First set the disconnected flag directly here to prevent any new transfers from starting. + dln2_stop(dln2); Then do all the completions (to speed things up
Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote: On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote: On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote: snip + * dln2_dev.mod_rx_slots and then the echo header field to index the + * slots field and find the receive context for a particular + * request. + */ +struct dln2_mod_rx_slots { + /* RX slots bitmap */ + unsigned long bmap; + + /* used to wait for a free RX slot */ + wait_queue_head_t wq; + + /* used to wait for an RX operation to complete */ + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS]; + + /* device has been disconnected */ + bool disconnected; This belongs in the dln2_dev struct. I think you're overcomplicating the disconnect handling by intertwining it with your slots. Add a lock, an active-transfer counter, a disconnected flag, and a wait queue to struct dln2_dev. I agree that disconnected is better suited in dln2_dev. However, I don't think that we need the active-transfer counter and a new wait queue. We can simply use the existing waiting queues and the implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still waiting for I/O. Just because you can reuse them doesn't mean it's a good idea. By separating a generic disconnect solution from your custom slot implementation we get something that is way easier to verify for correctness and that could also be reused in other drivers. snip + +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd, + const void *obuf, unsigned obuf_len, + void *ibuf, unsigned *ibuf_len) +{ + int ret = 0; + u16 result; + int rx_slot; + unsigned long flags; + struct dln2_response *rsp; + struct dln2_rx_context *rxc; + struct device *dev; + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000; + struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[handle]; + Check the disconnected flag before incrementing the transfer count (while holding the device lock) here. Then decrement counter before returning and wake up an waiters if disconnected below. That is sufficient to implement wait-until-io-has-completed. Anything else you do below and in the helper functions is only to speed things up at disconnect (and can be done without locking the device). + rx_slot = alloc_rx_slot(rxs); + if (rx_slot 0) + return rx_slot; + + dev = dln2-interface-dev; + + ret = dln2_send_wait(dln2, handle, cmd, rx_slot, obuf, obuf_len); + if (ret 0) { + free_rx_slot(dln2, rxs, rx_slot); goto out_free_rx_slot + dev_err(dev, USB write failed: %d, ret); + return ret; + } + + rxc = rxs-slots[rx_slot]; + + ret = wait_for_completion_interruptible_timeout(rxc-done, timeout); + if (ret = 0) { + if (!ret) + ret = -ETIMEDOUT; + goto out_free_rx_slot; + } + + spin_lock_irqsave(rxs-lock, flags); + + if (!rxc-urb) { Just check the disconnected flag directly here. Locking not needed (see below). I think we need the check here for the case when we cancel the completion and no response has been received yet. In that case rx-urb will be NULL (even if we remove the rx-urb = NULL statement in dln2_stop). But the disconnect flag will also be set and makes it more obvious what is going on. [...] +static void dln2_disconnect(struct usb_interface *interface) +{ + struct dln2_dev *dln2 = usb_get_intfdata(interface); + First set the disconnected flag directly here to prevent any new transfers from starting. + dln2_stop(dln2); Then do all the completions (to speed things up somewhat). Then wait for the transfer counter to reach 0. + mfd_remove_devices(interface-dev); + dln2_free(dln2); +} + As I mentioned above, I don't think we need the transfer counter, we can rely on the slots bitmap. Yes, a counter is simpler but it also requires adding a new waiting queue and a new lock. That's really not a big deal. See above. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_rndis: fix usb_interface_descriptor for rndis
On 2014-09-24 20:12, Heiko Schocher wrote: Hello Lars, Am 24.09.2014 14:25, schrieb Lars Melin: On 2014-09-24 13:48, Heiko Schocher wrote: use the values for RNDIS over Ethernet as defined in http://www.usb.org/developers/defined_class (search for RDNIS): - baseclass: 0xef (miscellaneous) - subclass: 0x04 - protocol: 0x01 That is usb class, it is not the same thing as communication device class. --- a/include/uapi/linux/usb/cdc.h +++ b/include/uapi/linux/usb/cdc.h @@ -12,6 +12,7 @@ #include linux/types.h #define USB_CDC_SUBCLASS_ACM 0x02 +#define USB_CDC_SUBCLASS_RNDIS 0x04 No, no, no. There is no CDC_SUBCLASS_RNDIS and you can not define one over an already used cdc subclass number, 0x04 is Multi-Channel Control Model Ah, ok, so I have to define this values in a new header file, as there is no current file for the USB_CLASS_MISC defines? Or is there a proper place for them? BTW: where do I find the cdc subclass number, 0x04 is Multi-Channel Control Model define? bye, Heiko You can still find the original specification usbcdc11.pdf on the net if you google for it, it has been pulled from usb.org where you could download it until a few years ago. It is old but covers a lot of what you need to know. Linux has afaik only the cdc.h definition file, everything else is coded by class/subclass in respectively drivers when needed. 02/02/ff or e0/01/03 are the most common interface attribute for rndis, both of them together with a data interface with attributes 0a/00/00. Please check the whitelisting in drivers/net/usb/rndis_host.c and also blacklistings in other net drivers under the same path, it should give you an idea how to bind an interface to a specific driver by interface attributes and/or usb vid:pid. You should be able to do the same for your particular device. -- 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: [resend PATCH 3/3] usb: dwc3: add ACPI support
On Wed, Sep 24, 2014 at 11:00:39AM +0300, Heikki Krogerus wrote: Adds ACPI ID used on newer Intel SoCs. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com --- drivers/usb/dwc3/core.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index d08cac5..c2cf2d8 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -32,6 +32,7 @@ #include linux/delay.h #include linux/dma-mapping.h #include linux/of.h +#include linux/acpi.h #include linux/usb/ch9.h #include linux/usb/gadget.h @@ -960,12 +961,21 @@ static const struct of_device_id of_dwc3_match[] = { MODULE_DEVICE_TABLE(of, of_dwc3_match); #endif +#ifdef CONFIG_ACPI +static const struct acpi_device_id dwc3_acpi_match[] = { + { 808622B7, 0 }, can you just provide a macro for this string ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
Hi, On Tue, Sep 23, 2014 at 10:09:22AM +0200, Michal Sojka wrote: +/** + * usb_gadget_giveback_request - give the request back to the gadget layer + * Context: in_interrupt() + * + * This is called by device controller drivers in order to return the + * completed request back to the gadget layer. + */ +void usb_gadget_giveback_request(struct usb_ep *ep, + struct usb_request *req) +{ + if (likely(req-complete)) + req-complete(ep, req); + else + pr_err(%s : req-complete must not be NULL\n, __func__); let it Oops. We require -complete to be valid, if there's any gadget driver not setting -complete, it deserves to oops so we can the error. The Oops was there before, but I removed it because greg k-h didn't want it. See http://marc.info/?l=linux-usbm=140917381611947w=2. Do you still want the oops here? you don't need a BUG_ON(), just call req-complete() directly without any checks, if it's not valid, it _will_ oops. -complete() is mandatory and anybody sending requests without complete deserve to oops. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
On Tue, 23 Sep 2014, Michal Sojka wrote: +/** + * usb_gadget_giveback_request - give the request back to the gadget layer + * Context: in_interrupt() + * + * This is called by device controller drivers in order to return the + * completed request back to the gadget layer. + */ +void usb_gadget_giveback_request(struct usb_ep *ep, + struct usb_request *req) +{ + if (likely(req-complete)) + req-complete(ep, req); + else + pr_err(%s : req-complete must not be NULL\n, __func__); let it Oops. We require -complete to be valid, if there's any gadget driver not setting -complete, it deserves to oops so we can the error. The Oops was there before, but I removed it because greg k-h didn't want it. See http://marc.info/?l=linux-usbm=140917381611947w=2. Do you still want the oops here? Greg didn't want you to add a BUG() statement, whereas Felipe wants you to leave out the if test and pr_err(). Just omit both, and everyone will be satisfied. 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: g_mass_storage bug ?
On Tue, 23 Sep 2014, Felipe Balbi wrote: Hi Alan, Need your help looking over this detail here. When I run g_mass_storage with stall=0 everything works fine. As soon as I remove it, things go bonkers. Looking at the bulk-only spec, I see: 6.7.2 Hi - Host expects to receive data from the device [ ... ] The specific device requirements are: 1. The device shall receive a CBW. 2. When the CBW is valid and meaningful, then: . The device shall attempt the command. . [Case (6)] If the device intends to send dCBWDataTransferLength, then: The device shall send dCBWDataTransferLength bytes of data. The device shall set bCSWStatus to 00h or 01h. The device shall set dCSWDataResidue to zero. Case (6) is when Hi == Di, looking at my logs, I have: 720 [ 286.843965] SCSI CDB: 1a 00 3f 00 c0 00 721 [ 286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); Dc=6, Di=192; Hc=6, Hi=192 722 [ 286.844018] g_mass_storage gadget: bulk-in set halt 723 [ 286.844034] g_mass_storage gadget: sending command-failure status 724 [ 286.844045] g_mass_storage gadget: sense data: SK x06, ASC x29, ASCQ x00; info x0 Isn't it wrong to halt in this condition ? No, it's correct. SK = 6 and ASC = 0x29 means Unit Attention, Reset Occurred. It occurs because this is the first command the gadget has received since starting up, which certainly is a form of reset. In effect, this is how the device tells the host that it was just powered on. So the case you should be looking at is Case 5: Hi Di. I realize the debugging output indicates Di = 192, but that line gets written before the driver checks for a Unit Attention condition. (The line is written near the start of check_command() in f_mass_storage.c, and the Unit Attention check is near the end of the same function.) The printed Di value indicates how much data the gadget thinks the command is asking for, not the amount of data the gadget actually intends to send. Therefore stalling is appropriate. Why it causes it problem for your system is a different matter. Is your UDC hardware capable of halting bulk endpoints? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC] storage: Simplify condition to reject bad target in usb_stor_control_thread()
On Wed, 24 Sep 2014, Mark Knibbs wrote: Hi, This follows on from something I mentioned in my recent post [PATCH] storage: Don't scan target 7 for SCM USB-SCSI converters. In drivers/usb/storage/usb.c, usb_stor_control_thread(): /* reject if target != 0 or if LUN is higher than * the maximum known LUN */ else if (us-srb-device-id !(us-fflags US_FL_SCM_MULT_TARG)) { usb_stor_dbg(us, Bad target number (%d:%llu)\n, us-srb-device-id, us-srb-device-lun); us-srb-result = DID_BAD_TARGET 16; } else if (us-srb-device-lun us-max_lun) { usb_stor_dbg(us, Bad LUN (%d:%llu)\n, ... The condition (us-srb-device-id !(us-fflags US_FL_SCM_MULT_TARG)) means: - If not SCM_MULT_TARG, reject if us-srb-device-id is non-zero. That works for the usual case. - If SCM_MULT_TARG, never reject. Hmm... Would it be better/clearer to change the condition to simply (us-srb-device-id = host-max_id) Then the command is rejected if us-srb-device-id is too large, and works for the SCM_MULT_TARG case too. (host-max_id is 1 for the non-SCM_MULT_TARG case.) Plus the condition matches nicely with the later LUN check. Untested patch below. Signed-off-by: Mark Knibbs ma...@clara.co.uk diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig linux-3.17-rc6/drivers/usb/storage/usb.c --- linux-3.17-rc6/drivers/usb/storage/usb.c.orig 2014-09-21 23:43:02.0 +0100 +++ linux-3.17-rc6/drivers/usb/storage/usb.c 2014-09-24 13:09:55.0 +0100 @@ -342,11 +342,10 @@ static int usb_stor_control_thread(void us-srb-result = DID_ERROR 16; } - /* reject if target != 0 or if LUN is higher than + /* reject if target is invalid or if LUN is higher than * the maximum known LUN */ - else if (us-srb-device-id - !(us-fflags US_FL_SCM_MULT_TARG)) { + else if (us-srb-device-id = host-max_id) { usb_stor_dbg(us, Bad target number (%d:%llu)\n, us-srb-device-id, us-srb-device-lun); That's probably okay. As far as I know, the SCSI layer doesn't generate commands for targets with id = host-max_id, so this entire test may be unnecessary. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Sep 24, 2014 at 05:54:15PM +0300, Octavian Purdila wrote: On Wed, Sep 24, 2014 at 4:54 PM, Johan Hovold jo...@kernel.org wrote: On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote: On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote: On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote: snip + * dln2_dev.mod_rx_slots and then the echo header field to index the + * slots field and find the receive context for a particular + * request. + */ +struct dln2_mod_rx_slots { + /* RX slots bitmap */ + unsigned long bmap; + + /* used to wait for a free RX slot */ + wait_queue_head_t wq; + + /* used to wait for an RX operation to complete */ + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS]; + + /* device has been disconnected */ + bool disconnected; This belongs in the dln2_dev struct. I think you're overcomplicating the disconnect handling by intertwining it with your slots. Add a lock, an active-transfer counter, a disconnected flag, and a wait queue to struct dln2_dev. I agree that disconnected is better suited in dln2_dev. However, I don't think that we need the active-transfer counter and a new wait queue. We can simply use the existing waiting queues and the implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still waiting for I/O. Just because you can reuse them doesn't mean it's a good idea. By separating a generic disconnect solution from your custom slot implementation we get something that is way easier to verify for correctness and that could also be reused in other drivers. Maybe I miss-understood what you are proposing, let me try to summarize it to see if I got it right. You are suggesting to add a counter, increment it in alloc_rx_slot(), decrement it in free_rx_slot(). No increment it at the start of _dln2_transfer, and decrement it before returning from that function. Then add a new waitqueue in dln2_dev and in free_rx_slot() wake it up while in disconnect do a wait_event() on it and check for the counter. Where you also wake the disconnect (or wait-until-sent) wait queue. Also, alloc_rx_slot() should fail if the disconnect flag is set. That is not required, but you can bail out early after alloc_rx_slot if the disconnect flag is set (no locking). In this case we are still coupled to the slots implementation, in the sense that you would need to understand the slots implementation to understand how the disconnect works. We are also doing two wake-up operations which I find redundant and which does not add much value in clarity (since we still need to wake-up all completions for each handle). I do agree that using a counter instead of checking the bitmaps is cleaner though. You only need to the wake up if disconnected is set when returning from _dln2_transfer. Sure, the optimisation bit -- to abort any ongoing transfer -- still requires some insight into the slot implementation. But this way everything disconnect related (correctness-wise) is isolated to _dln2_transfer and dln2_disconnect. 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: g_mass_storage bug ?
On Wed, 24 Sep 2014, Alan Stern wrote: Case (6) is when Hi == Di, looking at my logs, I have: 720 [ 286.843965] SCSI CDB: 1a 00 3f 00 c0 00 721 [ 286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); Dc=6, Di=192; Hc=6, Hi=192 722 [ 286.844018] g_mass_storage gadget: bulk-in set halt 723 [ 286.844034] g_mass_storage gadget: sending command-failure status 724 [ 286.844045] g_mass_storage gadget: sense data: SK x06, ASC x29, ASCQ x00; info x0 Isn't it wrong to halt in this condition ? No, it's correct. SK = 6 and ASC = 0x29 means Unit Attention, Reset Occurred. It occurs because this is the first command the gadget has received since starting up, which certainly is a form of reset. In effect, this is how the device tells the host that it was just powered on. Actually, looking at your log again, it seems more like this followed a genuine reset, not a power-on. Regardless, it's still appropriate. 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: g_mass_storage bug ?
Hi, On Wed, Sep 24, 2014 at 11:05:58AM -0400, Alan Stern wrote: On Tue, 23 Sep 2014, Felipe Balbi wrote: Hi Alan, Need your help looking over this detail here. When I run g_mass_storage with stall=0 everything works fine. As soon as I remove it, things go bonkers. Looking at the bulk-only spec, I see: 6.7.2 Hi - Host expects to receive data from the device [ ... ] The specific device requirements are: 1. The device shall receive a CBW. 2. When the CBW is valid and meaningful, then: . The device shall attempt the command. . [Case (6)] If the device intends to send dCBWDataTransferLength, then: The device shall send dCBWDataTransferLength bytes of data. The device shall set bCSWStatus to 00h or 01h. The device shall set dCSWDataResidue to zero. Case (6) is when Hi == Di, looking at my logs, I have: 720 [ 286.843965] SCSI CDB: 1a 00 3f 00 c0 00 721 [ 286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); Dc=6, Di=192; Hc=6, Hi=192 722 [ 286.844018] g_mass_storage gadget: bulk-in set halt 723 [ 286.844034] g_mass_storage gadget: sending command-failure status 724 [ 286.844045] g_mass_storage gadget: sense data: SK x06, ASC x29, ASCQ x00; info x0 Isn't it wrong to halt in this condition ? No, it's correct. SK = 6 and ASC = 0x29 means Unit Attention, Reset Occurred. It occurs because this is the first command the gadget has received since starting up, which certainly is a form of reset. In effect, this is how the device tells the host that it was just powered on. So the case you should be looking at is Case 5: Hi Di. I realize the debugging output indicates Di = 192, but that line gets written before the driver checks for a Unit Attention condition. (The line is written near the start of check_command() in f_mass_storage.c, and the Unit Attention check is near the end of the same function.) The printed Di value indicates how much data the gadget thinks the command is asking for, not the amount of data the gadget actually intends to send. Therefore stalling is appropriate. Why it causes it problem for your system is a different matter. Is your UDC hardware capable of halting bulk endpoints? yeah, that part is just fine; I also verified with my sniffer that bulk halt is happening as it should. The problem, however, is that after that halt condition happens, host (same board has xhci too, Linux 3.17-rc5) issues a reset recovery and it all happens again. I stay in that loop for a while until it finally enumerates correctly, but when I try to write to the block device with dd, it resets again. I'll try the same test against my desktop (3.16.1) and a Mac OS X I have here, and see if the same behavior shows up. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Sep 24, 2014 at 6:07 PM, Johan Hovold jo...@kernel.org wrote: On Wed, Sep 24, 2014 at 05:54:15PM +0300, Octavian Purdila wrote: On Wed, Sep 24, 2014 at 4:54 PM, Johan Hovold jo...@kernel.org wrote: On Wed, Sep 24, 2014 at 04:36:22PM +0300, Octavian Purdila wrote: On Wed, Sep 24, 2014 at 1:48 PM, Johan Hovold jo...@kernel.org wrote: On Fri, Sep 19, 2014 at 11:22:42PM +0300, Octavian Purdila wrote: snip + * dln2_dev.mod_rx_slots and then the echo header field to index the + * slots field and find the receive context for a particular + * request. + */ +struct dln2_mod_rx_slots { + /* RX slots bitmap */ + unsigned long bmap; + + /* used to wait for a free RX slot */ + wait_queue_head_t wq; + + /* used to wait for an RX operation to complete */ + struct dln2_rx_context slots[DLN2_MAX_RX_SLOTS]; + + /* device has been disconnected */ + bool disconnected; This belongs in the dln2_dev struct. I think you're overcomplicating the disconnect handling by intertwining it with your slots. Add a lock, an active-transfer counter, a disconnected flag, and a wait queue to struct dln2_dev. I agree that disconnected is better suited in dln2_dev. However, I don't think that we need the active-transfer counter and a new wait queue. We can simply use the existing waiting queues and the implicit alloc_rx_slot()/free_rx_slot() calls to see if we are still waiting for I/O. Just because you can reuse them doesn't mean it's a good idea. By separating a generic disconnect solution from your custom slot implementation we get something that is way easier to verify for correctness and that could also be reused in other drivers. Maybe I miss-understood what you are proposing, let me try to summarize it to see if I got it right. You are suggesting to add a counter, increment it in alloc_rx_slot(), decrement it in free_rx_slot(). No increment it at the start of _dln2_transfer, and decrement it before returning from that function. Then add a new waitqueue in dln2_dev and in free_rx_slot() wake it up while in disconnect do a wait_event() on it and check for the counter. Where you also wake the disconnect (or wait-until-sent) wait queue. Also, alloc_rx_slot() should fail if the disconnect flag is set. That is not required, but you can bail out early after alloc_rx_slot if the disconnect flag is set (no locking). In this case we are still coupled to the slots implementation, in the sense that you would need to understand the slots implementation to understand how the disconnect works. We are also doing two wake-up operations which I find redundant and which does not add much value in clarity (since we still need to wake-up all completions for each handle). I do agree that using a counter instead of checking the bitmaps is cleaner though. You only need to the wake up if disconnected is set when returning from _dln2_transfer. Sure, the optimisation bit -- to abort any ongoing transfer -- still requires some insight into the slot implementation. But this way everything disconnect related (correctness-wise) is isolated to _dln2_transfer and dln2_disconnect. OK, I see what you mean now. I'll give it a try and will follow up with a new patch set. Oh, and thanks for yet another review, it has been very educational to me just like the rest :) -- 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: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 11:17:18AM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Alan Stern wrote: Case (6) is when Hi == Di, looking at my logs, I have: 720 [ 286.843965] SCSI CDB: 1a 00 3f 00 c0 00 721 [ 286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); Dc=6, Di=192; Hc=6, Hi=192 722 [ 286.844018] g_mass_storage gadget: bulk-in set halt 723 [ 286.844034] g_mass_storage gadget: sending command-failure status 724 [ 286.844045] g_mass_storage gadget: sense data: SK x06, ASC x29, ASCQ x00; info x0 Isn't it wrong to halt in this condition ? No, it's correct. SK = 6 and ASC = 0x29 means Unit Attention, Reset Occurred. It occurs because this is the first command the gadget has received since starting up, which certainly is a form of reset. In effect, this is how the device tells the host that it was just powered on. Actually, looking at your log again, it seems more like this followed a genuine reset, not a power-on. Regardless, it's still appropriate. It seems to work better against my v3.16.1 desktop and Mac OS X then it does against v3.17-rc5 (running on the development board). I'll try using a USB stick attached to the board. -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 10:30:17AM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 11:17:18AM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Alan Stern wrote: Case (6) is when Hi == Di, looking at my logs, I have: 720 [ 286.843965] SCSI CDB: 1a 00 3f 00 c0 00 721 [ 286.844000] g_mass_storage gadget: SCSI command: MODE SENSE(6); Dc=6, Di=192; Hc=6, Hi=192 722 [ 286.844018] g_mass_storage gadget: bulk-in set halt 723 [ 286.844034] g_mass_storage gadget: sending command-failure status 724 [ 286.844045] g_mass_storage gadget: sense data: SK x06, ASC x29, ASCQ x00; info x0 Isn't it wrong to halt in this condition ? No, it's correct. SK = 6 and ASC = 0x29 means Unit Attention, Reset Occurred. It occurs because this is the first command the gadget has received since starting up, which certainly is a form of reset. In effect, this is how the device tells the host that it was just powered on. Actually, looking at your log again, it seems more like this followed a genuine reset, not a power-on. Regardless, it's still appropriate. It seems to work better against my v3.16.1 desktop and Mac OS X then it does against v3.17-rc5 (running on the development board). I'll try using a USB stick attached to the board. alright, USB stick attached to the board behaves perfectly, but then again I don't see any stalls from that device. Any chance I can persuade you into running a similar test with your net2272/2280 UDC ? I'm using a 500MiB tmpfs as storage backend, then just connecting to an XHCI host and looking at dmesg. A dd also triggers a few resets with v3.17-rc5 which don't happen on 3.16.1. cheers -- balbi signature.asc Description: Digital signature
Re: Poor performance with USB 1.1 drive connected to USB 3.0 port
On Wed, 24 Sep 2014, Mark Knibbs wrote: Hi, I did some benchmarks to check the maximum transfer rate of a USB-to-SCSI converter. The converter is USB 1.1, so limited to the 12Mbps full speed rate. The performance when connected to a USB 3.0 port is significantly worse than when connected to a USB 2.0 port, about 26.5% slower (0.63MB/sec vs 0.86MB/sec). The USB 3.0 port is provided by an ExpressCard which has a Renesas controller. It doesn't seem to be defective because I can get 138MB/sec from a USB 3.0 hard disk. lspci shows 05:00.0 USB Controller: Renesas Technology Corp. Device 0015 (rev 02) I didn't test with an unmodified mainline kernel, but the issue is present with both Ubuntu kernel 3.0 and Debian 3.14 (booted from Kali Linux live DVD). I used sg_rbuf from the sg3-utils package. That issues READ BUFFER commands instead of reading data from disk, so the result should be closer to the maximum achievable. The USB-SCSI converter was a Newer Technology uSCSI connected to an HP 2600fx MO drive. In a USB 2.0 port: # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 242.800241 secs, 0.86 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 242.800230 secs, 0.86 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) In a USB 3.0 port: # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 330.311337 secs, 0.63 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) # sg_rbuf --buffer=524288 -v -t -q /dev/sg3 Read buffer cdb: 3c 03 00 00 00 00 00 00 04 00 READ BUFFER reports: buffer capacity=983040, offset boundary=0 time to read data from buffer was 330.388556 secs, 0.63 MB/sec Read 200 MiB (actual: 209715200 bytes), buffer size=512 KiB (524288 bytes) By comparison, I checked the transfer rate in native Windows (Vista) with the WinSAT program. That reported 0.66MB/sec for both USB 2.0 and 3.0 ports. (WinSAT reads data from disk, probably in smaller chunks than 512KB. There's no sg_rbuf executable in the Windows sg3_utils archive.) Any ideas what the reason for the discrepancy might be? Can anyone else reproduce it? I guess USB 1.1-only devices aren't that common nowadays. You might be able to get more detailed timings if you collect usbmon traces of the two tests. They won't answer your question but they may help point in a particular direction. The xhci-hcd driver has been under active development. For the best results, you really should use the most up-to-date version of the kernel. 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] drivers/usb/host/ehci-xilinx-of.c: Include linux/of_irq.h to avoid compiling error
On Mon, 22 Sep 2014, Michal Simek wrote: Hi Alan and Greg, On 09/20/2014 06:19 AM, Chen Gang wrote: Hello Maintainers: Please help check this patch, when you have time. Thanks. On 09/08/2014 01:20 PM, Michal Simek wrote: On 09/03/2014 05:50 PM, Chen Gang wrote: Need include it for irq_of_parse_and_map(), the related error with allmodconfig under microblaze: drivers/usb/host/ehci-xilinx-of.c: In function �ehci_hcd_xilinx_of_probe�: drivers/usb/host/ehci-xilinx-of.c:156:2: error: implicit declaration of function �irq_of_parse_and_map� [-Werror=implicit-function-declaration] irq = irq_of_parse_and_map(dn, 0); ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- drivers/usb/host/ehci-xilinx-of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/ehci-xilinx-of.c b/drivers/usb/host/ehci-xilinx-of.c index fe57710..a232836 100644 --- a/drivers/usb/host/ehci-xilinx-of.c +++ b/drivers/usb/host/ehci-xilinx-of.c @@ -31,6 +31,7 @@ #include linux/of.h #include linux/of_platform.h #include linux/of_address.h +#include linux/of_irq.h /** * ehci_xilinx_port_handed_over - hand the port out if failed to enable it Acked-by: Michal Simek mon...@monstr.eu Alan: Can you please add this patch to your queue? Greg: If Alan is not maintaining this part of kernel, is this patch in your queue? I have also not a problem to add this patch through my microblaze tree but I think it will be much better to use any USB tree. Greg should be able to accept a trivial patch like this one directly. 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: g_mass_storage bug ?
On Wed, 24 Sep 2014, Felipe Balbi wrote: Therefore stalling is appropriate. Why it causes it problem for your system is a different matter. Is your UDC hardware capable of halting bulk endpoints? yeah, that part is just fine; I also verified with my sniffer that bulk halt is happening as it should. The problem, however, is that after that halt condition happens, host (same board has xhci too, Linux 3.17-rc5) issues a reset recovery It shouldn't; there's no reason for it to do so. Unless something else is going wrong on the host side. Have you tried capturing a usbmon trace on the host? and it all happens again. I stay in that loop for a while until it finally enumerates correctly, but when I try to write to the block device with dd, it resets again. I'll try the same test against my desktop (3.16.1) and a Mac OS X I have here, and see if the same behavior shows up. It seems to work better against my v3.16.1 desktop and Mac OS X then it does against v3.17-rc5 (running on the development board). Indicating that this really is a host-side problem. I'll try using a USB stick attached to the board. USB sticks probably won't generate the Unit Attention condition in response to a reset. They tend not to adhere terribly closely to the SCSI standard. 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: g_mass_storage bug ?
Hi, On Wed, Sep 24, 2014 at 11:40:54AM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: Therefore stalling is appropriate. Why it causes it problem for your system is a different matter. Is your UDC hardware capable of halting bulk endpoints? yeah, that part is just fine; I also verified with my sniffer that bulk halt is happening as it should. The problem, however, is that after that halt condition happens, host (same board has xhci too, Linux 3.17-rc5) issues a reset recovery It shouldn't; there's no reason for it to do so. Unless something else is going wrong on the host side. Have you tried capturing a usbmon trace on the host? I'll capture usbmon and send here shortly. and it all happens again. I stay in that loop for a while until it finally enumerates correctly, but when I try to write to the block device with dd, it resets again. I'll try the same test against my desktop (3.16.1) and a Mac OS X I have here, and see if the same behavior shows up. It seems to work better against my v3.16.1 desktop and Mac OS X then it does against v3.17-rc5 (running on the development board). Indicating that this really is a host-side problem. right. I'll try using a USB stick attached to the board. USB sticks probably won't generate the Unit Attention condition in response to a reset. They tend not to adhere terribly closely to the SCSI standard. yeah, I figured :-s -- balbi signature.asc Description: Digital signature
[PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend() which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend() sequentially. Then we do a tree wide update of current patterns which are present. As evident from log below this pattern is frequent in the kernel. This series can be found at git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git topic/pm_runtime_last_busy_and_autosuspend Fengguang's kbuild has tested it so it shouldn't break things for anyone. Barring one patch (explictyly mentioned in its changelog) rest are simple replacements. If all are okay, this should be merged thru PM tree as it depends on macro addition. Subhransu S. Prusty (1): PM: Add helper pm_runtime_last_busy_and_autosuspend() Vinod Koul (26): dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper drm/i915: use pm_runtime_last_busy_and_autosuspend helper drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper drm/radeon: use pm_runtime_last_busy_and_autosuspend helper vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper i2c: designware: use pm_runtime_last_busy_and_autosuspend helper i2c: omap: use pm_runtime_last_busy_and_autosuspend helper i2c: qup: use pm_runtime_last_busy_and_autosuspend helper mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper mei: use pm_runtime_last_busy_and_autosuspend helper mmc: use pm_runtime_last_busy_and_autosuspend helper mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper spi: orion: use pm_runtime_last_busy_and_autosuspend helper spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper spi: core: use pm_runtime_last_busy_and_autosuspend helper tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper video: fbdev: use pm_runtime_last_busy_and_autosuspend helper Documentation/power/runtime_pm.txt |4 ++ drivers/dma/ste_dma40.c | 30 - drivers/extcon/extcon-arizona.c |6 +-- drivers/gpu/drm/i915/intel_pm.c |3 +- drivers/gpu/drm/nouveau/nouveau_connector.c |3 +- drivers/gpu/drm/nouveau/nouveau_drm.c |9 +--- drivers/gpu/drm/radeon/radeon_connectors.c | 15 ++ drivers/gpu/drm/radeon/radeon_drv.c |5 +- drivers/gpu/drm/radeon/radeon_kms.c |6 +-- drivers/gpu/vga/vga_switcheroo.c|7 +-- drivers/i2c/busses/i2c-designware-core.c|3 +- drivers/i2c/busses/i2c-omap.c |6 +-- drivers/i2c/busses/i2c-qup.c|3 +- drivers/mfd/ab8500-gpadc.c |6 +-- drivers/mfd/arizona-irq.c |3 +- drivers/misc/mei/client.c | 12 ++ drivers/mmc/core/core.c |3 +- drivers/mmc/host/mmci.c | 12 ++ drivers/mmc/host/omap_hsmmc.c | 19 ++--- drivers/mmc/host/sdhci-pxav3.c |6 +-- drivers/mmc/host/sdhci.c|3 +- drivers/nfc/trf7970a.c |3 +- drivers/power/pm2301_charger.c |3 +- drivers/spi/spi-omap2-mcspi.c |9 +--- drivers/spi/spi-orion.c |3 +- drivers/spi/spi-ti-qspi.c |5 +- drivers/spi/spi.c |6 +-- drivers/tty/serial/omap-serial.c| 60 +-- drivers/usb/musb/omap2430.c |6 +-- drivers/video/fbdev/auo_k190x.c |9 +--- include/linux/pm_runtime.h |6 +++ 31 files changed, 97 insertions(+), 177 deletions(-) Thanks -- ~Vinod -- 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 26/27] usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
Use the new pm_runtime_last_busy_and_autosuspend helper instead of open coding the same code Signed-off-by: Vinod Koul vinod.k...@intel.com --- drivers/usb/musb/omap2430.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index d369bf1..17c6c49 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -293,8 +293,7 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue) musb-xceiv-last_event = USB_EVENT_NONE; if (musb-gadget_driver) { omap2430_musb_set_vbus(musb, 0); - pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); + pm_runtime_last_busy_and_autosuspend(dev); } if (data-interface_type == MUSB_INTERFACE_UTMI) @@ -321,8 +320,7 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work) pm_runtime_get_sync(dev); omap_musb_set_mailbox(glue); - pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); + pm_runtime_last_busy_and_autosuspend(dev); } static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci) -- 1.7.0.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: g_mass_storage bug ?
Hi, On Wed, Sep 24, 2014 at 11:20:31AM -0500, Felipe Balbi wrote: Therefore stalling is appropriate. Why it causes it problem for your system is a different matter. Is your UDC hardware capable of halting bulk endpoints? yeah, that part is just fine; I also verified with my sniffer that bulk halt is happening as it should. The problem, however, is that after that halt condition happens, host (same board has xhci too, Linux 3.17-rc5) issues a reset recovery It shouldn't; there's no reason for it to do so. Unless something else is going wrong on the host side. Have you tried capturing a usbmon trace on the host? I'll capture usbmon and send here shortly. here it is... Interesting part starts at line 73 (114 on this email) where the data transport received EPIPE (due to Stall). This time however, I was eventually able to talk to the device and managed to issue quite a few writes to it. so here's sequence of events so far: - Enumration goes fine - Get Max Lun - 0 (single lun) - Inquiry - Passed - Test Unit Ready - Failed - Request Sense (Unit Attention)- Passed - Test Unit Ready - Passed - Mode Sense- Stall of Data transport. - Clear Endpoint Feature (HALT) EP1 IN - After clear feature, a 16 bulk in completes. Shouldn't gadget driver have cancelled that ? - Bus reset This remains for a few iterations. One thing is very interesting ... [ snip ] ed2541c0 1239906485 S Bo:003:01 -115 31 = 55534243 1e00 1200 8603 0012 00 ed2541c0 1239906590 C Bo:003:01 0 31 ec1a8740 1239906770 S Bi:003:01 -115 18 ec1a8740 1239906871 C Bi:003:01 0 18 = 7600 000a 2900 ed2541c0 1239906975 S Bi:003:01 -115 13 ed2541c0 1239907026 C Bi:003:01 0 13 = 55534253 1e00 00 ed2541c0 1239907803 S Bo:003:01 -115 31 = 55534243 1f00 0002 8ca1 082e0001 ec00 00 0xa1 ? What is this ? Looks like XHCI corrupted the packet ? I can see the same SCSI opcode (0xa1) with my sniffer. -- balbi signature.asc Description: Digital signature
[PATCH v2] storage: Don't scan target 7 for SCM USB-SCSI converters
Hi, Regarding the patch I posted to prevent the bus scan of SCM USB-SCSI converters attempting to scan the converter itself, there is probably a neater way. In drivers/scsi/scsi_scan.c, function __scsi_scan_target(), there is: if (shost-this_id == id) /* * Don't scan the host adapter */ return; Since __scsi_scan_target() takes care to avoid scanning the host adapter, we just need to set this_id to 7. Untested patch below. Signed-off-by: Mark Knibbs ma...@clara.co.uk --- diff -up linux-3.17-rc6/drivers/usb/storage/usb.c.orig linux-3.17-rc6/drivers/usb/storage/usb.c --- linux-3.17-rc6/drivers/usb/storage/usb.c.orig 2014-09-21 23:43:02.0 +0100 +++ linux-3.17-rc6/drivers/usb/storage/usb.c2014-09-24 18:21:24.0 +0100 @@ -980,8 +980,12 @@ int usb_stor_probe2(struct us_data *us) if (us-fflags US_FL_SINGLE_LUN) us-max_lun = 0; - if (!(us-fflags US_FL_SCM_MULT_TARG)) + if (!(us-fflags US_FL_SCM_MULT_TARG)) { us_to_host(us)-max_id = 1; + } else { + /* SCM USB-SCSI converter ID is 7 */ + us_to_host(us)-this_id = 7; + } /* Find the endpoints and calculate pipe values */ result = get_pipes(us); -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] usb: core: log more general message on malformed LANGID descriptor
On Tue, 23 Sep 2014, Greg Kroah-Hartman wrote: On Tue, Sep 23, 2014 at 10:28:49PM +, Scot Doyle wrote: I'd like to change this error message: [3.325837] usb 1-4: string descriptor 0 malformed (err = -61), defaulting to 0x0409 into an error message followed by a debug message: [3.324726] usb 1-4: malformed string descriptor; unknown language, defaulting to English [3.327514] usb 1-4: string descriptor 0 malformed (err = -61), defaulting to 0x0409 in order to communicate more information from the log itself. Are there any problems with this approach? Would it be better to put all the information on a single line? Something else? How about just one line, why two? And something like device not implementing langid specifier, defaulting to English, but cleaned up to sound a bit better? That sounds good, I plan to submit this patch tomorrow unless anyone prefers different wording. --- diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0c8a7fc..5317081 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -770,9 +770,7 @@ static int usb_get_langid(struct usb_device *dev, unsigned char *tbuf) dev-string_langid = 0x0409; dev-have_langid = 1; dev_err(dev-dev, - string descriptor 0 malformed (err = %d), - defaulting to 0x%04x\n, - err, dev-string_langid); + language id specifier not provided by device, defaulting to English); return 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: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 12:19:13PM -0500, Felipe Balbi wrote: Hi, On Wed, Sep 24, 2014 at 11:20:31AM -0500, Felipe Balbi wrote: Therefore stalling is appropriate. Why it causes it problem for your system is a different matter. Is your UDC hardware capable of halting bulk endpoints? yeah, that part is just fine; I also verified with my sniffer that bulk halt is happening as it should. The problem, however, is that after that halt condition happens, host (same board has xhci too, Linux 3.17-rc5) issues a reset recovery It shouldn't; there's no reason for it to do so. Unless something else is going wrong on the host side. Have you tried capturing a usbmon trace on the host? I'll capture usbmon and send here shortly. here it is... Interesting part starts at line 73 (114 on this email) where the data transport received EPIPE (due to Stall). This time however, I was eventually able to talk to the device and managed to issue quite a few writes to it. so here's sequence of events so far: - Enumration goes fine - Get Max Lun - 0 (single lun) - Inquiry - Passed - Test Unit Ready - Failed - Request Sense (Unit Attention) - Passed - Test Unit Ready - Passed - Mode Sense - Stall of Data transport. - Clear Endpoint Feature (HALT) EP1 IN - After clear feature, a 16 bulk in completes. Shouldn't gadget driver have cancelled that ? - Bus reset looking into the SCSI glue, it seems like scsi is calling -eh_bus_reset_handler() instead of -eh_device_reset_handler(). Digging a little more. -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
On Wed, 24 Sep 2014, Felipe Balbi wrote: I'll capture usbmon and send here shortly. here it is... Interesting part starts at line 73 (114 on this email) where the data transport received EPIPE (due to Stall). This time however, I was eventually able to talk to the device and managed to issue quite a few writes to it. Here's where the unexpected stuff begins: ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 861a 003f00c0 00 ed2541c0 1237463431 C Bo:003:01 0 31 ec1a8540 1237463873 S Bi:003:01 -115 192 ec1a8540 1237464053 C Bi:003:01 -32 0 ed2541c0 1237464158 S Co:003:00 s 02 01 0081 0 ed2541c0 1237464359 C Co:003:00 0 0 ed2541c0 1237468607 S Bi:003:01 -115 13 ed2541c0 1237468802 C Bi:003:01 -75 0 This is the first MODE SENSE command. The gadget should send as much data as it can before halting the bulk-IN endpoint. Instead, the endpoint was halted first. Then, after the host cleared the halt, the gadget apparently sent the data that _should_ have been sent previously. The host was expecting to receive the CSW at this point, so there was an overflow error. That's what caused the host to perform a reset. Evidently this UDC implements the set_halt method incorrectly. According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. This didn't happen; the endpoint was halted before the host collected the pending data. Incidentally, even though the URB completed with -EOVERFLOW status, we still should see the first 13 bytes of data (i.e., the portion that could fit into the data buffer). But the actual_length value is 0. I don't know if this is a quirk of the xHCI hardware or a bug in xhci-hcd. ec1a8540 1237469361 S Co:001:00 s 23 03 0004 0001 0 ec1a8540 1237471551 C Co:001:00 0 0 ed2209c0 1237534064 S Ci:001:00 s a3 00 0001 0004 4 ed2209c0 1237537012 C Ci:001:00 0 4 = 03051000 ed2209c0 1237594113 S Co:001:00 s 23 01 0014 0001 0 ed2209c0 1237595037 C Co:001:00 0 0 ed2209c0 1237595434 S Co:001:00 s 23 01 0001 0001 0 ed2209c0 1237597480 C Co:001:00 0 0 Immediately after resetting the port, the host disabled it. No indication of why. ed2209c0 1237597823 S Co:001:00 s 23 03 0004 0001 0 ed2209c0 1237597890 C Co:001:00 0 0 ed2209c0 1237654005 S Ci:001:00 s a3 00 0001 0004 4 ed2209c0 1237654098 C Ci:001:00 0 4 = 03051000 ed2209c0 1237714084 S Co:001:00 s 23 01 0014 0001 0 ed2209c0 1237714151 C Co:001:00 0 0 ed2209c0 1237715894 S Co:001:00 s 23 01 0001 0001 0 ed2209c0 1237715985 C Co:001:00 0 0 Another reset followed by another port disable. ed2209c0 1237716244 S Co:001:00 s 23 03 0004 0001 0 ed2209c0 1237716308 C Co:001:00 0 0 ed2209c0 1237774094 S Ci:001:00 s a3 00 0001 0004 4 ed2209c0 1237775327 C Ci:001:00 0 4 = 03051000 ed2209c0 1237834107 S Co:001:00 s 23 01 0014 0001 0 ed2209c0 1237834183 C Co:001:00 0 0 ed2209c0 1237854094 S Ci:003:00 s 80 06 0100 0008 8 ed2209c0 1237854455 C Ci:003:00 0 8 = 12011002 0040 ed2209c0 1237854963 S Ci:003:00 s 80 06 0100 0012 18 ed2209c0 1237855219 C Ci:003:00 0 18 = 12011002 0040 2505a5a4 17030304 0001 ed2209c0 1237855544 S Ci:003:00 s 80 06 0f00 0005 5 ed2209c0 1237855771 C Ci:003:00 0 5 = 050f1600 02 ed2209c0 1237856062 S Ci:003:00 s 80 06 0f00 0016 22 ed2209c0 1237856265 C Ci:003:00 0 22 = 050f1600 02071002 0200 0a100300 0f000101 f401 ed2209c0 1237856548 S Ci:003:00 s 80 06 0200 0020 32 ed2209c0 1237858430 C Ci:003:00 0 32 = 09022000 010100c0 01090400 00020806 50010705 81020002 00070501 02000201 ed2200c0 1237860245 S Co:003:00 s 00 09 0001 0 ed2200c0 1237861785 C Co:003:00 0 0 Then a normal reset, like we should have seen originally. I have no idea what was going on. Maybe something involving warm vs. cold resets? ed2541c0 1237875505 S Bo:003:01 -115 31 = 55534243 0700 c000 861a 003f00c0 00 ed2541c0 1237875778 C Bo:003:01 0 31 ed2200c0 1237876448 S Bi:003:01 -115 192 ed2200c0 1237876534 C Bi:003:01 -32 0 ed2541c0 1237876703 S Co:003:00 s 02 01 0081 0 ed2541c0 1237876883 C Co:003:00 0 0 ed2541c0 1237876987 S Bi:003:01 -115 13 ed2541c0 1237877114 C Bi:003:01 0 0 ed2541c0 1237877486 S Bi:003:01 -115 13 ed2541c0 1237877572 C Bi:003:01 0 13 = 55534253 0700 c000 01 Here the MODE SENSE command was sent again, and this time the gadget responded in a way that the host could accept. I think it still wasn't _right_, because it appears the gadget tried to send a 0-length reply after the reset. But at least it didn't provoke another reset. ed2541c0 1237877915 S Bo:003:01 -115 31 = 55534243 0800 1200 8603 0012 00 ed2541c0 1237878041 C
Re: g_mass_storage bug ?
On Wed, 24 Sep 2014, Felipe Balbi wrote: so here's sequence of events so far: - Enumration goes fine - Get Max Lun - 0 (single lun) - Inquiry - Passed - Test Unit Ready - Failed - Request Sense (Unit Attention) - Passed - Test Unit Ready - Passed - Mode Sense - Stall of Data transport. - Clear Endpoint Feature (HALT) EP1 IN - After clear feature, a 16 bulk in completes. Shouldn't gadget driver have cancelled that ? No. The 16-byte transfer (which I presume was the response to the MODE SENSE) should have completed _before_ the halt feature was set. The UDC driver is buggy. - Bus reset This remains for a few iterations. One thing is very interesting ... [ snip ] ed2541c0 1239906485 S Bo:003:01 -115 31 = 55534243 1e00 1200 8603 0012 00 ed2541c0 1239906590 C Bo:003:01 0 31 ec1a8740 1239906770 S Bi:003:01 -115 18 ec1a8740 1239906871 C Bi:003:01 0 18 = 7600 000a 2900 ed2541c0 1239906975 S Bi:003:01 -115 13 ed2541c0 1239907026 C Bi:003:01 0 13 = 55534253 1e00 00 ed2541c0 1239907803 S Bo:003:01 -115 31 = 55534243 1f00 0002 8ca1 082e0001 ec00 00 0xa1 ? What is this ? Looks like XHCI corrupted the packet ? I can see the same SCSI opcode (0xa1) with my sniffer. 0xa1 is an ATA pass-through command. 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 v2] usb: core: downgrade log severity to info when descriptor unavailable
On Tue, 23 Sep 2014, Greg Kroah-Hartman wrote: On Tue, Sep 23, 2014 at 07:12:40PM +, Scot Doyle wrote: According to commit 0cce2eda19923e5e5ccc8b042dec5af87b3ffad0 USB: fix LANGID=0 regression usb devices are not required to report string descriptors. Since they are optional, log an info message instead of an error message. Signed-off-by: Scot Doyle lkm...@scotdoyle.com --- drivers/usb/core/message.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0c8a7fc..da2f1f2 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -780,8 +780,12 @@ static int usb_get_langid(struct usb_device *dev, unsigned char *tbuf) * deal with strings at all. Set string_langid to -1 in order to * prevent any string to be retrieved from the device */ if (err 0) { - dev_err(dev-dev, string descriptor 0 read error: %d\n, - err); + if (err == -EPIPE) + dev_info(dev-dev, +string descriptor 0 read error: -EPIPE\n); + else + dev_err(dev-dev, + string descriptor 0 read error: %d\n, err); What real difference does this patch make? What would a user do with -EPIPE? Not much difference, except e.g. a log line that isn't red. And USB devices _are_ required to provide a string descriptor if they provide a string id, so your changelog body doesn't make much sense. I suggest rewriting it to say they aren't required to provide a string descriptor for string 0. But even then, I don't understand the goal of this patch. I'm happy to drop this patch request, since in my case the return value is -ENODATA, which is covered by the other patch: [RFC PATCH] usb: core: log more general message on malformed LANGID descriptor -- 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: g_mass_storage bug ?
Hi, On Wed, Sep 24, 2014 at 01:56:21PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: so here's sequence of events so far: - Enumration goes fine - Get Max Lun - 0 (single lun) - Inquiry - Passed - Test Unit Ready - Failed - Request Sense (Unit Attention)- Passed - Test Unit Ready - Passed - Mode Sense- Stall of Data transport. - Clear Endpoint Feature (HALT) EP1 IN - After clear feature, a 16 bulk in completes. Shouldn't gadget driver have cancelled that ? No. The 16-byte transfer (which I presume was the response to the MODE SENSE) should have completed _before_ the halt feature was set. The UDC driver is buggy. ahaaa, now to figure out how to synchronize that. - Bus reset This remains for a few iterations. One thing is very interesting ... [ snip ] ed2541c0 1239906485 S Bo:003:01 -115 31 = 55534243 1e00 1200 8603 0012 00 ed2541c0 1239906590 C Bo:003:01 0 31 ec1a8740 1239906770 S Bi:003:01 -115 18 ec1a8740 1239906871 C Bi:003:01 0 18 = 7600 000a 2900 ed2541c0 1239906975 S Bi:003:01 -115 13 ed2541c0 1239907026 C Bi:003:01 0 13 = 55534253 1e00 00 ed2541c0 1239907803 S Bo:003:01 -115 31 = 55534243 1f00 0002 8ca1 082e0001 ec00 00 0xa1 ? What is this ? Looks like XHCI corrupted the packet ? I can see the same SCSI opcode (0xa1) with my sniffer. 0xa1 is an ATA pass-through command. ok, thanks. -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: I'll capture usbmon and send here shortly. here it is... Interesting part starts at line 73 (114 on this email) where the data transport received EPIPE (due to Stall). This time however, I was eventually able to talk to the device and managed to issue quite a few writes to it. Here's where the unexpected stuff begins: ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 861a 003f00c0 00 ed2541c0 1237463431 C Bo:003:01 0 31 ec1a8540 1237463873 S Bi:003:01 -115 192 ec1a8540 1237464053 C Bi:003:01 -32 0 ed2541c0 1237464158 S Co:003:00 s 02 01 0081 0 ed2541c0 1237464359 C Co:003:00 0 0 ed2541c0 1237468607 S Bi:003:01 -115 13 ed2541c0 1237468802 C Bi:003:01 -75 0 This is the first MODE SENSE command. The gadget should send as much data as it can before halting the bulk-IN endpoint. Instead, the endpoint was halted first. Then, after the host cleared the halt, the gadget apparently sent the data that _should_ have been sent previously. The host was expecting to receive the CSW at this point, so there was an overflow error. That's what caused the host to perform a reset. Evidently this UDC implements the set_halt method incorrectly. According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. This didn't happen; the endpoint was halted before the host collected the pending data. Incidentally, even though the URB completed with -EOVERFLOW status, we still should see the first 13 bytes of data (i.e., the portion that could fit into the data buffer). But the actual_length value is 0. I don't know if this is a quirk of the xHCI hardware or a bug in xhci-hcd. ec1a8540 1237469361 S Co:001:00 s 23 03 0004 0001 0 ec1a8540 1237471551 C Co:001:00 0 0 ed2209c0 1237534064 S Ci:001:00 s a3 00 0001 0004 4 ed2209c0 1237537012 C Ci:001:00 0 4 = 03051000 ed2209c0 1237594113 S Co:001:00 s 23 01 0014 0001 0 ed2209c0 1237595037 C Co:001:00 0 0 ed2209c0 1237595434 S Co:001:00 s 23 01 0001 0001 0 ed2209c0 1237597480 C Co:001:00 0 0 Immediately after resetting the port, the host disabled it. No indication of why. ed2209c0 1237597823 S Co:001:00 s 23 03 0004 0001 0 ed2209c0 1237597890 C Co:001:00 0 0 ed2209c0 1237654005 S Ci:001:00 s a3 00 0001 0004 4 ed2209c0 1237654098 C Ci:001:00 0 4 = 03051000 ed2209c0 1237714084 S Co:001:00 s 23 01 0014 0001 0 ed2209c0 1237714151 C Co:001:00 0 0 ed2209c0 1237715894 S Co:001:00 s 23 01 0001 0001 0 ed2209c0 1237715985 C Co:001:00 0 0 Another reset followed by another port disable. ed2209c0 1237716244 S Co:001:00 s 23 03 0004 0001 0 ed2209c0 1237716308 C Co:001:00 0 0 ed2209c0 1237774094 S Ci:001:00 s a3 00 0001 0004 4 ed2209c0 1237775327 C Ci:001:00 0 4 = 03051000 ed2209c0 1237834107 S Co:001:00 s 23 01 0014 0001 0 ed2209c0 1237834183 C Co:001:00 0 0 ed2209c0 1237854094 S Ci:003:00 s 80 06 0100 0008 8 ed2209c0 1237854455 C Ci:003:00 0 8 = 12011002 0040 ed2209c0 1237854963 S Ci:003:00 s 80 06 0100 0012 18 ed2209c0 1237855219 C Ci:003:00 0 18 = 12011002 0040 2505a5a4 17030304 0001 ed2209c0 1237855544 S Ci:003:00 s 80 06 0f00 0005 5 ed2209c0 1237855771 C Ci:003:00 0 5 = 050f1600 02 ed2209c0 1237856062 S Ci:003:00 s 80 06 0f00 0016 22 ed2209c0 1237856265 C Ci:003:00 0 22 = 050f1600 02071002 0200 0a100300 0f000101 f401 ed2209c0 1237856548 S Ci:003:00 s 80 06 0200 0020 32 ed2209c0 1237858430 C Ci:003:00 0 32 = 09022000 010100c0 01090400 00020806 50010705 81020002 00070501 02000201 ed2200c0 1237860245 S Co:003:00 s 00 09 0001 0 ed2200c0 1237861785 C Co:003:00 0 0 Then a normal reset, like we should have seen originally. I have no idea what was going on. Maybe something involving warm vs. cold resets? ed2541c0 1237875505 S Bo:003:01 -115 31 = 55534243 0700 c000 861a 003f00c0 00 ed2541c0 1237875778 C Bo:003:01 0 31 ed2200c0 1237876448 S Bi:003:01 -115 192 ed2200c0 1237876534 C Bi:003:01 -32 0 ed2541c0 1237876703 S Co:003:00 s 02 01 0081 0 ed2541c0 1237876883 C Co:003:00 0 0 ed2541c0 1237876987 S Bi:003:01 -115 13 ed2541c0 1237877114 C Bi:003:01 0 0 ed2541c0 1237877486 S Bi:003:01 -115 13 ed2541c0 1237877572 C Bi:003:01 0 13 = 55534253 0700 c000 01 Here the MODE SENSE command was sent again, and this time the gadget responded in a way that the host could accept. I think it still wasn't _right_, because it appears the gadget tried to send a
MUSB: extra cppi irq?
Hi Felipe and all, The musb driver musb_host_tx() has the following: 1244 /* with CPPI, DMA sometimes triggers extra irqs */ 1245 if (!urb) { 1246 dev_dbg(musb-controller, extra TX%d ready, csr %04x\n, epnum, tx_csr); 1247 return; 1248 } and 1321 /* second cppi case */ 1322 if (dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) { 1323 dev_dbg(musb-controller, extra TX%d ready, csr %04x\n, epnum, tx_csr); 1324 return; 1325 } which come with the very first commit of musb driver and never got changed. Is there any more information about the 'extra' irqs? and what is the 'second cppi case'? I ran into this problem and musb stops working after hits here. [13542.933563] musb-hdrc musb-hdrc.1: qh c261ad80 urb eec92bc0 dev4 ep7in-bulk, hw_ep 2, c2599000/4096 [13542.953552] musb-hdrc musb-hdrc.1: usbintr (0) epintr(4) [13542.953582] musb-hdrc musb-hdrc.1: extra TX2 ready, csr 0004 I bet this 'extra TX2 ready' log is printed by the first case, because only RX2 is used in this test, and TX2 is never got used, so there should not be any urb in TX2 queue. I am still waiting for another test failure to confirm this, but I am trying to understand why TX2 interrupt happened...which seems to be related RX2 request. Thanks for any help. -Bin. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] usb: rename phy to usb_phy in HCD
hello. On 09/24/2014 09:11 AM, Greg KH wrote: From: Antoine Tenart antoine.ten...@free-electrons.com The USB PHY member of the HCD structure is renamed to 'usb_phy' and modifications are done in all drivers accessing it. This is in preparation to adding the generic PHY support. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects caused by patch reordering, updated changelog.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Felipe Balbi ba...@ti.com --- Changes in version 5: - imported the patch from Antoine Tenart's series; - added missing 'drivers/usb/misc/lvstest.c' file; - resolved rejects caused by patch reordering; - refreshed patch; - updated changelog. drivers/usb/chipidea/host.c |2 +- drivers/usb/core/hcd.c| 20 ++-- drivers/usb/core/hub.c|8 drivers/usb/host/ehci-fsl.c | 16 drivers/usb/host/ehci-hub.c |2 +- drivers/usb/host/ehci-msm.c |4 ++-- drivers/usb/host/ehci-tegra.c | 16 drivers/usb/host/ohci-omap.c | 20 ++-- drivers/usb/misc/lvstest.c|8 include/linux/usb/hcd.h |2 +- 10 files changed, 49 insertions(+), 49 deletions(-) This doesn't apply to my tree at all anymore, Well, I'm seeing only a minor reject in the first file, easily fixable. can you refresh it and resend? OK, will re-post now. thanks, greg k-h WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/2] Add generic PHY support to USB HCD
Hello. This patchset is against the usb-next' branch of Greg KH's 'usb.git' repo. Here I add support for the generic PHY to the 'struct usb_hcd' (having to rename the existing 'phy' field to 'usb_phy' beforehand). This was mainly intended to be used with the PCI OHCI/EHCI drivers and also xHCI driver; however there are several more dependent patchsets now posted to 'linux-usb'. [1/2] usb: rename phy to usb_phy in HCD [2/2] usb: hcd: add generic PHY support WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/2] usb: rename phy to usb_phy in HCD
From: Antoine Tenart antoine.ten...@free-electrons.com The USB PHY member of the HCD structure is renamed to 'usb_phy' and modifications are done in all drivers accessing it. This is in preparation to adding the generic PHY support. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects, updated changelog.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Acked-by: Alan Stern st...@rowland.harvard.edu --- Changes in version 6: - resolved reject, refreshed the patch. Changes in version 5: - imported the patch from Antoine Tenart's series; - added missing 'drivers/usb/misc/lvstest.c' file; - resolved rejects caused by patch reordering; - refreshed patch; - updated changelog. drivers/usb/chipidea/host.c |2 +- drivers/usb/core/hcd.c| 20 ++-- drivers/usb/core/hub.c|8 drivers/usb/host/ehci-fsl.c | 16 drivers/usb/host/ehci-hub.c |2 +- drivers/usb/host/ehci-msm.c |4 ++-- drivers/usb/host/ehci-tegra.c | 16 drivers/usb/host/ohci-omap.c | 20 ++-- drivers/usb/misc/lvstest.c|8 include/linux/usb/hcd.h |2 +- 10 files changed, 49 insertions(+), 49 deletions(-) Index: usb/drivers/usb/chipidea/host.c === --- usb.orig/drivers/usb/chipidea/host.c +++ usb/drivers/usb/chipidea/host.c @@ -59,7 +59,7 @@ static int host_start(struct ci_hdrc *ci hcd-has_tt = 1; hcd-power_budget = ci-platdata-power_budget; - hcd-phy = ci-transceiver; + hcd-usb_phy = ci-transceiver; hcd-tpl_support = ci-platdata-tpl_support; ehci = hcd_to_ehci(hcd); Index: usb/drivers/usb/core/hcd.c === --- usb.orig/drivers/usb/core/hcd.c +++ usb/drivers/usb/core/hcd.c @@ -2627,7 +2627,7 @@ int usb_add_hcd(struct usb_hcd *hcd, int retval; struct usb_device *rhdev; - if (IS_ENABLED(CONFIG_USB_PHY) !hcd-phy) { + if (IS_ENABLED(CONFIG_USB_PHY) !hcd-usb_phy) { struct usb_phy *phy = usb_get_phy_dev(hcd-self.controller, 0); if (IS_ERR(phy)) { @@ -2640,7 +2640,7 @@ int usb_add_hcd(struct usb_hcd *hcd, usb_put_phy(phy); return retval; } - hcd-phy = phy; + hcd-usb_phy = phy; hcd-remove_phy = 1; } } @@ -2788,10 +2788,10 @@ err_allocate_root_hub: err_register_bus: hcd_buffer_destroy(hcd); err_remove_phy: - if (hcd-remove_phy hcd-phy) { - usb_phy_shutdown(hcd-phy); - usb_put_phy(hcd-phy); - hcd-phy = NULL; + if (hcd-remove_phy hcd-usb_phy) { + usb_phy_shutdown(hcd-usb_phy); + usb_put_phy(hcd-usb_phy); + hcd-usb_phy = NULL; } return retval; } @@ -2864,10 +2864,10 @@ void usb_remove_hcd(struct usb_hcd *hcd) usb_deregister_bus(hcd-self); hcd_buffer_destroy(hcd); - if (hcd-remove_phy hcd-phy) { - usb_phy_shutdown(hcd-phy); - usb_put_phy(hcd-phy); - hcd-phy = NULL; + if (hcd-remove_phy hcd-usb_phy) { + usb_phy_shutdown(hcd-usb_phy); + usb_put_phy(hcd-usb_phy); + hcd-usb_phy = NULL; } usb_put_invalidate_rhdev(hcd); Index: usb/drivers/usb/core/hub.c === --- usb.orig/drivers/usb/core/hub.c +++ usb/drivers/usb/core/hub.c @@ -4467,8 +4467,8 @@ hub_port_init (struct usb_hub *hub, stru if (retval) goto fail; - if (hcd-phy !hdev-parent) - usb_phy_notify_connect(hcd-phy, udev-speed); + if (hcd-usb_phy !hdev-parent) + usb_phy_notify_connect(hcd-usb_phy, udev-speed); /* * Some superspeed devices have finished the link training process @@ -4626,9 +4626,9 @@ static void hub_port_connect(struct usb_ /* Disconnect any existing devices under this port */ if (udev) { - if (hcd-phy !hdev-parent + if (hcd-usb_phy !hdev-parent !(portstatus USB_PORT_STAT_CONNECTION)) - usb_phy_notify_disconnect(hcd-phy, udev-speed); + usb_phy_notify_disconnect(hcd-usb_phy, udev-speed); usb_disconnect(port_dev-child); } Index: usb/drivers/usb/host/ehci-fsl.c === --- usb.orig/drivers/usb/host/ehci-fsl.c +++ usb/drivers/usb/host/ehci-fsl.c @@ -136,15 +136,15 @@ static int usb_hcd_fsl_probe(const struc if
[PATCH v6 2/2] usb: hcd: add generic PHY support
Add the generic PHY support, analogous to the USB PHY support. Intended it to be used with the PCI EHCI/OHCI drivers and the xHCI platform driver. Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com --- Changes in version 5: - renamed the new 'gen_phy' field of 'struct usb_phy' back to 'phy'; - resolved rejects occured due to a newly added patch. Changes in version 4: - refreshed the patch. Changes in version 3: - refreshed the patch. Changes in version 2: - renamed the new 'phy' field of 'struct usb_phy' to 'gen_phy'; - resolved rejects due to removal of the first patch in the series. drivers/usb/core/hcd.c | 42 -- include/linux/usb/hcd.h |1 + 2 files changed, 41 insertions(+), 2 deletions(-) Index: usb/drivers/usb/core/hcd.c === --- usb.orig/drivers/usb/core/hcd.c +++ usb/drivers/usb/core/hcd.c @@ -42,6 +42,7 @@ #include linux/pm_runtime.h #include linux/types.h +#include linux/phy/phy.h #include linux/usb.h #include linux/usb/hcd.h #include linux/usb/phy.h @@ -2645,6 +2646,29 @@ int usb_add_hcd(struct usb_hcd *hcd, } } + if (IS_ENABLED(CONFIG_GENERIC_PHY)) { + struct phy *phy = phy_get(hcd-self.controller, usb); + + if (IS_ERR(phy)) { + retval = PTR_ERR(phy); + if (retval == -EPROBE_DEFER) + goto err_phy; + } else { + retval = phy_init(phy); + if (retval) { + phy_put(phy); + goto err_phy; + } + retval = phy_power_on(phy); + if (retval) { + phy_exit(phy); + phy_put(phy); + goto err_phy; + } + hcd-phy = phy; + } + } + dev_info(hcd-self.controller, %s\n, hcd-product_desc); /* Keep old behaviour if authorized_default is not in [0, 1]. */ @@ -2660,7 +2684,7 @@ int usb_add_hcd(struct usb_hcd *hcd, */ if ((retval = hcd_buffer_create(hcd)) != 0) { dev_dbg(hcd-self.controller, pool alloc failed\n); - goto err_remove_phy; + goto err_create_buf; } if ((retval = usb_register_bus(hcd-self)) 0) @@ -2787,7 +2811,14 @@ err_allocate_root_hub: usb_deregister_bus(hcd-self); err_register_bus: hcd_buffer_destroy(hcd); -err_remove_phy: +err_create_buf: + if (IS_ENABLED(CONFIG_GENERIC_PHY) hcd-phy) { + phy_power_off(hcd-phy); + phy_exit(hcd-phy); + phy_put(hcd-phy); + hcd-phy = NULL; + } +err_phy: if (hcd-remove_phy hcd-usb_phy) { usb_phy_shutdown(hcd-usb_phy); usb_put_phy(hcd-usb_phy); @@ -2864,6 +2895,13 @@ void usb_remove_hcd(struct usb_hcd *hcd) usb_deregister_bus(hcd-self); hcd_buffer_destroy(hcd); + + if (IS_ENABLED(CONFIG_GENERIC_PHY) hcd-phy) { + phy_power_off(hcd-phy); + phy_exit(hcd-phy); + phy_put(hcd-phy); + hcd-phy = NULL; + } if (hcd-remove_phy hcd-usb_phy) { usb_phy_shutdown(hcd-usb_phy); usb_put_phy(hcd-usb_phy); Index: usb/include/linux/usb/hcd.h === --- usb.orig/include/linux/usb/hcd.h +++ usb/include/linux/usb/hcd.h @@ -107,6 +107,7 @@ struct usb_hcd { * other external phys should be software-transparent */ struct usb_phy *usb_phy; + struct phy *phy; /* Flags that need to be manipulated atomically because they can * change while the host controller is running. Always use -- 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: g_mass_storage bug ?
Hi, On Wed, Sep 24, 2014 at 01:08:15PM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: I'll capture usbmon and send here shortly. here it is... Interesting part starts at line 73 (114 on this email) where the data transport received EPIPE (due to Stall). This time however, I was eventually able to talk to the device and managed to issue quite a few writes to it. Here's where the unexpected stuff begins: ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 861a 003f00c0 00 ed2541c0 1237463431 C Bo:003:01 0 31 ec1a8540 1237463873 S Bi:003:01 -115 192 ec1a8540 1237464053 C Bi:003:01 -32 0 ed2541c0 1237464158 S Co:003:00 s 02 01 0081 0 ed2541c0 1237464359 C Co:003:00 0 0 ed2541c0 1237468607 S Bi:003:01 -115 13 ed2541c0 1237468802 C Bi:003:01 -75 0 This is the first MODE SENSE command. The gadget should send as much data as it can before halting the bulk-IN endpoint. Instead, the endpoint was halted first. Then, after the host cleared the halt, the gadget apparently sent the data that _should_ have been sent previously. The host was expecting to receive the CSW at this point, so there was an overflow error. That's what caused the host to perform a reset. Evidently this UDC implements the set_halt method incorrectly. According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 66f6256..834f524 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -210,6 +210,14 @@ #define DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(n) (((n) (0x0f 13)) 13) #define DWC3_MAX_HIBER_SCRATCHBUFS 15 +/* Global FIFO Space Register */ +#define DWC3_GDBGFIFOSPACE_TXFIFO (0 5) +#define DWC3_GDBGFIFOSPACE_RXFIFO (1 5) +#define DWC3_GDBGFIFOSPACE_TXREQ_Q (2 5) +#define DWC3_GDBGFIFOSPACE_RXREQ_Q (3 5) + +#define DWC3_GDBGFIFOSPACE_SPACE_AVAIL(num)(((num) 0x) 16) + /* Device Configuration Register */ #define DWC3_DCFG_DEVADDR(addr)((addr) 3) #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index de53361..5e89913 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -145,6 +145,75 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum dwc3_link_state state) } /** + * dwc3_gadget_ep_fifo_space - returns currently available space on FIFO + * @dwc: pointer to our context struct + * @dep: the endpoint to fetch FIFO space + * + * This function will return the currently available FIFO space + */ +static int dwc3_gadget_ep_fifo_space(struct dwc3 *dwc, struct dwc3_ep *dep) +{ + u32 current_fifo; + u32 reg; + + if (dep-direction) + reg = DWC3_GDBGFIFOSPACE_TXFIFO; + else + reg = DWC3_GDBGFIFOSPACE_RXFIFO; + + /* remove direction bit */ + reg |= dep-number 1; + + dwc3_writel(dwc-regs, DWC3_GDBGFIFOSPACE, reg); + reg = dwc3_readl(dwc-regs, DWC3_GDBGFIFOSPACE); + current_fifo = DWC3_GDBGFIFOSPACE_SPACE_AVAIL(reg); + + return current_fifo; +} + +/** + * dwc3_gadget_ep_fifo_size - returns allocated FIFO size + * @dwc: pointer to our context struct + * @dep: TX endpoint to fetch allocated FIFO size + * + * This function will read the correct TX FIFO register and + * return the FIFO size + */ +static int dwc3_gadget_ep_fifo_size(struct dwc3 *dwc, struct dwc3_ep *dep) +{ + if (WARN_ON(!dep-direction)) + return 0; + + return dwc3_readl(dwc-regs, DWC3_GTXFIFOSIZ(dep-number)) 0x; +} + +/** + * dwc3_gadget_ep_fifo_empty - returns true when FIFO is empty + * @dwc: pointer to our context struct + * @dep: endpoint to request FIFO space + * + * This function will return a TRUE when FIFO is empty and FALSE + * otherwise. + */ +static int dwc3_gadget_ep_fifo_empty(struct dwc3 *dwc, struct dwc3_ep *dep) +{ + u32 allocated_fifo; + u32 current_fifo; + + /* should not be called for RX endpoints */ + if (WARN_ON(!dep-direction)) + return false; + + current_fifo = dwc3_gadget_ep_fifo_space(dwc, dep); + allocated_fifo = dwc3_gadget_ep_fifo_size(dwc, dep); + + dev_vdbg(dwc-dev, %s: FIFO space %u/%u\n, dep-name, + current_fifo, allocated_fifo); + + return current_fifo == allocated_fifo; +} + +/** *
Re: g_mass_storage bug ?
Hi, On Wed, Sep 24, 2014 at 02:18:14PM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 01:08:15PM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 01:53:31PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: I'll capture usbmon and send here shortly. here it is... Interesting part starts at line 73 (114 on this email) where the data transport received EPIPE (due to Stall). This time however, I was eventually able to talk to the device and managed to issue quite a few writes to it. Here's where the unexpected stuff begins: ed2541c0 1237463240 S Bo:003:01 -115 31 = 55534243 0600 c000 861a 003f00c0 00 ed2541c0 1237463431 C Bo:003:01 0 31 ec1a8540 1237463873 S Bi:003:01 -115 192 ec1a8540 1237464053 C Bi:003:01 -32 0 ed2541c0 1237464158 S Co:003:00 s 02 01 0081 0 ed2541c0 1237464359 C Co:003:00 0 0 ed2541c0 1237468607 S Bi:003:01 -115 13 ed2541c0 1237468802 C Bi:003:01 -75 0 This is the first MODE SENSE command. The gadget should send as much data as it can before halting the bulk-IN endpoint. Instead, the endpoint was halted first. Then, after the host cleared the halt, the gadget apparently sent the data that _should_ have been sent previously. The host was expecting to receive the CSW at this point, so there was an overflow error. That's what caused the host to perform a reset. Evidently this UDC implements the set_halt method incorrectly. According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. OTOH, there's really no way to split it. It's all needed to fix a single bug. Do you want me to add Reported-by: Alan Stern ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 1/2] usb: rename phy to usb_phy in HCD
On Wed, Sep 24, 2014 at 11:05:50PM +0400, Sergei Shtylyov wrote: From: Antoine Tenart antoine.ten...@free-electrons.com The USB PHY member of the HCD structure is renamed to 'usb_phy' and modifications are done in all drivers accessing it. This is in preparation to adding the generic PHY support. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects, updated changelog.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 1/2] usb: rename phy to usb_phy in HCD
Hello. On 09/24/2014 11:28 PM, Felipe Balbi wrote: From: Antoine Tenart antoine.ten...@free-electrons.com The USB PHY member of the HCD structure is renamed to 'usb_phy' and modifications are done in all drivers accessing it. This is in preparation to adding the generic PHY support. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects, updated changelog.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Felipe Balbi ba...@ti.com Sorry, forgot to collect your ACK from the previous posting. :-/ WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: g_mass_storage bug ?
On Wed, 24 Sep 2014, Felipe Balbi wrote: According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. And this eliminates the problems you saw with g_mass_storage? 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: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 03:31:13PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. And this eliminates the problems you saw with g_mass_storage? yup, working with and without stall=0, with and without debugging on. On all three systems I tested before ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 1/2] usb: rename phy to usb_phy in HCD
On Wed, Sep 24, 2014 at 11:31:11PM +0400, Sergei Shtylyov wrote: Hello. On 09/24/2014 11:28 PM, Felipe Balbi wrote: From: Antoine Tenart antoine.ten...@free-electrons.com The USB PHY member of the HCD structure is renamed to 'usb_phy' and modifications are done in all drivers accessing it. This is in preparation to adding the generic PHY support. Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects, updated changelog.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Acked-by: Alan Stern st...@rowland.harvard.edu Acked-by: Felipe Balbi ba...@ti.com Sorry, forgot to collect your ACK from the previous posting. :-/ don't worry ;-) -- balbi signature.asc Description: Digital signature
[PATCH v6 partial 0/2] pxa27x_udc port to device-tree and gpio_desc
Hi Felipe and Linus, As you requested, Felipe, I made the translation for the driver's probing from gpio to gpio_desc. Even if the code is functional, I'm pretty unhappy about my patch 1, because I lost the active_low notion in the transformation. I have not found in the gpiolib anything for a driver to set that active_low value, only for machine code. The legacy behaviour was that that information was handed over to the driver. As transforming the different pxa27x_udc users will be not part of this serie, ie. because the mach info should be kept ISO for this serie, Linus, do you have an idea so that I still can port to gpio_desc this driver, and keep the gpio_inverted notion in it ? Ah, and I sent only 2 out of the 6 patches of the serie, mainly because Linus doesn't care about the remaining stuff, which remained constant from last version (at rebase conflicts resolution exception of course). Cheers. -- Robert Robert Jarzmik (2): usb: gadget: pxa27x_udc: prepare device-tree support usb: gadget: pxa27x_udc: add devicetree support drivers/usb/gadget/udc/pxa27x_udc.c | 61 + drivers/usb/gadget/udc/pxa27x_udc.h | 6 ++-- 2 files changed, 38 insertions(+), 29 deletions(-) -- 2.0.0.rc2 PS: For Felipe, the diff of the complete serie, for a one quick glance overview, and not for review is attached to this mail. drivers/usb/gadget/udc/pxa27x_udc.c | 96 + drivers/usb/gadget/udc/pxa27x_udc.h | 10 ++-- 2 files changed, 26 insertions(+), 80 deletions(-) diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c index 9506da8..adbe74a 100644 --- a/drivers/usb/gadget/udc/pxa27x_udc.c +++ b/drivers/usb/gadget/udc/pxa27x_udc.c @@ -22,6 +22,7 @@ #include linux/clk.h #include linux/irq.h #include linux/gpio.h +#include linux/gpio/consumer.h #include linux/slab.h #include linux/prefetch.h #include linux/byteorder/generic.h @@ -1509,18 +1510,13 @@ static struct usb_ep_ops pxa_ep_ops = { */ static void dplus_pullup(struct pxa_udc *udc, int on) { - if (on) { - if (gpio_is_valid(udc-gpio_pullup)) - gpio_set_value(udc-gpio_pullup, - !udc-gpio_pullup_inverted); - if (udc-mach udc-mach-udc_command) - udc-mach-udc_command(PXA2XX_UDC_CMD_CONNECT); - } else { - if (gpio_is_valid(udc-gpio_pullup)) - gpio_set_value(udc-gpio_pullup, - udc-gpio_pullup_inverted); - if (udc-mach udc-mach-udc_command) - udc-mach-udc_command(PXA2XX_UDC_CMD_DISCONNECT); + if (udc-gpiod) { + gpiod_set_value(udc-gpiod, on); + } else if (udc-udc_command) { + if (on) + udc-udc_command(PXA2XX_UDC_CMD_CONNECT); + else + udc-udc_command(PXA2XX_UDC_CMD_DISCONNECT); } udc-pullup_on = on; } @@ -1611,8 +1607,7 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int is_active) { struct pxa_udc *udc = to_gadget_udc(_gadget); - if (!gpio_is_valid(udc-gpio_pullup) -!(udc-mach udc-mach-udc_command)) + if (!udc-gpiod !udc-udc_command) return -EOPNOTSUPP; dplus_pullup(udc, is_active); @@ -2414,50 +2409,6 @@ static struct of_device_id udc_pxa_dt_ids[] = { MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids); /** - * pxa_udc_probe_dt - device tree specific probe - * @pdev: platform data - * @udc: pxa_udc structure to fill - * - * Fills udc from platform data out of device tree. - * - * Returns 0 if DT found, 1 if DT not found, and 0 on error - */ -static int pxa_udc_probe_dt(struct platform_device *pdev, struct pxa_udc *udc) -{ - struct device_node *np = pdev-dev.of_node; - const struct of_device_id *of_id = - of_match_device(udc_pxa_dt_ids, pdev-dev); - u32 gpio_pullup; - enum of_gpio_flags flags; - - if (!np || !of_id) - return 1; - pdev-id = -1; - - gpio_pullup = of_get_gpio_flags(np, 0, flags); - if (gpio_pullup = 0) { - udc-gpio_pullup = gpio_pullup; - udc-gpio_pullup_inverted = (flags OF_GPIO_ACTIVE_LOW); - } - return 0; -} - -/** - * pxa_udc_probe_pdata - legacy platform data probe - * @pdev: platform device - * @udc: pxa_udc structure to fill - * - * Simple copy of data from platform_data to udc control structure - */ -static void pxa_udc_probe_pdata(struct platform_device *pdev, - struct pxa_udc *udc) -{ - udc-mach = dev_get_platdata(pdev-dev); - udc-gpio_pullup = udc-mach-gpio_pullup; - udc-gpio_pullup_inverted = udc-mach-gpio_pullup_inverted; -} - -/** * pxa_udc_probe - probes the udc device * @_dev: platform device * @@ -2468,13 +2419,15 @@ static int
[PATCH v6 1/2] usb: gadget: pxa27x_udc: prepare device-tree support
For this preparation, a preliminary cleanup is done : - convert the probing of pxa27x_udc to gpio_desc. The conversion is partial because : - the platform data still provides a gpio number, not a gpio desc - the invert attribute is lost, hence a loss in the translation - convert the mach info, and store the udc_command in the pxa_udc control structure - loose the udc_is_connected() in mach info This was not really used, as mioa701 machine doesn't need it, balloon3 doesn't really use it, and most importantly the current driver never uses it. The drawback with the gpio_desc conversion is that the inverted gpio attribute is lost, as no gpiod_*() function exists to set the active_low state of a gpio, and that attribute was at driver's creation forecast to be set up by the driver and not the machine specific code. Signed-off-by: Robert Jarzmik robert.jarz...@free.fr --- drivers/usb/gadget/udc/pxa27x_udc.c | 51 - drivers/usb/gadget/udc/pxa27x_udc.h | 6 +++-- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c index 597d39f..e11f9c5 100644 --- a/drivers/usb/gadget/udc/pxa27x_udc.c +++ b/drivers/usb/gadget/udc/pxa27x_udc.c @@ -22,6 +22,7 @@ #include linux/clk.h #include linux/irq.h #include linux/gpio.h +#include linux/gpio/consumer.h #include linux/slab.h #include linux/prefetch.h #include linux/byteorder/generic.h @@ -1507,18 +1508,13 @@ static struct usb_ep_ops pxa_ep_ops = { */ static void dplus_pullup(struct pxa_udc *udc, int on) { - if (on) { - if (gpio_is_valid(udc-mach-gpio_pullup)) - gpio_set_value(udc-mach-gpio_pullup, - !udc-mach-gpio_pullup_inverted); - if (udc-mach-udc_command) - udc-mach-udc_command(PXA2XX_UDC_CMD_CONNECT); - } else { - if (gpio_is_valid(udc-mach-gpio_pullup)) - gpio_set_value(udc-mach-gpio_pullup, - udc-mach-gpio_pullup_inverted); - if (udc-mach-udc_command) - udc-mach-udc_command(PXA2XX_UDC_CMD_DISCONNECT); + if (udc-gpiod) { + gpiod_set_value(udc-gpiod, on); + } else if (udc-udc_command) { + if (on) + udc-udc_command(PXA2XX_UDC_CMD_CONNECT); + else + udc-udc_command(PXA2XX_UDC_CMD_DISCONNECT); } udc-pullup_on = on; } @@ -1609,7 +1605,7 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int is_active) { struct pxa_udc *udc = to_gadget_udc(_gadget); - if (!gpio_is_valid(udc-mach-gpio_pullup) !udc-mach-udc_command) + if (!udc-gpiod !udc-udc_command) return -EOPNOTSUPP; dplus_pullup(udc, is_active); @@ -2415,7 +2411,13 @@ static int pxa_udc_probe(struct platform_device *pdev) { struct resource *regs; struct pxa_udc *udc = memory; - int retval = 0, gpio; + struct pxa2xx_udc_mach_info *mach = dev_get_platdata(pdev-dev); + int retval = 0; + + if (mach) { + udc-gpiod = gpio_to_desc(mach-gpio_pullup); + udc-udc_command = mach-udc_command; + } regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!regs) @@ -2425,21 +2427,15 @@ static int pxa_udc_probe(struct platform_device *pdev) return udc-irq; udc-dev = pdev-dev; - udc-mach = dev_get_platdata(pdev-dev); udc-transceiver = usb_get_phy(USB_PHY_TYPE_USB2); - gpio = udc-mach-gpio_pullup; - if (gpio_is_valid(gpio)) { - retval = gpio_request(gpio, USB D+ pullup); - if (retval == 0) - gpio_direction_output(gpio, - udc-mach-gpio_pullup_inverted); - } - if (retval) { - dev_err(pdev-dev, Couldn't request gpio %d : %d\n, - gpio, retval); - return retval; + if (IS_ERR(udc-gpiod)) { + dev_err(pdev-dev, Couldn't find or request D+ gpio : %ld\n, + PTR_ERR(udc-gpiod)); + return PTR_ERR(udc-gpiod); } + if (udc-gpiod) + gpiod_direction_output(udc-gpiod, 0); udc-clk = clk_get(pdev-dev, NULL); if (IS_ERR(udc-clk)) { @@ -2501,14 +2497,11 @@ err_clk: static int pxa_udc_remove(struct platform_device *_dev) { struct pxa_udc *udc = platform_get_drvdata(_dev); - int gpio = udc-mach-gpio_pullup; usb_del_gadget_udc(udc-gadget); usb_gadget_unregister_driver(udc-driver); free_irq(udc-irq, udc); pxa_cleanup_debugfs(udc); - if (gpio_is_valid(gpio)) - gpio_free(gpio); usb_put_phy(udc-transceiver); diff --git
[PATCH v6 2/2] usb: gadget: pxa27x_udc: add devicetree support
Add support for device-tree device discovery. If devicetree is not provided, fallback to legacy platform data discovery. Signed-off-by: Robert Jarzmik robert.jarz...@free.fr Cc: devicet...@vger.kernel.org --- Since V1: change OF id mrvl,pxa27x_udc - marvell,pxa27x-udc This is a consequence of other DT reviews on the marvell namings. Since V2: address Mark's comments: - wildcard pxa27x becomes pxa270 - pullup gpio is described as standard dt gpio - bool XXX_probe_dt becomes int XXX_probe_dt Since v5: split out into 2 patches, this one being the true device-tree port --- drivers/usb/gadget/udc/pxa27x_udc.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c index e11f9c5..8961530 100644 --- a/drivers/usb/gadget/udc/pxa27x_udc.c +++ b/drivers/usb/gadget/udc/pxa27x_udc.c @@ -27,6 +27,8 @@ #include linux/prefetch.h #include linux/byteorder/generic.h #include linux/platform_data/pxa2xx_udc.h +#include linux/of_device.h +#include linux/of_gpio.h #include linux/usb.h #include linux/usb/ch9.h @@ -2400,6 +2402,12 @@ static struct pxa_udc memory = { } }; +static struct of_device_id udc_pxa_dt_ids[] = { + { .compatible = marvell,pxa270-udc }, + {} +}; +MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids); + /** * pxa_udc_probe - probes the udc device * @_dev: platform device @@ -2417,6 +2425,8 @@ static int pxa_udc_probe(struct platform_device *pdev) if (mach) { udc-gpiod = gpio_to_desc(mach-gpio_pullup); udc-udc_command = mach-udc_command; + } else { + udc-gpiod = devm_gpiod_get(pdev-dev, NULL); } regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -2608,6 +2618,7 @@ static struct platform_driver udc_driver = { .driver = { .name = pxa27x-udc, .owner = THIS_MODULE, + .of_match_table = of_match_ptr(udc_pxa_dt_ids), }, .probe = pxa_udc_probe, .remove = pxa_udc_remove, -- 2.0.0.rc2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: g_mass_storage bug ?
On Wed, 24 Sep 2014, Felipe Balbi wrote: alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. OTOH, there's really no way to split it. It's all needed to fix a single bug. Do you want me to add Reported-by: Alan Stern ? What we really need is a Diagnosed-by: tag. :-) I'll settle for Suggested-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: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 03:48:29PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. OTOH, there's really no way to split it. It's all needed to fix a single bug. Do you want me to add Reported-by: Alan Stern ? What we really need is a Diagnosed-by: tag. :-) heh, that's right. I'll settle for Suggested-by:. alright, I'll add that :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 1/2] usb: gadget: pxa27x_udc: prepare device-tree support
On Wed, Sep 24, 2014 at 09:41:12PM +0200, Robert Jarzmik wrote: For this preparation, a preliminary cleanup is done : - convert the probing of pxa27x_udc to gpio_desc. The conversion is partial because : - the platform data still provides a gpio number, not a gpio desc - the invert attribute is lost, hence a loss in the translation I asked for gpio to gpiod conversion to be a separate patch. It'll make things a lot easier to review. - convert the mach info, and store the udc_command in the pxa_udc control structure - loose the udc_is_connected() in mach info This was not really used, as mioa701 machine doesn't need it, balloon3 doesn't really use it, and most importantly the current driver never uses it. The drawback with the gpio_desc conversion is that the inverted gpio attribute is lost, as no gpiod_*() function exists to set the it's not lost, it's handled for you by the gpio library. Look at how GPIO_ACTIVE_LOW is used. -- balbi signature.asc Description: Digital signature
Re: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 02:40:12PM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 03:31:13PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. And this eliminates the problems you saw with g_mass_storage? yup, working with and without stall=0, with and without debugging on. On all three systems I tested before ;-) there is still one detail which I just caught and not sure if it's something we should care. When I run my msc.sh/msc.c tests [1], after each test I see a new sdX: unknown partition table. This doesn't happen with my USB stick. I'll fire up my sniffer again and see if I find anything peculiar. [1] https://gitorious.org/usb/usb-tools/source/7eb7ef21de6cd124e0e0d0e7df9ddfff0e2f548e:msc.sh -- balbi signature.asc Description: Digital signature
Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote: This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend() which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend() sequentially. Then we do a tree wide update of current patterns which are present. As evident from log below this pattern is frequent in the kernel. This series can be found at git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git topic/pm_runtime_last_busy_and_autosuspend Fengguang's kbuild has tested it so it shouldn't break things for anyone. Barring one patch (explictyly mentioned in its changelog) rest are simple replacements. If all are okay, this should be merged thru PM tree as it depends on macro addition. Subhransu S. Prusty (1): PM: Add helper pm_runtime_last_busy_and_autosuspend() Vinod Koul (26): dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper drm/i915: use pm_runtime_last_busy_and_autosuspend helper drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper drm/radeon: use pm_runtime_last_busy_and_autosuspend helper vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper i2c: designware: use pm_runtime_last_busy_and_autosuspend helper i2c: omap: use pm_runtime_last_busy_and_autosuspend helper i2c: qup: use pm_runtime_last_busy_and_autosuspend helper mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper mei: use pm_runtime_last_busy_and_autosuspend helper mmc: use pm_runtime_last_busy_and_autosuspend helper mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper spi: orion: use pm_runtime_last_busy_and_autosuspend helper spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper spi: core: use pm_runtime_last_busy_and_autosuspend helper tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper video: fbdev: use pm_runtime_last_busy_and_autosuspend helper Documentation/power/runtime_pm.txt |4 ++ drivers/dma/ste_dma40.c | 30 - drivers/extcon/extcon-arizona.c |6 +-- drivers/gpu/drm/i915/intel_pm.c |3 +- drivers/gpu/drm/nouveau/nouveau_connector.c |3 +- drivers/gpu/drm/nouveau/nouveau_drm.c |9 +--- drivers/gpu/drm/radeon/radeon_connectors.c | 15 ++ drivers/gpu/drm/radeon/radeon_drv.c |5 +- drivers/gpu/drm/radeon/radeon_kms.c |6 +-- drivers/gpu/vga/vga_switcheroo.c|7 +-- drivers/i2c/busses/i2c-designware-core.c|3 +- drivers/i2c/busses/i2c-omap.c |6 +-- drivers/i2c/busses/i2c-qup.c|3 +- drivers/mfd/ab8500-gpadc.c |6 +-- drivers/mfd/arizona-irq.c |3 +- drivers/misc/mei/client.c | 12 ++ drivers/mmc/core/core.c |3 +- drivers/mmc/host/mmci.c | 12 ++ drivers/mmc/host/omap_hsmmc.c | 19 ++--- drivers/mmc/host/sdhci-pxav3.c |6 +-- drivers/mmc/host/sdhci.c|3 +- drivers/nfc/trf7970a.c |3 +- drivers/power/pm2301_charger.c |3 +- drivers/spi/spi-omap2-mcspi.c |9 +--- drivers/spi/spi-orion.c |3 +- drivers/spi/spi-ti-qspi.c |5 +- drivers/spi/spi.c |6 +-- drivers/tty/serial/omap-serial.c| 60 +-- drivers/usb/musb/omap2430.c |6 +-- drivers/video/fbdev/auo_k190x.c |9 +--- include/linux/pm_runtime.h |6 +++ 31 files changed, 97 insertions(+), 177 deletions(-) OK, I guess this is as good as it gets. What tree would you like it go through? -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: g_mass_storage bug ?
On Wed, Sep 24, 2014 at 03:06:15PM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 02:40:12PM -0500, Felipe Balbi wrote: On Wed, Sep 24, 2014 at 03:31:13PM -0400, Alan Stern wrote: On Wed, 24 Sep 2014, Felipe Balbi wrote: According to the kerneldoc for usb_ep_set_halt: * Attempts to halt IN endpoints will fail (returning -EAGAIN) if any * transfer requests are still queued, or if the controller hardware * (usually a FIFO) still holds bytes that the host hasn't collected. damn old bugs :-) I'll fix that up and Cc stable. alright fixed. Below you can see a combined diff which I'll still split into patches so they can be applied properly. And this eliminates the problems you saw with g_mass_storage? yup, working with and without stall=0, with and without debugging on. On all three systems I tested before ;-) there is still one detail which I just caught and not sure if it's something we should care. When I run my msc.sh/msc.c tests [1], after each test I see a new sdX: unknown partition table. This doesn't happen with my USB stick. it happens with the USB stick after I destroy the partition table. So I guess that's normal. -- balbi signature.asc Description: Digital signature
Re: [PATCH v6 1/2] usb: gadget: pxa27x_udc: prepare device-tree support
Felipe Balbi ba...@ti.com writes: On Wed, Sep 24, 2014 at 09:41:12PM +0200, Robert Jarzmik wrote: For this preparation, a preliminary cleanup is done : - convert the probing of pxa27x_udc to gpio_desc. The conversion is partial because : - the platform data still provides a gpio number, not a gpio desc - the invert attribute is lost, hence a loss in the translation I asked for gpio to gpiod conversion to be a separate patch. It'll make things a lot easier to review. What does patch 1/2 do then ? There are 2 lines for udc_command (1 in .h and 1.c), and all the remaining is the gpiod conversion. Is it difficult for you to review a 51 lines changed patch you asked for ? Do you want me to ask other people to help you ? - convert the mach info, and store the udc_command in the pxa_udc control structure - loose the udc_is_connected() in mach info This was not really used, as mioa701 machine doesn't need it, balloon3 doesn't really use it, and most importantly the current driver never uses it. The drawback with the gpio_desc conversion is that the inverted gpio attribute is lost, as no gpiod_*() function exists to set the it's not lost, it's handled for you by the gpio library. Look at how GPIO_ACTIVE_LOW is used. It is so, the above assertion no gpiod_*() function exists is false. Therefore, which is the right function in the gpio library accessible to a driver ? Cheers. -- Robert -- 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