[patch 2/2 v2] USB: keyspan: add a sanity test on "len"
"len" comes from the USB transfer and it's probably correct. The thing is that we already have similar checks like: if (data[i] >= serial->num_ports) { So adding a sanity test here matches the rest of the code and is a good idea. Signed-off-by: Dan Carpenter --- v2: No change. Rebased. diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index 3d303f0..eb30d7b 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -727,14 +727,15 @@ static void usa49wg_indat_callback(struct urb *urb) if ((data[i] & 0x80) == 0) { /* no error on any byte */ i++; - for (x = 1; x < len ; ++x) + for (x = 1; x < len && i < urb->actual_length; ++x) tty_insert_flip_char(&port->port, data[i++], 0); } else { /* * some bytes had errors, every byte has status */ - for (x = 0; x + 1 < len; x += 2) { + for (x = 0; x + 1 < len && + i + 1 < urb->actual_length; x += 2) { int stat = data[i], flag = 0; if (stat & RXERROR_OVERRUN) -- 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 v2] USB: keyspan: pull in one indent level
We can remove the "if (urb->actual_length) {" check because checking for "while (i < urb->actual_length) {" is sufficient. This lets us pull the code in one indent level. Signed-off-by: Dan Carpenter --- v2: do some additional whitespace tweaks suggested by Sergei Shtylyov. diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index 5b912fc..3d303f0 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -712,45 +712,44 @@ static void usa49wg_indat_callback(struct urb *urb) i = 0; len = 0; - if (urb->actual_length) { - while (i < urb->actual_length) { + while (i < urb->actual_length) { - /* Check port number from message*/ - if (data[i] >= serial->num_ports) { - dev_dbg(&urb->dev->dev, "%s - Unexpected port number %d\n", - __func__, data[i]); - return; - } - port = serial->port[data[i++]]; - len = data[i++]; + /* Check port number from message */ + if (data[i] >= serial->num_ports) { + dev_dbg(&urb->dev->dev, "%s - Unexpected port number %d\n", + __func__, data[i]); + return; + } + port = serial->port[data[i++]]; + len = data[i++]; - /* 0x80 bit is error flag */ - if ((data[i] & 0x80) == 0) { - /* no error on any byte */ - i++; - for (x = 1; x < len ; ++x) - tty_insert_flip_char(&port->port, - data[i++], 0); - } else { - /* -* some bytes had errors, every byte has status -*/ - for (x = 0; x + 1 < len; x += 2) { - int stat = data[i], flag = 0; - if (stat & RXERROR_OVERRUN) - flag |= TTY_OVERRUN; - if (stat & RXERROR_FRAMING) - flag |= TTY_FRAME; - if (stat & RXERROR_PARITY) - flag |= TTY_PARITY; - /* XXX should handle break (0x10) */ - tty_insert_flip_char(&port->port, - data[i+1], flag); - i += 2; - } + /* 0x80 bit is error flag */ + if ((data[i] & 0x80) == 0) { + /* no error on any byte */ + i++; + for (x = 1; x < len ; ++x) + tty_insert_flip_char(&port->port, + data[i++], 0); + } else { + /* +* some bytes had errors, every byte has status +*/ + for (x = 0; x + 1 < len; x += 2) { + int stat = data[i], flag = 0; + + if (stat & RXERROR_OVERRUN) + flag |= TTY_OVERRUN; + if (stat & RXERROR_FRAMING) + flag |= TTY_FRAME; + if (stat & RXERROR_PARITY) + flag |= TTY_PARITY; + /* XXX should handle break (0x10) */ + tty_insert_flip_char(&port->port, data[i+1], +flag); + i += 2; } - tty_flip_buffer_push(&port->port); } + tty_flip_buffer_push(&port->port); } /* Resubmit urb so we continue receiving */ -- 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: USB3.0 Interrupt transfer in u-boot
I have changed my footer settings.. Thanks for informing.. -Puneet On Thu, 2013-04-04 at 19:30 +0530, Greg KH wrote: > On Thu, Apr 04, 2013 at 10:07:06AM +0530, Puneet Sharma wrote: > > Hello, > > > > I want to test USB keyboard for XHCI in u-boot and to do that i need the > > Interrupt transfer code in XHCI controller driver in u-boot. If > > possible, can you help me to give that piece of code or can you help me > > to figure out in kernel where can i find it so that i can try to > > replicate it in u-boot and make it work. > > > > > > Thanks & Regards > > -- > > Puneet Sharma > > > > > > The information contained in this email and any attachments is confidential > > and may be subject to copyright or other intellectual property protection. > > If you are not the intended recipient, you are not authorized to use or > > disclose this information, and we request that you notify us by reply mail > > or telephone and delete the original message from your mail system. > > Because of this footer, I am not allowed to answer your email, sorry. > Please fix it if you wish to interact with Linux kernel developers. > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: fix list access before init
On Thu, Apr 04, 2013 at 12:29:25PM -0700, Sarah Sharp wrote: > Someone else is already working on a patch to fix part of this: > > http://marc.info/?l=linux-usb&m=136509667003716&w=2 > > Please coordinate with them. > > Also, two people trying to solve the same simple bug and using almost > exactly the same commit messages seems like too much of a coincidence. > Are you working on the same project or did you both happen to run the > same static code checking tool at the same time, or ? This problem was met by chance while trying buggy out-of-tree USB stuff. So, we wanted to bypass xhci initialization gracefully instead of being stuck with null pointer dereference while list traversing in xhci_mem_cleanup(). It is why patch addressed not only initialization of cancel_cmd_list and lpm_failed_devs, but bw tables too. With proposed patch normal boot sequence is progressing. I've never heard of Sergio's patch, and never met him before :) What about picking up Sergio's patch and bw table part of this patch as separate commits? Best wishes Vladimir Murzin > > Sarah Sharp > > On Thu, Apr 04, 2013 at 08:31:26PM +0400, Vladimir Murzin wrote: > > Commits > > 9574323 xHCI: test USB2 software LPM > > b92cc66 xHCI: add aborting command ring function > > introduce useful functions which involves lists manipulations. > > > > If for whatever reason we fall into fail path in xhci_mem_init() we > > may access the lists in xhci_mem_cleanup() before they get > > initialized. > > > > The same init-fail-cleanup case is applicable to bw tables too. > > > > Fix this by moving list initialization code to the beginning of > > xhci_mem_init() > > > > Reported-by: Sergey Dyasly > > Tested-by: Sergey Dyasly > > Signed-off-by: Vladimir Murzin > > --- > > drivers/usb/host/xhci-mem.c | 37 - > > 1 file changed, 20 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > > index 6dc238c..7b5d2f5 100644 > > --- a/drivers/usb/host/xhci-mem.c > > +++ b/drivers/usb/host/xhci-mem.c > > @@ -2123,7 +2123,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd > > *xhci, gfp_t flags) > > __le32 __iomem *addr; > > u32 offset; > > unsigned int num_ports; > > - int i, j, port_index; > > + int i, port_index; > > > > addr = &xhci->cap_regs->hcc_params; > > offset = XHCI_HCC_EXT_CAPS(xhci_readl(xhci, addr)); > > @@ -2138,18 +2138,6 @@ static int xhci_setup_port_arrays(struct xhci_hcd > > *xhci, gfp_t flags) > > if (!xhci->port_array) > > return -ENOMEM; > > > > - xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); > > - if (!xhci->rh_bw) > > - return -ENOMEM; > > - for (i = 0; i < num_ports; i++) { > > - struct xhci_interval_bw_table *bw_table; > > - > > - INIT_LIST_HEAD(&xhci->rh_bw[i].tts); > > - bw_table = &xhci->rh_bw[i].bw_table; > > - for (j = 0; j < XHCI_MAX_INTERVAL; j++) > > - INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints); > > - } > > - > > /* > > * For whatever reason, the first capability offset is from the > > * capability register base, not from the HCCPARAMS register. > > @@ -2253,7 +2241,25 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > > u64 val_64; > > struct xhci_segment *seg; > > u32 page_size, temp; > > - int i; > > + int i, j, num_ports; > > + > > + INIT_LIST_HEAD(&xhci->cancel_cmd_list); > > + INIT_LIST_HEAD(&xhci->lpm_failed_devs); > > + > > + num_ports = HCS_MAX_PORTS(xhci_readl(xhci, > > &xhci->cap_regs->hcs_params1)); > > + > > + xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); > > + if (!xhci->rh_bw) > > + return -ENOMEM; > > + > > + for (i = 0; i < num_ports; i++) { > > + struct xhci_interval_bw_table *bw_table; > > + > > + INIT_LIST_HEAD(&xhci->rh_bw[i].tts); > > + bw_table = &xhci->rh_bw[i].bw_table; > > + for (j = 0; j < XHCI_MAX_INTERVAL; j++) > > + INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints); > > + } > > > > page_size = xhci_readl(xhci, &xhci->op_regs->page_size); > > xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); > > @@ -2333,7 +2339,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > > xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); > > if (!xhci->cmd_ring) > > goto fail; > > - INIT_LIST_HEAD(&xhci->cancel_cmd_list); > > xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); > > xhci_dbg(xhci, "First segment DMA is 0x%llx\n", > > (unsigned long long)xhci->cmd_ring->first_seg->dma); > > @@ -2444,8 +2449,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > > if (xhci_setup_port_arrays(xhci, flags)) > > goto fail; > > > > - INIT_LIST_HEAD(&xhci->lpm_failed_devs); > >
Re: [PATCH 0/8] Reorganize R8A7779/Marzen USB code
Hi >Here's the set of 4 patches against the Simon Horman's 'renesas.git' repo, > 'renesas-next-20130404v2' tag and the 2 Ether patches I've reposted yesterday. > It was created to fix the shortcomings in the R8A7779/Marzen USB platform code > and R8A7779 USB common PHY driver, and so spans both arch/arm/mach-shmobile/ > and drivers/usb/ subtrees (some patches have to touch both subtrees). > The patches were conceived with the complete bisectability goal in mind. > > [1/8] ARM: shmobile:Marzen: move USB EHCI, OHCI, and PHY devices to R8A7779 > code > [2/8] ehci-platform: add init() method to platform data > [3/8] ARM: shmobile: R8A7779: setup EHCI internal buffer > [4/8] rcar-phy: remove EHCI internal buffer setup > [5/8] rcar-phy: correct base address > [6/8] rcar-phy: add platform data > [7/8] ARM: shmobile: Marzen: pass platform data to USB PHY device > [8/8] rcar-phy: handle platform data > >I'm not sure thru which tree this patchset should be merged, however it > turns > out that it's too late now to push it thru Felipe Balbi's USB tree for 3.10, > so > maybe the patchset can be merged thru Simon's tree with Felipe's and Alan > Stern's ACKs. I guess you already got request about patch style from Simon. When you send v2 patch, could you please add "this patch is tested on bard" on each patch's comment area ? Then, for all patches Acked-by: Kuninori Morimoto Best regards --- Kuninori Morimoto -- 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/8] ARM: shmobile: Marzen: pass platform data to USB PHY device
On Fri, Apr 05, 2013 at 03:10:38AM +0400, Sergei Shtylyov wrote: > Since we're now going to setup the USBPCTRL0 register using the USB PHY > device's > platform data, we now need a way to pass those platform data from the board > file > to the device which is situated in setup-r8a7779.c -- and what I'm suggesting > is > r8a7779_add_usb_phy_device() that will register USB PHY platform device with > the > passed platform data using platform_device_register_resndata() call; creating > this function involves deletion of 'usb_phy_device' from r8a7779_devices_dt[], > so that it will no longer be registered for the generic R8A7779 machine (where > we can't provide the platform data anyway), hence EHCI/OHCI drivers will fail > to load as well. > > For the Marzen board, this new function will be called from marzen_init() to > register the USB PHY device early enough. As per my comment regarding patch 1, I wonder if this could be split into an SoC patch and a board patch. > > Signed-off-by: Sergei Shtylyov > > --- > arch/arm/mach-shmobile/board-marzen.c |5 + > arch/arm/mach-shmobile/include/mach/r8a7779.h |2 ++ > arch/arm/mach-shmobile/setup-r8a7779.c| 16 > 3 files changed, 15 insertions(+), 8 deletions(-) > > Index: renesas/arch/arm/mach-shmobile/board-marzen.c > === > --- renesas.orig/arch/arm/mach-shmobile/board-marzen.c > +++ renesas/arch/arm/mach-shmobile/board-marzen.c > @@ -56,6 +56,10 @@ static struct regulator_consumer_supply > REGULATOR_SUPPLY("vdd33a", "smsc911x"), > }; > > +static struct rcar_phy_platform_data usb_phy_platform_data = { > + .usbpctrl0 = 0, > +}; > + > /* SMSC LAN89218 */ > static struct resource smsc911x_resources[] = { > [0] = { > @@ -230,6 +234,7 @@ static void __init marzen_init(void) > r8a7779_pinmux_init(); > > r8a7779_add_standard_devices(); > + r8a7779_add_usb_phy_device(&usb_phy_platform_data); > platform_add_devices(marzen_devices, ARRAY_SIZE(marzen_devices)); > } > > Index: renesas/arch/arm/mach-shmobile/include/mach/r8a7779.h > === > --- renesas.orig/arch/arm/mach-shmobile/include/mach/r8a7779.h > +++ renesas/arch/arm/mach-shmobile/include/mach/r8a7779.h > @@ -4,6 +4,7 @@ > #include > #include > #include > +#include > > struct platform_device; > > @@ -33,6 +34,7 @@ extern void r8a7779_add_early_devices(vo > extern void r8a7779_add_standard_devices(void); > extern void r8a7779_add_standard_devices_dt(void); > extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata); > +extern void r8a7779_add_usb_phy_device(struct rcar_phy_platform_data *pdata); > extern void r8a7779_init_late(void); > extern void r8a7779_clock_init(void); > extern void r8a7779_pinmux_init(void); > Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c > === > --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c > +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c > @@ -407,13 +407,6 @@ static struct resource usb_phy_resources > }, > }; > > -static struct platform_device usb_phy_device = { > - .name = "rcar_usb_phy", > - .id = -1, > - .resource = usb_phy_resources, > - .num_resources = ARRAY_SIZE(usb_phy_resources), > -}; > - > /* USB */ > static struct usb_phy *phy; > > @@ -586,7 +579,6 @@ static struct platform_device *r8a7779_d > &scif5_device, > &tmu00_device, > &tmu01_device, > - &usb_phy_device, > }; > > static struct platform_device *r8a7779_standard_devices[] __initdata = { > @@ -621,6 +613,14 @@ void __init r8a7779_add_ether_device(str > pdata, sizeof(*pdata)); > } > > +void __init r8a7779_add_usb_phy_device(struct rcar_phy_platform_data *pdata) > +{ > + platform_device_register_resndata(&platform_bus, "rcar_usb_phy", -1, > + usb_phy_resources, > + ARRAY_SIZE(usb_phy_resources), > + pdata, sizeof(*pdata)); > +} > + > /* do nothing for !CONFIG_SMP or !CONFIG_HAVE_TWD */ > void __init __weak r8a7779_register_twd(void) { } > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sh" 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 5/8] rcar-phy: correct base address
On Fri, Apr 05, 2013 at 03:05:14AM +0400, Sergei Shtylyov wrote: > The memory region that is used by the driver overlaps EHCI and OHCI register > regions for absolutely no reason now -- fix it by adding offset of 0x800 to > the base address, changing the register #define's accordingly. This has extra > positive effect that we now can use devm_ioremap_resource(). > > Signed-off-by: Sergei Shtylyov As per my comment regarding patch 4, I wonder if this could be split into a driver patch and an SoC patch. > > --- > arch/arm/mach-shmobile/setup-r8a7779.c |2 +- > drivers/usb/phy/rcar-phy.c | 28 ++-- > 2 files changed, 11 insertions(+), 19 deletions(-) > > Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c > === > --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c > +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c > @@ -401,7 +401,7 @@ static struct platform_device sata_devic > /* USB PHY */ > static struct resource usb_phy_resources[] = { > [0] = { > - .start = 0xffe7, > + .start = 0xffe70800, > .end= 0xffe70900 - 1, > .flags = IORESOURCE_MEM, > }, > Index: renesas/drivers/usb/phy/rcar-phy.c > === > --- renesas.orig/drivers/usb/phy/rcar-phy.c > +++ renesas/drivers/usb/phy/rcar-phy.c > @@ -16,13 +16,13 @@ > #include > #include > > -/* USBH common register */ > -#define USBPCTRL00x0800 > -#define USBPCTRL10x0804 > -#define USBST0x0808 > -#define USBEH0 0x080C > -#define USBOH0 0x081C > -#define USBCTL0 0x0858 > +/* REGS block */ > +#define USBPCTRL00x00 > +#define USBPCTRL10x04 > +#define USBST0x08 > +#define USBEH0 0x0C > +#define USBOH0 0x1C > +#define USBCTL0 0x58 > > /* USBPCTRL1 */ > #define PHY_RST (1 << 2) > @@ -139,17 +139,9 @@ static int rcar_usb_phy_probe(struct pla > return -EINVAL; > } > > - /* > - * CAUTION > - * > - * Because this phy address is also mapped under OHCI/EHCI address area, > - * this driver can't use devm_request_and_ioremap(dev, res) here > - */ > - reg0 = devm_ioremap_nocache(dev, res0->start, resource_size(res0)); > - if (!reg0) { > - dev_err(dev, "ioremap error\n"); > - return -ENOMEM; > - } > + reg0 = devm_ioremap_resource(dev, res0); > + if (IS_ERR(reg0)) > + return PTR_ERR(reg0); > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > if (!priv) { > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] rcar-phy: remove EHCI internal buffer setup
On Fri, Apr 05, 2013 at 03:03:55AM +0400, Sergei Shtylyov wrote: > Now that the EHCI internal buffer setup is done by the platform code, we can > remove such code from this driver as it never really belonged here. We also > no longer need the 2nd memory region now (2nd EHCI controller is simply > missing > in e.g. R8A7778 SoC). I wonder if this patch could be split into a driver patch followed by an SoC patch. Or vice versa. > > Signed-off-by: Sergei Shtylyov > > --- > arch/arm/mach-shmobile/setup-r8a7779.c |5 - > drivers/usb/phy/rcar-phy.c | 28 > 2 files changed, 4 insertions(+), 29 deletions(-) > > Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c > === > --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c > +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c > @@ -405,11 +405,6 @@ static struct resource usb_phy_resources > .end= 0xffe70900 - 1, > .flags = IORESOURCE_MEM, > }, > - [1] = { > - .start = 0xfff7, > - .end= 0xfff70900 - 1, > - .flags = IORESOURCE_MEM, > - }, > }; > > static struct platform_device usb_phy_device = { > Index: renesas/drivers/usb/phy/rcar-phy.c > === > --- renesas.orig/drivers/usb/phy/rcar-phy.c > +++ renesas/drivers/usb/phy/rcar-phy.c > @@ -23,8 +23,6 @@ > #define USBEH0 0x080C > #define USBOH0 0x081C > #define USBCTL0 0x0858 > -#define EIIBC1 0x0094 > -#define EIIBC2 0x009C > > /* USBPCTRL1 */ > #define PHY_RST (1 << 2) > @@ -40,7 +38,6 @@ struct rcar_usb_phy_priv { > spinlock_t lock; > > void __iomem *reg0; > - void __iomem *reg1; > int counter; > }; > > @@ -59,7 +56,6 @@ static int rcar_usb_phy_init(struct usb_ > struct rcar_usb_phy_priv *priv = usb_phy_to_priv(phy); > struct device *dev = phy->dev; > void __iomem *reg0 = priv->reg0; > - void __iomem *reg1 = priv->reg1; > int i; > u32 val; > unsigned long flags; > @@ -97,19 +93,6 @@ static int rcar_usb_phy_init(struct usb_ > iowrite32(0x, (reg0 + USBPCTRL0)); > > /* > - * EHCI IP internal buffer setting > - * EHCI IP internal buffer enable > - * > - * These are recommended value of a datasheet > - * see [USB :: EHCI internal buffer setting] > - */ > - iowrite32(0x00ff0040, (reg0 + EIIBC1)); > - iowrite32(0x00ff0040, (reg1 + EIIBC1)); > - > - iowrite32(0x0001, (reg0 + EIIBC2)); > - iowrite32(0x0001, (reg1 + EIIBC2)); > - > - /* >* Bus alignment settings >*/ > > @@ -145,14 +128,13 @@ static void rcar_usb_phy_shutdown(struct > static int rcar_usb_phy_probe(struct platform_device *pdev) > { > struct rcar_usb_phy_priv *priv; > - struct resource *res0, *res1; > + struct resource *res0; > struct device *dev = &pdev->dev; > - void __iomem *reg0, *reg1; > + void __iomem *reg0; > int ret; > > res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - res1 = platform_get_resource(pdev, IORESOURCE_MEM, 1); > - if (!res0 || !res1) { > + if (!res0) { > dev_err(dev, "Not enough platform resources\n"); > return -EINVAL; > } > @@ -164,8 +146,7 @@ static int rcar_usb_phy_probe(struct pla >* this driver can't use devm_request_and_ioremap(dev, res) here >*/ > reg0 = devm_ioremap_nocache(dev, res0->start, resource_size(res0)); > - reg1 = devm_ioremap_nocache(dev, res1->start, resource_size(res1)); > - if (!reg0 || !reg1) { > + if (!reg0) { > dev_err(dev, "ioremap error\n"); > return -ENOMEM; > } > @@ -177,7 +158,6 @@ static int rcar_usb_phy_probe(struct pla > } > > priv->reg0 = reg0; > - priv->reg1 = reg1; > priv->counter = 0; > priv->phy.dev = dev; > priv->phy.label = dev_name(dev); > -- > To unsubscribe from this list: send the line "unsubscribe linux-sh" 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: Exporting iInterface (and associated string) in debugfs?
On Thu, 4 Apr 2013 11:26:16 -0700 Greg KH wrote: > > usbview predates both sysfs and libusb, it used only the usbfs 'devices' > file when it was created (way back in 1999 or so). That's why it > doesn't use those apis. > > My long-term goal is to merge usbview into usbutils, which does use > libusb, so that will resolve the dependancy on the debugfs 'devices' > file. It all makes sense now, thanks. > There really is no place in usbview to put the interface optional > strings at the moment, but 'lsusb' should show you them just fine, > right? I would have used it in interface->name if it would be "(none)" otherwise (as in my case). lsusb works well, yes. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] rcar-phy: handle platform data
Set the USBPCTRL0 register from the passed platform data in rcar_usb_phy_init(); don't reset it to 0 in rcar_usb_phy_shutdown() anymore as that does not make sense. Also, don't allow the driver's probe to succeed when the platform data are not supplied with a device. Signed-off-by: Sergei Shtylyov --- drivers/usb/phy/rcar-phy.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) Index: renesas/drivers/usb/phy/rcar-phy.c === --- renesas.orig/drivers/usb/phy/rcar-phy.c +++ renesas/drivers/usb/phy/rcar-phy.c @@ -1,8 +1,9 @@ /* * Renesas R-Car USB phy driver * - * Copyright (C) 2012 Renesas Solutions Corp. + * Copyright (C) 2012-2013 Renesas Solutions Corp. * Kuninori Morimoto + * Copyright (C) 2013 Cogent Embedded, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -11,10 +12,11 @@ #include #include -#include #include #include #include +#include +#include /* REGS block */ #define USBPCTRL0 0x00 @@ -55,6 +57,7 @@ static int rcar_usb_phy_init(struct usb_ { struct rcar_usb_phy_priv *priv = usb_phy_to_priv(phy); struct device *dev = phy->dev; + struct rcar_phy_platform_data *pdata = dev->platform_data; void __iomem *reg0 = priv->reg0; int i; u32 val; @@ -90,7 +93,7 @@ static int rcar_usb_phy_init(struct usb_ iowrite32(PHY_ENB | PLL_ENB | PHY_RST, (reg0 + USBPCTRL1)); /* set platform specific port settings */ - iowrite32(0x, (reg0 + USBPCTRL0)); + iowrite32(pdata->usbpctrl0, (reg0 + USBPCTRL0)); /* * Bus alignment settings @@ -117,10 +120,8 @@ static void rcar_usb_phy_shutdown(struct spin_lock_irqsave(&priv->lock, flags); - if (priv->counter-- == 1) { /* last user */ - iowrite32(0x, (reg0 + USBPCTRL0)); + if (priv->counter-- == 1) /* last user */ iowrite32(0x, (reg0 + USBPCTRL1)); - } spin_unlock_irqrestore(&priv->lock, flags); } @@ -133,6 +134,11 @@ static int rcar_usb_phy_probe(struct pla void __iomem *reg0; int ret; + if (!pdev->dev.platform_data) { + dev_err(dev, "No platform data\n"); + return -EINVAL; + } + res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res0) { dev_err(dev, "Not enough platform resources\n"); -- 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 7/8] ARM: shmobile: Marzen: pass platform data to USB PHY device
Since we're now going to setup the USBPCTRL0 register using the USB PHY device's platform data, we now need a way to pass those platform data from the board file to the device which is situated in setup-r8a7779.c -- and what I'm suggesting is r8a7779_add_usb_phy_device() that will register USB PHY platform device with the passed platform data using platform_device_register_resndata() call; creating this function involves deletion of 'usb_phy_device' from r8a7779_devices_dt[], so that it will no longer be registered for the generic R8A7779 machine (where we can't provide the platform data anyway), hence EHCI/OHCI drivers will fail to load as well. For the Marzen board, this new function will be called from marzen_init() to register the USB PHY device early enough. Signed-off-by: Sergei Shtylyov --- arch/arm/mach-shmobile/board-marzen.c |5 + arch/arm/mach-shmobile/include/mach/r8a7779.h |2 ++ arch/arm/mach-shmobile/setup-r8a7779.c| 16 3 files changed, 15 insertions(+), 8 deletions(-) Index: renesas/arch/arm/mach-shmobile/board-marzen.c === --- renesas.orig/arch/arm/mach-shmobile/board-marzen.c +++ renesas/arch/arm/mach-shmobile/board-marzen.c @@ -56,6 +56,10 @@ static struct regulator_consumer_supply REGULATOR_SUPPLY("vdd33a", "smsc911x"), }; +static struct rcar_phy_platform_data usb_phy_platform_data = { + .usbpctrl0 = 0, +}; + /* SMSC LAN89218 */ static struct resource smsc911x_resources[] = { [0] = { @@ -230,6 +234,7 @@ static void __init marzen_init(void) r8a7779_pinmux_init(); r8a7779_add_standard_devices(); + r8a7779_add_usb_phy_device(&usb_phy_platform_data); platform_add_devices(marzen_devices, ARRAY_SIZE(marzen_devices)); } Index: renesas/arch/arm/mach-shmobile/include/mach/r8a7779.h === --- renesas.orig/arch/arm/mach-shmobile/include/mach/r8a7779.h +++ renesas/arch/arm/mach-shmobile/include/mach/r8a7779.h @@ -4,6 +4,7 @@ #include #include #include +#include struct platform_device; @@ -33,6 +34,7 @@ extern void r8a7779_add_early_devices(vo extern void r8a7779_add_standard_devices(void); extern void r8a7779_add_standard_devices_dt(void); extern void r8a7779_add_ether_device(struct sh_eth_plat_data *pdata); +extern void r8a7779_add_usb_phy_device(struct rcar_phy_platform_data *pdata); extern void r8a7779_init_late(void); extern void r8a7779_clock_init(void); extern void r8a7779_pinmux_init(void); Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c === --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -407,13 +407,6 @@ static struct resource usb_phy_resources }, }; -static struct platform_device usb_phy_device = { - .name = "rcar_usb_phy", - .id = -1, - .resource = usb_phy_resources, - .num_resources = ARRAY_SIZE(usb_phy_resources), -}; - /* USB */ static struct usb_phy *phy; @@ -586,7 +579,6 @@ static struct platform_device *r8a7779_d &scif5_device, &tmu00_device, &tmu01_device, - &usb_phy_device, }; static struct platform_device *r8a7779_standard_devices[] __initdata = { @@ -621,6 +613,14 @@ void __init r8a7779_add_ether_device(str pdata, sizeof(*pdata)); } +void __init r8a7779_add_usb_phy_device(struct rcar_phy_platform_data *pdata) +{ + platform_device_register_resndata(&platform_bus, "rcar_usb_phy", -1, + usb_phy_resources, + ARRAY_SIZE(usb_phy_resources), + pdata, sizeof(*pdata)); +} + /* do nothing for !CONFIG_SMP or !CONFIG_HAVE_TWD */ void __init __weak r8a7779_register_twd(void) { } -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] rcar-phy: add platform data
Currently the driver hard-codes USBPCTRL0 register to 0. It is wrong since this register contains board-specific USB ports configuration and so its value should be somehow passed via the platform data. Add file with the USBPCTRL0 bit #define's and 'struct rcar_phy_platform_data' containing the value to be set by the driver to that register. Signed-off-by: Sergei Shtylyov --- include/linux/usb/rcar-phy.h | 39 +++ 1 file changed, 39 insertions(+) Index: renesas/include/linux/usb/rcar-phy.h === --- /dev/null +++ renesas/include/linux/usb/rcar-phy.h @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2013 Renesas Solutions Corp. + * Copyright (C) 2013 Cogent Embedded, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __RCAR_PHY_H +#define __RCAR_PHY_H + +#include + +/* USBPCTRL0 register bits */ +#define USBPCTRL0_OVC2 BIT(10) /* Switches the OVC input pin for port 2: */ + /* 1: USB_OVC2, 0: OVC2 */ +#define USBPCTRL0_OVC1_VBUS1 BIT(9) /* Switches the OVC input pin for port 1: */ + /* 1: USB_OVC1, 0: OVC1/VBUS1 */ +#define USBPCTRL0_OVC0 BIT(8) /* Switches the OVC input pin for port 0: */ + /* 1: USB_OVC0 pin, 0: OVC0 */ +#define USBPCTRL0_OVC2_ACT BIT(6) /* Host mode: OVC2 polarity: */ + /* 1: active-high, 0: active-low*/ + /* Function mode: be sure to set to 1 */ +#define USBPCTRL0_PENC BIT(4) /* Function mode: output level of PENC1 pin: */ + /* 1: high, 0: low */ +#define USBPCTRL0_OVC0_ACT BIT(3) /* Host mode: OVC0 polarity: */ + /* 1: active-high, 0: active-low*/ +#define USBPCTRL0_OVC1_ACT BIT(1) /* Host mode: OVC1 polarity: */ + /* 1: active-high, 0: active-low*/ + /* Function mode: be sure to set to 1 */ +#define USBPCTRL0_PORT1BIT(0) /* Selects port 1 mode: */ + /* 1: function, 0: host */ + +struct rcar_phy_platform_data { + u32 usbpctrl0; /* USBPCTRL0 register value */ +}; + +#endif /* __RCAR_PHY_H */ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] rcar-phy: correct base address
The memory region that is used by the driver overlaps EHCI and OHCI register regions for absolutely no reason now -- fix it by adding offset of 0x800 to the base address, changing the register #define's accordingly. This has extra positive effect that we now can use devm_ioremap_resource(). Signed-off-by: Sergei Shtylyov --- arch/arm/mach-shmobile/setup-r8a7779.c |2 +- drivers/usb/phy/rcar-phy.c | 28 ++-- 2 files changed, 11 insertions(+), 19 deletions(-) Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c === --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -401,7 +401,7 @@ static struct platform_device sata_devic /* USB PHY */ static struct resource usb_phy_resources[] = { [0] = { - .start = 0xffe7, + .start = 0xffe70800, .end= 0xffe70900 - 1, .flags = IORESOURCE_MEM, }, Index: renesas/drivers/usb/phy/rcar-phy.c === --- renesas.orig/drivers/usb/phy/rcar-phy.c +++ renesas/drivers/usb/phy/rcar-phy.c @@ -16,13 +16,13 @@ #include #include -/* USBH common register */ -#define USBPCTRL0 0x0800 -#define USBPCTRL1 0x0804 -#define USBST 0x0808 -#define USBEH0 0x080C -#define USBOH0 0x081C -#define USBCTL00x0858 +/* REGS block */ +#define USBPCTRL0 0x00 +#define USBPCTRL1 0x04 +#define USBST 0x08 +#define USBEH0 0x0C +#define USBOH0 0x1C +#define USBCTL00x58 /* USBPCTRL1 */ #define PHY_RST(1 << 2) @@ -139,17 +139,9 @@ static int rcar_usb_phy_probe(struct pla return -EINVAL; } - /* -* CAUTION -* -* Because this phy address is also mapped under OHCI/EHCI address area, -* this driver can't use devm_request_and_ioremap(dev, res) here -*/ - reg0 = devm_ioremap_nocache(dev, res0->start, resource_size(res0)); - if (!reg0) { - dev_err(dev, "ioremap error\n"); - return -ENOMEM; - } + reg0 = devm_ioremap_resource(dev, res0); + if (IS_ERR(reg0)) + return PTR_ERR(reg0); priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) { -- 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/8] rcar-phy: remove EHCI internal buffer setup
Now that the EHCI internal buffer setup is done by the platform code, we can remove such code from this driver as it never really belonged here. We also no longer need the 2nd memory region now (2nd EHCI controller is simply missing in e.g. R8A7778 SoC). Signed-off-by: Sergei Shtylyov --- arch/arm/mach-shmobile/setup-r8a7779.c |5 - drivers/usb/phy/rcar-phy.c | 28 2 files changed, 4 insertions(+), 29 deletions(-) Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c === --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -405,11 +405,6 @@ static struct resource usb_phy_resources .end= 0xffe70900 - 1, .flags = IORESOURCE_MEM, }, - [1] = { - .start = 0xfff7, - .end= 0xfff70900 - 1, - .flags = IORESOURCE_MEM, - }, }; static struct platform_device usb_phy_device = { Index: renesas/drivers/usb/phy/rcar-phy.c === --- renesas.orig/drivers/usb/phy/rcar-phy.c +++ renesas/drivers/usb/phy/rcar-phy.c @@ -23,8 +23,6 @@ #define USBEH0 0x080C #define USBOH0 0x081C #define USBCTL00x0858 -#define EIIBC1 0x0094 -#define EIIBC2 0x009C /* USBPCTRL1 */ #define PHY_RST(1 << 2) @@ -40,7 +38,6 @@ struct rcar_usb_phy_priv { spinlock_t lock; void __iomem *reg0; - void __iomem *reg1; int counter; }; @@ -59,7 +56,6 @@ static int rcar_usb_phy_init(struct usb_ struct rcar_usb_phy_priv *priv = usb_phy_to_priv(phy); struct device *dev = phy->dev; void __iomem *reg0 = priv->reg0; - void __iomem *reg1 = priv->reg1; int i; u32 val; unsigned long flags; @@ -97,19 +93,6 @@ static int rcar_usb_phy_init(struct usb_ iowrite32(0x, (reg0 + USBPCTRL0)); /* -* EHCI IP internal buffer setting -* EHCI IP internal buffer enable -* -* These are recommended value of a datasheet -* see [USB :: EHCI internal buffer setting] -*/ - iowrite32(0x00ff0040, (reg0 + EIIBC1)); - iowrite32(0x00ff0040, (reg1 + EIIBC1)); - - iowrite32(0x0001, (reg0 + EIIBC2)); - iowrite32(0x0001, (reg1 + EIIBC2)); - - /* * Bus alignment settings */ @@ -145,14 +128,13 @@ static void rcar_usb_phy_shutdown(struct static int rcar_usb_phy_probe(struct platform_device *pdev) { struct rcar_usb_phy_priv *priv; - struct resource *res0, *res1; + struct resource *res0; struct device *dev = &pdev->dev; - void __iomem *reg0, *reg1; + void __iomem *reg0; int ret; res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0); - res1 = platform_get_resource(pdev, IORESOURCE_MEM, 1); - if (!res0 || !res1) { + if (!res0) { dev_err(dev, "Not enough platform resources\n"); return -EINVAL; } @@ -164,8 +146,7 @@ static int rcar_usb_phy_probe(struct pla * this driver can't use devm_request_and_ioremap(dev, res) here */ reg0 = devm_ioremap_nocache(dev, res0->start, resource_size(res0)); - reg1 = devm_ioremap_nocache(dev, res1->start, resource_size(res1)); - if (!reg0 || !reg1) { + if (!reg0) { dev_err(dev, "ioremap error\n"); return -ENOMEM; } @@ -177,7 +158,6 @@ static int rcar_usb_phy_probe(struct pla } priv->reg0 = reg0; - priv->reg1 = reg1; priv->counter = 0; priv->phy.dev = dev; priv->phy.label = dev_name(dev); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] ARM: shmobile: R8A7779: setup EHCI internal buffer
Setup the EHCI internal buffer (before EHCI driver has a chance to touch the registers) using the init() method in 'struct usb_ehci_pdata'. Signed-off-by: Sergei Shtylyov --- arch/arm/mach-shmobile/setup-r8a7779.c | 16 1 file changed, 16 insertions(+) Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c === --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -445,10 +445,26 @@ static void usb_power_off(struct platfor pm_runtime_disable(&pdev->dev); } +static int ehci_init_internal_buffer(struct platform_device *pdev, +void __iomem *regs) +{ + /* +* Below are recommended values from the datasheet; +* see [USB :: Setting of EHCI Internal Buffer]. +*/ + /* EHCI IP internal buffer setting */ + iowrite32(0x00ff0040, regs + 0x0094); + /* EHCI IP internal buffer enable */ + iowrite32(0x0001, regs + 0x009C); + + return 0; +} + static struct usb_ehci_pdata ehcix_pdata = { .power_on = usb_power_on, .power_off = usb_power_off, .power_suspend = usb_power_off, + .init = ehci_init_internal_buffer, }; static struct resource ehci0_resources[] = { -- 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
usb-storage causes hibernation to freeze before snapshotting image
(Please CC as I am not subscribed.) Dear Linux USB folks, using Debian Wheezy/testing with Linux 3.2.39-2 and uswsusp 1.0+20110509-3, I see the following problem similar to the one described in some Ubuntu forum from 2009 [1] or a question from 2012 to Ask Ubuntu [2]. Running `pm-hibernate` and `s2disk`, sometimes everything works, but very often, it does not snapshot the system but just stalls. Looking for splash system... none s2disk: Snapshotting system Comparing the loaded modules listed in `/var/log/pm-suspend.log`, it turns out that the module `usb-storage` is the problem. Unloading this before hibernating, by doing for example $ echo "MODULES_SUSPEND=usb-storage" >> /etc/pm/config.d/60-modules the problem cannot be reproduced anymore. I attach some more information at the end of the message. Do you know if such a problem was fixed in later Linux kernels? How should that be debugged? Thanks, Paul [1] http://ubuntuforums.org/showthread.php?t=1176780 [2] http://askubuntu.com/questions/98866/hibernation-stalls $ lsusb -v Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 0 Full speed (or root) hub bMaxPacketSize064 idVendor 0x1d6b Linux Foundation idProduct 0x0002 2.0 root hub bcdDevice3.02 iManufacturer 3 Linux 3.2.0-4-686-pae ehci_hcd iProduct2 EHCI Host Controller iSerial 1 :00:1d.7 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 25 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0004 1x 4 bytes bInterval 12 Hub Descriptor: bLength 11 bDescriptorType 41 nNbrPorts 8 wHubCharacteristic 0x000a No power switching (usb 1.0) Per-port overcurrent protection bPwrOn2PwrGood 10 * 2 milli seconds bHubContrCurrent 0 milli Ampere DeviceRemovable0x00 0x00 PortPwrCtrlMask0xff 0xff Hub Port Status: Port 1: .0100 power Port 2: .0100 power Port 3: .0100 power Port 4: .0100 power Port 5: .0100 power Port 6: .0100 power Port 7: .0100 power Port 8: .0100 power Device Status: 0x0001 Self Powered Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 1.10 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 0 Full speed (or root) hub bMaxPacketSize064 idVendor 0x1d6b Linux Foundation idProduct 0x0001 1.1 root hub bcdDevice3.02 iManufacturer 3 Linux 3.2.0-4-686-pae uhci_hcd iProduct2 UHCI Host Controller iSerial 1 :00:1d.0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 25 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0002 1x 2 bytes bInterval 255 Hub Descriptor: bLen
[PATCH 2/8] ehci-platform: add init() method to platform data
Sometimes there is a need to initialize some non-standard registers mapped to the EHCI region before accessing the standard EHCI registers. Add the init() method to the 'ehci-platform' platform data for this purpose. Suggested-by: Kuninori Morimoto Signed-off-by: Sergei Shtylyov --- drivers/usb/host/ehci-platform.c |7 +++ include/linux/usb/ehci_pdriver.h |1 + 2 files changed, 8 insertions(+) Index: renesas/drivers/usb/host/ehci-platform.c === --- renesas.orig/drivers/usb/host/ehci-platform.c +++ renesas/drivers/usb/host/ehci-platform.c @@ -110,6 +110,13 @@ static int ehci_platform_probe(struct pl err = PTR_ERR(hcd->regs); goto err_put_hcd; } + + if (pdata->init) { + err = pdata->init(dev, hcd->regs); + if (err < 0) + goto err_put_hcd; + } + err = usb_add_hcd(hcd, irq, IRQF_SHARED); if (err) goto err_put_hcd; Index: renesas/include/linux/usb/ehci_pdriver.h === --- renesas.orig/include/linux/usb/ehci_pdriver.h +++ renesas/include/linux/usb/ehci_pdriver.h @@ -50,6 +50,7 @@ struct usb_ehci_pdata { /* Turn on only VBUS suspend power and hotplug detection, * turn off everything else */ void (*power_suspend)(struct platform_device *pdev); + int (*init)(struct platform_device *pdev, void __iomem *regs); }; #endif /* __USB_CORE_EHCI_PDRIVER_H */ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] Reorganize R8A7779/Marzen USB code
Hello. Here's the set of 4 patches against the Simon Horman's 'renesas.git' repo, 'renesas-next-20130404v2' tag and the 2 Ether patches I've reposted yesterday. It was created to fix the shortcomings in the R8A7779/Marzen USB platform code and R8A7779 USB common PHY driver, and so spans both arch/arm/mach-shmobile/ and drivers/usb/ subtrees (some patches have to touch both subtrees). The patches were conceived with the complete bisectability goal in mind. [1/8] ARM: shmobile:Marzen: move USB EHCI, OHCI, and PHY devices to R8A7779 code [2/8] ehci-platform: add init() method to platform data [3/8] ARM: shmobile: R8A7779: setup EHCI internal buffer [4/8] rcar-phy: remove EHCI internal buffer setup [5/8] rcar-phy: correct base address [6/8] rcar-phy: add platform data [7/8] ARM: shmobile: Marzen: pass platform data to USB PHY device [8/8] rcar-phy: handle platform data I'm not sure thru which tree this patchset should be merged, however it turns out that it's too late now to push it thru Felipe Balbi's USB tree for 3.10, so maybe the patchset can be merged thru Simon's tree with Felipe's and Alan Stern's ACKs. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: (Solved) cdc_acm device - unexpected characters sent to USB device
I wanted to mark this solved send a brief note of thanks. After a month (off and on) of troubleshooting, your insightful questions helped to narrow in on the problem and helped me to ultimately determine that it was a line discipline (somehow) attached to the this ttyACM device each time it was created. This was ultimately not a USB or ACM device problem. The code now calls tcsetattr() to set the port to raw mode (instead of improperly assuming that it already was which had worked previously). I've read this mailing list for the last week or so and it's amazing how much work you guys do. Please accept my thanks for your help with this problem and well as for all the work you all do for the community. Mike > After adding this code, the sample application appears to run > - yeah! I'll do some more complete testing today to verify. > > A couple of follow up questions if you don't mind: > - For the cdc_acm driver, is the bit rate in termios just ignored? > - Similarly, are VMIN and VTIME used? > - Any idea why previous releases worked fine but a line > discipline is now applied to this cdc_acm port in the current release? > > Thanks > > Mike-- 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] xhci: fix list access before init
> -Original Message- > From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] > Sent: Thursday, April 04, 2013 12:29 PM > To: Vladimir Murzin > Cc: linux-usb@vger.kernel.org; gre...@suse.de; dse...@gmail.com; Aguirre > Rodriguez, Sergio A > Subject: Re: [PATCH] xhci: fix list access before init > > Someone else is already working on a patch to fix part of this: > > http://marc.info/?l=linux-usb&m=136509667003716&w=2 > > Please coordinate with them. > > Also, two people trying to solve the same simple bug and using almost > exactly the same commit messages seems like too much of a coincidence. > Are you working on the same project or did you both happen to run the > same static code checking tool at the same time, or ? I had no idea about this patch, nor had the pleasure to meet Sergey/Vladimir :) I guess it was really a strange coincidence... Regards, Sergio > > Sarah Sharp > > On Thu, Apr 04, 2013 at 08:31:26PM +0400, Vladimir Murzin wrote: > > Commits > > 9574323 xHCI: test USB2 software LPM > > b92cc66 xHCI: add aborting command ring function introduce useful > > functions which involves lists manipulations. > > > > If for whatever reason we fall into fail path in xhci_mem_init() we > > may access the lists in xhci_mem_cleanup() before they get > > initialized. > > > > The same init-fail-cleanup case is applicable to bw tables too. > > > > Fix this by moving list initialization code to the beginning of > > xhci_mem_init() > > > > Reported-by: Sergey Dyasly > > Tested-by: Sergey Dyasly > > Signed-off-by: Vladimir Murzin > > --- > > drivers/usb/host/xhci-mem.c | 37 > - > > 1 file changed, 20 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > > index 6dc238c..7b5d2f5 100644 > > --- a/drivers/usb/host/xhci-mem.c > > +++ b/drivers/usb/host/xhci-mem.c > > @@ -2123,7 +2123,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd > *xhci, gfp_t flags) > > __le32 __iomem *addr; > > u32 offset; > > unsigned int num_ports; > > - int i, j, port_index; > > + int i, port_index; > > > > addr = &xhci->cap_regs->hcc_params; > > offset = XHCI_HCC_EXT_CAPS(xhci_readl(xhci, addr)); @@ -2138,18 > > +2138,6 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t > flags) > > if (!xhci->port_array) > > return -ENOMEM; > > > > - xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); > > - if (!xhci->rh_bw) > > - return -ENOMEM; > > - for (i = 0; i < num_ports; i++) { > > - struct xhci_interval_bw_table *bw_table; > > - > > - INIT_LIST_HEAD(&xhci->rh_bw[i].tts); > > - bw_table = &xhci->rh_bw[i].bw_table; > > - for (j = 0; j < XHCI_MAX_INTERVAL; j++) > > - INIT_LIST_HEAD(&bw_table- > >interval_bw[j].endpoints); > > - } > > - > > /* > > * For whatever reason, the first capability offset is from the > > * capability register base, not from the HCCPARAMS register. > > @@ -2253,7 +2241,25 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t > flags) > > u64 val_64; > > struct xhci_segment *seg; > > u32 page_size, temp; > > - int i; > > + int i, j, num_ports; > > + > > + INIT_LIST_HEAD(&xhci->cancel_cmd_list); > > + INIT_LIST_HEAD(&xhci->lpm_failed_devs); > > + > > + num_ports = HCS_MAX_PORTS(xhci_readl(xhci, > > +&xhci->cap_regs->hcs_params1)); > > + > > + xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); > > + if (!xhci->rh_bw) > > + return -ENOMEM; > > + > > + for (i = 0; i < num_ports; i++) { > > + struct xhci_interval_bw_table *bw_table; > > + > > + INIT_LIST_HEAD(&xhci->rh_bw[i].tts); > > + bw_table = &xhci->rh_bw[i].bw_table; > > + for (j = 0; j < XHCI_MAX_INTERVAL; j++) > > + INIT_LIST_HEAD(&bw_table- > >interval_bw[j].endpoints); > > + } > > > > page_size = xhci_readl(xhci, &xhci->op_regs->page_size); > > xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); > > @@ -2333,7 +2339,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t > flags) > > xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); > > if (!xhci->cmd_ring) > > goto fail; > > - INIT_LIST_HEAD(&xhci->cancel_cmd_list); > > xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); > > xhci_dbg(xhci, "First segment DMA is 0x%llx\n", > > (unsigned long long)xhci->cmd_ring->first_seg- > >dma); > > @@ -2444,8 +2449,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t > flags) > > if (xhci_setup_port_arrays(xhci, flags)) > > goto fail; > > > > - INIT_LIST_HEAD(&xhci->lpm_failed_devs); > > - > > /* Enable USB 3.0 device notifications for function remote wake, > which > > * is necessary for allowing USB 3.0 devices to do remote wakeup > from > > * U3 (devic
[PATCH] usb: xhci-dbg: Display endpoint number and direction in context dump
When CONFIG_XHCI_HCD_DEBUGGING is activated, the XHCI driver can dump device and input contexts to the console. The endpoint contexts in that dump are labeled "Endpoint N Context", where N is DCI - 1... this is very confusing, especially for people who are not that familiar with the XHCI specification. Let's change this to display the endpoint number and direction, which are much more commonly used concepts in USB (and map to XHCI DCIs 1-to-1). Signed-off-by: Julius Werner --- drivers/usb/host/xhci-dbg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c index 5f3a7c7..98b1bee 100644 --- a/drivers/usb/host/xhci-dbg.c +++ b/drivers/usb/host/xhci-dbg.c @@ -507,7 +507,8 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci, dma_addr_t dma = ctx->dma + ((unsigned long)ep_ctx - (unsigned long)ctx->bytes); - xhci_dbg(xhci, "Endpoint %02d Context:\n", i); + xhci_dbg(xhci, "Endpoint %02d %s Context:\n", + DIV_ROUND_UP(i, 2), i % 2 ? "OUT" : "IN"); xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - ep_info\n", &ep_ctx->ep_info, (unsigned long long)dma, ep_ctx->ep_info); -- 1.8.1.3 -- 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] TTY: io_ti, stop dereferencing potential NULL
tty_port_tty_get might return a tty which is NULL. But it is dereferenced unconditionally in edge_send. Stop dereferencing that by sending usb_serial_port pointer around. Signed-off-by: Jiri Slaby Cc: Johan Hovold Cc: linux-usb@vger.kernel.org --- drivers/usb/serial/io_ti.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c index 0ccc422..f2a1601 100644 --- a/drivers/usb/serial/io_ti.c +++ b/drivers/usb/serial/io_ti.c @@ -206,7 +206,7 @@ static int restart_read(struct edgeport_port *edge_port); static void edge_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios); -static void edge_send(struct tty_struct *tty); +static void edge_send(struct usb_serial_port *port, struct tty_struct *tty); /* sysfs attributes */ static int edge_create_sysfs_attrs(struct usb_serial_port *port); @@ -1712,7 +1712,7 @@ static void edge_bulk_out_callback(struct urb *urb) /* send any buffered data */ tty = tty_port_tty_get(&port->port); - edge_send(tty); + edge_send(port, tty); tty_kref_put(tty); } @@ -1940,14 +1940,13 @@ static int edge_write(struct tty_struct *tty, struct usb_serial_port *port, count = kfifo_in_locked(&edge_port->write_fifo, data, count, &edge_port->ep_lock); - edge_send(tty); + edge_send(port, tty); return count; } -static void edge_send(struct tty_struct *tty) +static void edge_send(struct usb_serial_port *port, struct tty_struct *tty) { - struct usb_serial_port *port = tty->driver_data; int count, result; struct edgeport_port *edge_port = usb_get_serial_port_data(port); unsigned long flags; -- 1.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] xhci-mem: init list heads at the beginning of init
On Thu, Apr 04, 2013 at 12:50:03PM -0700, David Cohen wrote: > On 04/04/2013 12:31 PM, Sarah Sharp wrote: > >On Thu, Apr 04, 2013 at 10:32:13AM -0700, Sergio Aguirre wrote: > >>+ INIT_LIST_HEAD(&xhci->lpm_failed_devs); > >>+ INIT_LIST_HEAD(&xhci->cancel_cmd_list); > >>+ > >>page_size = xhci_readl(xhci, &xhci->op_regs->page_size); > >>xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); > >>for (i = 0; i < 16; i++) { > >>@@ -2333,7 +2336,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > >>xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); > >>if (!xhci->cmd_ring) > >>goto fail; > >>- INIT_LIST_HEAD(&xhci->cancel_cmd_list); > >>xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); > >>xhci_dbg(xhci, "First segment DMA is 0x%llx\n", > >>(unsigned long long)xhci->cmd_ring->first_seg->dma); > >>@@ -2444,8 +2446,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > >>if (xhci_setup_port_arrays(xhci, flags)) > >>goto fail; > >>- INIT_LIST_HEAD(&xhci->lpm_failed_devs); > >>- > >Please don't remove newlines in bug fix patches. Try again? > > Isn't it going to have 2 consecutive empty lines otherwise? Oh, yes, you're right, the initial patch does show that. Why did you trim newlines in your reply? That was what confused me. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] usb: musb: factor out hcd initalization
The musb struct is currently allocated along with the hcd, which makes it difficult to build a driver that only acts as gadget device. Fix this by allocation musb directly, and keep the hcd around as pointer. struct hc_driver musb_hc_driver can now also be static to musb_host.c, and the macro musb_to_hcd() is just a pointer dereferencer for now, and will be elminiated later. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_core.c | 60 ++ drivers/usb/musb/musb_core.h | 1 + drivers/usb/musb/musb_gadget.c | 10 --- drivers/usb/musb/musb_host.c | 65 -- drivers/usb/musb/musb_host.h | 21 +++--- 5 files changed, 102 insertions(+), 55 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 37a261a..d5e9794 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -404,7 +404,8 @@ void musb_hnp_stop(struct musb *musb) break; case OTG_STATE_B_HOST: dev_dbg(musb->controller, "HNP: Disabling HR\n"); - hcd->self.is_b_host = 0; + if (hcd) + hcd->self.is_b_host = 0; musb->xceiv->state = OTG_STATE_B_PERIPHERAL; MUSB_DEV_MODE(musb); reg = musb_readb(mbase, MUSB_POWER); @@ -484,7 +485,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, musb->xceiv->state = OTG_STATE_A_HOST; musb->is_active = 1; - usb_hcd_resume_root_hub(musb_to_hcd(musb)); + musb_host_resume_root_hub(musb); break; case OTG_STATE_B_WAIT_ACON: musb->xceiv->state = OTG_STATE_B_PERIPHERAL; @@ -501,7 +502,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, case OTG_STATE_A_SUSPEND: /* possibly DISCONNECT is upcoming */ musb->xceiv->state = OTG_STATE_A_HOST; - usb_hcd_resume_root_hub(musb_to_hcd(musb)); + musb_host_resume_root_hub(musb); break; case OTG_STATE_B_WAIT_ACON: case OTG_STATE_B_PERIPHERAL: @@ -643,7 +644,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, * undesired detour through A_WAIT_BCON. */ musb_hnp_stop(musb); - usb_hcd_resume_root_hub(musb_to_hcd(musb)); + musb_host_resume_root_hub(musb); musb_root_disconnect(musb); musb_platform_try_idle(musb, jiffies + msecs_to_jiffies(musb->a_wait_bcon @@ -726,7 +727,8 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, dev_dbg(musb->controller, "HNP: CONNECT, now b_host\n"); b_host: musb->xceiv->state = OTG_STATE_B_HOST; - hcd->self.is_b_host = 1; + if (hcd) + hcd->self.is_b_host = 1; musb->ignore_disconnect = 0; del_timer(&musb->otg_timer); break; @@ -734,17 +736,13 @@ b_host: if ((devctl & MUSB_DEVCTL_VBUS) == (3 << MUSB_DEVCTL_VBUS_SHIFT)) { musb->xceiv->state = OTG_STATE_A_HOST; - hcd->self.is_b_host = 0; + if (hcd) + hcd->self.is_b_host = 0; } break; } - /* poke the root hub */ - MUSB_HST_MODE(musb); - if (hcd->status_urb) - usb_hcd_poll_rh_status(hcd); - else - usb_hcd_resume_root_hub(hcd); + musb_host_poke_root_hub(musb); dev_dbg(musb->controller, "CONNECT (%s) devctl %02x\n", usb_otg_state_string(musb->xceiv->state), devctl); @@ -759,7 +757,7 @@ b_host: switch (musb->xceiv->state) { case OTG_STATE_A_HOST: case OTG_STATE_A_SUSPEND: - usb_hcd_resume_root_hub(musb_to_hcd(musb)); + musb_host_resume_root_hub(musb); musb_root_disconnect(musb); if (musb->a_wait_bcon != 0) musb_platform_try_idle(musb, jiffies @@ -772,7 +770,8 @@ b_host: * in hnp_stop() is currently not used...
[PATCH 1/7] usb: gadget: drop unused USB_GADGET_MUSB_HDRC
The functionality meant to be represented by this symbol will be re-added later, but for now, USB_GADGET_MUSB_HDRC is in fact unused and can be dropped. Signed-off-by: Daniel Mack --- drivers/usb/gadget/Kconfig | 8 1 file changed, 8 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index a61d981..0702fa0 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -333,14 +333,6 @@ config USB_MV_U3D # Controllers available in both integrated and discrete versions # -# musb builds in ../musb along with host support -config USB_GADGET_MUSB_HDRC - tristate "Inventra HDRC USB Peripheral (TI, ADI, ...)" - depends on USB_MUSB_HDRC - help - This OTG-capable silicon IP is used in dual designs including - the TI DaVinci, OMAP 243x, OMAP 343x, TUSB 6010, and ADI Blackfin - config USB_M66592 tristate "Renesas M66592 USB Peripheral Controller" help -- 1.8.1.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 5/7] usb: musb: add musb_host_setup() and musb_host_cleanup()
This patch re-introduces the bits that are necessary to use the musb controller in host mode. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_core.c | 5 + drivers/usb/musb/musb_host.c | 21 + drivers/usb/musb/musb_host.h | 8 3 files changed, 34 insertions(+) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index c637d36..cb1631e 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1006,6 +1006,7 @@ static void musb_shutdown(struct platform_device *pdev) pm_runtime_get_sync(musb->controller); + musb_host_cleanup(musb); musb_gadget_cleanup(musb); spin_lock_irqsave(&musb->lock, flags); @@ -1957,6 +1958,10 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) musb->xceiv->state = OTG_STATE_B_IDLE; } + status = musb_host_setup(musb, plat->power); + if (status < 0) + goto fail3; + status = musb_gadget_setup(musb); if (status < 0) goto fail3; diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index d564580..d298be7 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -2649,6 +2649,27 @@ void musb_host_free(struct musb *musb) usb_put_hcd(musb->hcd); } +int musb_host_setup(struct musb *musb, int power_budget) +{ + int ret; + struct usb_hcd *hcd = musb->hcd; + + MUSB_HST_MODE(musb); + musb->xceiv->otg->default_a = 1; + musb->xceiv->state = OTG_STATE_A_IDLE; + + otg_set_host(musb->xceiv->otg, &hcd->self); + hcd->self.otg_port = 1; + musb->xceiv->otg->host = &hcd->self; + hcd->power_budget = 2 * (power_budget ? : 250); + + ret = usb_add_hcd(hcd, 0, 0); + if (ret < 0) + return ret; + + return 0; +} + void musb_host_resume_root_hub(struct musb *musb) { usb_hcd_resume_root_hub(musb->hcd); diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h index 591036a..d144839 100644 --- a/drivers/usb/musb/musb_host.h +++ b/drivers/usb/musb/musb_host.h @@ -81,6 +81,8 @@ static inline struct musb_qh *first_qh(struct list_head *q) extern struct musb *hcd_to_musb(struct usb_hcd *); extern irqreturn_t musb_h_ep0_irq(struct musb *); extern int musb_host_alloc(struct musb *); +extern int musb_host_setup(struct musb *, int); +extern void musb_host_cleanup(struct musb *); extern void musb_host_tx(struct musb *, u8); extern void musb_host_rx(struct musb *, u8); extern void musb_root_disconnect(struct musb *musb); @@ -108,6 +110,12 @@ static inline int musb_host_alloc(struct musb *musb) return 0; } +static inline int musb_host_setup(struct musb *musb, int power_budget) +{ + return 0; +} + +static inline void musb_host_cleanup(struct musb *musb){} static inline void musb_host_free(struct musb *musb) {} static inline void musb_host_tx(struct musb *musb, u8 epnum) {} static inline void musb_host_rx(struct musb *musb, u8 epnum) {} -- 1.8.1.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: musb: move function declarations to musb_{host,gadget}.h
Let the function declarations live in the header file the belong to, which makes it cleaner when they're stubbed out later. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_core.h | 17 - drivers/usb/musb/musb_gadget.h | 17 +++-- drivers/usb/musb/musb_host.h | 4 +++- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index 7fb4819..04d8974 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -82,23 +82,6 @@ struct musb_ep; #define MUSB_CONFIG_PROC_FS #endif -/** PERIPHERAL ROLE */ - -extern irqreturn_t musb_g_ep0_irq(struct musb *); -extern void musb_g_tx(struct musb *, u8); -extern void musb_g_rx(struct musb *, u8); -extern void musb_g_reset(struct musb *); -extern void musb_g_suspend(struct musb *); -extern void musb_g_resume(struct musb *); -extern void musb_g_wakeup(struct musb *); -extern void musb_g_disconnect(struct musb *); - -/** HOST ROLE ***/ - -extern irqreturn_t musb_h_ep0_irq(struct musb *); -extern void musb_host_tx(struct musb *, u8); -extern void musb_host_rx(struct musb *, u8); - /** CONSTANTS / #ifndef MUSB_C_NUM_EPS diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h index 66b7c5e..75f821c 100644 --- a/drivers/usb/musb/musb_gadget.h +++ b/drivers/usb/musb/musb_gadget.h @@ -37,6 +37,17 @@ #include +extern irqreturn_t musb_g_ep0_irq(struct musb *); +extern void musb_g_tx(struct musb *, u8); +extern void musb_g_rx(struct musb *, u8); +extern void musb_g_reset(struct musb *); +extern void musb_g_suspend(struct musb *); +extern void musb_g_resume(struct musb *); +extern void musb_g_wakeup(struct musb *); +extern void musb_g_disconnect(struct musb *); +extern void musb_gadget_cleanup(struct musb *); +extern int musb_gadget_setup(struct musb *); + enum buffer_map_state { UN_MAPPED = 0, PRE_MAPPED, @@ -106,14 +117,8 @@ static inline struct musb_request *next_request(struct musb_ep *ep) return container_of(queue->next, struct musb_request, list); } -extern void musb_g_tx(struct musb *musb, u8 epnum); -extern void musb_g_rx(struct musb *musb, u8 epnum); - extern const struct usb_ep_ops musb_g_ep0_ops; -extern int musb_gadget_setup(struct musb *); -extern void musb_gadget_cleanup(struct musb *); - extern void musb_g_giveback(struct musb_ep *, struct usb_request *, int); extern void musb_ep_restart(struct musb *, struct musb_request *); diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h index 5a9c8fe..9670269 100644 --- a/drivers/usb/musb/musb_host.h +++ b/drivers/usb/musb/musb_host.h @@ -84,7 +84,9 @@ static inline struct musb_qh *first_qh(struct list_head *q) return list_entry(q->next, struct musb_qh, ring); } - +extern irqreturn_t musb_h_ep0_irq(struct musb *); +extern void musb_host_tx(struct musb *, u8); +extern void musb_host_rx(struct musb *, u8); extern void musb_root_disconnect(struct musb *musb); struct usb_hcd; -- 1.8.1.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: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes
This makes building the actual object files optional to the selected mode, which saves users who know which kind of USB mode support they need some binary size. Unimplemented functions are stubbed out with static inline functions. Signed-off-by: Daniel Mack --- drivers/usb/musb/Kconfig | 29 + drivers/usb/musb/Makefile | 10 -- drivers/usb/musb/musb_core.c | 1 - drivers/usb/musb/musb_gadget.h | 21 + drivers/usb/musb/musb_host.h | 29 +++-- 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 47442d3..aab1568 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -28,6 +28,35 @@ config USB_MUSB_HDRC if USB_MUSB_HDRC choice + bool "MUSB Mode Selection" + default USB_MUSB_DUAL_ROLE if (USB && USB_GADGET) + default USB_MUSB_HOST if (USB && !USB_GADGET) + default USB_MUSB_GADGET if (!USB && USB_GADGET) + +config USB_MUSB_HOST + bool "Host only mode" + depends on USB + help + Select this when you want to use MUSB in host mode only, + thereby the gadget feature will be regressed. + +config USB_MUSB_GADGET + bool "Gadget only mode" + depends on USB_GADGET + help + Select this when you want to use MUSB in gadget mode only, + thereby the host feature will be regressed. + +config USB_MUSB_DUAL_ROLE + bool "Dual Role mode" + depends on (USB && USB_GADGET) + help + This is the default mode of working of MUSB controller where + both host and gadget features are enabled. + +endchoice + +choice prompt "Platform Glue Layer" config USB_MUSB_DAVINCI diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile index 3b85871..6b13a53 100644 --- a/drivers/usb/musb/Makefile +++ b/drivers/usb/musb/Makefile @@ -6,10 +6,16 @@ obj-$(CONFIG_USB_MUSB_HDRC) += musb_hdrc.o musb_hdrc-y := musb_core.o -musb_hdrc-y+= musb_gadget_ep0.o musb_gadget.o -musb_hdrc-y+= musb_virthub.o musb_host.o musb_hdrc-$(CONFIG_DEBUG_FS) += musb_debugfs.o +ifneq ($(filter y,$(CONFIG_USB_MUSB_HOST) $(CONFIG_USB_MUSB_DUAL_ROLE)),) + musb_hdrc-y += musb_virthub.o musb_host.o +endif + +ifneq ($(filter y,$(CONFIG_USB_MUSB_GADGET) $(CONFIG_USB_MUSB_DUAL_ROLE)),) + musb_hdrc-y += musb_gadget_ep0.o musb_gadget.o +endif + # Hardware Glue Layer obj-$(CONFIG_USB_MUSB_OMAP2PLUS) += omap2430.o obj-$(CONFIG_USB_MUSB_AM35X) += am35x.o diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d5e9794..c637d36 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1958,7 +1958,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) } status = musb_gadget_setup(musb); - if (status < 0) goto fail3; diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h index 75f821c..0314dfc 100644 --- a/drivers/usb/musb/musb_gadget.h +++ b/drivers/usb/musb/musb_gadget.h @@ -37,6 +37,7 @@ #include +#if IS_ENABLED(CONFIG_USB_MUSB_GADGET) || IS_ENABLED(CONFIG_USB_MUSB_DUAL_ROLE) extern irqreturn_t musb_g_ep0_irq(struct musb *); extern void musb_g_tx(struct musb *, u8); extern void musb_g_rx(struct musb *, u8); @@ -48,6 +49,26 @@ extern void musb_g_disconnect(struct musb *); extern void musb_gadget_cleanup(struct musb *); extern int musb_gadget_setup(struct musb *); +#else +static inline irqreturn_t musb_g_ep0_irq(struct musb *musb) +{ + return 0; +} + +static inline void musb_g_tx(struct musb *musb, u8 epnum) {} +static inline void musb_g_rx(struct musb *musb, u8 epnum) {} +static inline void musb_g_reset(struct musb *musb) {} +static inline void musb_g_suspend(struct musb *musb) {} +static inline void musb_g_resume(struct musb *musb){} +static inline void musb_g_wakeup(struct musb *musb){} +static inline void musb_g_disconnect(struct musb *musb){} +static inline void musb_gadget_cleanup(struct musb *musb) {} +static inline int musb_gadget_setup(struct musb *musb) +{ + return 0; +} +#endif + enum buffer_map_state { UN_MAPPED = 0, PRE_MAPPED, diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h index fb24422..591036a 100644 --- a/drivers/usb/musb/musb_host.h +++ b/drivers/usb/musb/musb_host.h @@ -39,8 +39,6 @@ #define musb_to_hcd(MUSB) ((MUSB)->hcd) -extern struct musb *hcd_to_musb(struct usb_hcd *); - /* stored in "usb_host_endpoint.hcpriv" for scheduled endpoints */ struct musb_qh { struct usb_host_endpoint *hep; /* usbcore info */ @@ -78,6 +76,9 @@ static inline struct musb_q
[PATCH 0/7] usb: musb: add support for host support back
Hi all, here are some patches to separate the HCD and gadget part of the musb driver so they can be deselected in Kconfig. They also make the driver keep track of the configured port mode that is set from DT, so the actual runtime configuration can be selected dynamically. One thing that is still broken is that once pm_suspend() was called on a musb device on a USB disconnect, the port won't wake up again when a device is plugged back in. I doubt this is related to my patches, but I might be wrong. If that effect rings a bell to anyone, please let me know. Thanks, Daniel Daniel Mack (7): usb: gadget: drop unused USB_GADGET_MUSB_HDRC usb: musb: move function declarations to musb_{host,gadget}.h usb: musb: factor out hcd initalization usb: musb: add Kconfig options for HOST, GAGDET or DUAL_ROLE modes usb: musb: add musb_host_setup() and musb_host_cleanup() usb: musb: re-introduce musb->port_mode usb: musb: eliminate musb_to_hcd drivers/usb/gadget/Kconfig | 8 drivers/usb/musb/Kconfig| 29 + drivers/usb/musb/Makefile | 10 - drivers/usb/musb/musb_core.c| 87 +- drivers/usb/musb/musb_core.h| 25 --- drivers/usb/musb/musb_gadget.c | 15 +++ drivers/usb/musb/musb_gadget.h | 38 ++--- drivers/usb/musb/musb_host.c| 94 ++--- drivers/usb/musb/musb_host.h| 58 - drivers/usb/musb/musb_virthub.c | 6 +-- 10 files changed, 267 insertions(+), 103 deletions(-) -- 1.8.1.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 6/7] usb: musb: re-introduce musb->port_mode
Initialize the host and gagdet subsystems of the musb driver only when the appropriate mode is selected from platform data, or device-tree information, respectively. Refuse to start the gadget part if the port is in host-only mode. Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_core.c | 25 + drivers/usb/musb/musb_core.h | 7 +++ drivers/usb/musb/musb_gadget.c | 5 + 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index cb1631e..c021058 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -943,10 +943,12 @@ void musb_start(struct musb *musb) * (b) vbus present/connect IRQ, peripheral mode; * (c) peripheral initiates, using SRP */ - if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) + if (musb->port_mode != MUSB_PORT_MODE_HOST && + (devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) { musb->is_active = 1; - else + } else { devctl |= MUSB_DEVCTL_SESSION; + } musb_platform_enable(musb); musb_writeb(regs, MUSB_DEVCTL, devctl); @@ -1868,6 +1870,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) musb->board_set_power = plat->set_power; musb->min_power = plat->min_power; musb->ops = plat->platform_ops; + musb->port_mode = plat->mode; /* The musb_platform_init() call: * - adjusts musb->mregs @@ -1958,13 +1961,19 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) musb->xceiv->state = OTG_STATE_B_IDLE; } - status = musb_host_setup(musb, plat->power); - if (status < 0) - goto fail3; + if (musb->port_mode == MUSB_PORT_MODE_HOST || + musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) { + status = musb_host_setup(musb, plat->power); + if (status < 0) + goto fail3; + } - status = musb_gadget_setup(musb); - if (status < 0) - goto fail3; + if (musb->port_mode == MUSB_PORT_MODE_GADGET || + musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) { + status = musb_gadget_setup(musb); + if (status < 0) + goto fail3; + } status = musb_init_debugfs(musb); if (status < 0) diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index ef5b4e6..ed1644f 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -77,6 +77,12 @@ struct musb_ep; #define is_peripheral_active(m)(!(m)->is_host) #define is_host_active(m) ((m)->is_host) +enum { + MUSB_PORT_MODE_HOST = 1, + MUSB_PORT_MODE_GADGET = 2, + MUSB_PORT_MODE_DUAL_ROLE= 3, +}; + #ifdef CONFIG_PROC_FS #include #define MUSB_CONFIG_PROC_FS @@ -356,6 +362,7 @@ struct musb { u8 min_power; /* vbus for periph, in mA/2 */ + int port_mode; /* MUSB_PORT_MODE_* */ boolis_host; int a_wait_bcon;/* VBUS timeout in msecs */ diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index 0414bc1..c606088 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1823,6 +1823,11 @@ static int musb_gadget_start(struct usb_gadget *g, unsigned long flags; int retval = 0; + if (musb->port_mode == MUSB_PORT_MODE_HOST) { + retval = -EINVAL; + goto err; + } + if (driver->max_speed < USB_SPEED_HIGH) { retval = -EINVAL; goto err; -- 1.8.1.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 7/7] usb: musb: eliminate musb_to_hcd
With the hcd is now a direct member of struct musb, we can now simply eliminate the musb_to_hcd() macro. There aren't that many users left anyway, as some where already fixed up when parts were factored out to musb_host.c Signed-off-by: Daniel Mack --- drivers/usb/musb/musb_core.c| 4 ++-- drivers/usb/musb/musb_host.c| 8 drivers/usb/musb/musb_virthub.c | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index c021058..7b6d976 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -389,7 +389,7 @@ static void musb_otg_timer_func(unsigned long data) */ void musb_hnp_stop(struct musb *musb) { - struct usb_hcd *hcd = musb_to_hcd(musb); + struct usb_hcd *hcd = musb->hcd; void __iomem*mbase = musb->mregs; u8 reg; @@ -686,7 +686,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, } if (int_usb & MUSB_INTR_CONNECT) { - struct usb_hcd *hcd = musb_to_hcd(musb); + struct usb_hcd *hcd = musb->hcd; handled = IRQ_HANDLED; musb->is_active = 1; diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index d298be7..0e0ff69 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -321,9 +321,9 @@ __acquires(musb->lock) urb->actual_length, urb->transfer_buffer_length ); - usb_hcd_unlink_urb_from_ep(musb_to_hcd(musb), urb); + usb_hcd_unlink_urb_from_ep(musb->hcd, urb); spin_unlock(&musb->lock); - usb_hcd_giveback_urb(musb_to_hcd(musb), urb, status); + usb_hcd_giveback_urb(musb->hcd, urb, status); spin_lock(&musb->lock); } @@ -1466,7 +1466,7 @@ done: if (length > qh->maxpacket) length = qh->maxpacket; /* Unmap the buffer so that CPU can use it */ - usb_hcd_unmap_urb_for_dma(musb_to_hcd(musb), urb); + usb_hcd_unmap_urb_for_dma(musb->hcd, urb); /* * We need to map sg if the transfer_buffer is @@ -1881,7 +1881,7 @@ void musb_host_rx(struct musb *musb, u8 epnum) unsigned int received_len; /* Unmap the buffer so that CPU can use it */ - usb_hcd_unmap_urb_for_dma(musb_to_hcd(musb), urb); + usb_hcd_unmap_urb_for_dma(musb->hcd, urb); /* * We need to map sg if the transfer_buffer is diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c index ef7d110..d86a9b2 100644 --- a/drivers/usb/musb/musb_virthub.c +++ b/drivers/usb/musb/musb_virthub.c @@ -170,7 +170,7 @@ static void musb_port_reset(struct musb *musb, bool do_reset) musb->port1_status |= USB_PORT_STAT_ENABLE | (USB_PORT_STAT_C_RESET << 16) | (USB_PORT_STAT_C_ENABLE << 16); - usb_hcd_poll_rh_status(musb_to_hcd(musb)); + usb_hcd_poll_rh_status(musb->hcd); musb->vbuserr_retry = VBUSERR_RETRY_COUNT; } @@ -183,7 +183,7 @@ void musb_root_disconnect(struct musb *musb) musb->port1_status = USB_PORT_STAT_POWER | (USB_PORT_STAT_C_CONNECTION << 16); - usb_hcd_poll_rh_status(musb_to_hcd(musb)); + usb_hcd_poll_rh_status(musb->hcd); musb->is_active = 0; switch (musb->xceiv->state) { @@ -337,7 +337,7 @@ int musb_hub_control( musb->port1_status &= ~(USB_PORT_STAT_SUSPEND | MUSB_PORT_STAT_RESUME); musb->port1_status |= USB_PORT_STAT_C_SUSPEND << 16; - usb_hcd_poll_rh_status(musb_to_hcd(musb)); + usb_hcd_poll_rh_status(musb->hcd); /* NOTE: it might really be A_WAIT_BCON ... */ musb->xceiv->state = OTG_STATE_A_HOST; } -- 1.8.1.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 v3] xhci-mem: init list heads at the beginning of init
On 04/04/2013 12:31 PM, Sarah Sharp wrote: On Thu, Apr 04, 2013 at 10:32:13AM -0700, Sergio Aguirre wrote: + INIT_LIST_HEAD(&xhci->lpm_failed_devs); + INIT_LIST_HEAD(&xhci->cancel_cmd_list); + page_size = xhci_readl(xhci, &xhci->op_regs->page_size); xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); for (i = 0; i < 16; i++) { @@ -2333,7 +2336,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); if (!xhci->cmd_ring) goto fail; - INIT_LIST_HEAD(&xhci->cancel_cmd_list); xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); xhci_dbg(xhci, "First segment DMA is 0x%llx\n", (unsigned long long)xhci->cmd_ring->first_seg->dma); @@ -2444,8 +2446,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) if (xhci_setup_port_arrays(xhci, flags)) goto fail; - INIT_LIST_HEAD(&xhci->lpm_failed_devs); - Please don't remove newlines in bug fix patches. Try again? Isn't it going to have 2 consecutive empty lines otherwise? Regards, David Cohen Sarah Sharp -- 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/5 v3] USB: regroup all depends on USB within an if USB block
On Thursday 04 April 2013, Felipe Balbi wrote: > On Thu, Apr 04, 2013 at 01:42:18PM -0400, Alan Stern wrote: > > > > diff --git a/drivers/usb/misc/sisusbvga/Kconfig > > > > b/drivers/usb/misc/sisusbvga/Kconfig > > > > index 30ea7ca..0d03a52 100644 > > > > --- a/drivers/usb/misc/sisusbvga/Kconfig > > > > +++ b/drivers/usb/misc/sisusbvga/Kconfig > > > > @@ -1,7 +1,7 @@ > > > > > > > > config USB_SISUSBVGA > > > > tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" > > > > - depends on USB && (USB_MUSB_HDRC || USB_EHCI_HCD) > > > > + depends on (USB_MUSB_HDRC || USB_EHCI_HCD) > > > > > > is it just me or would everybody agree that depending on MUSB or EHCI > > > here is wrong ? > > > > That line certainly looks like it could be removed entirely. Perhaps > > the original author can enlighten us. > > > > In any case, it's not relevant to the purpose of this patch set. > > right :-) > Originally it had just "depends on USB_EHCI_HCD" and the text mentions that it requires USB 2.0, so it probably comes from a time where EHCI was the only high speed capable USB HCD. 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 v3] xhci-mem: init list heads at the beginning of init
On Thu, Apr 04, 2013 at 10:32:13AM -0700, Sergio Aguirre wrote: > + INIT_LIST_HEAD(&xhci->lpm_failed_devs); > + INIT_LIST_HEAD(&xhci->cancel_cmd_list); > + > page_size = xhci_readl(xhci, &xhci->op_regs->page_size); > xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); > for (i = 0; i < 16; i++) { > @@ -2333,7 +2336,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); > if (!xhci->cmd_ring) > goto fail; > - INIT_LIST_HEAD(&xhci->cancel_cmd_list); > xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); > xhci_dbg(xhci, "First segment DMA is 0x%llx\n", > (unsigned long long)xhci->cmd_ring->first_seg->dma); > @@ -2444,8 +2446,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > if (xhci_setup_port_arrays(xhci, flags)) > goto fail; > > - INIT_LIST_HEAD(&xhci->lpm_failed_devs); > - Please don't remove newlines in bug fix patches. Try again? Sarah Sharp -- 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] xhci: fix list access before init
Someone else is already working on a patch to fix part of this: http://marc.info/?l=linux-usb&m=136509667003716&w=2 Please coordinate with them. Also, two people trying to solve the same simple bug and using almost exactly the same commit messages seems like too much of a coincidence. Are you working on the same project or did you both happen to run the same static code checking tool at the same time, or ? Sarah Sharp On Thu, Apr 04, 2013 at 08:31:26PM +0400, Vladimir Murzin wrote: > Commits > 9574323 xHCI: test USB2 software LPM > b92cc66 xHCI: add aborting command ring function > introduce useful functions which involves lists manipulations. > > If for whatever reason we fall into fail path in xhci_mem_init() we > may access the lists in xhci_mem_cleanup() before they get > initialized. > > The same init-fail-cleanup case is applicable to bw tables too. > > Fix this by moving list initialization code to the beginning of > xhci_mem_init() > > Reported-by: Sergey Dyasly > Tested-by: Sergey Dyasly > Signed-off-by: Vladimir Murzin > --- > drivers/usb/host/xhci-mem.c | 37 - > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 6dc238c..7b5d2f5 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -2123,7 +2123,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd > *xhci, gfp_t flags) > __le32 __iomem *addr; > u32 offset; > unsigned int num_ports; > - int i, j, port_index; > + int i, port_index; > > addr = &xhci->cap_regs->hcc_params; > offset = XHCI_HCC_EXT_CAPS(xhci_readl(xhci, addr)); > @@ -2138,18 +2138,6 @@ static int xhci_setup_port_arrays(struct xhci_hcd > *xhci, gfp_t flags) > if (!xhci->port_array) > return -ENOMEM; > > - xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); > - if (!xhci->rh_bw) > - return -ENOMEM; > - for (i = 0; i < num_ports; i++) { > - struct xhci_interval_bw_table *bw_table; > - > - INIT_LIST_HEAD(&xhci->rh_bw[i].tts); > - bw_table = &xhci->rh_bw[i].bw_table; > - for (j = 0; j < XHCI_MAX_INTERVAL; j++) > - INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints); > - } > - > /* >* For whatever reason, the first capability offset is from the >* capability register base, not from the HCCPARAMS register. > @@ -2253,7 +2241,25 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > u64 val_64; > struct xhci_segment *seg; > u32 page_size, temp; > - int i; > + int i, j, num_ports; > + > + INIT_LIST_HEAD(&xhci->cancel_cmd_list); > + INIT_LIST_HEAD(&xhci->lpm_failed_devs); > + > + num_ports = HCS_MAX_PORTS(xhci_readl(xhci, > &xhci->cap_regs->hcs_params1)); > + > + xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); > + if (!xhci->rh_bw) > + return -ENOMEM; > + > + for (i = 0; i < num_ports; i++) { > + struct xhci_interval_bw_table *bw_table; > + > + INIT_LIST_HEAD(&xhci->rh_bw[i].tts); > + bw_table = &xhci->rh_bw[i].bw_table; > + for (j = 0; j < XHCI_MAX_INTERVAL; j++) > + INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints); > + } > > page_size = xhci_readl(xhci, &xhci->op_regs->page_size); > xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); > @@ -2333,7 +2339,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); > if (!xhci->cmd_ring) > goto fail; > - INIT_LIST_HEAD(&xhci->cancel_cmd_list); > xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); > xhci_dbg(xhci, "First segment DMA is 0x%llx\n", > (unsigned long long)xhci->cmd_ring->first_seg->dma); > @@ -2444,8 +2449,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > if (xhci_setup_port_arrays(xhci, flags)) > goto fail; > > - INIT_LIST_HEAD(&xhci->lpm_failed_devs); > - > /* Enable USB 3.0 device notifications for function remote wake, which >* is necessary for allowing USB 3.0 devices to do remote wakeup from >* U3 (device suspend). > -- > 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 13/15] staging: usbip: userspace: libsrc: cleanup parsing
On Thu, Apr 04, 2013 at 10:10:10PM +0300, Dan Carpenter wrote: > > + if (buf[0] == 'P' && buf[1] == 'H' && buf[2] == 'Y' && > > + buf[3] == 'S' && buf[4] == 'D' && > > + buf[5] == 'E' && buf[6] == 'S' && /*isspace(buf[7])*/ > > + buf[7] == ' ') { > > continue; > > if (strncmp(buf, "PHYSDES ", 8) == 0) > continue; Btw, this is obviously something that could be cleaned up in a later patch. Generally, if you didn't introduce it and it's not a bug then fixing it in a later patch is fine. regards, dan carpenter -- 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 03/15] staging: usbip: reformat function pickup_urb_and_free_priv
On Thu, Apr 04, 2013 at 04:03:04PM +0200, Stefan Reif wrote: > + switch (status) { > + case -ENOENT: > + /* fall through */ > + case -ECONNRESET: If two case statements are right next to each other then they don't need a "fall through" comment. (Don't resend. I'm just pointing that out for future reference). regards, dan carpenter -- 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: [Libusbx-devel] libusbx v1.0.15-rc2 is now available for testing
Hi, merci beaucoup to everyone for your work... Le 04/04/13 20:35, Pete Batard a écrit : On 2013.04.04 09:27, nico wrote: Making all in libusb CC libusb_1_0_la-core.lo core.c:1755:30: warning: use of logical '&&' with constant operand Thanks for the report, and the test. This is now fixed in RC3, along with a couple minor issues. The new RC is available from: http://sourceforge.net/projects/libusbx/files/releases/1.0.15/source/ RC3 builds fine with Clang on macOS 10.7.5 ++ Nicolas Regards, /Pete -- Minimize network downtime and maximize team effectiveness. Reduce network management and security costs.Learn how to hire the most talented Cisco Certified professionals. Visit the Employer Resources Portal http://www.cisco.com/web/learning/employer_resources/index.html ___ libusbx-devel mailing list libusbx-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel -- 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 13/15] staging: usbip: userspace: libsrc: cleanup parsing
This one could probably have been broken into separate patches. > + if (buf[0] == 'P' && buf[1] == 'H' && buf[2] == 'Y' && > + buf[3] == 'S' && buf[4] == 'D' && > + buf[5] == 'E' && buf[6] == 'S' && /*isspace(buf[7])*/ > + buf[7] == ' ') { > continue; if (strncmp(buf, "PHYSDES ", 8) == 0) continue; regards, dan caprenter -- 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: linux-next: Tree for Apr 4 (usb/gadget/configfs)
On 04/04/13 00:22, Stephen Rothwell wrote: > Hi all, > > Changes since 20130403: > on x86_64, when CONFIG_BUG is not enabled: CC [M] drivers/usb/gadget/configfs.o drivers/usb/gadget/configfs.c: In function 'config_usb_cfg_unlink': drivers/usb/gadget/configfs.c:442:2: error: implicit declaration of function '__WARN_printf' [-Werror=implicit-function-declaration] drivers/usb/gadget/configfs.c: In function 'configfs_do_nothing': drivers/usb/gadget/configfs.c:733:2: error: implicit declaration of function '__WARN' [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[4]: *** [drivers/usb/gadget/configfs.o] Error 1 -- ~Randy -- 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: [Libusbx-devel] libusbx v1.0.15-rc2 is now available for testing
On 2013.04.04 09:27, nico wrote: Making all in libusb CC libusb_1_0_la-core.lo core.c:1755:30: warning: use of logical '&&' with constant operand Thanks for the report, and the test. This is now fixed in RC3, along with a couple minor issues. The new RC is available from: http://sourceforge.net/projects/libusbx/files/releases/1.0.15/source/ Regards, /Pete -- 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: Exporting iInterface (and associated string) in debugfs?
On Thu, Apr 04, 2013 at 04:13:20PM +0200, Stefan Tauner wrote: > Hi, > > I discovered that the optional strings and their indices of interface > descriptors are not exported in debugfs when I was looking at usbview's > source (https://github.com/gregkh/usbview/issues/5). > > I am not sure yet if they are cached (e.g. in rawdescriptors in struct > usb_device or even more conveniently) or if one would have to fetch > them with usb_get_descriptor(). More importantly though I wonder if you > think it is useful to export them at all? I haven't apprehended the > debugfs/sysfs differences regarding USB yet apparently. Maybe a better > question would have been "Why does usbview not use sysfs (or even > libusb)?". usbview predates both sysfs and libusb, it used only the usbfs 'devices' file when it was created (way back in 1999 or so). That's why it doesn't use those apis. My long-term goal is to merge usbview into usbutils, which does use libusb, so that will resolve the dependancy on the debugfs 'devices' file. There really is no place in usbview to put the interface optional strings at the moment, but 'lsusb' should show you them just fine, right? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5 v3] USB: regroup all depends on USB within an if USB block
Hi, On Thu, Apr 04, 2013 at 01:42:18PM -0400, Alan Stern wrote: > > > diff --git a/drivers/usb/misc/sisusbvga/Kconfig > > > b/drivers/usb/misc/sisusbvga/Kconfig > > > index 30ea7ca..0d03a52 100644 > > > --- a/drivers/usb/misc/sisusbvga/Kconfig > > > +++ b/drivers/usb/misc/sisusbvga/Kconfig > > > @@ -1,7 +1,7 @@ > > > > > > config USB_SISUSBVGA > > > tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" > > > - depends on USB && (USB_MUSB_HDRC || USB_EHCI_HCD) > > > + depends on (USB_MUSB_HDRC || USB_EHCI_HCD) > > > > is it just me or would everybody agree that depending on MUSB or EHCI > > here is wrong ? > > That line certainly looks like it could be removed entirely. Perhaps > the original author can enlighten us. > > In any case, it's not relevant to the purpose of this patch set. right :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 2/7] ARM: tegra: update device trees for USB binding rework
On 04/04/2013 07:01 AM, Venu Byravarasu wrote: >> -Original Message- >> From: Stephen Warren [mailto:swar...@wwwdotorg.org] >> Sent: Thursday, April 04, 2013 12:47 AM >> To: Venu Byravarasu >> Cc: gre...@linuxfoundation.org; ba...@ti.com; >> st...@rowland.harvard.edu; linux-te...@vger.kernel.org; linux- >> u...@vger.kernel.org; linux-ker...@vger.kernel.org >> Subject: Re: [PATCH v2 2/7] ARM: tegra: update device trees for USB binding >> rework >> >> On 04/03/2013 02:41 AM, Venu Byravarasu wrote: >>> This patch updates all Tegra board files so that they contain all the >> >> The binding documentation says that the vbus-supply property is required >> in all cases. However, many of the DT files still don't have that >> property even after this patch. That's inconsistent. > > Will edit the binding to have the vbus-supply only for otg cases. >>> diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts >>> b/arch/arm/boot/dts/tegra20-seaboard.dts >> >>> + usb-phy@c500 { >>> + vbus-supply = <&vbus_reg>; >>> + dr_mode = "otg"; >>> + }; >> >> Where is the nvidia,vbus-gpio property? Since your code changes don't >> actually implement use of the vbus-supply property yet, they will expect >> the vbus-gpio property to exist in the PHY node. > > As of now Vbus-gpio is being used by EHCI driver only. > As anyways we wanted to use vbus-supply, will implement its > support in PHY driver and remove vbus-gpio from EHCI node. > Hence did not add vbus-gpio to PHY DT nodes. Ah right. I thought that this patch series moved the use of vbus-gpio from the EHCI to the PHY driver, just like it moved the handling of other properties such as dr_mode. I guess that's not actually true. Just so we're clear, I'd expect the following then: * Update binding to remove references to vbus-gpio from EHCI binding, add references to vbus-supply to PHY binding. * Update DT to add vbus-supply to PHY nodes, but don't remove vbus-gpio from EHCI nodes. * The code in this series doesn't actually change the vbus-* usage yet. ... * Once this series is finalized, create another follow-on patch that adds vbus-supply handling in to the PHY driver, and removes vbus-gpio handling from the EHCI driver. * Finally, remove vbus-gpio from the DT files. I think we're in agreement on this now, and this series mostly already does exactly that. I just wanted to spell it out explicitly to make sure. -- 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/5 v3] USB: regroup all depends on USB within an if USB block
On Thu, 4 Apr 2013, Felipe Balbi wrote: > Hi, > > On Thu, Apr 04, 2013 at 05:57:24PM +0200, Florian Fainelli wrote: > > diff --git a/drivers/usb/misc/sisusbvga/Kconfig > > b/drivers/usb/misc/sisusbvga/Kconfig > > index 30ea7ca..0d03a52 100644 > > --- a/drivers/usb/misc/sisusbvga/Kconfig > > +++ b/drivers/usb/misc/sisusbvga/Kconfig > > @@ -1,7 +1,7 @@ > > > > config USB_SISUSBVGA > > tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" > > - depends on USB && (USB_MUSB_HDRC || USB_EHCI_HCD) > > + depends on (USB_MUSB_HDRC || USB_EHCI_HCD) > > is it just me or would everybody agree that depending on MUSB or EHCI > here is wrong ? That line certainly looks like it could be removed entirely. Perhaps the original author can enlighten us. In any case, it's not relevant to the purpose of this patch set. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FTDI FT2232C issue
On Thu, 4 Apr 2013, Yegor Yefremov wrote: > I reworked my test script, so that I can tell timeout from wrong > string. I have two cases now. On my am335x based system I get timeout > with 3.2 kernel and errors like here > (http://www.newit.co.uk/forum/index.php?topic=313.0) for 3.8. > > On x86 I get fully wrong string. For example, I send "test string" and > get something like that 57 D6 2E 17 A4 2E 5D AE 5A EB F6 on the other > port. > > I've also update the sniffs. They contain 20 cycles each. Let's take > this sniff ftdi_sniff_hub.out > > As far as I understand it, it is normal case, where the data was > received correctly: > > f1172780 1591104533 S Bo:1:004:2 -115 11 = 74657374 20737472 696e67 > f1172780 1591104593 C Bo:1:004:2 0 11 > > f1172400 1591105218 C Bi:1:004:3 0 8 = b1607465 73742073 > f1172400 1591105223 S Bi:1:004:3 -115 512 < > f1172b00 1591105226 C Bi:1:004:1 0 2 = b100 > f1172b00 1591105227 S Bi:1:004:1 -115 512 < > f1172400 1591106217 C Bi:1:004:3 0 7 = b1607472 696e67 > > Here everything will be received, but byte per byte: > > f1172780 1591706391 S Bo:1:004:2 -115 11 = 74657374 20737472 696e67 > f1172780 1591706487 C Bo:1:004:2 0 11 > > f1172400 1591707111 C Bi:1:004:3 0 2 = b160 > f1172400 1591707115 S Bi:1:004:3 -115 512 < > f1172b00 1591707117 C Bi:1:004:1 0 2 = b100 > f1172b00 1591707118 S Bi:1:004:1 -115 512 < > f1172400 1591708112 C Bi:1:004:3 0 3 = b16074 > f1172400 1591708117 S Bi:1:004:3 -115 512 < > f1172b00 1591708120 C Bi:1:004:1 0 2 = b100 > f1172b00 1591708121 S Bi:1:004:1 -115 512 < > f1172400 1591709112 C Bi:1:004:3 0 3 = b16065 ... > And here the second port seems to receive junk: > > f1172780 1592210316 S Bo:1:004:2 -115 11 = 74657374 20737472 696e67 > f1172780 1592210379 C Bo:1:004:2 0 11 > > f1172400 1592210878 C Bi:1:004:3 0 6 = b16895cd d181 > f1172400 1592210883 S Bi:1:004:3 -115 512 < > f1172400 1592211128 C Bi:1:004:3 0 3 = b1609a > f1172400 1592211132 S Bi:1:004:3 -115 512 < > f1172b00 1592211134 C Bi:1:004:1 0 2 = b100 > f1172b00 1592211135 S Bi:1:004:1 -115 512 < > f1172400 1592212128 C Bi:1:004:3 0 8 = b160d1c9 a5b99dff > > I've also found a hub, that looks like 12M hub, but it produces the > same error pattern (wrong chars): > > /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M > |__ Port 1: Dev 6, If 0, Class=hub, Driver=hub/4p, 12M > |__ Port 1: Dev 7, If 0, Class=vend., Driver=ftdi_sio, 12M > |__ Port 1: Dev 7, If 1, Class=vend., Driver=ftdi_sio, 12M > /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/6p, 480M > > On USB3.0 host I get 9 chars instead of 11 and they are also wrong. > > Rx/Tx test: baudrate 115200 > Wrong string len: 11 > 95 CD D1 81 9A D1 C9 A5 B9 9D FF > Rx/Tx /dev/ttyUSB0 > /dev/ttyUSB1: FAILED > Rx/Tx test: baudrate 115200 > Rx/Tx /dev/ttyUSB1 > /dev/ttyUSB0: O.K. > Rx/Tx test: baudrate 9600 > Rx/Tx /dev/ttyUSB0 > /dev/ttyUSB1: O.K. > Rx/Tx test: baudrate 9600 > Rx/Tx /dev/ttyUSB1 > /dev/ttyUSB0: O.K. > > Should I make a sniff with them? Should I make a sniff with FT4232H? This is a tough problem, no doubt about it. If you can get hold of a USB bus analyzer to see the actual data as it goes through the cable, it might help to pin down where the problem is -- before the hub or after it. But I suspect that this really is a bug in the FTDI part. This doesn't answer the question of why it works better when attached directly to a USB-1.1 port on the computer. Could it be a matter of available power? Maybe the hubs don't supply enough current. (But then the USB-3 port should work...) Can you reproduce these results under a different operating system (such as Windows)? If you can, you might try asking for help directly from FTDI. 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 v3] xhci-mem: init list heads at the beginning of init
Hi Sergio, On 04/04/2013 10:32 AM, Sergio Aguirre wrote: It is possible that we fail on xhci_mem_init, just before doing the INIT_LIST_HEAD, and calling xhci_mem_cleanup. Problem is that, the list_for_each_entry_safe macro, assumes list heads are initialized (not NULL), and dereferences their 'next' pointer, causing a kernel panic if this is not yet initialized. Let's protect from that by moving inits to the beginning. Signed-off-by: Sergio Aguirre --- Acked-by: David Cohen drivers/usb/host/xhci-mem.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6dc238c..ffbdefc 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2255,6 +2255,9 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) u32 page_size, temp; int i; + INIT_LIST_HEAD(&xhci->lpm_failed_devs); + INIT_LIST_HEAD(&xhci->cancel_cmd_list); + page_size = xhci_readl(xhci, &xhci->op_regs->page_size); xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); for (i = 0; i < 16; i++) { @@ -2333,7 +2336,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); if (!xhci->cmd_ring) goto fail; - INIT_LIST_HEAD(&xhci->cancel_cmd_list); xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); xhci_dbg(xhci, "First segment DMA is 0x%llx\n", (unsigned long long)xhci->cmd_ring->first_seg->dma); @@ -2444,8 +2446,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) if (xhci_setup_port_arrays(xhci, flags)) goto fail; - INIT_LIST_HEAD(&xhci->lpm_failed_devs); - /* Enable USB 3.0 device notifications for function remote wake, which * is necessary for allowing USB 3.0 devices to do remote wakeup from * U3 (device suspend). -- 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 v3] xhci-mem: init list heads at the beginning of init
It is possible that we fail on xhci_mem_init, just before doing the INIT_LIST_HEAD, and calling xhci_mem_cleanup. Problem is that, the list_for_each_entry_safe macro, assumes list heads are initialized (not NULL), and dereferences their 'next' pointer, causing a kernel panic if this is not yet initialized. Let's protect from that by moving inits to the beginning. Signed-off-by: Sergio Aguirre --- drivers/usb/host/xhci-mem.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6dc238c..ffbdefc 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2255,6 +2255,9 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) u32 page_size, temp; int i; + INIT_LIST_HEAD(&xhci->lpm_failed_devs); + INIT_LIST_HEAD(&xhci->cancel_cmd_list); + page_size = xhci_readl(xhci, &xhci->op_regs->page_size); xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); for (i = 0; i < 16; i++) { @@ -2333,7 +2336,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); if (!xhci->cmd_ring) goto fail; - INIT_LIST_HEAD(&xhci->cancel_cmd_list); xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); xhci_dbg(xhci, "First segment DMA is 0x%llx\n", (unsigned long long)xhci->cmd_ring->first_seg->dma); @@ -2444,8 +2446,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) if (xhci_setup_port_arrays(xhci, flags)) goto fail; - INIT_LIST_HEAD(&xhci->lpm_failed_devs); - /* Enable USB 3.0 device notifications for function remote wake, which * is necessary for allowing USB 3.0 devices to do remote wakeup from * U3 (device suspend). -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] xhci: prevent from potential null pointer dereference on failed init
Hi David, Thanks for the review. > -Original Message- > From: Cohen, David A > Sent: Thursday, April 04, 2013 10:03 AM > To: Aguirre Rodriguez, Sergio A > Cc: linux-usb@vger.kernel.org; Sarah Sharp; Kanigeri, Hari K > Subject: Re: [PATCH v2] xhci: prevent from potential null pointer > dereference on failed init > > Hi Sergio, > > On 04/04/2013 09:57 AM, Sergio Aguirre wrote: > > It is possible that we fail on xhci_mem_init, just before doing the > > INIT_LIST_HEAD, and calling xhci_mem_cleanup. > > > > Problem is that, the list_for_each_entry_safe macro, dereferences next > > assuming is not NULL (which is the case for a uninitialized list). > > > > Let's protect from that. > > > > Signed-off-by: Sergio Aguirre > > --- > > drivers/usb/host/xhci-mem.c |9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > > index 6dc238c..d110aeb 100644 > > --- a/drivers/usb/host/xhci-mem.c > > +++ b/drivers/usb/host/xhci-mem.c > > @@ -1820,9 +1820,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) > > scratchpad_free(xhci); > > > > spin_lock_irqsave(&xhci->lock, flags); > > - list_for_each_entry_safe(dev_info, next, &xhci->lpm_failed_devs, > list) { > > - list_del(&dev_info->list); > > - kfree(dev_info); > > + if (xhci->lpm_failed_devs.next) { > > IMO this check could be done without holding spin lock. > At this point, either we initialized list_head or not. I see your point. However, I think I'll go with Bjørn's suggestion to init the lists at the beginning, to avoid doing these null pointer checks altogether. Will resend v3 in a couple of seconds. Regards, Sergio > > Br, David > > > + list_for_each_entry_safe(dev_info, next, > > +&xhci->lpm_failed_devs, list) { > > + list_del(&dev_info->list); > > + kfree(dev_info); > > + } > > } > > spin_unlock_irqrestore(&xhci->lock, flags); > > -- 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/5 v3] USB: regroup all depends on USB within an if USB block
Hi, On Thu, Apr 04, 2013 at 05:57:24PM +0200, Florian Fainelli wrote: > diff --git a/drivers/usb/misc/sisusbvga/Kconfig > b/drivers/usb/misc/sisusbvga/Kconfig > index 30ea7ca..0d03a52 100644 > --- a/drivers/usb/misc/sisusbvga/Kconfig > +++ b/drivers/usb/misc/sisusbvga/Kconfig > @@ -1,7 +1,7 @@ > > config USB_SISUSBVGA > tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" > - depends on USB && (USB_MUSB_HDRC || USB_EHCI_HCD) > + depends on (USB_MUSB_HDRC || USB_EHCI_HCD) is it just me or would everybody agree that depending on MUSB or EHCI here is wrong ? > diff --git a/drivers/usb/mon/Kconfig b/drivers/usb/mon/Kconfig > index 635745f..5c6ffa2 100644 > --- a/drivers/usb/mon/Kconfig > +++ b/drivers/usb/mon/Kconfig > @@ -4,7 +4,6 @@ > > config USB_MON > tristate "USB Monitor" > - depends on USB > help > If you select this option, a component which captures the USB traffic > between peripheral-specific drivers and HC drivers will be built. > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig > index 05e5143..4636ab9 100644 > --- a/drivers/usb/musb/Kconfig > +++ b/drivers/usb/musb/Kconfig > @@ -6,7 +6,7 @@ > # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller > config USB_MUSB_HDRC > tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)' > - depends on USB && USB_GADGET > + depends on USB_GADGET great, except for the question above: Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
RE: [PATCH] xhci: prevent from potential null pointer dereference on failed init
> -Original Message- > From: Bjørn Mork [mailto:bj...@mork.no] > Sent: Thursday, April 04, 2013 10:01 AM > To: Sarah Sharp > Cc: Aguirre Rodriguez, Sergio A; linux-usb@vger.kernel.org > Subject: Re: [PATCH] xhci: prevent from potential null pointer dereference > on failed init > > Sarah Sharp writes: > > > Thanks for catching this! However, the inline comment is a bit much > > for a simple NULL pointer check. Can you remove the comment and > > resubmit this patch? > > And maybe handle failure to initialize cancel_cmd_list as well? It has the > same problem. > > But wouldn't it be better to move the list initializations to the beginning of > xhci_mem_init instead of relying on additional tests for a NULL next pointer? Yeah, that actually makes more sense to me. I'll do that in the next revision. Regards, Sergio > > > Bjørn N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: [PATCH 0/5 v3] USB: Kconfig cleanups
On Thu, 4 Apr 2013, Florian Fainelli wrote: > > These 5 patches contain my Kconfig cleanup on which I based the removal > of the USB_ARCH_HAS_* patches. They have been suggested by Alan Stern > as part of an earlier conversations. > > Let me know what you think about it so I can post subsequent work based > on it. > > Thanks! Fainelli (5): > USB: regroup all depends on USB within an if USB block > USB: remove USB_EHCI_BIG_ENDIAN_{DESC,MMIO} depends on architecture > symbol > USB: enclose EHCI HCD drivers within an if USB_EHCI_HCD block > USB: enclose all depends on USB_OHCI_HCD within an if USB_OHCI_HCD > block > USB: enclose USB_XHCI_HCD related symbols within a if USB_XHCI_HCD > block They all look good now. You can add my Acked-by to the remaining ones. 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] xhci: prevent from potential null pointer dereference on failed init
Sarah Sharp writes: > Thanks for catching this! However, the inline comment is a bit much for > a simple NULL pointer check. Can you remove the comment and resubmit > this patch? And maybe handle failure to initialize cancel_cmd_list as well? It has the same problem. But wouldn't it be better to move the list initializations to the beginning of xhci_mem_init instead of relying on additional tests for a NULL next pointer? Bjørn -- 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 video capture issue due to uvc_complete callback spends more time
On Thursday 04 April 2013 19:49:26 Felipe Balbi wrote: > On Thu, Apr 04, 2013 at 04:52:15PM +0200, Laurent Pinchart wrote: > > > Laurent/Ming Lei > > > > > > > Since the uvc_video_complete() callback handler called from > > > > interrupt > > > > context, video post processing or memcpy can be deferred to > > > > tasklet or > > > > bottom half, rather than doing it in interrupt context. > > > > > > If that's the only way to fix the issue, yes. However, given your > > > really poor memcpy() performances, I don't think you will be able > > > to sustain the incoming video bandwidth. The driver would soon run > > > out of URBs. > > > >>> > > > >>> I agree with you, let me check whether memcpy is the bottle here, > > > >> > > > >> typo mistake, read as "bottle-neck" > > > >> > > > >> I will try to get profile info on this. But in general it would be > > > >> good to move post processing to bottom half which helps to reduce > > > >> interrupt latency. > > > > In general it is, but the amount of data to be copied is pretty small, so > > I'm not sure if it's worth it for the uvcvideo driver. If reading from > > coherent memory is that slow you won't be able to sustain the required > > bandwidth, regardless of whether the memcpy is performed in interrupt > > context or not. The driver will be unusable in both case. > > why do we even have that memcpy() there ? Why don't we use the same > buffer for uvc_buffer->buf and urb->buffer ? That would cut all issues > to a halt. Anything which prevents a zero-copy implementation ? Per-packet headers that need to be interpreted and then removed. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH v2] xhci: prevent from potential null pointer dereference on failed init
Hi Sergio, On 04/04/2013 09:57 AM, Sergio Aguirre wrote: It is possible that we fail on xhci_mem_init, just before doing the INIT_LIST_HEAD, and calling xhci_mem_cleanup. Problem is that, the list_for_each_entry_safe macro, dereferences next assuming is not NULL (which is the case for a uninitialized list). Let's protect from that. Signed-off-by: Sergio Aguirre --- drivers/usb/host/xhci-mem.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6dc238c..d110aeb 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1820,9 +1820,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) scratchpad_free(xhci); spin_lock_irqsave(&xhci->lock, flags); - list_for_each_entry_safe(dev_info, next, &xhci->lpm_failed_devs, list) { - list_del(&dev_info->list); - kfree(dev_info); + if (xhci->lpm_failed_devs.next) { IMO this check could be done without holding spin lock. At this point, either we initialized list_head or not. Br, David + list_for_each_entry_safe(dev_info, next, +&xhci->lpm_failed_devs, list) { + list_del(&dev_info->list); + kfree(dev_info); + } } spin_unlock_irqrestore(&xhci->lock, flags); -- 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] xhci: prevent from potential null pointer dereference on failed init
It is possible that we fail on xhci_mem_init, just before doing the INIT_LIST_HEAD, and calling xhci_mem_cleanup. Problem is that, the list_for_each_entry_safe macro, dereferences next assuming is not NULL (which is the case for a uninitialized list). Let's protect from that. Signed-off-by: Sergio Aguirre --- drivers/usb/host/xhci-mem.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6dc238c..d110aeb 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1820,9 +1820,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) scratchpad_free(xhci); spin_lock_irqsave(&xhci->lock, flags); - list_for_each_entry_safe(dev_info, next, &xhci->lpm_failed_devs, list) { - list_del(&dev_info->list); - kfree(dev_info); + if (xhci->lpm_failed_devs.next) { + list_for_each_entry_safe(dev_info, next, +&xhci->lpm_failed_devs, list) { + list_del(&dev_info->list); + kfree(dev_info); + } } spin_unlock_irqrestore(&xhci->lock, flags); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] xhci: prevent from potential null pointer dereference on failed init
> -Original Message- > From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] > Sent: Thursday, April 04, 2013 9:33 AM > To: Aguirre Rodriguez, Sergio A > Cc: linux-usb@vger.kernel.org > Subject: Re: [PATCH] xhci: prevent from potential null pointer dereference > on failed init > > Hi Sergio, Hi Sarah, > > Thanks for catching this! However, the inline comment is a bit much for a > simple NULL pointer check. Can you remove the comment and resubmit this > patch? Sure, no problem. I'll do that and resubmit. Regards, Sergio > > Thanks, > Sarah Sharp > > On Wed, Apr 03, 2013 at 03:52:07PM -0700, Sergio Aguirre wrote: > > It is possible that we fail on xhci_mem_init, just before doing the > > INIT_LIST_HEAD, and calling xhci_mem_cleanup. > > > > Problem is that, the list_for_each_entry_safe macro, dereferences next > > assuming is not NULL (which is the case for a uninitialized list). > > > > Let's protect from that. > > > > Signed-off-by: Sergio Aguirre > > --- > > drivers/usb/host/xhci-mem.c | 14 +++--- > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > > index 6dc238c..0f701f7 100644 > > --- a/drivers/usb/host/xhci-mem.c > > +++ b/drivers/usb/host/xhci-mem.c > > @@ -1820,9 +1820,17 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) > > scratchpad_free(xhci); > > > > spin_lock_irqsave(&xhci->lock, flags); > > - list_for_each_entry_safe(dev_info, next, &xhci->lpm_failed_devs, > list) { > > - list_del(&dev_info->list); > > - kfree(dev_info); > > + /* > > +* It is possible that we fail during xhci_mem_init, just before > > +* initializing the list head, and causing a NULL pointer dereference > > +* on below macro. So, let's be safe, and do a simple null check here. > > +*/ > > + if (xhci->lpm_failed_devs.next) { > > + list_for_each_entry_safe(dev_info, next, > > +&xhci->lpm_failed_devs, list) { > > + list_del(&dev_info->list); > > + kfree(dev_info); > > + } > > } > > spin_unlock_irqrestore(&xhci->lock, flags); > > > > -- > > 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 video capture issue due to uvc_complete callback spends more time
Hi, On Thu, Apr 04, 2013 at 04:52:15PM +0200, Laurent Pinchart wrote: > > Laurent/Ming Lei > > > > > Since the uvc_video_complete() callback handler called from interrupt > > > context, video post processing or memcpy can be deferred to tasklet or > > > bottom half, rather than doing it in interrupt context. > > > > If that's the only way to fix the issue, yes. However, given your > > really poor memcpy() performances, I don't think you will be able to > > sustain the incoming video bandwidth. The driver would soon run out of > > URBs. > > >>> > > >>> I agree with you, let me check whether memcpy is the bottle here, > > >> > > >> typo mistake, read as "bottle-neck" > > >> > > >> I will try to get profile info on this. But in general it would be good > > >> to move post processing to bottom half which helps to reduce interrupt > > >> latency. > > In general it is, but the amount of data to be copied is pretty small, so I'm > not sure if it's worth it for the uvcvideo driver. If reading from coherent > memory is that slow you won't be able to sustain the required bandwidth, > regardless of whether the memcpy is performed in interrupt context or not. > The > driver will be unusable in both case. why do we even have that memcpy() there ? Why don't we use the same buffer for uvc_buffer->buf and urb->buffer ? That would cut all issues to a halt. Anything which prevents a zero-copy implementation ? -- balbi signature.asc Description: Digital signature
[PATCH] xhci: fix list access before init
Commits 9574323 xHCI: test USB2 software LPM b92cc66 xHCI: add aborting command ring function introduce useful functions which involves lists manipulations. If for whatever reason we fall into fail path in xhci_mem_init() we may access the lists in xhci_mem_cleanup() before they get initialized. The same init-fail-cleanup case is applicable to bw tables too. Fix this by moving list initialization code to the beginning of xhci_mem_init() Reported-by: Sergey Dyasly Tested-by: Sergey Dyasly Signed-off-by: Vladimir Murzin --- drivers/usb/host/xhci-mem.c | 37 - 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6dc238c..7b5d2f5 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2123,7 +2123,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags) __le32 __iomem *addr; u32 offset; unsigned int num_ports; - int i, j, port_index; + int i, port_index; addr = &xhci->cap_regs->hcc_params; offset = XHCI_HCC_EXT_CAPS(xhci_readl(xhci, addr)); @@ -2138,18 +2138,6 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags) if (!xhci->port_array) return -ENOMEM; - xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); - if (!xhci->rh_bw) - return -ENOMEM; - for (i = 0; i < num_ports; i++) { - struct xhci_interval_bw_table *bw_table; - - INIT_LIST_HEAD(&xhci->rh_bw[i].tts); - bw_table = &xhci->rh_bw[i].bw_table; - for (j = 0; j < XHCI_MAX_INTERVAL; j++) - INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints); - } - /* * For whatever reason, the first capability offset is from the * capability register base, not from the HCCPARAMS register. @@ -2253,7 +2241,25 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) u64 val_64; struct xhci_segment *seg; u32 page_size, temp; - int i; + int i, j, num_ports; + + INIT_LIST_HEAD(&xhci->cancel_cmd_list); + INIT_LIST_HEAD(&xhci->lpm_failed_devs); + + num_ports = HCS_MAX_PORTS(xhci_readl(xhci, &xhci->cap_regs->hcs_params1)); + + xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags); + if (!xhci->rh_bw) + return -ENOMEM; + + for (i = 0; i < num_ports; i++) { + struct xhci_interval_bw_table *bw_table; + + INIT_LIST_HEAD(&xhci->rh_bw[i].tts); + bw_table = &xhci->rh_bw[i].bw_table; + for (j = 0; j < XHCI_MAX_INTERVAL; j++) + INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints); + } page_size = xhci_readl(xhci, &xhci->op_regs->page_size); xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size); @@ -2333,7 +2339,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags); if (!xhci->cmd_ring) goto fail; - INIT_LIST_HEAD(&xhci->cancel_cmd_list); xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring); xhci_dbg(xhci, "First segment DMA is 0x%llx\n", (unsigned long long)xhci->cmd_ring->first_seg->dma); @@ -2444,8 +2449,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) if (xhci_setup_port_arrays(xhci, flags)) goto fail; - INIT_LIST_HEAD(&xhci->lpm_failed_devs); - /* Enable USB 3.0 device notifications for function remote wake, which * is necessary for allowing USB 3.0 devices to do remote wakeup from * U3 (device suspend). -- 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: [RFC][PATCH 1/2] ARM: OMAP4: clock: Add device tree support for AUXCLKs
* Roger Quadros [130404 00:39]: > On 04/04/2013 02:42 AM, Tony Lindgren wrote: > >> --- a/arch/arm/mach-omap2/cclock44xx_data.c > >> +++ b/arch/arm/mach-omap2/cclock44xx_data.c > >> @@ -27,6 +27,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> > >> #include "soc.h" > >> #include "iomap.h" > >> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = { > >>CLK(NULL, "cpufreq_ck", &dpll_mpu_ck, CK_443X), > >> }; > >> > >> +static struct clk *scrm_clks[] = { > >> + &auxclk0_ck, > >> + &auxclk1_ck, > >> + &auxclk2_ck, > >> + &auxclk3_ck, > >> + &auxclk4_ck, > >> + &auxclk5_ck, > >> +}; > > > > Hmm I don't like the idea of specifying the auxclk both in the > > cclock44xx_data.c and in DT.. > > Right, but till we have all clocks moved to DT we only need this > approach for general purpose clocks that are not mapped to devices > by hwmod. For v3.10, let's just make sure that USB works with DT as then after v3.10 we can make omap4 DT only and get rid of estimated 7K lines of code and data. I guess this is the last piece missing for that, or are we also missing something else? Can't you set up a clock alias for your device so it can find the auxclk when requesting it with the dev entry? For the DT clock driver if needed for v3.10, how about just do a minimal drivers/clock/omap/ that uses the standard binding? Then that driver can just do clk_get() from cclock44xx_data.c for now? And then later on we'll just move all the clocks to a combination of DT + /lib/firmware. > e.g. auxclk are required to be specified in DT nodes for USB PHY. > Without this we can't get USB host working on Panda. OK. So if the USB PHY has a dev entry, can't you just set up a clock alias in struct omap_clk omap44xx_clks[] for it? > As Rajendra points out, it seems moving entire clock data to DT is not > going to happen soon. So this is the simplistic way things can work. Right but seems like we can get started there without moving them all at once. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/11] usb: dwc3/xhci/phy: Enable runtime power management
On Thu, Apr 04, 2013 at 01:02:45PM +0530, Vivek Gautam wrote: > On Thu, Apr 4, 2013 at 12:40 PM, Felipe Balbi wrote: > > On Thu, Apr 04, 2013 at 10:34:57AM +0530, Vivek Gautam wrote: > >> Hi Sarah, > >> > >> > >> On Wed, Apr 3, 2013 at 10:57 PM, Sarah Sharp > >> wrote: > >> > Question: Do you still need this patch for 3.10? > >> > >> Felipe's 'next' is closed for 3.10, so this series won't be making it > >> to 3.10 now, as a whole. :-( > > > > right, besides we're still discussing what to do with the whole PHY > > part, right ? > > Right ofcourse. :-) Ok, so it sounds like I shouldn't merge that patch for 3.10. Please include that patch in your next round of revisions instead. And now I'm glad I'm slow. :) Sarah Sharp -- 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] ARM: OMAP4: clock: Add device tree support for AUXCLKs
* Rajendra Nayak [130403 22:25]: > On Thursday 04 April 2013 05:12 AM, Tony Lindgren wrote: > > > > How about just add a minimal drivers/clk/omap/clk-xyz.c that takes > > the configuration from DT and is based on the binding we already have in > > Documentation/devicetree/bindings/clock/clock-bindings.txt? > > > > Then as we add new bindings there we can drop them from current > > cclock44xx_data.c, no? That is after omap4 is DT only.. > > The patch just provides an alternative for clkdev mapping in case of DT. > Are you suggesting we move all *clock data* related to auxclks (and eventually > all clocks) into DT? Well I think we should have a driver that can take any defined clock binding from DT at least for boottime clocks. We need at least the timer clocks early during the boot, and probably the root device like MMC or possibly Ethernet depending on the board. The rest of the huge amount of clocks we can just load from /lib/firmware after mounting intramfs or root on MMC. As long as we can define any boottime clock in DT also, loading the rest of the clock data from /lib/firmware should not cause issues with booting. And as we get the clocks moved, we can drop them from cclock44xx_data.c. AFAIK the new driver can just clk_get the parent clocks so those can stay in cclock44xx_data.c for now? So basically we can do the same as we are already doing with pinctrl-single.c, but with addition of loading large amounts of data from /lib/firmware. And eventually we can do the same with hwmod too. Regarding the readability issue, we now have preprocessing support for .dts files merged AFAIK, so they can be as readable as data structures :) > We have discussed this multiple times in the past, and moving 250 clock nodes > with each needing multiple register offsets, masks, shifts etc into DT makes > it > completely un-readable. For me, having a way for devices to reference a clock > that they > use for a device using DT makes sense, but not moving all clock data into dts > files. Yes that's why we should also support loading clocks from /lib/firmware. Naturally the parent clocks can be allocated by the clock driver(s) at least initially. But the main reason I think we should do this is so we get out of the flame path with these huge data files for every new SoC. Regards, Tony -- 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] xhci: prevent from potential null pointer dereference on failed init
Hi Sergio, Thanks for catching this! However, the inline comment is a bit much for a simple NULL pointer check. Can you remove the comment and resubmit this patch? Thanks, Sarah Sharp On Wed, Apr 03, 2013 at 03:52:07PM -0700, Sergio Aguirre wrote: > It is possible that we fail on xhci_mem_init, just before doing > the INIT_LIST_HEAD, and calling xhci_mem_cleanup. > > Problem is that, the list_for_each_entry_safe macro, dereferences next > assuming is not NULL (which is the case for a uninitialized list). > > Let's protect from that. > > Signed-off-by: Sergio Aguirre > --- > drivers/usb/host/xhci-mem.c | 14 +++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 6dc238c..0f701f7 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -1820,9 +1820,17 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) > scratchpad_free(xhci); > > spin_lock_irqsave(&xhci->lock, flags); > - list_for_each_entry_safe(dev_info, next, &xhci->lpm_failed_devs, list) { > - list_del(&dev_info->list); > - kfree(dev_info); > + /* > + * It is possible that we fail during xhci_mem_init, just before > + * initializing the list head, and causing a NULL pointer dereference > + * on below macro. So, let's be safe, and do a simple null check here. > + */ > + if (xhci->lpm_failed_devs.next) { > + list_for_each_entry_safe(dev_info, next, > + &xhci->lpm_failed_devs, list) { > + list_del(&dev_info->list); > + kfree(dev_info); > + } > } > spin_unlock_irqrestore(&xhci->lock, flags); > > -- > 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 4/5 v3] USB: enclose all depends on USB_OHCI_HCD within an if USB_OHCI_HCD block
This patch removes the various depends on USB_OHCI_HCD from the OHCI HCD drivers and enclose them within an if USB_OHCI_HCD / endif block. The Octeon OHCI HCD driver has been moved around to remain in this block. Acked-by: Alan Stern Signed-off-by: Florian Fainelli --- Changes in v3: - added Alan's Acked-by tag drivers/usb/host/Kconfig | 51 +++--- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 415ebdd..8ff86d3 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -326,16 +326,18 @@ config USB_OHCI_HCD To compile this driver as a module, choose M here: the module will be called ohci-hcd. +if USB_OHCI_HCD + config USB_OHCI_HCD_OMAP1 bool "OHCI support for OMAP1/2 chips" - depends on USB_OHCI_HCD && ARCH_OMAP1 + depends on ARCH_OMAP1 default y ---help--- Enables support for the OHCI controller on OMAP1/2 chips. config USB_OHCI_HCD_OMAP3 bool "OHCI support for OMAP3 and later chips" - depends on USB_OHCI_HCD && (ARCH_OMAP3 || ARCH_OMAP4) + depends on (ARCH_OMAP3 || ARCH_OMAP4) default y ---help--- Enables support for the on-chip OHCI controller on @@ -343,7 +345,7 @@ config USB_OHCI_HCD_OMAP3 config USB_OHCI_ATH79 bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)" - depends on USB_OHCI_HCD && (SOC_AR71XX || SOC_AR724X) + depends on (SOC_AR71XX || SOC_AR724X) select USB_OHCI_HCD_PLATFORM default y help @@ -355,7 +357,7 @@ config USB_OHCI_ATH79 config USB_OHCI_HCD_PPC_OF_BE bool "OHCI support for OF platform bus (big endian)" - depends on USB_OHCI_HCD && PPC_OF + depends on PPC_OF select USB_OHCI_BIG_ENDIAN_DESC select USB_OHCI_BIG_ENDIAN_MMIO ---help--- @@ -364,7 +366,7 @@ config USB_OHCI_HCD_PPC_OF_BE config USB_OHCI_HCD_PPC_OF_LE bool "OHCI support for OF platform bus (little endian)" - depends on USB_OHCI_HCD && PPC_OF + depends on PPC_OF select USB_OHCI_LITTLE_ENDIAN ---help--- Enables support for little-endian USB controllers present on the @@ -372,12 +374,12 @@ config USB_OHCI_HCD_PPC_OF_LE config USB_OHCI_HCD_PPC_OF bool - depends on USB_OHCI_HCD && PPC_OF + depends on PPC_OF default USB_OHCI_HCD_PPC_OF_BE || USB_OHCI_HCD_PPC_OF_LE config USB_OHCI_HCD_PCI bool "OHCI support for PCI-bus USB controllers" - depends on USB_OHCI_HCD && PCI && (STB03xxx || PPC_MPC52xx || USB_OHCI_HCD_PPC_OF) + depends on PCI && (STB03xxx || PPC_MPC52xx || USB_OHCI_HCD_PPC_OF) default y select USB_OHCI_LITTLE_ENDIAN ---help--- @@ -386,7 +388,7 @@ config USB_OHCI_HCD_PCI config USB_OHCI_HCD_SSB bool "OHCI support for Broadcom SSB OHCI core (DEPRECATED)" - depends on USB_OHCI_HCD && (SSB = y || SSB = USB_OHCI_HCD) + depends on (SSB = y || SSB = USB_OHCI_HCD) select USB_HCD_SSB select USB_OHCI_HCD_PLATFORM default n @@ -404,7 +406,7 @@ config USB_OHCI_HCD_SSB config USB_OHCI_SH bool "OHCI support for SuperH USB controller (DEPRECATED)" - depends on USB_OHCI_HCD && SUPERH + depends on SUPERH select USB_OHCI_HCD_PLATFORM ---help--- This option is deprecated now and the driver was removed, use @@ -415,13 +417,13 @@ config USB_OHCI_SH config USB_OHCI_EXYNOS boolean "OHCI support for Samsung EXYNOS SoC Series" - depends on USB_OHCI_HCD && ARCH_EXYNOS + depends on ARCH_EXYNOS help Enable support for the Samsung Exynos SOC's on-chip OHCI controller. config USB_CNS3XXX_OHCI bool "Cavium CNS3XXX OHCI Module (DEPRECATED)" - depends on USB_OHCI_HCD && ARCH_CNS3XXX + depends on ARCH_CNS3XXX select USB_OHCI_HCD_PLATFORM ---help--- This option is deprecated now and the driver was removed, use @@ -432,7 +434,6 @@ config USB_CNS3XXX_OHCI config USB_OHCI_HCD_PLATFORM bool "Generic OHCI driver for a platform device" - depends on USB_OHCI_HCD default n ---help--- Adds an OHCI host driver for a generic platform device, which @@ -440,23 +441,33 @@ config USB_OHCI_HCD_PLATFORM If unsure, say N. +config USB_OCTEON_OHCI + bool "Octeon on-chip OHCI support" + depends on CPU_CAVIUM_OCTEON + default USB_OCTEON_EHCI + select USB_OHCI_BIG_ENDIAN_MMIO + select USB_OHCI_LITTLE_ENDIAN + help + Enable support for the Octeon II SOC's on-chip OHCI + controller. It is needed for low-speed USB 1.0 device + support. All CN6XXX based chips with USB are supported. + config USB_OHCI_BIG_ENDIAN_DESC bool - depends on USB_OHCI_HCD default n config USB_OHC
[PATCH 3/5 v3] USB: enclose EHCI HCD drivers within an if USB_EHCI_HCD block
Thist patch removes the depends on USB_EHCI_HCD that the various USB EHCI HCD drivers use and encloses every driver within an if USB_EHCI_HCD / endif block. The EHCI HCD platform and Octeon drivers have been moved around to remain enclosed within this block. Signed-off-by: Florian Fainelli --- Changes in v3: - move out USB_FSL_MPH_DR_OF from the USB_EHCI_HCD block - remove depends on USB_EHCI_HCD for USB_EHCI_BIG_ENDIAN_DESC drivers/usb/host/Kconfig | 81 -- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index a330549..415ebdd 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -93,14 +93,19 @@ config USB_EHCI_TT_NEWSCHED If unsure, say Y. +config USB_FSL_MPH_DR_OF + tristate + +if USB_EHCI_HCD + config USB_EHCI_PCI tristate - depends on USB_EHCI_HCD && PCI + depends on PCI default y config USB_EHCI_HCD_PMC_MSP tristate "EHCI support for on-chip PMC MSP71xx USB controller" - depends on USB_EHCI_HCD && MSP_HAS_USB + depends on MSP_HAS_USB default n select USB_EHCI_BIG_ENDIAN_DESC select USB_EHCI_BIG_ENDIAN_MMIO @@ -110,15 +115,13 @@ config USB_EHCI_HCD_PMC_MSP config USB_EHCI_BIG_ENDIAN_MMIO bool - depends on USB_EHCI_HCD config USB_EHCI_BIG_ENDIAN_DESC bool - depends on USB_EHCI_HCD config XPS_USB_HCD_XILINX bool "Use Xilinx usb host EHCI controller core" - depends on USB_EHCI_HCD && (PPC32 || MICROBLAZE) + depends on (PPC32 || MICROBLAZE) select USB_EHCI_BIG_ENDIAN_DESC select USB_EHCI_BIG_ENDIAN_MMIO ---help--- @@ -127,12 +130,9 @@ config XPS_USB_HCD_XILINX support both high speed and full speed devices, or high speed devices only. -config USB_FSL_MPH_DR_OF - tristate - config USB_EHCI_FSL bool "Support for Freescale PPC on-chip EHCI USB controller" - depends on USB_EHCI_HCD && FSL_SOC + depends on FSL_SOC select USB_EHCI_ROOT_HUB_TT select USB_FSL_MPH_DR_OF if OF ---help--- @@ -140,14 +140,14 @@ config USB_EHCI_FSL config USB_EHCI_MXC tristate "Support for Freescale i.MX on-chip EHCI USB controller" - depends on USB_EHCI_HCD && ARCH_MXC + depends on ARCH_MXC select USB_EHCI_ROOT_HUB_TT ---help--- Variation of ARC USB block used in some Freescale chips. config USB_EHCI_HCD_OMAP tristate "EHCI support for OMAP3 and later chips" - depends on USB_EHCI_HCD && ARCH_OMAP + depends on ARCH_OMAP select NOP_USB_XCEIV default y ---help--- @@ -156,7 +156,7 @@ config USB_EHCI_HCD_OMAP config USB_EHCI_MSM bool "Support for MSM on-chip EHCI USB controller" - depends on USB_EHCI_HCD && ARCH_MSM + depends on ARCH_MSM select USB_EHCI_ROOT_HUB_TT select USB_MSM_OTG ---help--- @@ -169,7 +169,7 @@ config USB_EHCI_MSM config USB_EHCI_TEGRA boolean "NVIDIA Tegra HCD support" - depends on USB_EHCI_HCD && ARCH_TEGRA + depends on ARCH_TEGRA select USB_EHCI_ROOT_HUB_TT help This driver enables support for the internal USB Host Controllers @@ -177,7 +177,7 @@ config USB_EHCI_TEGRA config USB_EHCI_HCD_PPC_OF bool "EHCI support for PPC USB controller on OF platform bus" - depends on USB_EHCI_HCD && PPC_OF + depends on PPC_OF default y ---help--- Enables support for the USB controller present on the PowerPC @@ -185,20 +185,20 @@ config USB_EHCI_HCD_PPC_OF config USB_EHCI_SH bool "EHCI support for SuperH USB controller" - depends on USB_EHCI_HCD && SUPERH + depends on SUPERH ---help--- Enables support for the on-chip EHCI controller on the SuperH. If you use the PCI EHCI controller, this option is not necessary. config USB_EHCI_S5P boolean "S5P EHCI support" - depends on USB_EHCI_HCD && PLAT_S5P + depends on PLAT_S5P help Enable support for the S5P SOC's on-chip EHCI controller. config USB_EHCI_MV bool "EHCI support for Marvell on-chip controller" - depends on USB_EHCI_HCD && (ARCH_PXA || ARCH_MMP) + depends on (ARCH_PXA || ARCH_MMP) select USB_EHCI_ROOT_HUB_TT ---help--- Enables support for Marvell (including PXA and MMP series) on-chip @@ -207,13 +207,13 @@ config USB_EHCI_MV config USB_W90X900_EHCI bool "W90X900(W90P910) EHCI support" - depends on USB_EHCI_HCD && ARCH_W90X900 + depends on ARCH_W90X900 ---help--- Enables support for the W90X900 USB controller config USB_CNS3XXX_EHCI bool "Cavium CNS3XXX EHCI Module (DEPRECATED)" - depends on USB_EHCI_HCD && ARCH_CNS3XXX + depends on ARCH_CNS3XXX
[PATCH 2/5 v3] USB: remove USB_EHCI_BIG_ENDIAN_{DESC,MMIO} depends on architecture symbol
Just like the OHCI counter part we just can remove the architecture specific symbols which prevent these configuration symbols from being selected by platforms/architectures requiring it. The original implementation did not scale at all since it required each and every single architecture to be added for these configuration symbols to be selected. Now it is up to the EHCI driver and/or platform to select these configuration symbols accordingly. Acked-by: Alan Stern Signed-off-by: Florian Fainelli --- Changes in v2: - added Alan's Acked-by tag Changes since RFC: - dropped default n for USB_EHCI_BIG_ENDIAN_{MMIO_DESC} as it is the default - properly patch MPC521x and not MPC52xx arch/arm/Kconfig|2 ++ arch/mips/Kconfig |3 +++ arch/powerpc/platforms/44x/Kconfig |2 ++ arch/powerpc/platforms/512x/Kconfig |2 ++ arch/sparc/Kconfig |2 ++ drivers/usb/host/Kconfig| 11 ++- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 13b7394..6100e6e 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -549,6 +549,8 @@ config ARCH_IXP4XX select GENERIC_CLOCKEVENTS select MIGHT_HAVE_PCI select NEED_MACH_IO_H + select USB_EHCI_BIG_ENDIAN_MMIO + select USB_EHCI_BIG_ENDIAN_DESC help Support for Intel's IXP4XX (XScale) family of processors. diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index cd2e21f..a46a0b0 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -404,6 +404,8 @@ config PMC_MSP select IRQ_CPU select SERIAL_8250 select SERIAL_8250_CONSOLE + select USB_EHCI_BIG_ENDIAN_MMIO + select USB_EHCI_BIG_ENDIAN_DESC help This adds support for the PMC-Sierra family of Multi-Service Processor System-On-A-Chips. These parts include a number @@ -1433,6 +1435,7 @@ config CPU_CAVIUM_OCTEON select CPU_SUPPORTS_HUGEPAGES select LIBFDT select USE_OF + select USB_EHCI_BIG_ENDIAN_MMIO help The Cavium Octeon processor is a highly integrated chip containing many ethernet hardware widgets for networking tasks. The processor diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig index 0effe9f..7be9336 100644 --- a/arch/powerpc/platforms/44x/Kconfig +++ b/arch/powerpc/platforms/44x/Kconfig @@ -274,6 +274,8 @@ config 440EPX select IBM_EMAC_EMAC4 select IBM_EMAC_RGMII select IBM_EMAC_ZMII + select USB_EHCI_BIG_ENDIAN_MMIO + select USB_EHCI_BIG_ENDIAN_DESC config 440GRX bool diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig index c169998..381a592 100644 --- a/arch/powerpc/platforms/512x/Kconfig +++ b/arch/powerpc/platforms/512x/Kconfig @@ -7,6 +7,8 @@ config PPC_MPC512x select PPC_PCI_CHOICE select FSL_PCI if PCI select ARCH_WANT_OPTIONAL_GPIOLIB + select USB_EHCI_BIG_ENDIAN_MMIO + select USB_EHCI_BIG_ENDIAN_DESC config MPC5121_ADS bool "Freescale MPC5121E ADS" diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index 3d361f2..66dc562 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -407,6 +407,8 @@ config SERIAL_CONSOLE config SPARC_LEON bool "Sparc Leon processor family" depends on SPARC32 + select USB_EHCI_BIG_ENDIAN_MMIO + select USB_EHCI_BIG_ENDIAN_DESC ---help--- If you say Y here if you are running on a SPARC-LEON processor. The LEON processor is a synthesizable VHDL model of the diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 7f4ccf7..a330549 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -110,18 +110,11 @@ config USB_EHCI_HCD_PMC_MSP config USB_EHCI_BIG_ENDIAN_MMIO bool - depends on USB_EHCI_HCD && (PPC_CELLEB || PPC_PS3 || 440EPX || \ - ARCH_IXP4XX || XPS_USB_HCD_XILINX || \ - PPC_MPC512x || CPU_CAVIUM_OCTEON || \ - PMC_MSP || SPARC_LEON || MIPS_SEAD3) - default y + depends on USB_EHCI_HCD config USB_EHCI_BIG_ENDIAN_DESC bool - depends on USB_EHCI_HCD && (440EPX || ARCH_IXP4XX || XPS_USB_HCD_XILINX || \ - PPC_MPC512x || PMC_MSP || SPARC_LEON || \ - MIPS_SEAD3) - default y + depends on USB_EHCI_HCD config XPS_USB_HCD_XILINX bool "Use Xilinx usb host EHCI controller core" -- 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 1/5 v3] USB: regroup all depends on USB within an if USB block
This patch removes the depends on USB from all config symbols in drivers/usb/host/Kconfig and replace that with an if USB / endif block as suggested by Alan Stern. Some source ... Kconfig lines have been shuffled around to permit a better regroupment of the Kconfig files depending on "config USB" item. No functionnal change is introduced. Signed-off-by: Florian Fainelli --- Changes since v2: - properly keep USB_MUSB_HDRC dependency Changes since v1: - add missing if USB in drivers/usb/Kconfig before USB_USS720 Changes since RFC: - only remove the "depends on USB" conditionnals and not more as the RFC patch did drivers/usb/Kconfig| 23 +++ drivers/usb/atm/Kconfig|2 +- drivers/usb/class/Kconfig |6 +- drivers/usb/core/Kconfig |6 -- drivers/usb/host/Kconfig | 30 +++--- drivers/usb/image/Kconfig |4 +--- drivers/usb/misc/Kconfig | 21 - drivers/usb/misc/sisusbvga/Kconfig |2 +- drivers/usb/mon/Kconfig|1 - drivers/usb/musb/Kconfig |2 +- drivers/usb/renesas_usbhs/Kconfig |2 +- drivers/usb/serial/Kconfig |2 +- drivers/usb/storage/Kconfig|7 +++ drivers/usb/wusbcore/Kconfig |2 -- 14 files changed, 36 insertions(+), 74 deletions(-) diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 640ae6c..24c14fc 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -122,9 +122,9 @@ config USB To compile this driver as a module, choose M here: the module will be called usbcore. -source "drivers/usb/core/Kconfig" +if USB -source "drivers/usb/dwc3/Kconfig" +source "drivers/usb/core/Kconfig" source "drivers/usb/mon/Kconfig" @@ -134,8 +134,6 @@ source "drivers/usb/host/Kconfig" source "drivers/usb/musb/Kconfig" -source "drivers/usb/chipidea/Kconfig" - source "drivers/usb/renesas_usbhs/Kconfig" source "drivers/usb/class/Kconfig" @@ -144,12 +142,19 @@ source "drivers/usb/storage/Kconfig" source "drivers/usb/image/Kconfig" +endif + +source "drivers/usb/dwc3/Kconfig" + +source "drivers/usb/chipidea/Kconfig" + comment "USB port drivers" - depends on USB + +if USB config USB_USS720 tristate "USS720 parport driver" - depends on USB && PARPORT + depends on PARPORT select PARPORT_NOT_PC ---help--- This driver is for USB parallel port adapters that use the Lucent @@ -180,10 +185,12 @@ source "drivers/usb/serial/Kconfig" source "drivers/usb/misc/Kconfig" -source "drivers/usb/phy/Kconfig" - source "drivers/usb/atm/Kconfig" +endif # USB + +source "drivers/usb/phy/Kconfig" + source "drivers/usb/gadget/Kconfig" source "drivers/usb/otg/Kconfig" diff --git a/drivers/usb/atm/Kconfig b/drivers/usb/atm/Kconfig index be0b8da..0f92294 100644 --- a/drivers/usb/atm/Kconfig +++ b/drivers/usb/atm/Kconfig @@ -4,7 +4,7 @@ menuconfig USB_ATM tristate "USB DSL modem support" - depends on USB && ATM + depends on ATM select CRC32 default n help diff --git a/drivers/usb/class/Kconfig b/drivers/usb/class/Kconfig index 316aac8..bb8b736 100644 --- a/drivers/usb/class/Kconfig +++ b/drivers/usb/class/Kconfig @@ -2,11 +2,10 @@ # USB Class driver configuration # comment "USB Device Class drivers" - depends on USB config USB_ACM tristate "USB Modem (CDC ACM) support" - depends on USB && TTY + depends on TTY ---help--- This driver supports USB modems and ISDN adapters which support the Communication Device Class Abstract Control Model interface. @@ -21,7 +20,6 @@ config USB_ACM config USB_PRINTER tristate "USB Printer support" - depends on USB help Say Y here if you want to connect a USB printer to your computer's USB port. @@ -31,7 +29,6 @@ config USB_PRINTER config USB_WDM tristate "USB Wireless Device Management support" - depends on USB ---help--- This driver supports the WMC Device Management functionality of cell phones compliant to the CDC WMC specification. You can use @@ -42,7 +39,6 @@ config USB_WDM config USB_TMC tristate "USB Test and Measurement Class support" - depends on USB help Say Y here if you want to connect a USB device that follows the USB.org specification for USB Test and Measurement devices diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig index 7b7305e..8772b36 100644 --- a/drivers/usb/core/Kconfig +++ b/drivers/usb/core/Kconfig @@ -3,7 +3,6 @@ # config USB_DEBUG bool "USB verbose debug messages" - depends on USB help Say Y here if you want the USB core & hub drivers to produce a bunch of debug messages to the system log. Select this if you are having a @@ -11,7 +10,6
[PATCH 5/5 v3] USB: enclose USB_XHCI_HCD related symbols within a if USB_XHCI_HCD block
This patch encloses all symbols depending on USB_XHCI_HCD within an if USB_XHCI_HCD / endif block. Acked-by: Alan Stern Signed-off-by: Florian Fainelli --- Changes in v3: - added Alan's Acked-by tag drivers/usb/host/Kconfig |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 8ff86d3..c7cb18d 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -25,13 +25,13 @@ config USB_XHCI_HCD To compile this driver as a module, choose M here: the module will be called xhci-hcd. +if USB_XHCI_HCD + config USB_XHCI_PLATFORM tristate - depends on USB_XHCI_HCD config USB_XHCI_HCD_DEBUGGING bool "Debugging for the xHCI host controller" - depends on USB_XHCI_HCD ---help--- Say 'Y' to turn on debugging for the xHCI host controller driver. This will spew debugging output, even in interrupt context. @@ -39,6 +39,8 @@ config USB_XHCI_HCD_DEBUGGING If unsure, say N. +endif # USB_XHCI_HCD + config USB_EHCI_HCD tristate "EHCI HCD (USB 2.0) support" depends on USB_ARCH_HAS_EHCI -- 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 0/5 v3] USB: Kconfig cleanups
These 5 patches contain my Kconfig cleanup on which I based the removal of the USB_ARCH_HAS_* patches. They have been suggested by Alan Stern as part of an earlier conversations. Let me know what you think about it so I can post subsequent work based on it. Thanks! Fainelli (5): USB: regroup all depends on USB within an if USB block USB: remove USB_EHCI_BIG_ENDIAN_{DESC,MMIO} depends on architecture symbol USB: enclose EHCI HCD drivers within an if USB_EHCI_HCD block USB: enclose all depends on USB_OHCI_HCD within an if USB_OHCI_HCD block USB: enclose USB_XHCI_HCD related symbols within a if USB_XHCI_HCD block arch/arm/Kconfig|2 + arch/mips/Kconfig |3 + arch/powerpc/platforms/44x/Kconfig |2 + arch/powerpc/platforms/512x/Kconfig |2 + arch/sparc/Kconfig |2 + drivers/usb/Kconfig | 23 +++-- drivers/usb/atm/Kconfig |2 +- drivers/usb/class/Kconfig |6 +- drivers/usb/core/Kconfig|6 -- drivers/usb/host/Kconfig| 169 +-- drivers/usb/image/Kconfig |4 +- drivers/usb/misc/Kconfig| 21 - drivers/usb/misc/sisusbvga/Kconfig |2 +- drivers/usb/mon/Kconfig |1 - drivers/usb/musb/Kconfig|2 +- drivers/usb/renesas_usbhs/Kconfig |2 +- drivers/usb/serial/Kconfig |2 +- drivers/usb/storage/Kconfig |7 +- drivers/usb/wusbcore/Kconfig|2 - 19 files changed, 116 insertions(+), 144 deletions(-) -- 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/RFC] uvcvideo: Disable USB autosuspend for Creative Live! Cam Optia AF
Hi Alan, On Thursday 28 March 2013 10:45:27 Alan Stern wrote: > On Thu, 28 Mar 2013, Laurent Pinchart wrote: > > The camera fails to start video streaming after having been autosuspend. > > Add a new quirk to selectively disable autosuspend for devices that > > don't support it. > > > > Signed-off-by: Laurent Pinchart > > --- > > > > drivers/media/usb/uvc/uvc_driver.c | 14 +- > > drivers/media/usb/uvc/uvcvideo.h | 1 + > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > I've tried to set the reset resume quirk for this device in the USB core > > but the camera still failed to start video streaming after having been > > autosuspended. Regardless of whether the reset resume quirk was set, it > > would respond to control messages but wouldn't send video data. > > Presumably the camera won't work after a system suspend, either. That was my expectation as well, but the device has survived system suspend without being reenumerated. I don't know if the USB port power got cut off during system suspend. > > This solution below is a hack, but I'm not sure what else I can try. Crazy > > ideas are welcome. > > It's not a hack; it's a normal use for a quirk flag. Of course, if you > can figure out what's wrong with the camera and see how to fix it, that > would be best. I've tried to but I can't figure out what goes wrong exactly. > How does the camera perform on a Windows system after being put to > sleep and then woken up? I don't know, I have no Windows system to test the camera on. -- Regards, Laurent Pinchart -- 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
Exporting iInterface (and associated string) in debugfs?
Hi, I discovered that the optional strings and their indices of interface descriptors are not exported in debugfs when I was looking at usbview's source (https://github.com/gregkh/usbview/issues/5). I am not sure yet if they are cached (e.g. in rawdescriptors in struct usb_device or even more conveniently) or if one would have to fetch them with usb_get_descriptor(). More importantly though I wonder if you think it is useful to export them at all? I haven't apprehended the debugfs/sysfs differences regarding USB yet apparently. Maybe a better question would have been "Why does usbview not use sysfs (or even libusb)?". -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- 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 video capture issue due to uvc_complete callback spends more time
Hi Ravi, On Friday 29 March 2013 14:14:35 B, Ravi wrote: > Laurent/Ming Lei > > > Since the uvc_video_complete() callback handler called from interrupt > > context, video post processing or memcpy can be deferred to tasklet or > > bottom half, rather than doing it in interrupt context. > > If that's the only way to fix the issue, yes. However, given your > really poor memcpy() performances, I don't think you will be able to > sustain the incoming video bandwidth. The driver would soon run out of > URBs. > >>> > >>> I agree with you, let me check whether memcpy is the bottle here, > >> > >> typo mistake, read as "bottle-neck" > >> > >> I will try to get profile info on this. But in general it would be good > >> to move post processing to bottom half which helps to reduce interrupt > >> latency. In general it is, but the amount of data to be copied is pretty small, so I'm not sure if it's worth it for the uvcvideo driver. If reading from coherent memory is that slow you won't be able to sustain the required bandwidth, regardless of whether the memcpy is performed in interrupt context or not. The driver will be unusable in both case. > > You are correct, I have profiled, the timing I have posted in previous > > mail are due to the memcpy() in uvc_video_decode_data(). Which is the > > bottle-neck and consumes most of the time. > > There is an improvement in timing after defining CONFIG_DMA_NONCOHERENT, the > coherent memory access is very slow leads to this issue. > > #ifndef CONFIG_DMA_NONCOHERENT > stream->urb_buffer[i] = usb_alloc_coherent( > stream->dev->udev, stream->urb_size, > gfp_flags | __GFP_NOWARN, > &stream->urb_dma[i]); #else > stream->urb_buffer[i] = > kmalloc(stream->urb_size, gfp_flags | > __GFP_NOWARN); #endif Can your platform really DMA from non-coherent memory ? If so, why isn't CONFIG_DMA_NONCOHERENT unset ? > The mentor host controller (driver/usb/musb) puts all overhead on SW to > schedule the next urb, unlike like ehci/xhci where the multiple urbs (TDs) > can be queued and HW executes the transfers on the bus without SW > intervention. In case of musb host controller, the SW has to program the > urb one by one, hence it is critical that urb completion callback called in > interrupt context must be very thin. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
On Thu, 4 Apr 2013, Felipe Balbi wrote: > > >> Some subsystems handle this issue by calling pm_runtime_get_sync() > > >> before probing a driver and pm_runtime_put_sync() after unbinding the > > >> driver. If the driver is runtime-PM-enabled, it then does its own > > >> put_sync near the end of its probe routine and get_sync in its release > > >> routine. > > > > > > sounds a bit 'fishy' to me... So a separate entity would call > > > pm_runtime_get_sync(), even when we don't have registered dev_pm_ops, I don't know what you mean by "separate entity". The PHY's subsystem would handle this. After all, the subsystem has to handle registering the PHY in the first place. If the PHY doesn't have a dev_pm_ops, why are you talking about doing runtime PM on it? That doesn't make any sense. > > > then drivers need to check if runtime_pm is enabled and call > > > pm_runtime_put*() conditionally before returning from probe(). One They don't have to check. If CONFIG_PM_RUNTIME isn't set or the target is runtime-PM-disabled then pm_runtime_put* is essentially a no-op (in the disabled case it decrements the usage counter but doesn't do anything else). One possible complication I did not mention: The scheme described above assumes the device that uses the PHY will always be registered on the same type of bus. If the device can be used on multiple bus types (and hence in multiple subsystems) then things aren't so simple, because some of the subsystems might support runtime PM and others might not. (You may very well run into this problem with USB controllers that are sometimes on a PCI bus and sometimes on a platform bus.) In this case, the driver needs to adapt to the subsystem's capabilities. Presumably the bus-glue part of the driver takes care of this. > > > remove, we might have another issue: device is already runtime_suspended > > > (due to e.g. autosuspend) when module is removed, a call to > > > pm_runtime_put_sync() will be unbalanced. No ? No. I left out some of the details. For one thing, the subsystem is careful to do a runtime resume before calling the driver's remove method. (Also, if you look over my original description carefully, you'll see that there are no unbalanced calls -- even if the device is already runtime suspended when the driver is unbound.) For another, the subsystem needs to check before calling the driver's runtime-PM methods. There are two brief windows during which the driver isn't in charge of the device even though dev->driver is set. Those windows occur in the subsystem's probe routine (before it calls the driver's probe method) and in the subsystem's remove routine (after it calls the driver's remove method). At such times, the subsystem's runtime-PM handlers must be careful _not_ to call the driver's runtime-PM routines. > > May be i am misinterpreting !! > > If PHYs are runtime-PM enabled (PHY probe calls *runtime_enable*), > > then the consumers > > need to call pm_runtime_get_sync whever they want to access PHY. No, because in addition to being runtime-PM enabled, the PHY should automatically be runtime resumed when the consumer registers itself as a user of the PHY. Therefore the consumer doesn't need to do anything at all -- which is good for consumers that aren't runtime-PM aware. > Alright, so here's my understanding: > > I suggested letting e.g. DWC3 enable the PHY's runtime_pm; Alan said > that it could be done before that so that DWC3 sees an enabled PHY > during probe. Basically right. Help me to understand the overall situation a little better: What code registers the PHY initially? What routine does the DWC3 driver call to register itself as a consumer of the PHY? Likewise, what routine does it call to unregister itself? 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 02/15] staging: usbip: reformat function stub_recv_cmd_unlink()
From: Kurt Kanzenbach Reformat function stub_recv_cmd_unlink() to improve readability. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/stub_rx.c | 98 - 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/drivers/staging/usbip/stub_rx.c b/drivers/staging/usbip/stub_rx.c index 715e8a7..9fc62b3 100644 --- a/drivers/staging/usbip/stub_rx.c +++ b/drivers/staging/usbip/stub_rx.c @@ -228,61 +228,61 @@ static void tweak_special_requests(struct urb *urb) static int stub_recv_cmd_unlink(struct stub_device *sdev, struct usbip_header *pdu) { + int ret; unsigned long flags; - struct stub_priv *priv; spin_lock_irqsave(&sdev->priv_lock, flags); list_for_each_entry(priv, &sdev->priv_init, list) { - if (priv->seqnum == pdu->u.cmd_unlink.seqnum) { - int ret; - - dev_info(&priv->urb->dev->dev, "unlink urb %p\n", -priv->urb); - - /* -* This matched urb is not completed yet (i.e., be in -* flight in usb hcd hardware/driver). Now we are -* cancelling it. The unlinking flag means that we are -* now not going to return the normal result pdu of a -* submission request, but going to return a result pdu -* of the unlink request. -*/ - priv->unlinking = 1; - - /* -* In the case that unlinking flag is on, prev->seqnum -* is changed from the seqnum of the cancelling urb to -* the seqnum of the unlink request. This will be used -* to make the result pdu of the unlink request. -*/ - priv->seqnum = pdu->base.seqnum; - - spin_unlock_irqrestore(&sdev->priv_lock, flags); - - /* -* usb_unlink_urb() is now out of spinlocking to avoid -* spinlock recursion since stub_complete() is -* sometimes called in this context but not in the -* interrupt context. If stub_complete() is executed -* before we call usb_unlink_urb(), usb_unlink_urb() -* will return an error value. In this case, stub_tx -* will return the result pdu of this unlink request -* though submission is completed and actual unlinking -* is not executed. OK? -*/ - /* In the above case, urb->status is not -ECONNRESET, -* so a driver in a client host will know the failure -* of the unlink request ? -*/ - ret = usb_unlink_urb(priv->urb); - if (ret != -EINPROGRESS) - dev_err(&priv->urb->dev->dev, - "failed to unlink a urb %p, ret %d\n", - priv->urb, ret); - return 0; - } + if (priv->seqnum != pdu->u.cmd_unlink.seqnum) + continue; + + dev_info(&priv->urb->dev->dev, "unlink urb %p\n", +priv->urb); + + /* +* This matched urb is not completed yet (i.e., be in +* flight in usb hcd hardware/driver). Now we are +* cancelling it. The unlinking flag means that we are +* now not going to return the normal result pdu of a +* submission request, but going to return a result pdu +* of the unlink request. +*/ + priv->unlinking = 1; + + /* +* In the case that unlinking flag is on, prev->seqnum +* is changed from the seqnum of the cancelling urb to +* the seqnum of the unlink request. This will be used +* to make the result pdu of the unlink request. +*/ + priv->seqnum = pdu->base.seqnum; + + spin_unlock_irqrestore(&sdev->priv_lock, flags); + + /* +* usb_unlink_urb() is now out of spinlocking to avoid +* spinlock recursion since stub_complete() is +* sometimes called in this context but not in the +* interrupt context. If stub_complete() is executed +* before we call usb_unlink_urb(), usb_unlink_urb() +* will return an error value. In this case, stub_tx +* will return the r
[PATCH 01/15] staging: usbip: userspace: libsrc: replace numbers by ascii
replace numbers in code by ascii text constants as suggested by Dan Carpenter: http://driverdev.linuxdriverproject.org/pipermail/devel/2013-February/035907.html Signed-off-by: Stefan Reif --- drivers/staging/usbip/userspace/libsrc/names.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/usbip/userspace/libsrc/names.c b/drivers/staging/usbip/userspace/libsrc/names.c index 3b151df..72480fb 100644 --- a/drivers/staging/usbip/userspace/libsrc/names.c +++ b/drivers/staging/usbip/userspace/libsrc/names.c @@ -491,10 +491,10 @@ static void parse(FILE *f) while (fgets(buf, sizeof(buf), f)) { linectr++; /* remove line ends */ - cp = strchr(buf, 13); + cp = strchr(buf, '\r'); if (cp) *cp = 0; - cp = strchr(buf, 10); + cp = strchr(buf, '\n'); if (cp) *cp = 0; if (buf[0] == '#' || !buf[0]) -- 1.8.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 04/15] staging: usbip: removed enumeration of comments
From: Kurt Kanzenbach Enumerations for one comment makes no sense. This is why this should be removed. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/stub_dev.c | 2 +- drivers/staging/usbip/stub_rx.c | 2 +- drivers/staging/usbip/vhci_rx.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c index 0a70b8e..471cd2a 100644 --- a/drivers/staging/usbip/stub_dev.c +++ b/drivers/staging/usbip/stub_dev.c @@ -452,7 +452,7 @@ static void shutdown_busid(struct bus_id_priv *busid_priv) busid_priv->shutdown_busid = 1; usbip_event_add(&busid_priv->sdev->ud, SDEV_EVENT_REMOVED); - /* 2. wait for the stop of the event handler */ + /* wait for the stop of the event handler */ usbip_stop_eh(&busid_priv->sdev->ud); } } diff --git a/drivers/staging/usbip/stub_rx.c b/drivers/staging/usbip/stub_rx.c index 9fc62b3..db48a78 100644 --- a/drivers/staging/usbip/stub_rx.c +++ b/drivers/staging/usbip/stub_rx.c @@ -552,7 +552,7 @@ static void stub_rx_pdu(struct usbip_device *ud) memset(&pdu, 0, sizeof(pdu)); - /* 1. receive a pdu header */ + /* receive a pdu header */ ret = usbip_recv(ud->tcp_socket, &pdu, sizeof(pdu)); if (ret != sizeof(pdu)) { dev_err(dev, "recv a header, %d\n", ret); diff --git a/drivers/staging/usbip/vhci_rx.c b/drivers/staging/usbip/vhci_rx.c index 50d6fd5..d07fcb5 100644 --- a/drivers/staging/usbip/vhci_rx.c +++ b/drivers/staging/usbip/vhci_rx.c @@ -206,7 +206,7 @@ static void vhci_rx_pdu(struct usbip_device *ud) memset(&pdu, 0, sizeof(pdu)); - /* 1. receive a pdu header */ + /* receive a pdu header */ ret = usbip_recv(ud->tcp_socket, &pdu, sizeof(pdu)); if (ret < 0) { if (ret == -ECONNRESET) -- 1.8.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 00/15] staging: usbip: code cleanup
Code cleanup for the usbip driver. - delete unused code - improve readability - improve usability of the command line tools Kurt and I did this work together. Kurt Kanzenbach (10): staging: usbip: reformat function stub_recv_cmd_unlink() staging: usbip: removed enumeration of comments staging: usbip: simplified errorhandling staging: usbip: removed unnecessary if-else-statements staging: usbip: removed unnecessary for loop staging: usbip: simplified cleanup function staging: usbip: userspace: removed unnecessary code staging: usbip: userspace: libsrc: cleanup parsing staging: usbip: userspace: unified command line arguments staging: usbip: userspace: show product name in `list -l' command Stefan Reif (5): staging: usbip: userspace: libsrc: replace numbers by ascii staging: usbip: reformat function pickup_urb_and_free_priv staging: usbip: remove unnused, broken macro staging: usbip: simple indent fix staging: usbip: userspace: avoid memory leaks drivers/staging/usbip/stub_dev.c | 32 +- drivers/staging/usbip/stub_main.c | 34 +- drivers/staging/usbip/stub_rx.c| 100 ++--- drivers/staging/usbip/usbip_common.c | 2 +- drivers/staging/usbip/userspace/README | 4 +- drivers/staging/usbip/userspace/doc/usbip.8| 4 +- drivers/staging/usbip/userspace/libsrc/names.c | 499 +++-- drivers/staging/usbip/userspace/libsrc/names.h | 21 +- drivers/staging/usbip/userspace/src/usbip_attach.c | 12 +- drivers/staging/usbip/userspace/src/usbip_list.c | 12 + .../staging/usbip/userspace/src/usbip_network.c| 4 +- drivers/staging/usbip/userspace/src/usbipd.c | 11 +- drivers/staging/usbip/vhci.h | 1 - drivers/staging/usbip/vhci_rx.c| 56 +-- 14 files changed, 211 insertions(+), 581 deletions(-) -- 1.8.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 03/15] staging: usbip: reformat function pickup_urb_and_free_priv
re-indent funtion "pickup_urb_and_free_priv" to improve readability. Signed-off-by: Stefan Reif --- drivers/staging/usbip/vhci_rx.c | 54 ++--- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/drivers/staging/usbip/vhci_rx.c b/drivers/staging/usbip/vhci_rx.c index faf8e60..50d6fd5 100644 --- a/drivers/staging/usbip/vhci_rx.c +++ b/drivers/staging/usbip/vhci_rx.c @@ -31,33 +31,37 @@ struct urb *pickup_urb_and_free_priv(struct vhci_device *vdev, __u32 seqnum) int status; list_for_each_entry_safe(priv, tmp, &vdev->priv_rx, list) { - if (priv->seqnum == seqnum) { - urb = priv->urb; - status = urb->status; - - usbip_dbg_vhci_rx("find urb %p vurb %p seqnum %u\n", - urb, priv, seqnum); - - /* TODO: fix logic here to improve indent situtation */ - if (status != -EINPROGRESS) { - if (status == -ENOENT || - status == -ECONNRESET) - dev_info(&urb->dev->dev, -"urb %p was unlinked " -"%ssynchronuously.\n", urb, -status == -ENOENT ? "" : "a"); - else - dev_info(&urb->dev->dev, -"urb %p may be in a error, " -"status %d\n", urb, status); - } - - list_del(&priv->list); - kfree(priv); - urb->hcpriv = NULL; - + if (priv->seqnum != seqnum) + continue; + + urb = priv->urb; + status = urb->status; + + usbip_dbg_vhci_rx("find urb %p vurb %p seqnum %u\n", + urb, priv, seqnum); + + switch (status) { + case -ENOENT: + /* fall through */ + case -ECONNRESET: + dev_info(&urb->dev->dev, +"urb %p was unlinked %ssynchronuously.\n", urb, +status == -ENOENT ? "" : "a"); + break; + case -EINPROGRESS: + /* no info output */ break; + default: + dev_info(&urb->dev->dev, +"urb %p may be in a error, status %d\n", urb, +status); } + + list_del(&priv->list); + kfree(priv); + urb->hcpriv = NULL; + + break; } return urb; -- 1.8.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 09/15] staging: usbip: removed unnecessary for loop
From: Kurt Kanzenbach This for loop is not needed, since STUB_BUSID_OTHER is defined as 0. In Addition added a comment if STUB_BUSID_OTHER changes sometime. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/stub_main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/usbip/stub_main.c b/drivers/staging/usbip/stub_main.c index 629bfcb..33027cc 100644 --- a/drivers/staging/usbip/stub_main.c +++ b/drivers/staging/usbip/stub_main.c @@ -38,11 +38,11 @@ static spinlock_t busid_table_lock; static void init_busid_table(void) { - int i; - + /* +* This also sets the bus_table[i].status to +* STUB_BUSID_OTHER, which is 0. +*/ memset(busid_table, 0, sizeof(busid_table)); - for (i = 0; i < MAX_BUSID; i++) - busid_table[i].status = STUB_BUSID_OTHER; spin_lock_init(&busid_table_lock); } -- 1.8.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 15/15] staging: usbip: userspace: show product name in `list -l' command
From: Kurt Kanzenbach The `usbip list -l' command shows your local usb-devices. Example: $ usbip list -l $ Local USB devices $ = $ - busid 1-1 (13fe:1d00) $ 1-1:1.0 -> usb-storage $ $ - busid 1-2 (0409:55aa) $ 1-2:1.0 -> hub However this list command doesn't show which device is connected to this busid. Therefore you have to use another tool e.g. lsusb to determine that. This patches adds the possibility to see which device that is. Example: $ usbip list -l $ Local USB devices $ = $ - busid 1-1 (13fe:1d00) $ Kingston Technology Company Inc. : DataTraveler 2.0 1GB/4GB Flash Drive / Patriot Xporter 4GB Flash $ 1-1:1.0 -> usb-storage $ $ - busid 1-2 (0409:55aa) $ NEC Corp. : Hub (0409:55aa) $ 1-2:1.0 -> hub If parsable is specified the info will be not printed. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/userspace/src/usbip_list.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/staging/usbip/userspace/src/usbip_list.c b/drivers/staging/usbip/userspace/src/usbip_list.c index ed30d91..ff56255 100644 --- a/drivers/staging/usbip/userspace/src/usbip_list.c +++ b/drivers/staging/usbip/userspace/src/usbip_list.c @@ -159,6 +159,12 @@ static void print_device(char *busid, char *vendor, char *product, printf(" - busid %s (%.4s:%.4s)\n", busid, vendor, product); } +static void print_product_name(char *product_name, bool parsable) +{ + if (!parsable) + printf(" %s\n", product_name); +} + static void print_interface(char *busid, char *driver, bool parsable) { if (parsable) @@ -189,6 +195,7 @@ static int list_devices(bool parsable) { char bus_type[] = "usb"; char busid[SYSFS_BUS_ID_SIZE]; + char product_name[128]; struct sysfs_bus *ubus; struct sysfs_device *dev; struct sysfs_device *intf; @@ -231,8 +238,13 @@ static int list_devices(bool parsable) goto err_out; } + /* get product name */ + usbip_names_get_product(product_name, sizeof(product_name), + strtol(idVendor->value, NULL, 16), + strtol(idProduct->value, NULL, 16)); print_device(dev->bus_id, idVendor->value, idProduct->value, parsable); + print_product_name(product_name, parsable); for (i = 0; i < atoi(bNumIntfs->value); i++) { snprintf(busid, sizeof(busid), "%s:%.1s.%d", -- 1.8.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 12/15] staging: usbip: userspace: removed unnecessary code
From: Kurt Kanzenbach Since no usbip_name function is used in usbipd, it's not necessary to parse "usb.ids" file at startup. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/userspace/src/usbipd.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c index 3f10c51..3e913b8 100644 --- a/drivers/staging/usbip/userspace/src/usbipd.c +++ b/drivers/staging/usbip/userspace/src/usbipd.c @@ -436,9 +436,6 @@ static int do_standalone_mode(int daemonize) struct timespec timeout; sigset_t sigmask; - if (usbip_names_init(USBIDS_FILE)) - err("failed to open %s", USBIDS_FILE); - if (usbip_host_driver_open()) { err("please load " USBIP_CORE_MOD_NAME ".ko and " USBIP_HOST_DRV_NAME ".ko!"); @@ -507,7 +504,6 @@ static int do_standalone_mode(int daemonize) free(fds); freeaddrinfo(ai_head); usbip_host_driver_close(); - usbip_names_free(); return 0; } -- 1.8.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 08/15] staging: usbip: removed unnecessary if-else-statements
From: Kurt Kanzenbach In each if-else case "return" is called. This is why these if-else-statements are useless. Removing them improves understanding and readability. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/stub_main.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/staging/usbip/stub_main.c b/drivers/staging/usbip/stub_main.c index 705a9e5..629bfcb 100644 --- a/drivers/staging/usbip/stub_main.c +++ b/drivers/staging/usbip/stub_main.c @@ -167,22 +167,22 @@ static ssize_t store_match_busid(struct device_driver *dev, const char *buf, strncpy(busid, buf + 4, BUSID_SIZE); if (!strncmp(buf, "add ", 4)) { - if (add_match_busid(busid) < 0) { + if (add_match_busid(busid) < 0) return -ENOMEM; - } else { - pr_debug("add busid %s\n", busid); - return count; - } - } else if (!strncmp(buf, "del ", 4)) { - if (del_match_busid(busid) < 0) { + + pr_debug("add busid %s\n", busid); + return count; + } + + if (!strncmp(buf, "del ", 4)) { + if (del_match_busid(busid) < 0) return -ENODEV; - } else { - pr_debug("del busid %s\n", busid); - return count; - } - } else { - return -EINVAL; + + pr_debug("del busid %s\n", busid); + return count; } + + return -EINVAL; } static DRIVER_ATTR(match_busid, S_IRUSR | S_IWUSR, show_match_busid, store_match_busid); -- 1.8.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 14/15] staging: usbip: userspace: unified command line arguments
From: Kurt Kanzenbach The command `usbip attach' uses --host for specifing the remote host, while `usbip list' uses --remote. This is confusing and this patch adapts this. In Addition changed the manpage and README accordingly. Before: $ usbip attach --host -b $ usbip list --remote Now: $ usbip attach --remote -b $ usbip list --remote Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/userspace/README | 4 ++-- drivers/staging/usbip/userspace/doc/usbip.8| 4 ++-- drivers/staging/usbip/userspace/src/usbip_attach.c | 12 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/usbip/userspace/README b/drivers/staging/usbip/userspace/README index 233d1d7..00a1658 100644 --- a/drivers/staging/usbip/userspace/README +++ b/drivers/staging/usbip/userspace/README @@ -54,7 +54,7 @@ client:# usbip list --remote - List exported USB devices on the . -client:# usbip attach --host --busid 1-2 +client:# usbip attach --remote --busid 1-2 - Connect the remote USB device. client:# usbip port @@ -163,7 +163,7 @@ exportable on the host. Attach a remote USB device: -deux:# usbip attach --host 10.0.0.3 --busid 1-1 +deux:# usbip attach --remote 10.0.0.3 --busid 1-1 port 0 attached Show the devices attached to this client: diff --git a/drivers/staging/usbip/userspace/doc/usbip.8 b/drivers/staging/usbip/userspace/doc/usbip.8 index 6e0d745..ccdadc8 100644 --- a/drivers/staging/usbip/userspace/doc/usbip.8 +++ b/drivers/staging/usbip/userspace/doc/usbip.8 @@ -38,7 +38,7 @@ then exit. .PP .HP -\fBattach\fR \-\-host=<\fIhost\fR> \-\-busid=<\fIbus_id\fR> +\fBattach\fR \-\-remote=<\fIhost\fR> \-\-busid=<\fIbus_id\fR> .IP Attach a remote USB device. .PP @@ -79,7 +79,7 @@ List local USB devices. client:# usbip list --remote=server - List exportable usb devices on the server. -client:# usbip attach --host=server --busid=1-2 +client:# usbip attach --remote=server --busid=1-2 - Connect the remote USB device. client:# usbip detach --port=0 diff --git a/drivers/staging/usbip/userspace/src/usbip_attach.c b/drivers/staging/usbip/userspace/src/usbip_attach.c index 2da4e44..0ec16e5 100644 --- a/drivers/staging/usbip/userspace/src/usbip_attach.c +++ b/drivers/staging/usbip/userspace/src/usbip_attach.c @@ -36,7 +36,7 @@ static const char usbip_attach_usage_string[] = "usbip attach \n" - "-h, --host= The machine with exported USB devices\n" + "-r, --remote= The machine with exported USB devices\n" "-b, --busid=Busid of the device on \n"; void usbip_attach_usage(void) @@ -201,9 +201,9 @@ static int attach_device(char *host, char *busid) int usbip_attach(int argc, char *argv[]) { static const struct option opts[] = { - { "host", required_argument, NULL, 'h' }, - { "busid", required_argument, NULL, 'b' }, - { NULL, 0, NULL, 0 } + { "remote", required_argument, NULL, 'r' }, + { "busid", required_argument, NULL, 'b' }, + { NULL, 0, NULL, 0 } }; char *host = NULL; char *busid = NULL; @@ -211,13 +211,13 @@ int usbip_attach(int argc, char *argv[]) int ret = -1; for (;;) { - opt = getopt_long(argc, argv, "h:b:", opts, NULL); + opt = getopt_long(argc, argv, "r:b:", opts, NULL); if (opt == -1) break; switch (opt) { - case 'h': + case 'r': host = optarg; break; case 'b': -- 1.8.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 05/15] staging: usbip: remove unnused, broken macro
Remove broken preprocessor macro "hardware". It is unused and it references an element (pdev in vhci_hcd) that does not exist. Signed-off-by: Stefan Reif --- drivers/staging/usbip/vhci.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/usbip/vhci.h b/drivers/staging/usbip/vhci.h index 5dddc4d..a863a98 100644 --- a/drivers/staging/usbip/vhci.h +++ b/drivers/staging/usbip/vhci.h @@ -95,7 +95,6 @@ struct vhci_hcd { extern struct vhci_hcd *the_controller; extern const struct attribute_group dev_attr_group; -#define hardware (&the_controller->pdev.dev) /* vhci_hcd.c */ void rh_port_connect(int rhport, enum usb_device_speed speed); -- 1.8.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 13/15] staging: usbip: userspace: libsrc: cleanup parsing
From: Kurt Kanzenbach Since the names.c/names.h are taken from another project, some functions which names.c provides aren't used by usbipd. This patch fixes: - removed useless comments - unified debug/error messages by using the macros provided by usbip_common.h - removed unnused code The code cleanup includes: - remove unused data structures - remove code to create them - remove code to access them The file names.c is used to parse the `usb.ids' file. The parser stores a lot of information about usb devices that is never used. The `usb.ids' file has several sections. Some variables (like `lasthut') store the ID of the current section, and those variables are used to decide which section is currently being parsed (i.e. in which data structure the current line will be stored). We removed the code to read those IDs because they are never used anyway. We replaced them by the pseudo-ID `1' (instead of reading the ID from the file) to indicate that the parser is in a section that can be ignored. If the parser is in such a section, the current line (which contains sub-items for this section) is discarded. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/userspace/libsrc/names.c | 495 - drivers/staging/usbip/userspace/libsrc/names.h | 21 +- 2 files changed, 67 insertions(+), 449 deletions(-) diff --git a/drivers/staging/usbip/userspace/libsrc/names.c b/drivers/staging/usbip/userspace/libsrc/names.c index 72480fb..3c8d28b 100644 --- a/drivers/staging/usbip/userspace/libsrc/names.c +++ b/drivers/staging/usbip/userspace/libsrc/names.c @@ -1,4 +1,3 @@ -/*/ /* * names.c -- USB name database manipulation routines * @@ -19,15 +18,14 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. * * - */ - -/* + * + * + * * Copyright (C) 2005 Takahiro Hirofuchi * - names_deinit() is added. + * */ -/*/ - #include #include #include @@ -39,11 +37,8 @@ #include #include - #include "names.h" - - -/* -- */ +#include "usbip_common.h" struct vendor { struct vendor *next; @@ -75,19 +70,12 @@ struct protocol { char name[1]; }; -struct audioterminal { - struct audioterminal *next; - u_int16_t termt; - char name[1]; -}; - struct genericstrtable { struct genericstrtable *next; unsigned int num; char name[1]; }; -/* -- */ #define HASH1 0x10 #define HASH2 0x02 @@ -103,75 +91,12 @@ static unsigned int hashnum(unsigned int num) return num & (HASHSZ-1); } -/* -- */ static struct vendor *vendors[HASHSZ] = { NULL, }; static struct product *products[HASHSZ] = { NULL, }; static struct class *classes[HASHSZ] = { NULL, }; static struct subclass *subclasses[HASHSZ] = { NULL, }; static struct protocol *protocols[HASHSZ] = { NULL, }; -static struct audioterminal *audioterminals[HASHSZ] = { NULL, }; -static struct genericstrtable *hiddescriptors[HASHSZ] = { NULL, }; -static struct genericstrtable *reports[HASHSZ] = { NULL, }; -static struct genericstrtable *huts[HASHSZ] = { NULL, }; -static struct genericstrtable *biass[HASHSZ] = { NULL, }; -static struct genericstrtable *physdess[HASHSZ] = { NULL, }; -static struct genericstrtable *hutus[HASHSZ] = { NULL, }; -static struct genericstrtable *langids[HASHSZ] = { NULL, }; -static struct genericstrtable *countrycodes[HASHSZ] = { NULL, }; - -/* -- */ - -static const char *names_genericstrtable(struct genericstrtable *t[HASHSZ], -unsigned int index) -{ - struct genericstrtable *h; - - for (h = t[hashnum(index)]; h; h = h->next) - if (h->num == index) - return h->name; - return NULL; -} - -const char *names_hid(u_int8_t hidd) -{ - return names_genericstrtable(hiddescriptors, hidd); -} - -const char *names_reporttag(u_int8_t rt) -{ - return names_genericstrtable(reports, rt); -} - -const char *names_huts(unsigned int data) -{ - return names_genericstrtable(huts, data); -} - -const char *names_hutus(unsigned int data) -{ - return names_genericstrtable(hutus, data); -} - -const char *names_langid(u_int16_t langid) -{ - return names_genericstrtable(langids, langid); -} - -const char *names_physdes(u_int8_t ph) -{ - return names_genericstrtable(physdess, ph); -} - -const char *names_bias(u_int8_t b) -{ - return names_genericstrtable(biass, b); -} - -const char *names_countrycode(unsigned int countrycode) -{ - return names_
[PATCH 10/15] staging: usbip: simplified cleanup function
From: Kurt Kanzenbach This patch simplified "stub_device_free" cleanup function: - changed return type to void, since the return value is not checked anywhere - kfree is NULL-safe, so removed if statement - deleted debug-message Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/stub_dev.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c index c75ae63..83d629a 100644 --- a/drivers/staging/usbip/stub_dev.c +++ b/drivers/staging/usbip/stub_dev.c @@ -327,15 +327,9 @@ static struct stub_device *stub_device_alloc(struct usb_device *udev, return sdev; } -static int stub_device_free(struct stub_device *sdev) +static void stub_device_free(struct stub_device *sdev) { - if (!sdev) - return -EINVAL; - kfree(sdev); - pr_debug("kfree udev ok\n"); - - return 0; } /* -- 1.8.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 06/15] staging: usbip: simple indent fix
Fix an indent. Signed-off-by: Stefan Reif --- drivers/staging/usbip/usbip_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/usbip/usbip_common.c b/drivers/staging/usbip/usbip_common.c index 539fa57..7b97df6 100644 --- a/drivers/staging/usbip/usbip_common.c +++ b/drivers/staging/usbip/usbip_common.c @@ -389,7 +389,7 @@ int usbip_recv(struct socket *sock, void *buf, int size) pr_debug("receiving\n"); usbip_dump_buffer(bp, osize); pr_debug("received, osize %d ret %d size %d total %d\n", - osize, result, size, total); +osize, result, size, total); } return total; -- 1.8.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 11/15] staging: usbip: userspace: avoid memory leaks
Call freeaddrinfo when connect/listen fails. Call usbip_host_driver_close on error. Signed-off-by: Stefan Reif --- drivers/staging/usbip/userspace/src/usbip_network.c | 4 ++-- drivers/staging/usbip/userspace/src/usbipd.c| 7 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/staging/usbip/userspace/src/usbip_network.c b/drivers/staging/usbip/userspace/src/usbip_network.c index 4cb76e5..b12448e 100644 --- a/drivers/staging/usbip/userspace/src/usbip_network.c +++ b/drivers/staging/usbip/userspace/src/usbip_network.c @@ -248,10 +248,10 @@ int usbip_net_tcp_connect(char *hostname, char *service) close(sockfd); } + freeaddrinfo(res); + if (!rp) return EAI_SYSTEM; - freeaddrinfo(res); - return sockfd; } diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c index cc3be17..3f10c51 100644 --- a/drivers/staging/usbip/userspace/src/usbipd.c +++ b/drivers/staging/usbip/userspace/src/usbipd.c @@ -448,6 +448,7 @@ static int do_standalone_mode(int daemonize) if (daemonize) { if (daemon(0, 0) < 0) { err("daemonizing failed: %s", strerror(errno)); + usbip_host_driver_close(); return -1; } umask(0); @@ -456,14 +457,18 @@ static int do_standalone_mode(int daemonize) set_signal(); ai_head = do_getaddrinfo(NULL, PF_UNSPEC); - if (!ai_head) + if (!ai_head) { + usbip_host_driver_close(); return -1; + } info("starting " PROGNAME " (%s)", usbip_version_string); nsockfd = listen_all_addrinfo(ai_head, sockfdlist); if (nsockfd <= 0) { err("failed to open a listening socket"); + freeaddrinfo(ai_head); + usbip_host_driver_close(); return -1; } fds = calloc(nsockfd, sizeof(struct pollfd)); -- 1.8.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 07/15] staging: usbip: simplified errorhandling
From: Kurt Kanzenbach In each errorcase spin_unlock_irq is called and -EINVAL is returned. To simplify that I created a label called "err" doing that. On Success count will be returned. Signed-off-by: Kurt Kanzenbach Signed-off-by: Stefan Reif --- drivers/staging/usbip/stub_dev.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c index 471cd2a..c75ae63 100644 --- a/drivers/staging/usbip/stub_dev.c +++ b/drivers/staging/usbip/stub_dev.c @@ -86,6 +86,7 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr, struct stub_device *sdev = dev_get_drvdata(dev); int sockfd = 0; struct socket *socket; + ssize_t err = -EINVAL; if (!sdev) { dev_err(dev, "sdev is null\n"); @@ -101,15 +102,13 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr, if (sdev->ud.status != SDEV_ST_AVAILABLE) { dev_err(dev, "not ready\n"); - spin_unlock_irq(&sdev->ud.lock); - return -EINVAL; + goto err; } socket = sockfd_to_socket(sockfd); - if (!socket) { - spin_unlock_irq(&sdev->ud.lock); - return -EINVAL; - } + if (!socket) + goto err; + sdev->ud.tcp_socket = socket; spin_unlock_irq(&sdev->ud.lock); @@ -127,16 +126,19 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr, dev_info(dev, "stub down\n"); spin_lock_irq(&sdev->ud.lock); - if (sdev->ud.status != SDEV_ST_USED) { - spin_unlock_irq(&sdev->ud.lock); - return -EINVAL; - } + if (sdev->ud.status != SDEV_ST_USED) + goto err; + spin_unlock_irq(&sdev->ud.lock); usbip_event_add(&sdev->ud, SDEV_EVENT_DOWN); } return count; + +err: + spin_unlock_irq(&sdev->ud.lock); + return err; } static DEVICE_ATTR(usbip_sockfd, S_IWUSR, NULL, store_sockfd); -- 1.8.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB3.0 Interrupt transfer in u-boot
On Thu, Apr 04, 2013 at 10:07:06AM +0530, Puneet Sharma wrote: > Hello, > > I want to test USB keyboard for XHCI in u-boot and to do that i need the > Interrupt transfer code in XHCI controller driver in u-boot. If > possible, can you help me to give that piece of code or can you help me > to figure out in kernel where can i find it so that i can try to > replicate it in u-boot and make it work. > > > Thanks & Regards > -- > Puneet Sharma > > > The information contained in this email and any attachments is confidential > and may be subject to copyright or other intellectual property protection. If > you are not the intended recipient, you are not authorized to use or disclose > this information, and we request that you notify us by reply mail or > telephone and delete the original message from your mail system. Because of this footer, I am not allowed to answer your email, sorry. Please fix it if you wish to interact with Linux kernel developers. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2] USB: keyspan: pull in one indent level
Hello. On 04-04-2013 10:33, Dan Carpenter wrote: We can remove the "if (urb->actual_length) {" check because checking for "while (i < urb->actual_length) {" is sufficient. This lets us pull the code in one indent level. Signed-off-by: Dan Carpenter diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index 5b912fc..c773ac7 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -712,45 +712,43 @@ static void usa49wg_indat_callback(struct urb *urb) i = 0; len = 0; - if (urb->actual_length) { - while (i < urb->actual_length) { + while (i < urb->actual_length) { - /* Check port number from message*/ - if (data[i] >= serial->num_ports) { - dev_dbg(&urb->dev->dev, "%s - Unexpected port number %d\n", - __func__, data[i]); - return; - } - port = serial->port[data[i++]]; - len = data[i++]; + /* Check port number from message*/ Minor nit. Could you add a space before '*/'? + if (data[i] >= serial->num_ports) { + dev_dbg(&urb->dev->dev, "%s - Unexpected port number %d\n", + __func__, data[i]); + return; + } + port = serial->port[data[i++]]; + len = data[i++]; - /* 0x80 bit is error flag */ - if ((data[i] & 0x80) == 0) { - /* no error on any byte */ - i++; - for (x = 1; x < len ; ++x) - tty_insert_flip_char(&port->port, - data[i++], 0); - } else { - /* -* some bytes had errors, every byte has status -*/ - for (x = 0; x + 1 < len; x += 2) { - int stat = data[i], flag = 0; - if (stat & RXERROR_OVERRUN) - flag |= TTY_OVERRUN; - if (stat & RXERROR_FRAMING) - flag |= TTY_FRAME; - if (stat & RXERROR_PARITY) - flag |= TTY_PARITY; - /* XXX should handle break (0x10) */ - tty_insert_flip_char(&port->port, - data[i+1], flag); - i += 2; - } + /* 0x80 bit is error flag */ + if ((data[i] & 0x80) == 0) { + /* no error on any byte */ + i++; + for (x = 1; x < len ; ++x) + tty_insert_flip_char(&port->port, + data[i++], 0); + } else { + /* +* some bytes had errors, every byte has status +*/ + for (x = 0; x + 1 < len; x += 2) { + int stat = data[i], flag = 0; Empty line wouldn't hurt here. + if (stat & RXERROR_OVERRUN) + flag |= TTY_OVERRUN; + if (stat & RXERROR_FRAMING) + flag |= TTY_FRAME; + if (stat & RXERROR_PARITY) + flag |= TTY_PARITY; + /* XXX should handle break (0x10) */ + tty_insert_flip_char(&port->port, data[i+1], +flag); + i += 2; } WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 2/7] ARM: tegra: update device trees for USB binding rework
> -Original Message- > From: Stephen Warren [mailto:swar...@wwwdotorg.org] > Sent: Thursday, April 04, 2013 12:47 AM > To: Venu Byravarasu > Cc: gre...@linuxfoundation.org; ba...@ti.com; > st...@rowland.harvard.edu; linux-te...@vger.kernel.org; linux- > u...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v2 2/7] ARM: tegra: update device trees for USB binding > rework > > On 04/03/2013 02:41 AM, Venu Byravarasu wrote: > > This patch updates all Tegra board files so that they contain all the > > The binding documentation says that the vbus-supply property is required > in all cases. However, many of the DT files still don't have that > property even after this patch. That's inconsistent. Will edit the binding to have the vbus-supply only for otg cases. > > > diff --git a/arch/arm/boot/dts/tegra20-iris-512.dts > b/arch/arm/boot/dts/tegra20-iris-512.dts > > index 52f1103..c99eccc 100644 > > --- a/arch/arm/boot/dts/tegra20-iris-512.dts > > +++ b/arch/arm/boot/dts/tegra20-iris-512.dts > > @@ -41,6 +41,10 @@ > > dr_mode = "otg"; > > }; > > > > + usb-phy@c500 { > > + dr_mode = "otg"; > > + }; > > Since this port claims to be OTG-capable, presumably a vbus-supply > property is mandatory here? If you don't know enough about the board to > correctly set up such a regulator, lets just mark this port as host-only > for now; we can switch it back to OTG-mode later once someone implements > the required regulator. This won't lose any functionality, since we > don't support OTG-mode yet anyway. However, it will allow the device > tree to be correct/consistent in the mean time. Agree completely. Will update the patch accordingly and send for review. > > > diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts > b/arch/arm/boot/dts/tegra20-seaboard.dts > > > + usb-phy@c500 { > > + vbus-supply = <&vbus_reg>; > > + dr_mode = "otg"; > > + }; > > Where is the nvidia,vbus-gpio property? Since your code changes don't > actually implement use of the vbus-supply property yet, they will expect > the vbus-gpio property to exist in the PHY node. As of now Vbus-gpio is being used by EHCI driver only. As anyways we wanted to use vbus-supply, will implement its support in PHY driver and remove vbus-gpio from EHCI node. Hence did not add vbus-gpio to PHY DT nodes. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FTDI FT2232C issue
On Wed, Apr 3, 2013 at 12:16 PM, Yegor Yefremov wrote: > On Wed, Apr 3, 2013 at 9:12 AM, Yegor Yefremov > wrote: >> On Tue, Apr 2, 2013 at 4:36 PM, Alan Stern wrote: >>> On Tue, 2 Apr 2013, Yegor Yefremov wrote: >>> On Tue, Apr 2, 2013 at 4:14 PM, Alan Stern wrote: > On Tue, 2 Apr 2013, Yegor Yefremov wrote: > >> I'm making a burn-in test (see serialtest.py >> http://pastebin.com/pz47gaar) for our devices, that have built-in FTDI >> chips. If FT2232C is attached directly to the USB host controller >> port, the tests run properly. If I connect the chip to a USB-hub some >> of the tests show timeout errors (read() routine). On the newer host >> with USB3.0 this issue occurs on all USB ports where I attach FT2232C. >> I tried kernels 3.2, 3.5 and 3.8. >> >> The test procedure opens two ports, exchanges data and closes ports. >> If I don't open/close ports I don't encounter this issue. >> >> The FT4232H has no problems whether I attach it to USB HC or via HUB. >> Any idea how to workaround the problem? > > If you use a different type of hub, does the test work? So far I tested with 3 different hub chips SMSC, GL (USB2.0) and one USB3.0 hub - no difference. >>> >>> In that case, try collecting two usbmon traces showing what happens >>> during the test. In one trace, connect the FT2232C directly to the >>> computer port; in the other, go through the GL hub. See >>> Documentation/usb/usbmon.txt for instructions. >> >> Will do. > > The proper script with cycles support '-n' > http://dl.dropbox.com/u/6486923/serialtest.py > raw text HCD: http://dl.dropbox.com/u/6486923/ftdi_sniff_hcd.out > tcpdump HCD: http://dl.dropbox.com/u/6486923/ftdi_sniff_hcd.pcap > > HCD sniffs show 2 cycles > > raw text HUB: http://dl.dropbox.com/u/6486923/ftdi_sniff_hub.out > tcpdump HUB: http://dl.dropbox.com/u/6486923/ftdi_sniff_hub.pcap > > HUB sniffs have different number of cycles regarding when the test failed. > > Let me know if you need more info or sniffs. I reworked my test script, so that I can tell timeout from wrong string. I have two cases now. On my am335x based system I get timeout with 3.2 kernel and errors like here (http://www.newit.co.uk/forum/index.php?topic=313.0) for 3.8. On x86 I get fully wrong string. For example, I send "test string" and get something like that 57 D6 2E 17 A4 2E 5D AE 5A EB F6 on the other port. I've also update the sniffs. They contain 20 cycles each. Let's take this sniff ftdi_sniff_hub.out As far as I understand it, it is normal case, where the data was received correctly: f1172780 1591104533 S Bo:1:004:2 -115 11 = 74657374 20737472 696e67 f1172780 1591104593 C Bo:1:004:2 0 11 > f1172400 1591105218 C Bi:1:004:3 0 8 = b1607465 73742073 f1172400 1591105223 S Bi:1:004:3 -115 512 < f1172b00 1591105226 C Bi:1:004:1 0 2 = b100 f1172b00 1591105227 S Bi:1:004:1 -115 512 < f1172400 1591106217 C Bi:1:004:3 0 7 = b1607472 696e67 Here everything will be received, but byte per byte: f1172780 1591706391 S Bo:1:004:2 -115 11 = 74657374 20737472 696e67 f1172780 1591706487 C Bo:1:004:2 0 11 > f1172400 1591707111 C Bi:1:004:3 0 2 = b160 f1172400 1591707115 S Bi:1:004:3 -115 512 < f1172b00 1591707117 C Bi:1:004:1 0 2 = b100 f1172b00 1591707118 S Bi:1:004:1 -115 512 < f1172400 1591708112 C Bi:1:004:3 0 3 = b16074 f1172400 1591708117 S Bi:1:004:3 -115 512 < f1172b00 1591708120 C Bi:1:004:1 0 2 = b100 f1172b00 1591708121 S Bi:1:004:1 -115 512 < f1172400 1591709112 C Bi:1:004:3 0 3 = b16065 f1172400 1591709116 S Bi:1:004:3 -115 512 < f1172b00 1591709118 C Bi:1:004:1 0 2 = b100 f1172b00 1591709119 S Bi:1:004:1 -115 512 < f1172400 1591710111 C Bi:1:004:3 0 3 = b16073 f1172400 1591710116 S Bi:1:004:3 -115 512 < f1172b00 1591710118 C Bi:1:004:1 0 2 = b100 f1172b00 1591710119 S Bi:1:004:1 -115 512 < f1172400 159172 C Bi:1:004:3 0 3 = b16074 f1172400 159176 S Bi:1:004:3 -115 512 < f1172b00 159178 C Bi:1:004:1 0 2 = b100 f1172b00 159179 S Bi:1:004:1 -115 512 < f1172400 1591712113 C Bi:1:004:3 0 3 = b16020 f1172400 1591712117 S Bi:1:004:3 -115 512 < f1172b00 1591712119 C Bi:1:004:1 0 2 = b100 f1172b00 1591712120 S Bi:1:004:1 -115 512 < f1172400 1591713112 C Bi:1:004:3 0 3 = b16073 f1172400 1591713116 S Bi:1:004:3 -115 512 < f1172b00 1591713119 C Bi:1:004:1 0 2 = b100 f1172b00 1591713120 S Bi:1:004:1 -115 512 < f1172400 1591714112 C Bi:1:004:3 0 3 = b16074 f1172400 1591714117 S Bi:1:004:3 -115 512 < f1172b00 1591714119 C Bi:1:004:1 0 2 = b100 f1172b00 1591714120 S Bi:1:004:1 -115 512 < f1172400 1591715111 C Bi:1:004:3 0 3 = b16072 f1172400 1591715116 S Bi:1:004:3 -115 512 < f1172b00 1591715118 C Bi:1:004:1 0 2 = b100 f1172b00 1591715119 S Bi:1:004:1 -115 512 < f1172400 1591716115 C Bi:1:004:3 0 2 = b160 f1172400 1591716118 S Bi:1:004:3 -115 512 < f1172b00 1591716121 C Bi:1:004:1 0 2 = b160 f1172b00 1591716122 S Bi:1:004:1 -115 512 < f1172400 1591717111 C Bi:1:004:3 0 3 = b16069 f1172400
RE: [PATCH v2 6/7] usb: phy: tegra: Add error handling & clean up.
> -Original Message- > From: Stephen Warren [mailto:swar...@wwwdotorg.org] > Sent: Thursday, April 04, 2013 1:06 AM > To: Venu Byravarasu > Cc: gre...@linuxfoundation.org; ba...@ti.com; > st...@rowland.harvard.edu; linux-te...@vger.kernel.org; linux- > u...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v2 6/7] usb: phy: tegra: Add error handling & clean up. > > On 04/03/2013 02:41 AM, Venu Byravarasu wrote: > > Check return values from all GPIO APIs and handle errors accordingly. > > Remove clk_disable_unprepare which is no more needed. > > checkpatch fails: As these are checkpatch warnings only, did not pay much attention on that. Any how will fix it in next update. > > > WARNING: line over 80 characters > > #27: FILE: drivers/usb/phy/phy-tegra-usb.c:547: > > + dev_err(phy->dev, "gpio %d direction not set\n", phy- > >reset_gpio); > > > > WARNING: line over 80 characters > > #34: FILE: drivers/usb/phy/phy-tegra-usb.c:553: > > + dev_err(phy->dev, "gpio %d direction not set\n", phy- > >reset_gpio); > > > > total: 0 errors, 2 warnings, 72 lines checked > > > > 0006-usb-phy-tegra-Add-error-handling-clean-up.patch has style problems, > please review. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/7] ARM: tegra: finalize USB EHCI and PHY bindings
> -Original Message- > From: linux-tegra-ow...@vger.kernel.org [mailto:linux-tegra- > ow...@vger.kernel.org] On Behalf Of Stephen Warren > Sent: Thursday, April 04, 2013 12:38 AM > To: Venu Byravarasu > Cc: gre...@linuxfoundation.org; ba...@ti.com; > st...@rowland.harvard.edu; linux-te...@vger.kernel.org; linux- > u...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v2 1/7] ARM: tegra: finalize USB EHCI and PHY bindings > > On 04/03/2013 02:41 AM, Venu Byravarasu wrote: > > The existing Tegra USB bindings have a few issues: > ... > > This patch fixes the binding definition to resolve these issues. > > > diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb- > phy.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb- > phy.txt > > > Required properties : > ... > > + - vbus-supply: regulator for VBUS > > Doesn't the driver only need to control VBUS if the port is in OTG mode? > > If there is no VBUS control, and the HW provides VBUS, I think that the > port can only operate in host mode. > > If there is no VBUS control, and the HW does not provide VBUS, I think > that the port can only operate in peripheral mode. > > If there is VBUS control, then shouldn't the port always operate in OTG > mode, or are there other reasons to control VBUS even in host-only mode? > > If VBUS control is only useful for OTG mode, then I think the > vbus-supply property should be documented in a "Required properties for > dr_mode == otg" section. Agree, will do it in next patch update. > > I assume that VBUS control makes no sense for a peripheral-mode-only > port, so if VBUS control is useful for host-only mode as well as OTG > mode, then I think the vbus-supply property should be documented in a > "Required properties for dr_mode != peripheral" section. > > Is the following table correct? > > Port operating mode: hostperipheral otg > -- --- > VBUS control required:no no yes > VBUS control useful: yes[1]? no yes > > [1] perhaps for power-saving/suspend??? For waking system up from sleep via devices connected to USB, I think Vbus is always kept ON in host mode. > -- > To unsubscribe from this list: send the line "unsubscribe linux-tegra" 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
USB ehci suspend/resume on beagleboard
correcting misleading subject line. was (Re: MUSB regression in linux next at least for pandboard) On 04/04/2013 02:36 PM, Roger Quadros wrote: > Hi, > > On 02/07/2013 04:10 PM, Grazvydas Ignotas wrote: >> On Thu, Feb 7, 2013 at 11:16 AM, Roger Quadros wrote: >> >>> It seems the beagleboard problem is related to OMAP silicon errata [1]. >>> Apparently, remote wakeup as well as host issued wakeup break omap-ehci and >>> have >>> nothing to do with the hub or it's driver. >>> >>> I'll work on this issue after I'm done with device tree migration. >> >> Looking forward to this, mainline has been suffering from this since >> almost forever.. >> >> > > Unfortunately, there is another errata [2] that makes it impossible to > support suspend/resume on the beagleboard. > > cheers, > -roger > > [2] Advisory 3.1.1.195 HSUSB Interoperability Issue With SMSC USB3320 PHY > http://www.ti.com/general/docs/lit/getliterature.tsp?literatureNumber=sprz278f&fileType=pdf > -- 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: MUSB regression in linux next at least for pandboard
Hi, On 02/07/2013 04:10 PM, Grazvydas Ignotas wrote: > On Thu, Feb 7, 2013 at 11:16 AM, Roger Quadros wrote: > >> It seems the beagleboard problem is related to OMAP silicon errata [1]. >> Apparently, remote wakeup as well as host issued wakeup break omap-ehci and >> have >> nothing to do with the hub or it's driver. >> >> I'll work on this issue after I'm done with device tree migration. > > Looking forward to this, mainline has been suffering from this since > almost forever.. > > Unfortunately, there is another errata [2] that makes it impossible to support suspend/resume on the beagleboard. cheers, -roger [2] Advisory 3.1.1.195 HSUSB Interoperability Issue With SMSC USB3320 PHY http://www.ti.com/general/docs/lit/getliterature.tsp?literatureNumber=sprz278f&fileType=pdf -- 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
Odp: Re: Odp: Re: Odp: Re: kernel 3.9-rc5 - system does not show information about the new drive.
Dnia Środa, 3 Kwietnia 2013 21:33 Greg KH napisał(a) > On Wed, Apr 03, 2013 at 09:02:50PM +0200, Tomasz Miś wrote: > > Dnia Środa, 3 Kwietnia 2013 20:26 Greg KH napisał(a) > > > > > > A: No. > > > Q: Should I include quotations after my reply? > > > > > > http://daringfireball.net/2007/07/on_top > > > > > > On Wed, Apr 03, 2013 at 08:16:47PM +0200, Tomasz Miś wrote: > > > > Welcome. Yes You're right. However, I reported that at this point, > > > > because of the effects it causes. You can not see absolutely no > > > > negative side effects of network solutions. However, there are > > > > "effects" about which I wrote on the USB drive. > > > > > > I agree, but this is not a USB issue, it's a networking / udev issue, > > > there's nothing we can do here. > > > > > > > Established "symptoms" appear both in my kernels [pure vanilla sources > > > > + patch removes the message fail tsc ...] and the mainline kernels for > > > > Ubuntu. > > > > > > This works just fine for me here with a newer version of udev, perhaps > > > your distro needs to update to a more recent version? > > > > > > Again, there's nothing the USB developers can do here, sorry, > > > > > > good luck, > > > > > > greg k-h > > > > That's what I was beginning to suspect - udevd version. But why under > > the rc4 is ok?!?. I just wondered if this irresistible desire to > > remain by Canonical at the "old" udevd = and "not accept" by them > > systemd not rebound soon hiccup ... so I have version 175 udev. But... > > what could be changed between rc4 and rc5 that this effect was made - > > on the side of notifications about USB drives? > > The kernel change is commit 14134f6584212d585b310ce95428014b653dfaf6 if > you are curious. As to why that causes the change in udev, I do not > know, ask the udev and networking developers about this. > > Good luck, > > greg k-h Thank You for response. As for the changes in the kernel ... I follow up changes in git ;) ... but this time I can not even track down where to look. I thought that you show me more precisely the possible search area ;). I rather, try to look for any errors and try to remove them by myself - I submit it, if I can not do it myself - for example, errors in Realtek rts5139 driver [by the way the code of this driver still is "medium" and crashes ehci and sometimes usb-storage after about 1 hour of action]. There are also "mistakes" that I myself repair, but I do not submit it - eg eth driver: Intel: e1000e and IGB and from the 3.9.x kernel in Marvell`s driver ... but they "is revealed" only if the kernel is subjected to optimization [fairly deep] at the level of the compiler. I builds four versions of the kernel for the various machines including version i7 specially optimized for Intel processors [i3, i5 and i7] and Brazos [for newer AMD processors - from Athlon II X4] and publish the Ubuntu community - mainly in Poland. I asked Colleague which works with me on our project [NeteXt'73 - Ubuntu optymizator] to check how the kernel 3.9-rc5 is working [generic from Ubuntu ppa mailine] under Ubuntu 13.04 [under virtual machine] - there is udev in version 175 and libudev in version 198 ... unfortunately again "shows" udevd `errors`. https://dl.dropbox.com/u/72313101/screeny/udevd-fail.png Regards Tomasz Miś -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
Hi, On Thursday 04 April 2013 04:11 PM, Sylwester Nawrocki wrote: Hi, On 04/04/2013 11:21 AM, Kishon Vijay Abraham I wrote: On Thursday 04 April 2013 03:16 AM, Sylwester Nawrocki wrote: On 04/03/2013 02:53 PM, Kishon Vijay Abraham I wrote: +4. Getting a reference to the PHY + +Before the controller can make use of the PHY, it has to get a reference to +it. This framework provides 6 APIs to get a reference to the PHY. + +struct phy *phy_get(struct device *dev, int index); +struct phy *devm_phy_get(struct device *dev, int index); +struct phy *of_phy_get(struct device *dev, const char *phandle, int index); +struct phy *devm_of_phy_get(struct device *dev, const char *phandle, int index); Why do we need separate functions for dt and non-dt ? Couldn't it be handled internally by the framework ? So the PHY users can use same calls for dt and non-dt, like in case of e.g. the regulators API ? yeah. Indeed it makes sense to do it that way. Also signatures of some functions are different now: extern struct phy *phy_get(struct device *dev, int index); extern struct phy *devm_phy_get(struct device *dev, int index); extern struct phy *of_phy_get(struct device *dev, int index); extern struct phy *devm_of_phy_get(struct device *dev, int index); My bad :-( BTW, I think "extern" could be dropped, does it have any significance in function declarations in header files ? it shouldn't have any effect I guess. It's harmless nevertheless. Yup. +struct phy *of_phy_get_byname(struct device *dev, const char *string); +struct phy *devm_of_phy_get_byname(struct device *dev, const char *string); --- /dev/null +++ b/drivers/phy/phy-core.c @@ -0,0 +1,616 @@ +static struct phy *of_phy_lookup(struct device_node *node) +{ +struct phy *phy; +struct device *dev; +struct class_dev_iter iter; + +class_dev_iter_init(&iter, phy_class, NULL, NULL); There is currently nothing preventing a call to this function before phy_class is initialized. It happened during my tests, and the nice stack dump showed that it was the PHY user attempting to get the PHY before the framework got initialized. So either phy_core_init should be a subsys_initcall or we need a better protection against phy_class being NULL or ERR_PTR in more places. Whats the initcall used in your PHY user? Looks like more people prefer having It happened in a regular platform driver probe() callback. module_init and any issue because of it should be fixed in PHY providers and PHY users. OK. In fact this uncovered some issues in the MIPI DSI interface driver (some unexpected interrupts). But this should just be fixed in those drivers anyway. Now the DSI interface driver needs to wait for the PHY to be registered and ready, so the initialization will likely be changed regardless the framework initializes in module_init or earlier. +while ((dev = class_dev_iter_next(&iter))) { +phy = container_of(dev, struct phy, dev); +if (node != phy->of_node) +continue; + +class_dev_iter_exit(&iter); +return phy; +} + +class_dev_iter_exit(&iter); +return ERR_PTR(-EPROBE_DEFER); +} +/** + * of_phy_get() - lookup and obtain a reference to a phy by phandle + * @dev: device that requests this phy + * @index: the index of the phy + * + * Returns the phy associated with the given phandle value, + * after getting a refcount to it or -ENODEV if there is no such phy or + * -EPROBE_DEFER if there is a phandle to the phy, but the device is + * not yet loaded. + */ +struct phy *of_phy_get(struct device *dev, int index) +{ +int ret; +struct phy *phy = NULL; +struct phy_bind *phy_map = NULL; +struct of_phandle_args args; +struct device_node *node; + +if (!dev->of_node) { +dev_dbg(dev, "device does not have a device node entry\n"); +return ERR_PTR(-EINVAL); +} + +ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells", +index,&args); +if (ret) { +dev_dbg(dev, "failed to get phy in %s node\n", +dev->of_node->full_name); +return ERR_PTR(-ENODEV); +} + +phy = of_phy_lookup(args.np); +if (IS_ERR(phy) || !try_module_get(phy->ops->owner)) { +phy = ERR_PTR(-EPROBE_DEFER); +goto err0; +} + +phy = phy->ops->of_xlate(phy,&args); +if (IS_ERR(phy)) +goto err1; + +phy_map = phy_bind(dev_name(dev), index, dev_name(&phy->dev)); +if (!IS_ERR(phy_map)) { +phy_map->phy = phy; +phy_map->auto_bind = true; Shouldn't it be done with the phy_bind_mutex held ? I guess an unlocked version of the phy_bind functions is needed, so it can be used internally. The locking is done inside phy_bind function. I'm not sure but I vaguely remember getting a dump stack (incorrect mutex ordering or something) when trying to have phy_bind_mutex here. I can check it again. Yes, IIUC the locking needs to be reworked a bit so "phy_map" is not modified without the mute
Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
Hi, On 04/04/2013 11:21 AM, Kishon Vijay Abraham I wrote: > On Thursday 04 April 2013 03:16 AM, Sylwester Nawrocki wrote: >> On 04/03/2013 02:53 PM, Kishon Vijay Abraham I wrote: >>> +4. Getting a reference to the PHY >>> + >>> +Before the controller can make use of the PHY, it has to get a >>> reference to >>> +it. This framework provides 6 APIs to get a reference to the PHY. >>> + >>> +struct phy *phy_get(struct device *dev, int index); >>> +struct phy *devm_phy_get(struct device *dev, int index); >>> +struct phy *of_phy_get(struct device *dev, const char *phandle, int >>> index); >>> +struct phy *devm_of_phy_get(struct device *dev, const char *phandle, >>> int index); >> >> Why do we need separate functions for dt and non-dt ? Couldn't it be >> handled >> internally by the framework ? So the PHY users can use same calls for dt >> and non-dt, like in case of e.g. the regulators API ? > > yeah. Indeed it makes sense to do it that way. > >> Also signatures of some functions are different now: >> >> extern struct phy *phy_get(struct device *dev, int index); >> extern struct phy *devm_phy_get(struct device *dev, int index); >> extern struct phy *of_phy_get(struct device *dev, int index); >> extern struct phy *devm_of_phy_get(struct device *dev, int index); > > My bad :-( > >> BTW, I think "extern" could be dropped, does it have any significance in >> function declarations in header files ? > > it shouldn't have any effect I guess. It's harmless nevertheless. Yup. >>> +struct phy *of_phy_get_byname(struct device *dev, const char *string); >>> +struct phy *devm_of_phy_get_byname(struct device *dev, const char >>> *string); >>> --- /dev/null >>> +++ b/drivers/phy/phy-core.c >>> @@ -0,0 +1,616 @@ >> >>> +static struct phy *of_phy_lookup(struct device_node *node) >>> +{ >>> +struct phy *phy; >>> +struct device *dev; >>> +struct class_dev_iter iter; >>> + >>> +class_dev_iter_init(&iter, phy_class, NULL, NULL); >> >> There is currently nothing preventing a call to this function before >> phy_class is initialized. It happened during my tests, and the nice >> stack dump showed that it was the PHY user attempting to get the PHY >> before the framework got initialized. So either phy_core_init should >> be a subsys_initcall or we need a better protection against phy_class >> being NULL or ERR_PTR in more places. > > Whats the initcall used in your PHY user? Looks like more people prefer having It happened in a regular platform driver probe() callback. > module_init and any issue because of it should be fixed in PHY providers and > PHY users. OK. In fact this uncovered some issues in the MIPI DSI interface driver (some unexpected interrupts). But this should just be fixed in those drivers anyway. Now the DSI interface driver needs to wait for the PHY to be registered and ready, so the initialization will likely be changed regardless the framework initializes in module_init or earlier. >>> +while ((dev = class_dev_iter_next(&iter))) { >>> +phy = container_of(dev, struct phy, dev); >>> +if (node != phy->of_node) >>> +continue; >>> + >>> +class_dev_iter_exit(&iter); >>> +return phy; >>> +} >>> + >>> +class_dev_iter_exit(&iter); >>> +return ERR_PTR(-EPROBE_DEFER); >>> +} >> >>> +/** >>> + * of_phy_get() - lookup and obtain a reference to a phy by phandle >>> + * @dev: device that requests this phy >>> + * @index: the index of the phy >>> + * >>> + * Returns the phy associated with the given phandle value, >>> + * after getting a refcount to it or -ENODEV if there is no such phy or >>> + * -EPROBE_DEFER if there is a phandle to the phy, but the device is >>> + * not yet loaded. >>> + */ >>> +struct phy *of_phy_get(struct device *dev, int index) >>> +{ >>> +int ret; >>> +struct phy *phy = NULL; >>> +struct phy_bind *phy_map = NULL; >>> +struct of_phandle_args args; >>> +struct device_node *node; >>> + >>> +if (!dev->of_node) { >>> +dev_dbg(dev, "device does not have a device node entry\n"); >>> +return ERR_PTR(-EINVAL); >>> +} >>> + >>> +ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells", >>> +index,&args); >>> +if (ret) { >>> +dev_dbg(dev, "failed to get phy in %s node\n", >>> +dev->of_node->full_name); >>> +return ERR_PTR(-ENODEV); >>> +} >>> + >>> +phy = of_phy_lookup(args.np); >>> +if (IS_ERR(phy) || !try_module_get(phy->ops->owner)) { >>> +phy = ERR_PTR(-EPROBE_DEFER); >>> +goto err0; >>> +} >>> + >>> +phy = phy->ops->of_xlate(phy,&args); >>> +if (IS_ERR(phy)) >>> +goto err1; >>> + >>> +phy_map = phy_bind(dev_name(dev), index, dev_name(&phy->dev)); >>> +if (!IS_ERR(phy_map)) { >>> +phy_map->phy = phy; >>> +phy_map->auto_bind = true; >> >> Shouldn't it be done with the phy_bind_mutex held ? I guess an unlocked >> version of the phy_bind functions is needed, so