Re: [PATCH 2/5] usb: gadget: s3c2410_udc: Use pr_* functions
Hi, On Wed, Aug 22, 2012 at 11:13 AM, Sachin Kamat sachin.ka...@linaro.org wrote: Replace printk with corresponding pr_* functions. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/usb/gadget/s3c2410_udc.c | 12 +++- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c index 7acecc0..7e2ce44 100644 --- a/drivers/usb/gadget/s3c2410_udc.c +++ b/drivers/usb/gadget/s3c2410_udc.c @@ -12,6 +12,8 @@ * (at your option) any later version. */ +#define pr_fmt(fmt) s3c2410_udc: fmt + Is this a stray change? I dont see pr_fmt being used anywhere in this patch.. 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: [PATCHv3 3/9] serial: vt8500: Add devicetree support for
On Tue, 2012-08-21 at 23:12 +0100, Alan Cox wrote: On Wed, 22 Aug 2012 08:47:32 +1200 Tony Prisk li...@prisktech.co.nz wrote: Signed-off-by: Tony Prisk li...@prisktech.co.nz --- drivers/tty/serial/vt8500_serial.c | 37 1 file changed, 33 insertions(+), 4 deletions(-) Can we have a comment attached to a change this size. In particular one describing why it gone from 4 to 6 ports, and why the port id twiddling. Is there a reason you can't use the device tree port id ? What are the regression risks for existing users expecting the pdev-id binding ? ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel Sorry Alan, The original patch was very simple, but I revisited it to fix other issues and forgot to add the relevant comments. Port size is changed to fix a problem - WM8505 actually had 6 uart's defined in platform data but the vt8500_ports variable was only 4. I have added devicetree port id support as well. No regression risks as the entire platform is being converted to devicetree and existing code dropped in this patchset. Patchv4 3/9 to follow. Regards Tony Prisk -- 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: [PATCHv3 3/9] serial: vt8500: Add devicetree support for
On Wednesday 22 August 2012, Tony Prisk wrote: The original patch was very simple, but I revisited it to fix other issues and forgot to add the relevant comments. Port size is changed to fix a problem - WM8505 actually had 6 uart's defined in platform data but the vt8500_ports variable was only 4. I have added devicetree port id support as well. If you do multiple things in one driver, you should normally send multiple patches as well, each with a description why that change is done. It may seem silly at first to send out a one-line patch next to a 100-line patch for the same file, but those cases are actually the ones where it's most important. 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 0/3]: ezusb cleanup, FX2 support, firmware downloading support
- Although i removed the dependency from ezusb to usb_serial, ezusb.c still resides in drivers/usb/serial. Can you give me a hint, where to put it instead? I would like to do that with an additional patch. Why do you want to move it? because is has nothing to do with any serial stuff anymore... - is it fine to base the soon-to-be-resent patches on 3.6-rc2? You should redo them against the linux-next tree, which has many more patches than 3.6-rc2. ok, thanks! greg k-h René Bürgel -- 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 v4 2/2] arm/dts: omap4: Add ocp2scp data
Add ocp2scp data node in omap4 device tree file. Acked-by: Felipe Balbi ba...@ti.com Signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- arch/arm/boot/dts/omap4.dtsi |8 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi index 04cbbcb..8a780b2 100644 --- a/arch/arm/boot/dts/omap4.dtsi +++ b/arch/arm/boot/dts/omap4.dtsi @@ -295,5 +295,13 @@ interrupt-parent = gic; ti,hwmods = dmic; }; + + ocp2scp { + compatible = ti,omap-ocp2scp; + #address-cells = 1; + #size-cells = 1; + ranges; + ti,hwmods = ocp2scp_usb_phy; + }; }; }; -- 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
[RESEND PATCH v4 0/2] omap: add ocp2scp as a bus driver
This patch series has been lying here for long. If no one has any comments on this patch series, can someone pick it up? This patch series is done as a preparatory step for adding phy drivers for dwc3 and musb. This series adds a new driver for ocp2scp (only dt) to which phy drivers are connected. Since currently there is no generic way to create a child device along with doing a pm_runtime_enable (the exact requirement for ocp2scp), I'm creating a separate driver for ocp2scp. Changes from v3: No functional changes. Fixed few comments on filling *module* details. Changes from v2: Fixed Felipe's comments to avoid using arch_initcall and make dependent drivers return -EPROBE_DEFER case this isn't ready yet. Changes from v1: Fixed Sergei's comments to remove the address in the node name of ocp2scp since the ocp2scp node doesn't have a reg property. Changes from [RFC PATCH v2 0/2] omap: add ocp2scp as a misc driver: Created a new folder drivers/bus and moved ocp2scp driver from misc to drivers/bus. This patch was developed and tested on git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git Kishon Vijay Abraham I (2): drivers: bus: add a new driver for omap-ocp2scp arm/dts: omap4: Add ocp2scp data .../devicetree/bindings/bus/omap-ocp2scp.txt | 10 +++ arch/arm/boot/dts/omap4.dtsi |8 ++ drivers/Kconfig|2 + drivers/Makefile |2 + drivers/bus/Kconfig| 15 drivers/bus/Makefile |5 ++ drivers/bus/omap-ocp2scp.c | 88 7 files changed, 130 insertions(+) create mode 100644 Documentation/devicetree/bindings/bus/omap-ocp2scp.txt create mode 100644 drivers/bus/Kconfig create mode 100644 drivers/bus/Makefile create mode 100644 drivers/bus/omap-ocp2scp.c -- 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
[RESEND PATCH v4 1/2] drivers: bus: add a new driver for omap-ocp2scp
Adds a new driver *omap-ocp2scp*. This driver takes the responsibility of creating all the devices that is connected to OCP2SCP. In the case of OMAP4, USB2PHY is connected to ocp2scp. This also includes device tree support for ocp2scp driver and the documentation with device tree binding information is updated. Acked-by: Felipe Balbi ba...@ti.com Acked-by: Arnd Bergmann a...@arndb.de Signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- .../devicetree/bindings/bus/omap-ocp2scp.txt | 10 +++ drivers/Kconfig|2 + drivers/Makefile |2 + drivers/bus/Kconfig| 15 drivers/bus/Makefile |5 ++ drivers/bus/omap-ocp2scp.c | 88 6 files changed, 122 insertions(+) create mode 100644 Documentation/devicetree/bindings/bus/omap-ocp2scp.txt create mode 100644 drivers/bus/Kconfig create mode 100644 drivers/bus/Makefile create mode 100644 drivers/bus/omap-ocp2scp.c diff --git a/Documentation/devicetree/bindings/bus/omap-ocp2scp.txt b/Documentation/devicetree/bindings/bus/omap-ocp2scp.txt new file mode 100644 index 000..d2fe064 --- /dev/null +++ b/Documentation/devicetree/bindings/bus/omap-ocp2scp.txt @@ -0,0 +1,10 @@ +* OMAP OCP2SCP - ocp interface to scp interface + +properties: +- compatible : Should be ti,omap-ocp2scp +- #address-cells, #size-cells : Must be present if the device has sub-nodes +- ranges : the child address space are mapped 1:1 onto the parent address space +- ti,hwmods : must be ocp2scp_usb_phy + +Sub-nodes: +All the devices connected to ocp2scp are described using sub-node to ocp2scp diff --git a/drivers/Kconfig b/drivers/Kconfig index ece958d..324e958 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -2,6 +2,8 @@ menu Device Drivers source drivers/base/Kconfig +source drivers/bus/Kconfig + source drivers/connector/Kconfig source drivers/mtd/Kconfig diff --git a/drivers/Makefile b/drivers/Makefile index 5b42184..f8cdeeb 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -5,6 +5,8 @@ # Rewritten to use lists instead of if-statements. # +obj-y += bus/ + # GPIO must come after pinctrl as gpios may need to mux pins etc obj-y += pinctrl/ obj-y += gpio/ diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig new file mode 100644 index 000..6270415 --- /dev/null +++ b/drivers/bus/Kconfig @@ -0,0 +1,15 @@ +# +# Bus Devices +# + +menu Bus devices + +config OMAP_OCP2SCP + tristate OMAP OCP2SCP DRIVER + help + Driver to enable ocp2scp module which transforms ocp interface + protocol to scp protocol. In OMAP4, USB PHY is connected via + OCP2SCP and in OMAP5, both USB PHY and SATA PHY is connected via + OCP2SCP. + +endmenu diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile new file mode 100644 index 000..0ec50bc --- /dev/null +++ b/drivers/bus/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the bus drivers. +# + +obj-$(CONFIG_OMAP_OCP2SCP) += omap-ocp2scp.o diff --git a/drivers/bus/omap-ocp2scp.c b/drivers/bus/omap-ocp2scp.c new file mode 100644 index 000..881d5bb --- /dev/null +++ b/drivers/bus/omap-ocp2scp.c @@ -0,0 +1,88 @@ +/* + * omap-ocp2scp.c - transform ocp interface protocol to scp protocol + * + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com + * 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; either version 2 of the License, or + * (at your option) any later version. + * + * Author: Kishon Vijay Abraham I kis...@ti.com + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include linux/module.h +#include linux/platform_device.h +#include linux/err.h +#include linux/pm_runtime.h +#include linux/of.h +#include linux/of_platform.h + +static int ocp2scp_remove_devices(struct device *dev, void *c) +{ + struct platform_device *pdev = to_platform_device(dev); + + platform_device_unregister(pdev); + + return 0; +} + +static int __devinit omap_ocp2scp_probe(struct platform_device *pdev) +{ + int ret; + struct device_node *np = pdev-dev.of_node; + + if (np) { + ret = of_platform_populate(np, NULL, NULL, pdev-dev); + if (ret) { + dev_err(pdev-dev, failed to add resources for ocp2scp child\n); + goto err0; + } + } + pm_runtime_enable(pdev-dev); + + return 0; + +err0: + device_for_each_child(pdev-dev, NULL,
Re: [PATCH 2/5] usb: gadget: s3c2410_udc: Use pr_* functions
On 22 August 2012 11:48, Felipe Balbi ba...@ti.com wrote: Hi, On Wed, Aug 22, 2012 at 11:13:16AM +0530, Sachin Kamat wrote: Replace printk with corresponding pr_* functions. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/usb/gadget/s3c2410_udc.c | 12 +++- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c index 7acecc0..7e2ce44 100644 --- a/drivers/usb/gadget/s3c2410_udc.c +++ b/drivers/usb/gadget/s3c2410_udc.c @@ -12,6 +12,8 @@ * (at your option) any later version. */ +#define pr_fmt(fmt) s3c2410_udc: fmt + #include linux/module.h #include linux/kernel.h #include linux/delay.h @@ -115,7 +117,7 @@ static int dprintk(int level, const char *fmt, ...) sizeof(printk_buf)-len, fmt, args); va_end(args); - return printk(KERN_DEBUG %s, printk_buf); + return pr_debug(%s, printk_buf); } #else static int dprintk(int level, const char *fmt, ...) @@ -1683,13 +1685,13 @@ static int s3c2410_udc_start(struct usb_gadget_driver *driver, return -EBUSY; if (!bind || !driver-setup || driver-max_speed USB_SPEED_FULL) { - printk(KERN_ERR Invalid driver: bind %p setup %p speed %d\n, + pr_err(Invalid driver: bind %p setup %p speed %d\n, bind, driver-setup, driver-max_speed); you have access to a struct device *. Please use dev_* instead. Ok. I will re-send this one after changing to dev_err. -- balbi -- With warm regards, Sachin -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] usb: gadget: s3c2410_udc: Move assignment outside if condition
Fixes the following checkpatch errors: ERROR: do not use assignment in if condition Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/usb/gadget/s3c2410_udc.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c index d9e3a21..e895ea1 100644 --- a/drivers/usb/gadget/s3c2410_udc.c +++ b/drivers/usb/gadget/s3c2410_udc.c @@ -1696,7 +1696,8 @@ static int s3c2410_udc_start(struct usb_gadget_driver *driver, udc-gadget.dev.driver = driver-driver; /* Bind the driver */ - if ((retval = device_add(udc-gadget.dev)) != 0) { + retval = device_add(udc-gadget.dev); + if (retval) { dev_err(udc-gadget.dev, Error in device_add() : %d\n, retval); goto register_error; } @@ -1704,7 +1705,8 @@ static int s3c2410_udc_start(struct usb_gadget_driver *driver, dprintk(DEBUG_NORMAL, binding gadget driver '%s'\n, driver-driver.name); - if ((retval = bind(udc-gadget)) != 0) { + retval = bind(udc-gadget); + if (retval) { device_del(udc-gadget.dev); goto register_error; } -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] usb: gadget: s3c2410_udc: Silence checkpatch errors and warnings
Silences about 75 errors and warnings related to - Spacing - Alignment of braces - Line over 80 characters Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/usb/gadget/s3c2410_udc.c | 124 ++--- 1 files changed, 60 insertions(+), 64 deletions(-) diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c index 55a571e..d9e3a21 100644 --- a/drivers/usb/gadget/s3c2410_udc.c +++ b/drivers/usb/gadget/s3c2410_udc.c @@ -127,10 +127,10 @@ static int dprintk(int level, const char *fmt, ...) #endif static int s3c2410_udc_debugfs_seq_show(struct seq_file *m, void *p) { - u32 addr_reg,pwr_reg,ep_int_reg,usb_int_reg; + u32 addr_reg, pwr_reg, ep_int_reg, usb_int_reg; u32 ep_int_en_reg, usb_int_en_reg, ep0_csr; - u32 ep1_i_csr1,ep1_i_csr2,ep1_o_csr1,ep1_o_csr2; - u32 ep2_i_csr1,ep2_i_csr2,ep2_o_csr1,ep2_o_csr2; + u32 ep1_i_csr1, ep1_i_csr2, ep1_o_csr1, ep1_o_csr2; + u32 ep2_i_csr1, ep2_i_csr2, ep2_o_csr1, ep2_o_csr2; addr_reg = udc_read(S3C2410_UDC_FUNC_ADDR_REG); pwr_reg= udc_read(S3C2410_UDC_PWR_REG); @@ -166,10 +166,10 @@ static int s3c2410_udc_debugfs_seq_show(struct seq_file *m, void *p) EP2_I_CSR2 : 0x%04X\n EP2_O_CSR1 : 0x%04X\n EP2_O_CSR2 : 0x%04X\n, - addr_reg,pwr_reg,ep_int_reg,usb_int_reg, + addr_reg, pwr_reg, ep_int_reg, usb_int_reg, ep_int_en_reg, usb_int_en_reg, ep0_csr, - ep1_i_csr1,ep1_i_csr2,ep1_o_csr1,ep1_o_csr2, - ep2_i_csr1,ep2_i_csr2,ep2_o_csr1,ep2_o_csr2 + ep1_i_csr1, ep1_i_csr2, ep1_o_csr1, ep1_o_csr2, + ep2_i_csr1, ep2_i_csr2, ep2_o_csr1, ep2_o_csr2 ); return 0; @@ -232,7 +232,7 @@ static inline void s3c2410_udc_set_ep0_de_out(void __iomem *base) { udc_writeb(base, S3C2410_UDC_INDEX_EP0, S3C2410_UDC_INDEX_REG); - udc_writeb(base,(S3C2410_UDC_EP0_CSR_SOPKTRDY + udc_writeb(base, (S3C2410_UDC_EP0_CSR_SOPKTRDY | S3C2410_UDC_EP0_CSR_DE), S3C2410_UDC_EP0_CSR_REG); } @@ -265,7 +265,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep, list_del_init(req-queue); - if (likely (req-req.status == -EINPROGRESS)) + if (likely(req-req.status == -EINPROGRESS)) req-req.status = status; else status = req-req.status; @@ -282,9 +282,9 @@ static void s3c2410_udc_nuke(struct s3c2410_udc *udc, if (ep-queue == NULL) return; - while (!list_empty (ep-queue)) { + while (!list_empty(ep-queue)) { struct s3c2410_request *req; - req = list_entry (ep-queue.next, struct s3c2410_request, + req = list_entry(ep-queue.next, struct s3c2410_request, queue); s3c2410_udc_done(ep, req, status); } @@ -391,10 +391,10 @@ static int s3c2410_udc_write_fifo(struct s3c2410_ep *ep, if (idx == 0) { /* Reset signal = no need to say 'data sent' */ - if (! (udc_read(S3C2410_UDC_USB_INT_REG) + if (!(udc_read(S3C2410_UDC_USB_INT_REG) S3C2410_UDC_USBINT_RESET)) s3c2410_udc_set_ep0_de_in(base_addr); - ep-dev-ep0state=EP0_IDLE; + ep-dev-ep0state = EP0_IDLE; } else { udc_write(idx, S3C2410_UDC_INDEX_REG); ep_csr = udc_read(S3C2410_UDC_IN_CSR1_REG); @@ -408,7 +408,7 @@ static int s3c2410_udc_write_fifo(struct s3c2410_ep *ep, } else { if (idx == 0) { /* Reset signal = no need to say 'data sent' */ - if (! (udc_read(S3C2410_UDC_USB_INT_REG) + if (!(udc_read(S3C2410_UDC_USB_INT_REG) S3C2410_UDC_USBINT_RESET)) s3c2410_udc_set_ep0_ipr(base_addr); } else { @@ -444,7 +444,7 @@ static int s3c2410_udc_read_fifo(struct s3c2410_ep *ep, u8 *buf; u32 ep_csr; unsignedbufferspace; - int is_last=1; + int is_last = 1; unsignedavail; int fifo_count = 0; u32 idx; @@ -512,7 +512,7 @@ static int s3c2410_udc_read_fifo(struct s3c2410_ep *ep, /* Only ep0 debug messages are interesting */ if (idx == 0) dprintk(DEBUG_VERBOSE, %s fifo count : %d [last %d]\n, - __func__, fifo_count,is_last); + __func__, fifo_count, is_last); if (is_last) {
[PATCH v2 1/5] usb: gadget: s3c2410_udc: Replace asm/io.h with linux/io.h
Fixes the following warning: WARNING: Use #include linux/io.h instead of asm/io.h Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/usb/gadget/s3c2410_udc.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c index f2e51f5..7acecc0 100644 --- a/drivers/usb/gadget/s3c2410_udc.c +++ b/drivers/usb/gadget/s3c2410_udc.c @@ -27,6 +27,7 @@ #include linux/clk.h #include linux/gpio.h #include linux/prefetch.h +#include linux/io.h #include linux/debugfs.h #include linux/seq_file.h @@ -35,7 +36,6 @@ #include linux/usb/gadget.h #include asm/byteorder.h -#include asm/io.h #include asm/irq.h #include asm/unaligned.h #include mach/irqs.h -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] usb: gadget: s3c2410_udc: Fixes and cleanup
This patch series fixes several errors and warnings related to coding style and compilation in s3c2410_udc driver. It is compile tested using s3c2410_defconfig based on linux-next tree. Changes since v1: Patch2: Used dev_err in function where struct device was accessible. Other patches rebased due to this change. Sachin Kamat (5): usb: gadget: s3c2410_udc: Replace asm/io.h with linux/io.h usb: gadget: s3c2410_udc: Use pr_* and dev_err functions usb: gadget: s3c2410_udc: Silence checkpatch errors and warnings usb: gadget: s3c2410_udc: Move assignment outside if condition usb: gadget: s3c2410_udc: Do not use integer for NULL drivers/usb/gadget/s3c2410_udc.c | 144 +++--- 1 files changed, 72 insertions(+), 72 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
when missing USB PHY clock, kernel booting up will hang during USB initialization. We should check USBGP[PHY_CLK_VALID] bit to avoid CPU hanging in this case. Signed-off-by: Shengzhou Liu shengzhou@freescale.com --- v2 changes: use spin_event_timeout() instead. drivers/usb/host/ehci-fsl.c | 58 +- drivers/usb/host/ehci-fsl.h |1 + include/linux/fsl_devices.h |1 + 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index b7451b2..11ff4b4 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -210,11 +210,11 @@ static void usb_hcd_fsl_remove(struct usb_hcd *hcd, usb_put_hcd(hcd); } -static void ehci_fsl_setup_phy(struct usb_hcd *hcd, +static int ehci_fsl_setup_phy(struct usb_hcd *hcd, enum fsl_usb2_phy_modes phy_mode, unsigned int port_offset) { - u32 portsc, temp; + u32 portsc; struct ehci_hcd *ehci = hcd_to_ehci(hcd); void __iomem *non_ehci = hcd-regs; struct device *dev = hcd-self.controller; @@ -232,9 +232,15 @@ static void ehci_fsl_setup_phy(struct usb_hcd *hcd, case FSL_USB2_PHY_ULPI: if (pdata-controller_ver) { /* controller version 1.6 or above */ - temp = in_be32(non_ehci + FSL_SOC_USB_CTRL); - out_be32(non_ehci + FSL_SOC_USB_CTRL, temp | - USB_CTRL_USB_EN | ULPI_PHY_CLK_SEL); + setbits32(non_ehci + FSL_SOC_USB_CTRL, + ULPI_PHY_CLK_SEL); + /* +* Due to controller issue of PHY_CLK_VALID in ULPI +* mode, we set USB_CTRL_USB_EN before checking +* PHY_CLK_VALID, otherwise PHY_CLK_VALID doesn't work. +*/ + clrsetbits_be32(non_ehci + FSL_SOC_USB_CTRL, + UTMI_PHY_EN, USB_CTRL_USB_EN); } portsc |= PORT_PTS_ULPI; break; @@ -247,9 +253,7 @@ static void ehci_fsl_setup_phy(struct usb_hcd *hcd, case FSL_USB2_PHY_UTMI: if (pdata-controller_ver) { /* controller version 1.6 or above */ - temp = in_be32(non_ehci + FSL_SOC_USB_CTRL); - out_be32(non_ehci + FSL_SOC_USB_CTRL, temp | - UTMI_PHY_EN | USB_CTRL_USB_EN); + setbits32(non_ehci + FSL_SOC_USB_CTRL, UTMI_PHY_EN); mdelay(FSL_UTMI_PHY_DLY); /* Delay for UTMI PHY CLK to become stable - 10ms*/ } @@ -262,23 +266,34 @@ static void ehci_fsl_setup_phy(struct usb_hcd *hcd, case FSL_USB2_PHY_NONE: break; } + + if ((pdata-controller_ver) ((phy_mode == FSL_USB2_PHY_ULPI) || + (phy_mode == FSL_USB2_PHY_UTMI))) { + /* check PHY_CLK_VALID to get phy clk valid */ + if (!spin_event_timeout(in_be32(non_ehci + FSL_SOC_USB_CTRL) + PHY_CLK_VALID, FSL_USB_PHY_CLK_TIMEOUT, 0)) { + printk(KERN_WARNING fsl-ehci: USB PHY clock invalid\n); + return -EINVAL; + } + } + ehci_writel(ehci, portsc, ehci-regs-port_status[port_offset]); + + if (phy_mode != FSL_USB2_PHY_ULPI) + setbits32(non_ehci + FSL_SOC_USB_CTRL, USB_CTRL_USB_EN); + + return 0; } -static void ehci_fsl_usb_setup(struct ehci_hcd *ehci) +static int ehci_fsl_usb_setup(struct ehci_hcd *ehci) { struct usb_hcd *hcd = ehci_to_hcd(ehci); struct fsl_usb2_platform_data *pdata; void __iomem *non_ehci = hcd-regs; - u32 temp; pdata = hcd-self.controller-platform_data; - /* Enable PHY interface in the control reg. */ if (pdata-have_sysif_regs) { - temp = in_be32(non_ehci + FSL_SOC_USB_CTRL); - out_be32(non_ehci + FSL_SOC_USB_CTRL, temp | 0x0004); - /* * Turn on cache snooping hardware, since some PowerPC platforms * wholly rely on hardware to deal with cache coherent @@ -293,7 +308,8 @@ static void ehci_fsl_usb_setup(struct ehci_hcd *ehci) if ((pdata-operating_mode == FSL_USB2_DR_HOST) || (pdata-operating_mode == FSL_USB2_DR_OTG)) - ehci_fsl_setup_phy(hcd, pdata-phy_mode, 0); + if (ehci_fsl_setup_phy(hcd, pdata-phy_mode, 0)) + return -EINVAL; if (pdata-operating_mode == FSL_USB2_MPH_HOST) { unsigned int chip, rev, svr; @@ -307,9 +323,12 @@ static void ehci_fsl_usb_setup(struct ehci_hcd *ehci)
Re: USB: DWC3: Missed Isoc issue
Hi, On Wed, Aug 22, 2012 at 02:39:50PM +0530, Pratyush Anand wrote: Hi Felip, I am already discussing it with SNPS, but if you have observed following with current code.. Out of two generation condition for missed isoc, only first is handled with current code. 1. when the host does not poll for all the data. 2. because of application-side delays that prevent all the data from being transferred in programmed microframe. I have observed that 2nd case does not work. I tried following , still it does not work. a. issue end transfer (dwc3_stop_active_transfer) when first missed trb is observed and wait for 100 us. b. Now do not issue start transfer from ep_queue (sceneario 3), rather wait for xfernotready and then issue start transfer. I see that second start transfer is isused with sufficient future frame number, but no xferinprogress is received. all TRBs remains with HWO=1. :( Aha, that's a great finding :-) When you miss the ISOC, I believe you should make sure to reclaim all TRBs (drop the HWO bit) and then issue another start transfer. No ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/9] usb/gadgets: move bind() callback back to struct usb_composite_driver
Hi, On Wed, Aug 15, 2012 at 09:59:13PM +0200, Sebastian Andrzej Siewior wrote: This pertly reverts 07a18bd7 (usb gadget: don't save bind callback in struct usb_composite_driver) and fixes new drivers. The section missmatch problems was solved by whitelisting bind callback in modpost. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de I don't think you should split this from patch 1, otherwise before this patch no gadget driver will work, right ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 6/9] usb/gadget: remove global struct usb_composite_driver in composite
On Wed, Aug 15, 2012 at 09:59:16PM +0200, Sebastian Andrzej Siewior wrote: The direct user of the gadget driver (composite, dbgp, inode, file_storage) keeps a global variable where it stores a pointer to something which identifies the current instance from the time calling usb_gadget_probe_driver() and later in its -bind() callback. This patch passes the struct usb_gadget_driver as an argument. Since it is embedded in the private struct of its user (composite, …), the user can restore its original data structure and avoid having a global pointer. Composite is embedding struct usb_gadget_driver. Others are unchanged. Only the old-style UDC drivers have to be touched here, new style are doing it right because this change is made in udc-core. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de looks good. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] staging/ccg: Allow to overwrite composite's setup function
Hi, On Mon, Aug 20, 2012 at 09:35:22AM +0200, Sebastian Andrzej Siewior wrote: I'm not going to swear here. The Android gadget includes composite.c from the main tree and overwrites functions. This would still work if I rename it and remove the const attribute but the problem rises again once we stop including composite.c and use it as a library function. Cc: de...@driverdev.osuosl.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de If ccg needs a different setup method, it's completely non-standard. I don't think it should be using composite.c at all. Just make ccg a standalone gadget non-dependent on this composite.c and let's move on. If later ccg wants to continue to exist in the kernel, they need to adapt to composite.c properly, instead of hacking its own stuff on composite.c's setup method. -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/9] usb/gadgets: move bind() callback back to struct usb_composite_driver
On 08/22/2012 01:33 PM, Felipe Balbi wrote: I don't think you should split this from patch 1, otherwise before this patch no gadget driver will work, right ? In #1 I have @@ -1639,7 +1640,8 @@ int usb_composite_probe(struct usb_composite_driver *driver, composite_driver.max_speed = min_t(u8, composite_driver.max_speed, driver-max_speed); composite = driver; - composite_gadget_bind = bind; + if (!driver-bind) + driver-bind = bind; return usb_gadget_probe_driver(composite_driver, composite_bind); } which sets the callback for all users which do not set it. So no, nothing should break. Sebastian -- 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 8/9] usb/gadget: Add I2C dependency for USB_LPC32XX
On Fri, Aug 17, 2012 at 03:14:10PM +0200, Sebastian Andrzej Siewior wrote: On 08/16/2012 11:31 AM, Roland Stigge wrote: USB_ISP1301 already depends on I2C. Maybe we should rather let USB_LPC32XX depends on USB_ISP1301 instead of select USB_ISP1301? And your OHCI select has the same problem. So we make both things depend on the transceiver instead of selecting it right? But now that I've seen the transceiver driver it is probably the smallest one. if it depends on the transceiver, why do we have i2c transfers on those drivers ? They should be hidden by the usb_phy_*() calls. Further questions: - why is there normal_i2c ? - why is that thing not doing anything at all? Couldn't you just move the transiver logic out of the UDC? And fail in probe if it does not find one and get probed later again? I think we do something like that in the device case where the interrupt controller is not yet probed but the currently probed device requires it. Felipe, do we have a phy framework yet or is it still in work? it's still in the works, but it's getting there. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] staging/ccg: Allow to overwrite composite's setup function
On 08/22/2012 01:42 PM, Felipe Balbi wrote: If ccg needs a different setup method, it's completely non-standard. I don't think it should be using composite.c at all. Just make ccg a standalone gadget non-dependent on this composite.c and let's move on. If later ccg wants to continue to exist in the kernel, they need to adapt to composite.c properly, instead of hacking its own stuff on composite.c's setup method. Okay. Just for the protocol: - the setup callback has a list_for_each() of a private ccg member which has _no_ users at all. - the state tracking of the udc/gadget which can be connected, disconnected, configured, … was discussed here before and it should be implemented in udc-core. So I will move it as it has been done for keucr driver. Sebastian -- 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 8/9] usb/gadget: Add I2C dependency for USB_LPC32XX
On 08/22/2012 01:43 PM, Felipe Balbi wrote: But now that I've seen the transceiver driver it is probably the smallest one. if it depends on the transceiver, why do we have i2c transfers on those drivers ? They should be hidden by the usb_phy_*() calls. Unless Roland has other plans now I would hide both (the gadget and ohci) behind the PHY bar. Further questions: - why is there normal_i2c ? - why is that thing not doing anything at all? Couldn't you just move the transiver logic out of the UDC? And fail in probe if it does not find one and get probed later again? I think we do something like that in the device case where the interrupt controller is not yet probed but the currently probed device requires it. Felipe, do we have a phy framework yet or is it still in work? it's still in the works, but it's getting there. Okay. Sebastian -- 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 8/9] usb/gadget: Add I2C dependency for USB_LPC32XX
On Wed, Aug 22, 2012 at 01:56:56PM +0200, Sebastian Andrzej Siewior wrote: On 08/22/2012 01:43 PM, Felipe Balbi wrote: But now that I've seen the transceiver driver it is probably the smallest one. if it depends on the transceiver, why do we have i2c transfers on those drivers ? They should be hidden by the usb_phy_*() calls. Unless Roland has other plans now I would hide both (the gadget and ohci) behind the PHY bar. correct. that isp1301.c is ridiculous. It's not using the (simple) phy layer we have now at all. It's also completely preventing multiple instances of this driver to be used. Quite silly implementation. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usbfs: Add a new disconnect-and-claim ioctl
On Wednesday 22 August 2012 13:42:47 Hans de Goede wrote: Apps which deal with devices which also have a kernel driver, need to do the following: 1) Check which driver is attached, so as to not detach the wrong driver (ie detaching usbfs while another instance of the app is using the device) 2) Detach the kernel driver 3) Claim the interface Where moving from one step to the next for both 1-2 and 2-3 consists of a (small) race window. So currently such apps are racy and people just live with it. This patch adds a new ioctl which makes it possible for apps to do this in a race free manner. For flexibility apps can choose to: 1) Specify the driver to disconnect 2) Specify to disconnect any driver except for the one named by the app 3) Disconnect any driver Note that if there is no driver attached, the ioctl will just act like the regular claim-interface ioctl, this is by design, as returning an error for this condition would open a new bag of race-conditions. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/usb/core/devio.c | 35 +++ include/linux/usbdevice_fs.h | 14 ++ 2 files changed, 49 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index ebb8a9d..829edce 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1928,6 +1928,38 @@ static int proc_get_capabilities(struct dev_state *ps, void __user *arg) return 0; } +static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) +{ + struct usbdevfs_disconnect_claim dc; + struct usb_interface *intf; + + if (copy_from_user(dc, arg, sizeof(dc))) + return -EFAULT; + + intf = usb_ifnum_to_if(ps-dev, dc.interface); + if (!intf) + return -EINVAL; + + if (intf-dev.driver) { + struct usb_driver *driver = to_usb_driver(intf-dev.driver); + + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) != 0) You have no idea what is in the memory behind dev.driver-name which you will happily compare to and thus access. Potentially you violate the DMA coherency rules here. + return -EBUSY; + + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) == 0) + return -EBUSY; Both flags could be set. You should catch that case. + + dev_dbg(intf-dev, disconnect by usbfs\n); + usb_driver_release_interface(driver, intf); + } + + return claimintf(ps, dc.interface); So you may return an error and yet execute a part of the command. +} + /* * NOTE: All requests here that have interface numbers as parameters * are assuming that somehow the configuration has been prevented from @@ -2101,6 +2133,9 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, case USBDEVFS_GET_CAPABILITIES: ret = proc_get_capabilities(ps, p); break; + case USBDEVFS_DISCONNECT_CLAIM: + ret = proc_disconnect_claim(ps, p); + break; } usb_unlock_device(dev); if (ret = 0) diff --git a/include/linux/usbdevice_fs.h b/include/linux/usbdevice_fs.h index 3b74666..4abe28e 100644 --- a/include/linux/usbdevice_fs.h +++ b/include/linux/usbdevice_fs.h @@ -131,6 +131,19 @@ struct usbdevfs_hub_portinfo { #define USBDEVFS_CAP_NO_PACKET_SIZE_LIM 0x04 #define USBDEVFS_CAP_BULK_SCATTER_GATHER 0x08 +/* USBDEVFS_DISCONNECT_CLAIM flags struct */ + +/* disconnect-and-claim if the driver matches the driver field */ +#define USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER 0x01 +/* disconnect-and-claim except when the driver matches the driver field */ +#define USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER 0x02 + +struct usbdevfs_disconnect_claim { + unsigned int interface; + unsigned int flags; + char driver[USBDEVFS_MAXDRIVERNAME + 1]; +}; You export this to user space. Please, please use u32 and u8. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] g_file_storage: implement -reset method
Even though it makes me wonder whether f_mass_storage isn't missing something in its original code. I wondered about that too. The difference is that f_mass_storage uses the composite framework. How does composite handle disconnects and port resets? The framework might need to be modified to pass these events down to the function drivers in separate ways. Chen Peter-B29397 b29...@freescale.com writes: f_mass_storage does not have disconnect callback, and do noop at FSG_STATE_DISCONNECT case. I can't find where f_mass_storage calls fsg_lun_fsync_sub when disconnect occurs. Yep, that seems to be the issue. I'll try to take a look at it later this week to figure out what f_mass_storage should do. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo-- pgpdNoP9glIsc.pgp Description: PGP signature
Re: [BUG] - USB3 bluetooth device not working properly?
On Wed, Aug 22, 2012 at 7:30 PM, Miroslav Sabljic miroslav.sabl...@avl.com wrote: On 08/21/2012 01:33 PM, Andiry Xu wrote: Not sure. it's hard to debug via software side because software seems does not do anything wrong. If you disable USB3 support in BIOS the ports are handled by EHCI instead. Perhaps there is something wrong with the Intel Panther Point EHCI-xHCI hand over mechanism. I don't have such a platform and I cannot tell. Maybe Sarah can give you some suggestions when she get back from vacation. Thanks for your effort Andiry! In meantime I've upgraded my BIOS but with no change regarding this issue. I'm attaching Sarah in CC and I hope she will have some more ideas how to resolve this. Also I'm attaching two more dmesg logs (for Sarah or someone else) with debugging enabled. One is aftear a clean boot when my device is working properly and you can see in log file that device VID:PID 0930:0219 is found and it is working but after I suspend my machine device is no longer working at all and I get those Timeout errors and not accepting address errors. If the device is working properly on boot up and fails after suspend/resume, you can try the patch attached to see if it helps on the failure after system resume. Thanks, Andiry -- Best regards, Miroslav test.patch Description: Binary data
Re: [PATCH] usbfs: Add a new disconnect-and-claim ioctl
Hi, Thanks for the review! On 08/22/2012 02:04 PM, Oliver Neukum wrote: On Wednesday 22 August 2012 13:42:47 Hans de Goede wrote: Apps which deal with devices which also have a kernel driver, need to do the following: 1) Check which driver is attached, so as to not detach the wrong driver (ie detaching usbfs while another instance of the app is using the device) 2) Detach the kernel driver 3) Claim the interface Where moving from one step to the next for both 1-2 and 2-3 consists of a (small) race window. So currently such apps are racy and people just live with it. This patch adds a new ioctl which makes it possible for apps to do this in a race free manner. For flexibility apps can choose to: 1) Specify the driver to disconnect 2) Specify to disconnect any driver except for the one named by the app 3) Disconnect any driver Note that if there is no driver attached, the ioctl will just act like the regular claim-interface ioctl, this is by design, as returning an error for this condition would open a new bag of race-conditions. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/usb/core/devio.c | 35 +++ include/linux/usbdevice_fs.h | 14 ++ 2 files changed, 49 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index ebb8a9d..829edce 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1928,6 +1928,38 @@ static int proc_get_capabilities(struct dev_state *ps, void __user *arg) return 0; } +static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) +{ + struct usbdevfs_disconnect_claim dc; + struct usb_interface *intf; + + if (copy_from_user(dc, arg, sizeof(dc))) + return -EFAULT; + + intf = usb_ifnum_to_if(ps-dev, dc.interface); + if (!intf) + return -EINVAL; + + if (intf-dev.driver) { + struct usb_driver *driver = to_usb_driver(intf-dev.driver); + + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) != 0) You have no idea what is in the memory behind dev.driver-name which you will happily compare to and thus access. Potentially you violate the DMA coherency rules here. Actually what is there is valid, kernel mapped (readable) memory containing a 0 terminated string. See for example also the implementation of proc_getdriver() in devio.c: if (!intf || !intf-dev.driver) ret = -ENODATA; else { strncpy(gd.driver, intf-dev.driver-name, sizeof(gd.driver)); ret = (copy_to_user(arg, gd, sizeof(gd)) ? -EFAULT : 0); } Exactly the same usage of intf-dev.driver-name except as a strncpy source rather then a strncmp argument. + return -EBUSY; + + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) == 0) + return -EBUSY; Both flags could be set. You should catch that case. I already thought about that, if both flags are set userspace will always get an -EBUSY return. I see that as userspace's problem, if they don't like it they should not set both flags. I don't think adding a separate check and returning -EINVAL is worth the trouble. + + dev_dbg(intf-dev, disconnect by usbfs\n); + usb_driver_release_interface(driver, intf); + } + + return claimintf(ps, dc.interface); So you may return an error and yet execute a part of the command. Good point, but no, claimintf will always succeed here, it will only fail if: 1) dc.interface does not point to a valid interface which we already checked 2) usb_driver_claim_interface() returns -EBUSY, which it won't since we just detached the driver. And the USB device lock is held by do_ioctl while we're called so nothing can change things underneath us. I guess this could be made more clear by changing the end of the function to: /* claimintf cannot fail here, as all conditions it checks are met */ claimintf(ps, dc.interface); return 0; } +} + /* * NOTE: All requests here that have interface numbers as parameters * are assuming that somehow the configuration has been prevented from @@ -2101,6 +2133,9 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, case USBDEVFS_GET_CAPABILITIES: ret = proc_get_capabilities(ps, p); break; + case USBDEVFS_DISCONNECT_CLAIM: + ret = proc_disconnect_claim(ps, p); + break; } usb_unlock_device(dev); if (ret = 0) diff --git a/include/linux/usbdevice_fs.h b/include/linux/usbdevice_fs.h index 3b74666..4abe28e 100644
Re: [PATCH 8/9] usb/gadget: Add I2C dependency for USB_LPC32XX
On 08/22/2012 01:56 PM, Sebastian Andrzej Siewior wrote: On 08/22/2012 01:43 PM, Felipe Balbi wrote: But now that I've seen the transceiver driver it is probably the smallest one. if it depends on the transceiver, why do we have i2c transfers on those drivers ? They should be hidden by the usb_phy_*() calls. Unless Roland has other plans now I would hide both (the gadget and ohci) behind the PHY bar. Fine with that. Thanks for working on this! Roland -- 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: [rtc-linux] [PATCHv3 8/9] arm: vt8500: gpio: Devicetree support for arch-vt8500
Converted the existing arch-vt8500 gpio to a platform_device. Added support for WM8505 and WM8650 GPIO controllers. (...) + unsigned val; I asked about the datatype for this val, it sure isn't unsigned. I suspected the registers were only 8bit and so it should be u8. But atleast use u32 if you must use all these bits. Sorry - missed adding this to my list of fixes. I've used u32 as the datasheet advises these are 32-bit registers and must be read/written as such. Changes included in v4. (...) + val = readl(vt8500_chip-base + vt8500_chip-regs-en); + val |= BIT(offset); + writel(val, vt8500_chip-base + vt8500_chip-regs-en); BTW: have you considered [readl|writel]_relaxed? I haven't - and to be honest I didn't know what difference it would make until I read up on it. Most of this code already existed in mainline in arch/arm/mach-vt8500/gpio.c and I simply moved it across to drivers/gpio and made some changes/additions. Having looked, I don't see any reason why it can't use the _relaxed variants. Changes included in v4. I'll post up v4 for review when I get home in about 7 hours. Work always gets in the way of being productive :) Regards Tony Prisk-- 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: Set usb port's DevicerRemovable according acpi information in EHCI
On Wed, 22 Aug 2012, Lan Tianyu wrote: @@ -1560,6 +1560,10 @@ static int hub_configure(struct usb_hub *hub, dev_err(hub-intfdev, couldn't create port%d device.\n, i + 1); + /* Get hub descriptor again to sync port's DeviceRemovable + * after the usb port devices being created. + */ + get_hub_descriptor(hdev, hub-descriptor); Why is this needed? Do you have any reason to think the hub descriptor has changed since the first time we retrieved it? At first time, the usb port devices have not been created and not bound with acpi. So at that time, port's DeviceRemovable is not set according acpi information. After ports' devices are created, binding procedure is completed. Request hub descriptor again and DeviceRemovable will be updated by acpi information. Why not just update the ACPI information using the current hub descriptor? You don't need to fetch the hub descriptor again. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] gadget/composite: move bind callback into driver struct
It was moved to be an argument in 07a18bd716ed5 (usb gadget: don't save bind callback in struct usb_composite_driver). The reason was to avoid the section missmatch. The warning was shown because -bind is marked as __init becuase it is a one time init. The warning can be also suppresed by whitelisting the variable i.e. rename it to lets say _probe. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/usb/gadget/composite.c | 10 ++ include/linux/usb/composite.h | 13 + 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 2cb1030..de78ca8 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -32,7 +32,6 @@ #define USB_BUFSIZ 1024 static struct usb_composite_driver *composite; -static int (*composite_gadget_bind)(struct usb_composite_dev *cdev); /* Some systems will need runtime overrides for the product identifiers * published in the device descriptor, either numbers or strings or both. @@ -1468,7 +1467,7 @@ static int composite_bind(struct usb_gadget *gadget) * serial number), register function drivers, potentially update * power state and consumption, etc */ - status = composite_gadget_bind(cdev); + status = composite-bind(cdev); if (status 0) goto fail; @@ -1627,7 +1626,9 @@ static struct usb_gadget_driver composite_driver = { int usb_composite_probe(struct usb_composite_driver *driver, int (*bind)(struct usb_composite_dev *cdev)) { - if (!driver || !driver-dev || !bind || composite) + if (!driver || !driver-dev || composite) + return -EINVAL; + if (!bind !driver-bind) return -EINVAL; if (!driver-name) @@ -1639,7 +1640,8 @@ int usb_composite_probe(struct usb_composite_driver *driver, composite_driver.max_speed = min_t(u8, composite_driver.max_speed, driver-max_speed); composite = driver; - composite_gadget_bind = bind; + if (!driver-bind) + driver-bind = bind; return usb_gadget_probe_driver(composite_driver, composite_bind); } diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h index 9d8c3b6..3153f73 100644 --- a/include/linux/usb/composite.h +++ b/include/linux/usb/composite.h @@ -257,12 +257,16 @@ void usb_remove_config(struct usb_composite_dev *, * not set. * @dev: Template descriptor for the device, including default device * identifiers. - * @strings: tables of strings, keyed by identifiers assigned during bind() + * @strings: tables of strings, keyed by identifiers assigned during @bind * and language IDs provided in control requests * @max_speed: Highest speed the driver supports. * @needs_serial: set to 1 if the gadget needs userspace to provide * a serial number. If one is not provided, warning will be printed. - * @unbind: Reverses bind; called as a side effect of unregistering + * @bind: (REQUIRED) Used to allocate resources that are shared across the + * whole device, such as string IDs, and add its configurations using + * @usb_add_config(). This may fail by returning a negative errno + * value; it should return zero on successful initialization. + * @unbind: Reverses @bind; called as a side effect of unregistering * this driver. * @disconnect: optional driver disconnect method * @suspend: Notifies when the host stops sending USB traffic, @@ -271,9 +275,9 @@ void usb_remove_config(struct usb_composite_dev *, * before function notifications * * Devices default to reporting self powered operation. Devices which rely - * on bus powered operation should report this in their @bind() method. + * on bus powered operation should report this in their @bind method. * - * Before returning from bind, various fields in the template descriptor + * Before returning from @bind, various fields in the template descriptor * may be overridden. These include the idVendor/idProduct/bcdDevice values * normally to bind the appropriate host side driver, and the three strings * (iManufacturer, iProduct, iSerialNumber) normally used to provide user @@ -291,6 +295,7 @@ struct usb_composite_driver { enum usb_device_speed max_speed; unsignedneeds_serial:1; + int (*bind)(struct usb_composite_dev *cdev); int (*unbind)(struct usb_composite_dev *); void(*disconnect)(struct usb_composite_dev *); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] usb/gadget: push all usb_composite_driver structs into __refdata
As it turns out, Sam's comment was better than I initially assumed. This patch pushes as struct usb_composite_driver data structures into __refdata section to avoid a section missmatch report from modpost because the -bind() can be marked __init. The only downside is that modpost does not check between -bind() and other member. However, it is temporary. Cc: Sam Ravnborg s...@ravnborg.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/usb/gadget/acm_ms.c |2 +- drivers/usb/gadget/audio.c |2 +- drivers/usb/gadget/cdc2.c |2 +- drivers/usb/gadget/dbgp.c |2 +- drivers/usb/gadget/ether.c |2 +- drivers/usb/gadget/file_storage.c |2 +- drivers/usb/gadget/g_ffs.c |2 +- drivers/usb/gadget/gmidi.c |2 +- drivers/usb/gadget/hid.c|2 +- drivers/usb/gadget/mass_storage.c |2 +- drivers/usb/gadget/multi.c |2 +- drivers/usb/gadget/ncm.c|2 +- drivers/usb/gadget/nokia.c |2 +- drivers/usb/gadget/printer.c|2 +- drivers/usb/gadget/serial.c |2 +- drivers/usb/gadget/tcm_usb_gadget.c |2 +- drivers/usb/gadget/webcam.c |2 +- drivers/usb/gadget/zero.c |2 +- 18 files changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c index 75b8a69..dc5cd51 100644 --- a/drivers/usb/gadget/acm_ms.c +++ b/drivers/usb/gadget/acm_ms.c @@ -232,7 +232,7 @@ static int __exit acm_ms_unbind(struct usb_composite_dev *cdev) return 0; } -static struct usb_composite_driver acm_ms_driver = { +static __refdata struct usb_composite_driver acm_ms_driver = { .name = g_acm_ms, .dev= device_desc, .max_speed = USB_SPEED_SUPER, diff --git a/drivers/usb/gadget/audio.c b/drivers/usb/gadget/audio.c index 9889924..e539490 100644 --- a/drivers/usb/gadget/audio.c +++ b/drivers/usb/gadget/audio.c @@ -198,7 +198,7 @@ static int __exit audio_unbind(struct usb_composite_dev *cdev) return 0; } -static struct usb_composite_driver audio_driver = { +static __refdata struct usb_composite_driver audio_driver = { .name = g_audio, .dev= device_desc, .strings= audio_strings, diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c index 725550f..00b65ac 100644 --- a/drivers/usb/gadget/cdc2.c +++ b/drivers/usb/gadget/cdc2.c @@ -232,7 +232,7 @@ static int __exit cdc_unbind(struct usb_composite_dev *cdev) return 0; } -static struct usb_composite_driver cdc_driver = { +static __refdata struct usb_composite_driver cdc_driver = { .name = g_cdc, .dev= device_desc, .strings= dev_strings, diff --git a/drivers/usb/gadget/dbgp.c b/drivers/usb/gadget/dbgp.c index 19d7bb0..2d2cff3 100644 --- a/drivers/usb/gadget/dbgp.c +++ b/drivers/usb/gadget/dbgp.c @@ -402,7 +402,7 @@ fail: return err; } -static struct usb_gadget_driver dbgp_driver = { +static __refdata struct usb_gadget_driver dbgp_driver = { .function = dbgp, .max_speed = USB_SPEED_HIGH, .unbind = dbgp_unbind, diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index a28f6ff..49a7dac 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -388,7 +388,7 @@ static int __exit eth_unbind(struct usb_composite_dev *cdev) return 0; } -static struct usb_composite_driver eth_driver = { +static __refdata struct usb_composite_driver eth_driver = { .name = g_ether, .dev= device_desc, .strings= dev_strings, diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c index a896d73..683234b 100644 --- a/drivers/usb/gadget/file_storage.c +++ b/drivers/usb/gadget/file_storage.c @@ -3603,7 +3603,7 @@ static void fsg_resume(struct usb_gadget *gadget) /*-*/ -static struct usb_gadget_driverfsg_driver = { +static __refdata struct usb_gadget_driver fsg_driver = { .max_speed = USB_SPEED_SUPER, .function = (char *) fsg_string_product, .unbind = fsg_unbind, diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index d3ace90..d1312c4 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -163,7 +163,7 @@ static int gfs_bind(struct usb_composite_dev *cdev); static int gfs_unbind(struct usb_composite_dev *cdev); static int gfs_do_config(struct usb_configuration *c); -static struct usb_composite_driver gfs_driver = { +static __refdata struct usb_composite_driver gfs_driver = { .name = DRIVER_NAME, .dev= gfs_dev_desc, .strings= gfs_dev_strings, diff --git
[PATCH 3/7] usb/gadget: move bind() callback back to struct usb_composite_driver
This partly reverts 07a18bd7 (usb gadget: don't save bind callback in struct usb_composite_driver) and fixes new drivers. The section missmatch problems was solved by whitelisting structs in question via __ref. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/staging/ccg/ccg.c |3 ++- drivers/usb/gadget/acm_ms.c |3 ++- drivers/usb/gadget/audio.c |3 ++- drivers/usb/gadget/cdc2.c |3 ++- drivers/usb/gadget/composite.c |9 ++--- drivers/usb/gadget/ether.c |3 ++- drivers/usb/gadget/g_ffs.c |3 ++- drivers/usb/gadget/gmidi.c |3 ++- drivers/usb/gadget/hid.c|3 ++- drivers/usb/gadget/mass_storage.c |3 ++- drivers/usb/gadget/multi.c |3 ++- drivers/usb/gadget/ncm.c|3 ++- drivers/usb/gadget/nokia.c |3 ++- drivers/usb/gadget/printer.c|3 ++- drivers/usb/gadget/serial.c |3 ++- drivers/usb/gadget/tcm_usb_gadget.c |3 ++- drivers/usb/gadget/webcam.c |3 ++- drivers/usb/gadget/zero.c |3 ++- include/linux/usb/composite.h |3 +-- 19 files changed, 37 insertions(+), 26 deletions(-) diff --git a/drivers/staging/ccg/ccg.c b/drivers/staging/ccg/ccg.c index 6a7aab8..eadda55 100644 --- a/drivers/staging/ccg/ccg.c +++ b/drivers/staging/ccg/ccg.c @@ -1162,6 +1162,7 @@ static int ccg_usb_unbind(struct usb_composite_dev *cdev) static struct usb_composite_driver ccg_usb_driver = { .name = configurable_usb, .dev= device_desc, + .bind = ccg_bind, .unbind = ccg_usb_unbind, .needs_serial = true, .iManufacturer = Linux Foundation, @@ -1275,7 +1276,7 @@ static int __init init(void) composite_driver.setup = ccg_setup; composite_driver.disconnect = ccg_disconnect; - err = usb_composite_probe(ccg_usb_driver, ccg_bind); + err = usb_composite_probe(ccg_usb_driver); if (err) { class_destroy(ccg_class); kfree(dev); diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c index dc5cd51..65a2f3c 100644 --- a/drivers/usb/gadget/acm_ms.c +++ b/drivers/usb/gadget/acm_ms.c @@ -237,6 +237,7 @@ static __refdata struct usb_composite_driver acm_ms_driver = { .dev= device_desc, .max_speed = USB_SPEED_SUPER, .strings= dev_strings, + .bind = acm_ms_bind, .unbind = __exit_p(acm_ms_unbind), }; @@ -246,7 +247,7 @@ MODULE_LICENSE(GPL v2); static int __init init(void) { - return usb_composite_probe(acm_ms_driver, acm_ms_bind); + return usb_composite_probe(acm_ms_driver); } module_init(init); diff --git a/drivers/usb/gadget/audio.c b/drivers/usb/gadget/audio.c index e539490..dd339bc 100644 --- a/drivers/usb/gadget/audio.c +++ b/drivers/usb/gadget/audio.c @@ -203,12 +203,13 @@ static __refdata struct usb_composite_driver audio_driver = { .dev= device_desc, .strings= audio_strings, .max_speed = USB_SPEED_HIGH, + .bind = audio_bind, .unbind = __exit_p(audio_unbind), }; static int __init init(void) { - return usb_composite_probe(audio_driver, audio_bind); + return usb_composite_probe(audio_driver); } module_init(init); diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c index 00b65ac..b7d984b 100644 --- a/drivers/usb/gadget/cdc2.c +++ b/drivers/usb/gadget/cdc2.c @@ -237,6 +237,7 @@ static __refdata struct usb_composite_driver cdc_driver = { .dev= device_desc, .strings= dev_strings, .max_speed = USB_SPEED_HIGH, + .bind = cdc_bind, .unbind = __exit_p(cdc_unbind), }; @@ -246,7 +247,7 @@ MODULE_LICENSE(GPL); static int __init init(void) { - return usb_composite_probe(cdc_driver, cdc_bind); + return usb_composite_probe(cdc_driver); } module_init(init); diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index de78ca8..ba87057 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -1623,12 +1623,9 @@ static struct usb_gadget_driver composite_driver = { * while it was binding. That would usually be done in order to wait for * some userspace participation. */ -int usb_composite_probe(struct usb_composite_driver *driver, - int (*bind)(struct usb_composite_dev *cdev)) +int usb_composite_probe(struct usb_composite_driver *driver) { - if (!driver || !driver-dev || composite) - return -EINVAL; - if (!bind !driver-bind) + if (!driver || !driver-dev || composite || !driver-bind) return -EINVAL; if (!driver-name) @@ -1640,8 +1637,6 @@ int usb_composite_probe(struct
[PATCH 5/7] staging/ccg: initialize ret in functionfs_ready_callback()
The ret variable is not initialized and therefore random. I guess the Android compiler is doing this right :P Cc: de...@driverdev.osuosl.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/staging/ccg/ccg.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ccg/ccg.c b/drivers/staging/ccg/ccg.c index eadda55..8fb8663 100644 --- a/drivers/staging/ccg/ccg.c +++ b/drivers/staging/ccg/ccg.c @@ -226,7 +226,7 @@ static int functionfs_ready_callback(struct ffs_data *ffs) goto done; } ffs_obj-desc_ready = true; - + ret = 0; done: mutex_unlock(_ccg_dev-mutex); return ret; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] usb/gadget: move bind() callback back to struct usb_gadget_driver
This partly reverts 07a18bd7 (usb gadget: don't save bind callback in struct usb_composite_driver) and fixes new drivers. The section missmatch problems was solved by whitelisting bind callback in modpost. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/usb/gadget/composite.c|3 ++- drivers/usb/gadget/dbgp.c |3 ++- drivers/usb/gadget/file_storage.c |4 +++- drivers/usb/gadget/inode.c|6 -- drivers/usb/gadget/udc-core.c |9 - include/linux/usb/gadget.h|6 +++--- 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index ba87057..29d5f70 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -1591,6 +1591,7 @@ static struct usb_gadget_driver composite_driver = { .max_speed = USB_SPEED_HIGH, #endif + .bind = composite_bind, .unbind = composite_unbind, .setup = composite_setup, @@ -1638,7 +1639,7 @@ int usb_composite_probe(struct usb_composite_driver *driver) min_t(u8, composite_driver.max_speed, driver-max_speed); composite = driver; - return usb_gadget_probe_driver(composite_driver, composite_bind); + return usb_gadget_probe_driver(composite_driver); } /** diff --git a/drivers/usb/gadget/dbgp.c b/drivers/usb/gadget/dbgp.c index 2d2cff3..df9a010 100644 --- a/drivers/usb/gadget/dbgp.c +++ b/drivers/usb/gadget/dbgp.c @@ -405,6 +405,7 @@ fail: static __refdata struct usb_gadget_driver dbgp_driver = { .function = dbgp, .max_speed = USB_SPEED_HIGH, + .bind = dbgp_bind, .unbind = dbgp_unbind, .setup = dbgp_setup, .disconnect = dbgp_disconnect, @@ -416,7 +417,7 @@ static __refdata struct usb_gadget_driver dbgp_driver = { static int __init dbgp_init(void) { - return usb_gadget_probe_driver(dbgp_driver, dbgp_bind); + return usb_gadget_probe_driver(dbgp_driver); } static void __exit dbgp_exit(void) diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c index 683234b..ce186fa 100644 --- a/drivers/usb/gadget/file_storage.c +++ b/drivers/usb/gadget/file_storage.c @@ -3606,6 +3606,7 @@ static void fsg_resume(struct usb_gadget *gadget) static __refdata struct usb_gadget_driver fsg_driver = { .max_speed = USB_SPEED_SUPER, .function = (char *) fsg_string_product, + .bind = fsg_bind, .unbind = fsg_unbind, .disconnect = fsg_disconnect, .setup = fsg_setup, @@ -3653,7 +3654,8 @@ static int __init fsg_init(void) if ((rc = fsg_alloc()) != 0) return rc; fsg = the_fsg; - if ((rc = usb_gadget_probe_driver(fsg_driver, fsg_bind)) != 0) + rc = usb_gadget_probe_driver(fsg_driver); + if (rc != 0) kref_put(fsg-ref, fsg_release); return rc; } diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c index e58b164..83c62a7 100644 --- a/drivers/usb/gadget/inode.c +++ b/drivers/usb/gadget/inode.c @@ -1769,6 +1769,7 @@ static struct usb_gadget_driver gadgetfs_driver = { .max_speed = USB_SPEED_FULL, #endif .function = (char *) driver_desc, + .bind = gadgetfs_bind, .unbind = gadgetfs_unbind, .setup = gadgetfs_setup, .disconnect = gadgetfs_disconnect, @@ -1791,6 +1792,7 @@ static int gadgetfs_probe (struct usb_gadget *gadget) static struct usb_gadget_driver probe_driver = { .max_speed = USB_SPEED_HIGH, + .bind = gadgetfs_probe, .unbind = gadgetfs_nop, .setup = (void *)gadgetfs_nop, .disconnect = gadgetfs_nop, @@ -1900,7 +1902,7 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) /* triggers gadgetfs_bind(); then we can enumerate. */ spin_unlock_irq (dev-lock); - value = usb_gadget_probe_driver(gadgetfs_driver, gadgetfs_bind); + value = usb_gadget_probe_driver(gadgetfs_driver); if (value != 0) { kfree (dev-buf); dev-buf = NULL; @@ -2039,7 +2041,7 @@ gadgetfs_fill_super (struct super_block *sb, void *opts, int silent) return -ESRCH; /* fake probe to determine $CHIP */ - (void) usb_gadget_probe_driver(probe_driver, gadgetfs_probe); + usb_gadget_probe_driver(probe_driver); if (!CHIP) return -ENODEV; diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c index bae243c..85e1e2f 100644 --- a/drivers/usb/gadget/udc-core.c +++ b/drivers/usb/gadget/udc-core.c @@ -311,13 +311,12 @@ EXPORT_SYMBOL_GPL(usb_del_gadget_udc); /* - */ -int
[PATCH 6/7] staging/ccg: provide a copy of composite.c for ccg
The Android gadgets does a few things wrong and keeping them working is a bad example _and_ they should be solved differntly. Since it makes further composite.c changes very difficult, ccg has its own copy for now. Cc: de...@driverdev.osuosl.org Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/staging/ccg/ccg.c |2 +- drivers/staging/ccg/composite.c | 1695 +++ 2 files changed, 1696 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/ccg/composite.c diff --git a/drivers/staging/ccg/ccg.c b/drivers/staging/ccg/ccg.c index 8fb8663..d726a8d 100644 --- a/drivers/staging/ccg/ccg.c +++ b/drivers/staging/ccg/ccg.c @@ -47,7 +47,7 @@ #include ../../usb/gadget/usbstring.c #include ../../usb/gadget/config.c #include ../../usb/gadget/epautoconf.c -#include ../../usb/gadget/composite.c +#include composite.c #include ../../usb/gadget/f_mass_storage.c #include ../../usb/gadget/u_serial.c diff --git a/drivers/staging/ccg/composite.c b/drivers/staging/ccg/composite.c new file mode 100644 index 000..9391f3b --- /dev/null +++ b/drivers/staging/ccg/composite.c @@ -0,0 +1,1695 @@ +/* + * composite.c - infrastructure for Composite USB Gadgets + * + * Copyright (C) 2006-2008 David Brownell + * + * 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; either version 2 of the License, or + * (at your option) any later version. + */ + +/* #define VERBOSE_DEBUG */ + +#include linux/kallsyms.h +#include linux/kernel.h +#include linux/slab.h +#include linux/module.h +#include linux/device.h +#include linux/utsname.h + +#include linux/usb/composite.h +#include asm/unaligned.h + +/* + * The code in this file is utility code, used to build a gadget driver + * from one or more function drivers, one or more configuration + * objects, and a usb_composite_driver by gluing them together along + * with the relevant device-wide data. + */ + +/* big enough to hold our biggest descriptor */ +#define USB_BUFSIZ 1024 + +static struct usb_composite_driver *composite; + +/* Some systems will need runtime overrides for the product identifiers + * published in the device descriptor, either numbers or strings or both. + * String parameters are in UTF-8 (superset of ASCII's 7 bit characters). + */ + +static ushort idVendor; +module_param(idVendor, ushort, 0644); +MODULE_PARM_DESC(idVendor, USB Vendor ID); + +static ushort idProduct; +module_param(idProduct, ushort, 0644); +MODULE_PARM_DESC(idProduct, USB Product ID); + +static ushort bcdDevice; +module_param(bcdDevice, ushort, 0644); +MODULE_PARM_DESC(bcdDevice, USB Device version (BCD)); + +static char *iManufacturer; +module_param(iManufacturer, charp, 0644); +MODULE_PARM_DESC(iManufacturer, USB Manufacturer string); + +static char *iProduct; +module_param(iProduct, charp, 0644); +MODULE_PARM_DESC(iProduct, USB Product string); + +static char *iSerialNumber; +module_param(iSerialNumber, charp, 0644); +MODULE_PARM_DESC(iSerialNumber, SerialNumber string); + +static char composite_manufacturer[50]; + +/*-*/ +/** + * next_ep_desc() - advance to the next EP descriptor + * @t: currect pointer within descriptor array + * + * Return: next EP descriptor or NULL + * + * Iterate over @t until either EP descriptor found or + * NULL (that indicates end of list) encountered + */ +static struct usb_descriptor_header** +next_ep_desc(struct usb_descriptor_header **t) +{ + for (; *t; t++) { + if ((*t)-bDescriptorType == USB_DT_ENDPOINT) + return t; + } + return NULL; +} + +/* + * for_each_ep_desc()- iterate over endpoint descriptors in the + * descriptors list + * @start: pointer within descriptor array. + * @ep_desc: endpoint descriptor to use as the loop cursor + */ +#define for_each_ep_desc(start, ep_desc) \ + for (ep_desc = next_ep_desc(start); \ + ep_desc; ep_desc = next_ep_desc(ep_desc+1)) + +/** + * config_ep_by_speed() - configures the given endpoint + * according to gadget speed. + * @g: pointer to the gadget + * @f: usb function + * @_ep: the endpoint to configure + * + * Return: error code, 0 on success + * + * This function chooses the right descriptors for a given + * endpoint according to gadget speed and saves it in the + * endpoint desc field. If the endpoint already has a descriptor + * assigned to it - overwrites it with currently corresponding + * descriptor. The endpoint maxpacket field is updated according + * to the chosen descriptor. + * Note: the supplied function should hold all the descriptors + * for supported speeds + */ +int config_ep_by_speed(struct usb_gadget *g, + struct usb_function *f, + struct usb_ep *_ep) +{ + struct usb_composite_dev*cdev =
[PATCH 7/7] usb/gadget: remove global struct usb_composite_driver in composite
The direct user of the gadget driver (composite, dbgp, inode, file_storage) keeps a global variable where it stores a pointer to something which identifies the current instance from the time calling usb_gadget_probe_driver() and later in its -bind() callback. This patch passes the struct usb_gadget_driver as an argument. Since it is embedded in the private struct of its user (composite, …), the user can restore its original data structure and avoid having a global pointer. Composite is embedding struct usb_gadget_driver. Others are unchanged. Only the old-style UDC drivers have to be touched here, new style are doing it right because this change is made in udc-core. Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/usb/gadget/amd5536udc.c |6 +-- drivers/usb/gadget/composite.c| 82 - drivers/usb/gadget/dbgp.c |3 +- drivers/usb/gadget/file_storage.c |3 +- drivers/usb/gadget/fsl_udc_core.c |6 +-- drivers/usb/gadget/fusb300_udc.c |4 +- drivers/usb/gadget/goku_udc.c |6 +-- drivers/usb/gadget/inode.c|7 ++-- drivers/usb/gadget/lpc32xx_udc.c |6 +-- drivers/usb/gadget/m66592-udc.c |4 +- drivers/usb/gadget/mv_udc_core.c |6 +-- drivers/usb/gadget/omap_udc.c |6 +-- drivers/usb/gadget/pch_udc.c |6 +-- drivers/usb/gadget/pxa25x_udc.c |6 +-- drivers/usb/gadget/pxa27x_udc.c |6 +-- drivers/usb/gadget/s3c2410_udc.c |7 ++-- drivers/usb/gadget/udc-core.c |4 +- include/linux/usb/composite.h |1 + include/linux/usb/gadget.h|6 ++- 19 files changed, 95 insertions(+), 80 deletions(-) diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c index 187d211..fc0ec5e 100644 --- a/drivers/usb/gadget/amd5536udc.c +++ b/drivers/usb/gadget/amd5536udc.c @@ -1401,7 +1401,7 @@ static int udc_wakeup(struct usb_gadget *gadget) } static int amd5536_start(struct usb_gadget_driver *driver, - int (*bind)(struct usb_gadget *)); + int (*bind)(struct usb_gadget *, struct usb_gadget_driver *)); static int amd5536_stop(struct usb_gadget_driver *driver); /* gadget operations */ static const struct usb_gadget_ops udc_ops = { @@ -1914,7 +1914,7 @@ static int setup_ep0(struct udc *dev) /* Called by gadget driver to register itself */ static int amd5536_start(struct usb_gadget_driver *driver, - int (*bind)(struct usb_gadget *)) + int (*bind)(struct usb_gadget *, struct usb_gadget_driver *)) { struct udc *dev = udc; int retval; @@ -1932,7 +1932,7 @@ static int amd5536_start(struct usb_gadget_driver *driver, dev-driver = driver; dev-gadget.dev.driver = driver-driver; - retval = bind(dev-gadget); + retval = bind(dev-gadget, driver); /* Some gadget drivers use both ep0 directions. * NOTE: to gadget driver, ep0 is just one endpoint... diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 29d5f70..598df69 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -31,8 +31,6 @@ /* big enough to hold our biggest descriptor */ #define USB_BUFSIZ 1024 -static struct usb_composite_driver *composite; - /* Some systems will need runtime overrides for the product identifiers * published in the device descriptor, either numbers or strings or both. * String parameters are in UTF-8 (superset of ASCII's 7 bit characters). @@ -889,6 +887,7 @@ static int lookup_string( static int get_string(struct usb_composite_dev *cdev, void *buf, u16 language, int id) { + struct usb_composite_driver *cdriver = cdev-driver; struct usb_configuration*c; struct usb_function *f; int len; @@ -907,7 +906,7 @@ static int get_string(struct usb_composite_dev *cdev, memset(s, 0, 256); s-bDescriptorType = USB_DT_STRING; - sp = composite-strings; + sp = cdriver-strings; if (sp) collect_langs(sp, s-wData); @@ -936,12 +935,12 @@ static int get_string(struct usb_composite_dev *cdev, * check if the string has not been overridden. */ if (cdev-manufacturer_override == id) - str = iManufacturer ?: composite-iManufacturer ?: + str = iManufacturer ?: cdriver-iManufacturer ?: composite_manufacturer; else if (cdev-product_override == id) - str = iProduct ?: composite-iProduct; + str = iProduct ?: cdriver-iProduct; else if (cdev-serial_override == id) - str = iSerialNumber ?: composite-iSerialNumber; + str = iSerialNumber ?: cdriver-iSerialNumber; else str = NULL; if (str)
Re: [RFC PATCH 2/2] USB: Set usb port's DevicerRemovable according acpi information in EHCI
于 2012/8/22 22:32, Alan Stern 写道: On Wed, 22 Aug 2012, Lan Tianyu wrote: @@ -1560,6 +1560,10 @@ static int hub_configure(struct usb_hub *hub, dev_err(hub-intfdev, couldn't create port%d device.\n, i + 1); + /* Get hub descriptor again to sync port's DeviceRemovable +* after the usb port devices being created. +*/ + get_hub_descriptor(hdev, hub-descriptor); Why is this needed? Do you have any reason to think the hub descriptor has changed since the first time we retrieved it? At first time, the usb port devices have not been created and not bound with acpi. So at that time, port's DeviceRemovable is not set according acpi information. After ports' devices are created, binding procedure is completed. Request hub descriptor again and DeviceRemovable will be updated by acpi information. Why not just update the ACPI information using the current hub descriptor? You don't need to fetch the hub descriptor again. You mean to set DeviceRemovable directly rather than via hub descriptor request here, right? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 1/2] usb: usb port power off mechanism add auto option
于 2012/8/22 22:39, Alan Stern 写道: On Wed, 22 Aug 2012, Lan Tianyu wrote: You forgot to change the logic in hub_power_on(). You have to handle the USB_PORT_POWER_AUTO case. Yeah. Thanks for reminder. How about following? @@ -858,7 +860,14 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay) if (hub-ports[port1 - 1]-port_power_policy == USB_PORT_POWER_ON) set_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER); - else + else if (hub-ports[port1 - 1]-port_power_policy + == USB_PORT_POWER_AUTO) { + if (hub-ports[port1 - 1]-connect_type + == USB_PORT_NOT_USED +!hub-ports[port1 - 1]-child) + clear_port_feature(hub-hdev, port1, + USB_PORT_FEAT_POWER); + } else clear_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER); This doesn't handle all the cases, and it has duplicated code. It would be better to do something like this: p = hub-ports[port1 - 1]; if (p-port_power_policy == USB_PORT_POWER_OFF || (p-port_power_policy == USB_PORT_POWER_AUTO p-connect_type == USB_PORT_NOT_USED !p-child)) clear_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER); else set_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER); Alan Stern OK. I get it. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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] usbfs: Add a new disconnect-and-claim ioctl
On Wed, 22 Aug 2012, Hans de Goede wrote: Apps which deal with devices which also have a kernel driver, need to do the following: 1) Check which driver is attached, so as to not detach the wrong driver (ie detaching usbfs while another instance of the app is using the device) 2) Detach the kernel driver 3) Claim the interface Where moving from one step to the next for both 1-2 and 2-3 consists of a (small) race window. So currently such apps are racy and people just live with it. This patch adds a new ioctl which makes it possible for apps to do this in a race free manner. For flexibility apps can choose to: 1) Specify the driver to disconnect 2) Specify to disconnect any driver except for the one named by the app 3) Disconnect any driver Note that if there is no driver attached, the ioctl will just act like the regular claim-interface ioctl, this is by design, as returning an error for this condition would open a new bag of race-conditions. ... + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) != 0) + return -EBUSY; + + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) == 0) + return -EBUSY; Please don't indent continuation lines by exactly one tab stop. It makes them look like the start of a sub-block. On 08/22/2012 02:04 PM, Oliver Neukum wrote: ... + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) != 0) You have no idea what is in the memory behind dev.driver-name which you will happily compare to and thus access. Potentially you violate the DMA coherency rules here. Actually what is there is valid, kernel mapped (readable) memory containing a 0 terminated string. See for example also the implementation of proc_getdriver() in devio.c: if (!intf || !intf-dev.driver) ret = -ENODATA; else { strncpy(gd.driver, intf-dev.driver-name, sizeof(gd.driver)); ret = (copy_to_user(arg, gd, sizeof(gd)) ? -EFAULT : 0); } Exactly the same usage of intf-dev.driver-name except as a strncpy source rather then a strncmp argument. Right. Oliver, driver-name is used in other places too, like dev_driver_string() in the device core (its complications concern whether or not the device is bound to a driver, not retrieving the driver's name). +struct usbdevfs_disconnect_claim { + unsigned int interface; + unsigned int flags; + char driver[USBDEVFS_MAXDRIVERNAME + 1]; +}; You export this to user space. Please, please use u32 and u8. I can understand where you're coming from, but: 1) If you search for interface in include/linux/usbdevice_fs.h, all declarations of it are unsigned int, so using u32 would be inconsistent 2) If you look at the existing struct usbdevfs_getdriver it contains: char driver[USBDEVFS_MAXDRIVERNAME + 1]; 3) If you look at the existing struct usbdevfs_urb it contains: unsigned int flags; So making the requested change would be inconsistent. With that said I'm fine either way, but it seems better to keep things as is. There's no reason to specify an explicit size. Sizes are needed when structures have to match up with external data (such as USB descriptors), but that's not true here. The only requirement is that the compilers used to build the kernel and the application agree on the size of unsigned int. If they didn't then things like struct dirent wouldn't work (it contains an unsigned short). Now if the type were unsigned long then you'd have to worry about 32-bit vs. 64-bit compatibility. In that case an explicit size would be justified. But not for unsigned int. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb composite: fix locking in usb_function_activate
On Wed, 22 Aug 2012, Michael Grzeschik wrote: On Thu, Jul 19, 2012 at 12:20:11AM +0200, Michael Grzeschik wrote: The lockdep hunter mentions a non consistent usage of spin_lock and spin_lock_irqsafe in the composite_disconnect and usb_function_activate function: [ 15.700897] = [ 15.705255] [ INFO: inconsistent lock state ] [ 15.709617] 3.5.0-rc5+ #413 Not tainted [ 15.713453] - [ 15.717812] inconsistent {HARDIRQ-ON-W} - {IN-HARDIRQ-W} usage. [ 15.723822] uvc-gadget/116 [HC1[1]:SC0[0]:HE0:SE1] takes: [ 15.729222] ((cdev-lock)-rlock){?.+...}, at: [7f0049e8] composite_disconnect+0x2c/0x74 [g_webcam] [ 15.738797] {HARDIRQ-ON-W} state was registered at: [ 15.743677] [8006de3c] mark_lock+0x148/0x688 [ 15.748325] [8006ecb0] __lock_acquire+0x934/0x1b74 [ 15.753481] [8007047c] lock_acquire+0x98/0x138 [ 15.758288] [804c776c] _raw_spin_lock+0x4c/0x84 [ 15.763188] [7f006ae4] usb_function_activate+0x28/0x94 [g_webcam] [ 15.769652] [7f00820c] usb_ep_autoconfig_reset+0x78/0x98 [g_webcam] [ 15.776287] [7f0082a4] uvc_v4l2_open+0x78/0x94 [g_webcam] --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -300,9 +300,10 @@ int usb_function_deactivate(struct usb_function *function) int usb_function_activate(struct usb_function *function) { struct usb_composite_dev*cdev = function-config-cdev; + unsigned long flags; int status = 0; - spin_lock(cdev-lock); + spin_lock_irqsave(cdev-lock, flags); if (WARN_ON(cdev-deactivations == 0)) status = -EINVAL; @@ -312,7 +313,7 @@ int usb_function_activate(struct usb_function *function) status = usb_gadget_connect(cdev-gadget); } - spin_unlock(cdev-lock); + spin_unlock_irqrestore(cdev-lock, flags); return status; } Wouldn't it be better to fix the uvc-gadget driver to avoid calling these functions in interrupt context? Or have I misunderstood the flow of control? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb composite: fix locking in usb_function_activate
On Thu, Jul 19, 2012 at 12:20:11AM +0200, Michael Grzeschik wrote: The lockdep hunter mentions a non consistent usage of spin_lock and spin_lock_irqsafe in the composite_disconnect and usb_function_activate function: [...] Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Acked-by: Michal Nazarewicz min...@mina86.com --- drivers/usb/gadget/composite.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 3f72110..2cb1030 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -300,9 +300,10 @@ int usb_function_deactivate(struct usb_function *function) int usb_function_activate(struct usb_function *function) { struct usb_composite_dev*cdev = function-config-cdev; + unsigned long flags; int status = 0; - spin_lock(cdev-lock); + spin_lock_irqsave(cdev-lock, flags); if (WARN_ON(cdev-deactivations == 0)) status = -EINVAL; @@ -312,7 +313,7 @@ int usb_function_activate(struct usb_function *function) status = usb_gadget_connect(cdev-gadget); } - spin_unlock(cdev-lock); + spin_unlock_irqrestore(cdev-lock, flags); return status; } -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo-- pgppbAlgxJhhU.pgp Description: PGP signature
Re: [RFC PATCH 2/2] USB: Set usb port's DevicerRemovable according acpi information in EHCI
On Wed, 22 Aug 2012, Lan Tianyu wrote: At first time, the usb port devices have not been created and not bound with acpi. So at that time, port's DeviceRemovable is not set according acpi information. After ports' devices are created, binding procedure is completed. Request hub descriptor again and DeviceRemovable will be updated by acpi information. Why not just update the ACPI information using the current hub descriptor? You don't need to fetch the hub descriptor again. You mean to set DeviceRemovable directly rather than via hub descriptor request here, right? Right. DeviceRemovable should be set in only one place, and this should be that place. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb composite: fix locking in usb_function_activate
Alan Stern st...@rowland.harvard.edu writes: On Wed, 22 Aug 2012, Michael Grzeschik wrote: On Thu, Jul 19, 2012 at 12:20:11AM +0200, Michael Grzeschik wrote: The lockdep hunter mentions a non consistent usage of spin_lock and spin_lock_irqsafe in the composite_disconnect and usb_function_activate function: [ 15.700897] = [ 15.705255] [ INFO: inconsistent lock state ] [ 15.709617] 3.5.0-rc5+ #413 Not tainted [ 15.713453] - [ 15.717812] inconsistent {HARDIRQ-ON-W} - {IN-HARDIRQ-W} usage. [ 15.723822] uvc-gadget/116 [HC1[1]:SC0[0]:HE0:SE1] takes: [ 15.729222] ((cdev-lock)-rlock){?.+...}, at: [7f0049e8] composite_disconnect+0x2c/0x74 [g_webcam] [ 15.738797] {HARDIRQ-ON-W} state was registered at: [ 15.743677] [8006de3c] mark_lock+0x148/0x688 [ 15.748325] [8006ecb0] __lock_acquire+0x934/0x1b74 [ 15.753481] [8007047c] lock_acquire+0x98/0x138 [ 15.758288] [804c776c] _raw_spin_lock+0x4c/0x84 [ 15.763188] [7f006ae4] usb_function_activate+0x28/0x94 [g_webcam] [ 15.769652] [7f00820c] usb_ep_autoconfig_reset+0x78/0x98 [g_webcam] [ 15.776287] [7f0082a4] uvc_v4l2_open+0x78/0x94 [g_webcam] --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -300,9 +300,10 @@ int usb_function_deactivate(struct usb_function *function) int usb_function_activate(struct usb_function *function) { struct usb_composite_dev*cdev = function-config-cdev; + unsigned long flags; int status = 0; - spin_lock(cdev-lock); + spin_lock_irqsave(cdev-lock, flags); if (WARN_ON(cdev-deactivations == 0)) status = -EINVAL; @@ -312,7 +313,7 @@ int usb_function_activate(struct usb_function *function) status = usb_gadget_connect(cdev-gadget); } - spin_unlock(cdev-lock); + spin_unlock_irqrestore(cdev-lock, flags); return status; } Wouldn't it be better to fix the uvc-gadget driver to avoid calling these functions in interrupt context? Or have I misunderstood the flow of control? It goes the other way around. f_uvc calls usb_function_activate outside of interrupt context. Ie. it is called from uvc_v4l2_open() which is an open fop. In either case though, I feel like it's hardly a critical path, so there's not much sense to worry about performance, and the only drawback of irqsave is performance. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo-- pgpgp5m0zXcyy.pgp Description: PGP signature
Re: [PATCH] usbfs: Add a new disconnect-and-claim ioctl
Hi, On 08/22/2012 05:05 PM, Alan Stern wrote: On Wed, 22 Aug 2012, Hans de Goede wrote: Apps which deal with devices which also have a kernel driver, need to do the following: 1) Check which driver is attached, so as to not detach the wrong driver (ie detaching usbfs while another instance of the app is using the device) 2) Detach the kernel driver 3) Claim the interface Where moving from one step to the next for both 1-2 and 2-3 consists of a (small) race window. So currently such apps are racy and people just live with it. This patch adds a new ioctl which makes it possible for apps to do this in a race free manner. For flexibility apps can choose to: 1) Specify the driver to disconnect 2) Specify to disconnect any driver except for the one named by the app 3) Disconnect any driver Note that if there is no driver attached, the ioctl will just act like the regular claim-interface ioctl, this is by design, as returning an error for this condition would open a new bag of race-conditions. ... + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) != 0) + return -EBUSY; + + if ((dc.flags USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER) + strncmp(dc.driver, intf-dev.driver-name, + sizeof(dc.driver)) == 0) + return -EBUSY; Please don't indent continuation lines by exactly one tab stop. It makes them look like the start of a sub-block. Ok, so you want 4 spaces there to but the 's' of strncmp below the second '(' I assume? I actually had that first, then changed to this :) Before I respin the patch, any other remarks from you? Regards, Hams -- 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] xhci: Unconditionally switch ports to xHCI on powerup
If this is a must-to-do thing for Intel Panther Point platform, then we need to make sure it's called on power up and resume. Yes, I think moving the code below hc_init label should work and I think it's a better solution than your original patch. Yes it effects a lot of machines (Lenovo), I will send out a new seperate patch with the fix. Thanks Manoj Thanks, Andiry On Tue, 21 Aug 2012, Andiry Xu wrote: On Tue, Aug 21, 2012 at 12:06 PM, manoj.i...@canonical.com wrote: From: Manoj Iyer manoj.i...@canonical.com USB 3.0 devices show up as high-speed devices on powerup, after an s3 cycle they are correctly recognized as SuperSpeed. At powerup unconditionally switch the port to xHCI like we do when we resume from suspend. BugLink: http://bugs.launchpad.net/bugs/1000424 Signed-off-by: Manoj Iyer manoj.i...@canonical.com --- drivers/usb/host/xhci-pci.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 9bfd4ca11..5c8dbea 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -48,6 +48,14 @@ static int xhci_pci_reinit(struct xhci_hcd *xhci, struct pci_dev *pdev) if (!pci_set_mwi(pdev)) xhci_dbg(xhci, MWI active\n); + /* +* USB SuperSpeed ports are recognized as HighSpeed ports on powerup +* unconditionally switch the ports to xHCI like we do when resume +* from suspend. +*/ + if (usb_is_intel_switchable_xhci(pdev)) + usb_enable_xhci_ports(pdev); + Strange. This should have been called during system power up, in quirk_usb_handoff_xhci() of pci_quirks.c. Do you see that routine get called during power up? Thanks, Andiry xhci_dbg(xhci, Finished xhci_pci_reinit\n); return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Manoj Iyer Ubuntu/Canonical Hardware Enablement -- Manoj Iyer Ubuntu/Canonical Hardware Enablement -- 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 7/7] usb/gadget: remove global struct usb_composite_driver in composite
On 08/22/2012 05:28 PM, Alan Stern wrote: Pardon me for being stupid, but I don't understand this description at all. Example one, on the first line: How is file_storage a direct user of the gadget driver? It _is_ the gadget driver! I meant the fact that that file_storage is using struct usb_gadget_driver directly. In contrast to this most other do use composite as the glue layer. Example two, on the fifth line: The patch passes the usb_gadget_driver structure as an argument to what function? There are lots of other examples. It's clear that the purpose of this patch is to change composite.c. The other source files just go along for the ride -- their bind routines get an additional, unused argument. Therefore the patch description should explain what part of composite.c is being changed and why the change is needed. Okay. I will provide a better description. Alan Stern Sebastian -- 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: PROBLEM: Certain USB devices wake up the system after power off
On Wed, 22 Aug 2012, Àlex Magaz Graça wrote: Hi, [1.] One line summary of the problem: Certain USB devices wake up the system after power off [2.] Full description of the problem/report: The computer no longer shuts down properly after upgrading from Ubuntu 11.04 to 11.10 (this is from kernel 2.6.38 to 3.0.20 (*)), it powers on immediately after being shut down. WORKAROUND: It doesn't happen if any of the problematic USB devices are not plugged. I perform the following the following steps: 1. Switch to a virtual terminal (no XWindow) and log in. 2. Run: sudo poweroff 3. The computer shuts down (I see how all LEDs switch off and hear the hard drive stopping). 4. Immediately the computer powers on again. It only fails when the device is plugged in on the back USB ports, NOT in the ones in the front panel. - Fails with: WiFi card or pendrive. - Doesn't fail with: printer, keyboard/mouse wireless receiver, bluetooth dongle. If I boot passing acpi=off to the kernel command line, the computer doesn't shut down. After bisecting I've found the problem was introduced in commit c61875977458637226ab093a35d200f2d5789787. OHCI: final fix for NVIDIA problems (I hope). That patch affects shutdown for all OHCI controllers, not just NVIDIA controllers. On the other hand, it's strange that the problem is triggered only by devices that don't use OHCI. Have you checked for any BIOS updates from the manufacturer? The lsusb output attached to your bug report indicates that only two devices were plugged in: the WiFi card and the wireless receiver. What happens if the only USB device attached is the WiFi card? (For best testing, unplug the receiver and other things before you boot.) Please post the output from: ls -d /sys/bus/pci/drivers/?hci_hcd/*/usb? Also, post the output from lsusb with the WiFi card plugged into a rear port and the pendrive plugged into a front port. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1] xhci: Recognize USB 3.0 devices as superspeed at powerup
From: Manoj Iyer manoj.i...@canonical.com On Intel Panther Point chipset USB 3.0 devices show up as high-speed devices on powerup, but after an s3 cycle they are correctly recognized as SuperSpeed. At powerup switch the port to xHCI so that USB 3.0 devices are correctly recognized. This is a second attempt at fixing this based on review comments to my earlier patch from Andiry Xu BugLink: http://bugs.launchpad.net/bugs/1000424 Test results after applying patch == USB 3.0 device connected to SuperSpeed port [ 34.694033] usb 4-1: new SuperSpeed USB device number 2 using xhci_hcd [ 34.712925] scsi6 : usb-storage 4-1:1.0 [ 35.709947] scsi 6:0:0:0: Direct-Access JetFlash Transcend 16GB 1.00 PQ: 0 ANSI: 5 [ 35.710886] sd 6:0:0:0: Attached scsi generic sg3 type 0 [ 35.711475] sd 6:0:0:0: [sdc] 30871552 512-byte logical blocks: (15.8 GB/14.7 GiB) [ 35.712721] sd 6:0:0:0: [sdc] Write Protect is off [ 35.712725] sd 6:0:0:0: [sdc] Mode Sense: 23 00 00 00 [ 35.712841] sd 6:0:0:0: [sdc] Write cache: disabled, read cache: disabled, doesn't support DPO or FUA [ 35.714627] sdc: sdc1 USB 3.0 device connected to highspeed port --- [ 148.128616] usb 1-1.2: new high-speed USB device number 5 using ehci_hcd [ 148.226040] scsi7 : usb-storage 1-1.2:1.0 [ 149.225645] scsi 7:0:0:0: Direct-Access JetFlash Transcend 16GB 1.00 PQ: 0 ANSI: 5 [ 149.227690] sd 7:0:0:0: Attached scsi generic sg3 type 0 [ 149.230196] sd 7:0:0:0: [sdc] 30871552 512-byte logical blocks: (15.8 GB/14.7 GiB) [ 149.230555] sd 7:0:0:0: [sdc] Write Protect is off [ 149.230563] sd 7:0:0:0: [sdc] Mode Sense: 23 00 00 00 [ 149.231333] sd 7:0:0:0: [sdc] Write cache: disabled, read cache: disabled, doesn't support DPO or FUA [ 149.235822] sdc: sdc1 [ 149.238035] sd 7:0:0:0: [sdc] Attached SCSI removable disk Manoj Iyer (1): xhci: Recognize USB 3.0 devices as superspeed at powerup drivers/usb/host/pci-quirks.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] xhci: Recognize USB 3.0 devices as superspeed at powerup
From: Manoj Iyer manoj.i...@canonical.com On Intel Panther Point chipset USB 3.0 devices show up as high-speed devices on powerup, but after an s3 cycle they are correctly recognized as SuperSpeed. At powerup switch the port to xHCI so that USB 3.0 devices are correctly recognized. BugLink: http://bugs.launchpad.net/bugs/1000424 Signed-off-by: Manoj Iyer manoj.i...@canonical.com --- drivers/usb/host/pci-quirks.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index c5e9e4a..486e812 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -870,9 +870,10 @@ static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev) /* Disable any BIOS SMIs and clear all SMI events*/ writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET); +hc_init: if (usb_is_intel_switchable_xhci(pdev)) usb_enable_xhci_ports(pdev); -hc_init: + op_reg_base = base + XHCI_HC_LENGTH(readl(base)); /* Wait for the host controller to be ready before writing any -- 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: USB: DWC3: Missed Isoc issue
On 8/22/2012 5:01 PM, Felipe Balbi wrote: Hi, On Wed, Aug 22, 2012 at 02:39:50PM +0530, Pratyush Anand wrote: Hi Felip, I am already discussing it with SNPS, but if you have observed following with current code.. Out of two generation condition for missed isoc, only first is handled with current code. 1. when the host does not poll for all the data. 2. because of application-side delays that prevent all the data from being transferred in programmed microframe. I have observed that 2nd case does not work. I tried following , still it does not work. a. issue end transfer (dwc3_stop_active_transfer) when first missed trb is observed and wait for 100 us. b. Now do not issue start transfer from ep_queue (sceneario 3), rather wait for xfernotready and then issue start transfer. I see that second start transfer is isused with sufficient future frame number, but no xferinprogress is received. all TRBs remains with HWO=1. :( Aha, that's a great finding :-) When you miss the ISOC, I believe you should make sure to reclaim all TRBs (drop the HWO bit) and then issue another start transfer. No ? Hummm...Not sure.. When missed isoc is received HWO is already reset for that TRB. So with current code flow goes like this: -- I use usbtest (test 16 ISOC IN) to test this. -- use pattern=2 bInterval=1 in f_sourcesink -- Now put deliberately 125 us delay in case 2 of reinit_write_data to cause missed isoc after some time -- When set interface is received, gadget issues 8 ep_queue -- They all are added to dep-request_list -- When xfernotready is received, TRBs (TRB0 to TRB7) are prepared from request_list. start transfer is issued with TRB0. -- When first xferinprogress is received, update xfer is issued with TRB1 [This is wrong, it should be issued with TRB8, comment in code is perfect that req points to the first request where HWO changed from 0 to 1, however it does not work that way. I will correct it.] -- anyway, this flow goes on till TRB25, and they all are transferred. -- TRB26-30 and TRB0-TRB2 has missed isoc. When missed isoc is set, HWO is also reset to zero by controller. -- Now I made some modifications: when missed isoc happens, issue end transfer. When TRB26 was cleaned, end transfer was issued. During giveback/ep_queue of TRB26-30 TRB0-2 new request was only added in the request_list. So, we again have 8 new request in list. -- Now, when 2nd xfernotready was received, again TRB0-7 are prepared and start transfer is issued with trb0. At this moment, hwo=1 for TRB0-7 and 0 for TRB8-30. TRB31 is linked and not touched. I expect TRB0-7 to be transferred, but it does not happen. Regards Pratyush -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301
The gadget case is, just depend on the ISP1301 instead of selecting it. The OHCI case is little more difficult. It is not possible to say select if and on top of it, the phy depends on USB which depends on OHCI. This started as a fix for: | drivers/usb/gadget/lpc32xx_udc.c: In function ‘isp1301_udc_configure’: | drivers/usb/gadget/lpc32xx_udc.c:606: error: implicit declaration of function ‘i2c_smbus_write_byte_data’ | drivers/usb/gadget/lpc32xx_udc.c:665: error: implicit declaration of function ‘i2c_smbus_read_word_data’ | drivers/usb/gadget/lpc32xx_udc.c: In function ‘vbus_work’: | drivers/usb/gadget/lpc32xx_udc.c:2959: error: implicit declaration of function ‘i2c_smbus_read_byte_data’ Cc: Alexandre Pereira da Silva aletes@gmail.com Cc: Roland Stigge sti...@antcom.de Cc: Alan Stern st...@rowland.harvard.edu Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/usb/gadget/Kconfig |3 +-- drivers/usb/host/Kconfig| 10 +- drivers/usb/host/ohci-hcd.c |2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 51ab5fd..1494112 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -143,8 +143,7 @@ config USB_AT91 config USB_LPC32XX tristate LPC32XX USB Peripheral Controller - depends on ARCH_LPC32XX - select USB_ISP1301 + depends on ARCH_LPC32XX USB_ISP1301 help This option selects the USB device controller in the LPC32xx SoC. diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index c3f619b..cac3ee2 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -292,7 +292,6 @@ config USB_OHCI_HCD depends on USB USB_ARCH_HAS_OHCI select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3 select USB_OTG_UTILS if ARCH_OMAP - select USB_ISP1301 if ARCH_LPC32XX || ARCH_PNX4008 ---help--- The Open Host Controller Interface (OHCI) is a standard for accessing USB 1.1 host controller hardware. It does more in hardware than Intel's @@ -412,6 +411,15 @@ config USB_CNS3XXX_OHCI Enable support for the CNS3XXX SOC's on-chip OHCI controller. It is needed for low-speed USB 1.0 device support. +config USB_OHCI_LPC32XX_PNX4008 + bool LPC32XX or PNX4008 OHCI Module + depends on USB_OHCI_HCD + depends on USB_ISP1301 + ---help--- + Enable support for the LPC32XX or PNX4008 SOC's on-chip + OHCI controller. + It is needed for low-speed USB 1.0 device support. + config USB_OHCI_HCD_PLATFORM bool Generic OHCI driver for a platform device depends on USB_OHCI_HCD EXPERIMENTAL diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 2b1e8d8..95cb858 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1049,7 +1049,7 @@ MODULE_LICENSE (GPL); #define PLATFORM_DRIVERohci_hcd_at91_driver #endif -#if defined(CONFIG_ARCH_PNX4008) || defined(CONFIG_ARCH_LPC32XX) +#ifdef USB_OHCI_LPC32XX_PNX4008 #include ohci-nxp.c #define PLATFORM_DRIVERusb_hcd_nxp_driver #endif -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb/pxa25x: make it compile with debug again
|drivers/usb/gadget/pxa25x_udc.h: In function 'dump_state': |drivers/usb/gadget/pxa25x_udc.h:228:20: error: invalid type argument of '-' (have 'struct usb_ep') Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/usb/gadget/pxa25x_udc.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/pxa25x_udc.h b/drivers/usb/gadget/pxa25x_udc.h index 861f4df..2eca1e7 100644 --- a/drivers/usb/gadget/pxa25x_udc.h +++ b/drivers/usb/gadget/pxa25x_udc.h @@ -225,7 +225,7 @@ dump_state(struct pxa25x_udc *dev) dev-stats.read.bytes, dev-stats.read.ops); for (i = 1; i PXA_UDC_NUM_ENDPOINTS; i++) { - if (dev-ep [i].desc == NULL) + if (dev-ep[i].ep.desc == NULL) continue; DMSG (udccs%d = %02x\n, i, *dev-ep-reg_udccs); } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301
On Wed, 22 Aug 2012, Sebastian Andrzej Siewior wrote: The gadget case is, just depend on the ISP1301 instead of selecting it. The OHCI case is little more difficult. It is not possible to say select if and on top of it, the phy depends on USB which depends on OHCI. This Let's see if I understand this. On the host side, the key point is here: diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index c3f619b..cac3ee2 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -292,7 +292,6 @@ config USB_OHCI_HCD depends on USB USB_ARCH_HAS_OHCI select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3 select USB_OTG_UTILS if ARCH_OMAP - select USB_ISP1301 if ARCH_LPC32XX || ARCH_PNX4008 You want to avoid selecting USB_ISP1301, right? @@ -412,6 +411,15 @@ config USB_CNS3XXX_OHCI Enable support for the CNS3XXX SOC's on-chip OHCI controller. It is needed for low-speed USB 1.0 device support. +config USB_OHCI_LPC32XX_PNX4008 + bool LPC32XX or PNX4008 OHCI Module + depends on USB_OHCI_HCD + depends on USB_ISP1301 + ---help--- + Enable support for the LPC32XX or PNX4008 SOC's on-chip + OHCI controller. + It is needed for low-speed USB 1.0 device support. Instead you introduce a new symbol to control whether or not ohci-nxp.c gets compiled. The new symbol depends on USB_ISP1301 but not on ARCH_LPC32XX or ARCH_PNX4008, which means in theory it could be defined even under a totally different arch. diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 2b1e8d8..95cb858 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1049,7 +1049,7 @@ MODULE_LICENSE (GPL); #define PLATFORM_DRIVER ohci_hcd_at91_driver #endif -#if defined(CONFIG_ARCH_PNX4008) || defined(CONFIG_ARCH_LPC32XX) +#ifdef USB_OHCI_LPC32XX_PNX4008 (... should be CONFIG_USB_OHCI_LPC32XX_PNX4008) #include ohci-nxp.c #define PLATFORM_DRIVER usb_hcd_nxp_driver #endif And then you compile ohci-nxp.c whenever the new symbol is defined. Is this really what you want? It doesn't seem right. Couldn't it lead to a conflict if USB_OHCI_LPC32XX_PNX4008 is defined under the wrong arch? Also, why do you want the new symbol to be configurable whereas now everything is decided automatically? Couldn't you get the effect you want without the new symbol, like this: -#if defined(CONFIG_ARCH_PNX4008) || defined(CONFIG_ARCH_LPC32XX) +#if (defined(CONFIG_ARCH_PNX4008) || defined(CONFIG_ARCH_LPC32XX)) \ defined(CONFIG_USB_ISP1301) ? 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: Kernel oops on usbhid_submit_report (updates)
On Sat, Aug 18, 2012 at 08:16:12PM -0400, Alan Stern wrote: On Sat, 18 Aug 2012, Alan Stern wrote: Looking at the output of usbmon, the kernel re-uses URB addresses. Is it possible that the urb is freed while the instruction is in *implement()*? In fact, the usbhid driver does not free any URBs until it is unbound from the device. It keeps a circular queue of URBs and uses them in sequence, over and over. Correction: The usbhid driver keeps a circular queue of report structures and related data and uses _them_ in sequence, over and over. There is only one URB, which gets used for all the reports. More accurately, there is one URB for the interrupt-IN endpoint, one URB for the interrupt-OUT endpoint (if there is one), and one URB for endpoint 0. Each URB gets used for all the reports on its endpoint. One thing to look out for: Evidently usbhid_submit_report() does not check the HID_DISCONNECTED flag and will happily allow reports to be submitted after usbhid_stop() returns. This appears to be a bug. It could account for the behavior you're seeing. This makes sense. This could be what I am seeing. I am trying to write a patch for this but I'm wondering how do I test this issue? It is quite hard to reproduce, so is there a way I can force the HID to disconnect? Thanks, Amit -- 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: [PATCHv3 7/9] arm: vt8500: doc: Add device tree bindings for arch-vt8500 devices
On 08/21/2012 02:47 PM, Tony Prisk wrote: Bindings for gpio, interrupt controller, power management controller, timer, realtime clock, serial uart, ehci and uhci controllers and framebuffer controllers used on the arch-vt8500 platform. Framebuffer binding also specifies a 'display' node which is required for determining the lcd panel data. diff --git a/Documentation/devicetree/bindings/gpio/gpio_vt8500.txt b/Documentation/devicetree/bindings/gpio/gpio_vt8500.txt +- #gpio-cells : should be 3. + 1) bank + 2) pin number + 3) flags Should this enumerate what legal values are for flags, or point at a standard document that does? diff --git a/Documentation/devicetree/bindings/tty/serial/via,vt8500-uart.txt b/Documentation/devicetree/bindings/tty/serial/via,vt8500-uart.txt + uart@d821 { + compatible = via,vt8500-uart; + reg = 0xd821 0x1040; + interrupts = 47; + }; How does the UART know what frequency its clock input is, in order to calculate dividers? Should there be a clocks property to link to the input clock, so the rate can be queried? If so, a reference to the common clock binding, plus a specification of which clocks must be listed in the clock property should be included here. diff --git a/Documentation/devicetree/bindings/video/via,vt8500-fb.txt b/Documentation/devicetree/bindings/video/via,vt8500-fb.txt +VIA VT8500 Display +- +Required properties: +- xres : lcd panel horizontal resolution +- yres : lcd panel vertical resolution +- left-margin, +- right-margin, +- hsync-len: lcd panel horizontal timings in pixels +- upper-margin, +- lower-margin, +- vsync-len: lcd panel verticals timings in pixels +- bpp: lcd panel bit-depth. + 16 for RGB565, 32 for RGB888 Shouldn't this reference Sascha Hauer's binding document (although I suppose it isn't checked in yet), and just document the additions? I wonder if this binding should be written assuming Sascha's binding doc will be checked in? -- 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: Kernel oops on usbhid_submit_report (updates)
On Wed, 22 Aug 2012, Amit Uttamchandani wrote: One thing to look out for: Evidently usbhid_submit_report() does not check the HID_DISCONNECTED flag and will happily allow reports to be submitted after usbhid_stop() returns. This appears to be a bug. It could account for the behavior you're seeing. This makes sense. This could be what I am seeing. I am trying to write a patch for this but I'm wondering how do I test this issue? It is quite hard to reproduce, so is there a way I can force the HID to disconnect? An easy way is to rmmod usbhid. 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: [PATCHv3 7/9] arm: vt8500: doc: Add device tree bindings for arch-vt8500 devices
On Wed, 2012-08-22 at 15:07 -0600, Stephen Warren wrote: On 08/21/2012 02:47 PM, Tony Prisk wrote: Bindings for gpio, interrupt controller, power management controller, timer, realtime clock, serial uart, ehci and uhci controllers and framebuffer controllers used on the arch-vt8500 platform. Framebuffer binding also specifies a 'display' node which is required for determining the lcd panel data. diff --git a/Documentation/devicetree/bindings/gpio/gpio_vt8500.txt b/Documentation/devicetree/bindings/gpio/gpio_vt8500.txt +- #gpio-cells : should be 3. + 1) bank + 2) pin number + 3) flags Should this enumerate what legal values are for flags, or point at a standard document that does? There currently are no supported flags - guess this was an oversight. I'll take a look if there are any that we might require in the future and implement them otherwise I'll drop the flags reference. diff --git a/Documentation/devicetree/bindings/tty/serial/via,vt8500-uart.txt b/Documentation/devicetree/bindings/tty/serial/via,vt8500-uart.txt + uart@d821 { + compatible = via,vt8500-uart; + reg = 0xd821 0x1040; + interrupts = 47; + }; How does the UART know what frequency its clock input is, in order to calculate dividers? Should there be a clocks property to link to the input clock, so the rate can be queried? If so, a reference to the common clock binding, plus a specification of which clocks must be listed in the clock property should be included here. I didn't revisit the code that had already been posted in v1/v2 when I added the common clock code and there was no clock handling code at that point. The uart's are all clocked via a clkgen that outputs 24Mhz and this was hardcoded in the driver. I will correct this and add the appropriate device tree entries and documentation. diff --git a/Documentation/devicetree/bindings/video/via,vt8500-fb.txt b/Documentation/devicetree/bindings/video/via,vt8500-fb.txt +VIA VT8500 Display +- +Required properties: +- xres : lcd panel horizontal resolution +- yres : lcd panel vertical resolution +- left-margin, +- right-margin, +- hsync-len: lcd panel horizontal timings in pixels +- upper-margin, +- lower-margin, +- vsync-len: lcd panel verticals timings in pixels +- bpp: lcd panel bit-depth. + 16 for RGB565, 32 for RGB888 Shouldn't this reference Sascha Hauer's binding document (although I suppose it isn't checked in yet), and just document the additions? I wonder if this binding should be written assuming Sascha's binding doc will be checked in? I haven't seen Sascha's binding yet - I'm not even sure its been agreed upon but it does seem to be the most likely candidate so far. Hopefully it will get accepted in this window, and I will have time to do exactly as you commented, which was the plan from the start. This binding was based on the most current one I had seen suggested at the time. Given the additional comments for this patch set, and the required changes I'll hold off posting v4 until everything is tidied up. Regards Tony Prisk -- 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
[RFC ebeam PATCH v4 0/2] new USB eBeam input driver
Hi, New USB input driver for eBeam devices. Currently supported (tested) : - Luidia eBeam classic projection and edge projection models - Nec interactive solution NP01Wi1 NP01Wi2 accessories. In fact, from basic usb point of view, all these devices are indistinguishable : they have the same usb ids and (blank) hid report descriptors. There's other re-branded hardware that need to be test, but i bet on same ids. So, the code : Patch 1 to blacklist the devices for hid generic-usb. Patch 2 is the actual driver. Notable stuff : - use div64_s64 for portable 64/64-bits divisions. - 13 sysfs custom files : 9 values for the transformation matrix, 4 for xy ranges and a calibration trigger. The module run fine with a 3.3.6 and a 3.5.0 kernel, both x86_32 and 64. Thanks for your help. -- 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
[RFC ebeam PATCH v4 1/2] hid: Blacklist eBeam devices
Signed-off-by: Yann Cantin yann.can...@laposte.net --- drivers/hid/hid-core.c |3 +++ drivers/hid/hid-ids.h |3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 60ea284..efc68c8 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1908,6 +1908,9 @@ static const struct hid_device_id hid_ignore_list[] = { { HID_USB_DEVICE(USB_VENDOR_ID_DELORME, USB_DEVICE_ID_DELORME_EM_LT20) }, { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x0004) }, { HID_USB_DEVICE(USB_VENDOR_ID_DREAM_CHEEKY, 0x000a) }, +#if defined(CONFIG_INPUT_EBEAM_USB) + { HID_USB_DEVICE(USB_VENDOR_ID_EFI, USB_DEVICE_ID_EFI_EBEAM) }, +#endif { HID_USB_DEVICE(USB_VENDOR_ID_ESSENTIAL_REALITY, USB_DEVICE_ID_ESSENTIAL_REALITY_P5) }, { HID_USB_DEVICE(USB_VENDOR_ID_ETT, USB_DEVICE_ID_TC5UH) }, { HID_USB_DEVICE(USB_VENDOR_ID_ETT, USB_DEVICE_ID_TC4UM) }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 1dcb76f..93f5b83 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -271,6 +271,9 @@ #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_7349 0x7349 #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_A001 0xa001 +#define USB_VENDOR_ID_EFI 0x2650 +#define USB_DEVICE_ID_EFI_EBEAM0x1311 + #define USB_VENDOR_ID_ELECOM 0x056e #define USB_DEVICE_ID_ELECOM_BM084 0x0061 -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC ebeam PATCH v4 2/2] input: misc: New USB eBeam input driver.
Signed-off-by: Yann Cantin yann.can...@laposte.net --- drivers/input/misc/Kconfig | 22 ++ drivers/input/misc/Makefile |1 + drivers/input/misc/ebeam.c | 766 +++ 3 files changed, 789 insertions(+) create mode 100644 drivers/input/misc/ebeam.c diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 7c0f1ec..c226b83 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -83,6 +83,28 @@ config INPUT_BMA150 To compile this driver as a module, choose M here: the module will be called bma150. +config INPUT_EBEAM_USB + tristate USB eBeam driver + depends on USB_ARCH_HAS_HCD + select USB + help + Say Y here if you have a USB eBeam pointing device and want to + use it without any proprietary user space tools. + + Have a look at http://sourceforge.net/projects/ebeam/ for + a usage description and the required user-space tools. + + Supported devices : + - Luidia eBeam Classic Projection and eBeam Edge Projection + - Nec NP01Wi1 NP01Wi2 interactive solution + + Supposed working devices, need test, may lack functionnality : + - Luidia eBeam Edge Whiteboard and eBeam Engage + - Hitachi Starboard FX-63, FX-77, FX-82, FX-77GII + + To compile this driver as a module, choose M here: the + module will be called ebeam. + config INPUT_PCSPKR tristate PC Speaker support depends on PCSPKR_PLATFORM diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 83fe6f5..2aa9813 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_INPUT_CMA3000_I2C) += cma3000_d0x_i2c.o obj-$(CONFIG_INPUT_COBALT_BTNS)+= cobalt_btns.o obj-$(CONFIG_INPUT_DA9052_ONKEY) += da9052_onkey.o obj-$(CONFIG_INPUT_DM355EVM) += dm355evm_keys.o +obj-$(CONFIG_INPUT_EBEAM_USB) += ebeam.o obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o diff --git a/drivers/input/misc/ebeam.c b/drivers/input/misc/ebeam.c new file mode 100644 index 000..80e3397 --- /dev/null +++ b/drivers/input/misc/ebeam.c @@ -0,0 +1,766 @@ +/** + * + * eBeam driver + * + * Copyright (C) 2012 Yann Cantin (yann.can...@laposte.net) + * + * 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; either version 2 of the + * License, or (at your option) any later version. + * + * based on + * + * usbtouchscreen.c by Daniel Ritz daniel.r...@gmx.ch + * aiptek.c (sysfs/settings) by Chris Atenasio ch...@crud.net + * Bryan W. Headley bwhead...@earthlink.net + * + */ + +#include linux/kernel.h +#include linux/slab.h +#include linux/math64.h +#include linux/input.h +#include linux/module.h +#include linux/init.h +#include linux/usb.h +#include linux/usb/input.h + +#define DRIVER_AUTHOR Yann Cantin yann.can...@laposte.net +#define DRIVER_DESCUSB eBeam Driver + +/* Common values for eBeam devices */ +#define REPT_SIZE 8 /* packet size*/ +#define MIN_RAW_X 0 /* raw coordinates ranges */ +#define MAX_RAW_X 0x +#define MIN_RAW_Y 0 +#define MAX_RAW_Y 0x + + +#define USB_VENDOR_ID_EFI 0x2650 /* Electronics For Imaging, Inc */ +#define USB_DEVICE_ID_EFI_EBEAM0x1311 /* eBeam hardware : */ + /* Luidia Classic and Edge */ + /* Nec NP01Wi1 NP01Wi2 */ + +#define EBEAM_BTN_TIP 0x1 /* tip*/ +#define EBEAM_BTN_LIT 0x2 /* little */ +#define EBEAM_BTN_BIG 0x4 /* big*/ + +/* ebeam settings */ +struct ebeam_settings { + int min_x; /* computed coordinates ranges for the input layer */ + int max_x; + int min_y; + int max_y; + + /* H matrix */ + s64 h1; + s64 h2; + s64 h3; + s64 h4; + s64 h5; + s64 h6; + s64 h7; + s64 h8; + s64 h9; +}; + +/* ebeam device */ +struct ebeam_device { + unsigned char*data; + dma_addr_t data_dma; + unsigned char*buffer; + int buf_len; + struct urb *irq; + struct usb_interface *interface; + struct input_dev *input; + char name[128]; + char phys[64]; + void *priv; + +
Re: [PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301
Hi, On 22/08/12 21:49, Alan Stern wrote: diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index c3f619b..cac3ee2 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -292,7 +292,6 @@ config USB_OHCI_HCD depends on USB USB_ARCH_HAS_OHCI select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3 select USB_OTG_UTILS if ARCH_OMAP -select USB_ISP1301 if ARCH_LPC32XX || ARCH_PNX4008 You want to avoid selecting USB_ISP1301, right? The ultimate goal is just to prevent the compile error that currently occurs when I2C isn't selected on LPC32XX but UDC is. It's an invalid config combination, not easy to prevent by current Kconfig mechanism. @@ -412,6 +411,15 @@ config USB_CNS3XXX_OHCI Enable support for the CNS3XXX SOC's on-chip OHCI controller. It is needed for low-speed USB 1.0 device support. +config USB_OHCI_LPC32XX_PNX4008 +bool LPC32XX or PNX4008 OHCI Module +depends on USB_OHCI_HCD +depends on USB_ISP1301 +---help--- + Enable support for the LPC32XX or PNX4008 SOC's on-chip + OHCI controller. + It is needed for low-speed USB 1.0 device support. Instead you introduce a new symbol to control whether or not ohci-nxp.c gets compiled. The new symbol depends on USB_ISP1301 but not on ARCH_LPC32XX or ARCH_PNX4008, which means in theory it could be defined even under a totally different arch. This issue could be resolved in a different way: Have you followed the discussion at linux-arm-ker...@lists.infradead.org on 2012-08-20, Subject i2c: pnx: Fix bit definitions? If nothing unexpected turns out, PNX4008 should probably be removed altogether. (Practically abandoned arch for 6 years.) Easily possible for v3.7. Then, the resulting solution is much simpler and Sebastian's patch obsolete. Thanks, Roland -- 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: 10 seconds lag when connecting UPS device (usb_submit_urb(ctrl) failed: -1)
Le Wed, 15 Aug 2012 10:53:07 -0400 (EDT), Alan Stern st...@rowland.harvard.edu a écrit : On Tue, 14 Aug 2012, Laurent Bigonville wrote: But does the UPS work okay when you use the parameter? If it does, the quirk information can be added to a permanent table in the usbhid driver. Yes it seems to work, the userspace driver can communicate with the device (at least I can see the status of the UPS). Okay. Below is a patch to add a permanent blacklist entry for your UPS. With this patch you shouldn't need to use the module parameter any more. Please try it out and let me know if it works; if it does I will submit it for inclusion in the kernel. Alright, I've recompiled linux kernel 3.5 with the patch and it seems to work. Thanks! Laurent Bigonville -- 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: renesas_usbhs: fixup DMA transport data alignment
renesas_usbhs dma can transport 8byte alignment data, not 4byte. This patch fixup it. Reported-by: Sugnan Prabhu S sugnan.pra...@renesasmobile.com Signed-off-by: Kuninori Morimoto kuninori.morimoto...@renesas.com --- drivers/usb/renesas_usbhs/fifo.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c index ecd1730..143c4e9 100644 --- a/drivers/usb/renesas_usbhs/fifo.c +++ b/drivers/usb/renesas_usbhs/fifo.c @@ -818,7 +818,7 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, int *is_done) usbhs_pipe_is_dcp(pipe)) goto usbhsf_pio_prepare_push; - if (len % 4) /* 32bit alignment */ + if (len 0x7) /* 8byte alignment */ goto usbhsf_pio_prepare_push; if ((uintptr_t)(pkt-buf + pkt-actual) 0x7) /* 8byte alignment */ @@ -905,7 +905,7 @@ static int usbhsf_dma_try_pop(struct usbhs_pkt *pkt, int *is_done) /* use PIO if packet is less than pio_dma_border */ len = usbhsf_fifo_rcv_len(priv, fifo); len = min(pkt-length - pkt-actual, len); - if (len % 4) /* 32bit alignment */ + if (len 0x7) /* 8byte alignment */ goto usbhsf_pio_prepare_pop_unselect; if (len usbhs_get_dparam(priv, pio_dma_border)) -- 1.7.5.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: [RFT] xhci: Switch PPT ports to EHCI on shutdown.
On 08/07/2012 11:39 AM, Sarah Sharp wrote: The Intel desktop boards DH77EB and DH77DF have a hardware issue that can be worked around by BIOS. If the USB ports are switched to xHCI on shutdown, the xHCI host will send a spurious interrupt, which will wake the system. Some BIOS will work around this, but not all. The bug can be avoided if the USB ports are switched back to EHCI on shutdown. The Intel Windows driver switches the ports back to EHCI, so change the Linux xHCI driver to do the same. Unfortunately, we can't tell the two effected boards apart from other working motherboards, because the vendors will change the DMI strings for the DH77EB and DH77DF boards to their own custom names. One example is Compulab's mini-desktop, the Intense-PC. Instead, key off the Panther Point xHCI host PCI vendor and device ID, and switch the ports over for all PPT xHCI hosts. The only impact this will have on non-effected boards is to add a couple hundred milliseconds delay on boot when the BIOS has to switch the ports over from EHCI to xHCI. This patch should be backported to kernels as old as 3.0, that contain the commit 69e848c2090aebba5698a1620604c7dccb448684 Intel xhci: Support EHCI/xHCI port switching. Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com Reported-by: Denis Turischev de...@compulab.co.il Cc: sta...@vger.kernel.org --- drivers/usb/host/pci-quirks.c |7 +++ drivers/usb/host/pci-quirks.h |1 + drivers/usb/host/xhci-pci.c |9 + drivers/usb/host/xhci.c |3 +++ drivers/usb/host/xhci.h |1 + 5 files changed, 21 insertions(+), 0 deletions(-) diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index df0828c..c5e9e4a 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -800,6 +800,13 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev) } EXPORT_SYMBOL_GPL(usb_enable_xhci_ports); +void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) +{ + pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN, 0x0); + pci_write_config_dword(xhci_pdev, USB_INTEL_XUSB2PR, 0x0); +} +EXPORT_SYMBOL_GPL(usb_disable_xhci_ports); + /** * PCI Quirks for xHCI. * diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h index b1002a8..ef004a5 100644 --- a/drivers/usb/host/pci-quirks.h +++ b/drivers/usb/host/pci-quirks.h @@ -10,6 +10,7 @@ void usb_amd_quirk_pll_disable(void); void usb_amd_quirk_pll_enable(void); bool usb_is_intel_switchable_xhci(struct pci_dev *pdev); void usb_enable_xhci_ports(struct pci_dev *xhci_pdev); +void usb_disable_xhci_ports(struct pci_dev *xhci_pdev); #else static inline void usb_amd_quirk_pll_disable(void) {} static inline void usb_amd_quirk_pll_enable(void) {} diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 92eaff6..9bfd4ca11 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -94,6 +94,15 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) xhci-quirks |= XHCI_EP_LIMIT_QUIRK; xhci-limit_active_eps = 64; xhci-quirks |= XHCI_SW_BW_CHECKING; + /* +* PPT desktop boards DH77EB and DH77DF will power back on after +* a few seconds of being shutdown. The fix for this is to +* switch the ports from xHCI to EHCI on shutdown. We can't use +* DMI information to find those particular boards (since each +* vendor will change the board name), so we have to key off all +* PPT chipsets. +*/ + xhci-quirks |= XHCI_SPURIOUS_REBOOT; } if (pdev-vendor == PCI_VENDOR_ID_ETRON pdev-device == PCI_DEVICE_ID_ASROCK_P67) { diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 95394e5..81aa10c 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -659,6 +659,9 @@ void xhci_shutdown(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); + if (xhci-quirks XHCI_SPURIOUS_REBOOT) + usb_disable_xhci_ports(to_pci_dev(hcd-self.controller)); This looks like a typo, think it should be not . With this code, it appears the quirk will always be triggered since XHCI_SPURIOUS_REBOOT is non-zero. + spin_lock_irq(xhci-lock); xhci_halt(xhci); spin_unlock_irq(xhci-lock); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 96f49db..c713256 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1494,6 +1494,7 @@ struct xhci_hcd { #define XHCI_TRUST_TX_LENGTH (1 10) #define XHCI_LPM_SUPPORT (1 11) #define XHCI_INTEL_HOST (1 12) +#define XHCI_SPURIOUS_REBOOT (1 13) unsigned intnum_active_eps; unsigned intlimit_active_eps; /* There are two roothubs to keep track of