Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver
Hi, On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski Signed-off-by: Kyungmin Park --- .../devicetree/bindings/phy/samsung-usbphy.txt | 52 drivers/phy/Kconfig| 23 +- drivers/phy/Makefile |4 + drivers/phy/phy-exynos-usb2.c | 234 ++ drivers/phy/phy-exynos-usb2.h | 87 ++ drivers/phy/phy-exynos4210-usb2.c | 272 drivers/phy/phy-exynos4212-usb2.c | 324 7 files changed, 995 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb2.c create mode 100644 drivers/phy/phy-exynos-usb2.h create mode 100644 drivers/phy/phy-exynos4210-usb2.c create mode 100644 drivers/phy/phy-exynos4212-usb2.c diff --git a/Documentation/devicetree/bindings/phy/samsung-usbphy.txt b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt new file mode 100644 index 000..c8fbc70 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung-usbphy.txt @@ -0,0 +1,52 @@ +Samsung S5P/EXYNOS SoC series USB PHY +- + +Required properties: +- compatible : should be one of the listed compatibles: + - "samsung,exynos4210-usbphy" + - "samsung,exynos4212-usbphy" +- reg : a list of registers used by phy driver + - first and obligatory is the location of phy modules registers + - second and also required is the location of isolation registers + (isolation registers control the physical connection between the in + SoC modules and outside of the SoC, this also can be called enable + control in the documentation of the SoC) + - third is the location of the mode switch register, this only applies + to SoCs that have such a feature; mode switching enables to have + both host and device used the same SoC pins and is commonly used + when OTG is supported +- #phy-cells : from the generic phy bindings, must be 1; +- clocks and clock-names: + - the "phy" clocks is required by the phy module + - other clocks are associated by name with their respective phys and + are used to determine the value of the clock settings register + +The second cell in the PHY specifier identifies the PHY, its meaning is +compatible dependent. For the currently supported SoCs (Exynos 4210 and +Exynos 4212) it is as follows: + 0 - USB device, + 1 - USB host, + 2 - HSIC0, + 3 - HSIC1, + +Example: + +For Exynos 4412 (compatible with Exynos 4212): + +exynos_usbphy: exynos-usbphy@125B { + compatible = "samsung,exynos4212-usbphy"; + reg = <0x125B 0x100 0x10020704 0x0c 0x1001021c 0x4>; + clocks = <&clock 305>, <&clock 2>, <&clock 2>, <&clock 2>, + <&clock 2>; + clock-names = "phy", "device", "host", "hsic0", "hsic1"; + status = "okay"; + #phy-cells = <1>; +}; + +Then the PHY can be used in other nodes such as: + +ehci@1258 { + status = "okay"; + phys = <&exynos_usbphy 2>; + phy-names = "hsic0"; +}; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index a344f3d..bdf0fab 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -14,7 +14,7 @@ config GENERIC_PHY API by which phy drivers can create PHY using the phy framework and phy users can obtain reference to the PHY. All the users of this framework should select this config. - + spurious change config PHY_EXYNOS_MIPI_VIDEO tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver" help @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO help Support for Display Port PHY found on Samsung EXYNOS SoCs. +config PHY_EXYNOS_USB2 + tristate "Samsung USB 2.0 PHY driver" + help + Enable this to support Samsung USB phy helper driver for Samsung SoCs. + This driver provides common interface to interact, for Samsung + USB 2.0 PHY driver. I still think we can get rid of this helper driver and have a single driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2. + +config PHY_EXYNOS4210_USB2 + bool "Support for Exynos 4210" + depends on PHY_EXYNOS_USB2 + depends on CPU_EXYNOS4210 + help + Enable USB PHY support for Exynos 4210 + +config PHY_EXYNOS4212_USB2 + bool "Support for Exynos 4212" + depends on PHY_EXYNOS_USB2 + depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) + help + Enable USB PHY support for Exynos 4212 + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Ma
[PATCH] usb: musb: only cancel work if it is initialized
Since commit c5340bd14 ("usb: musb: cancel work on removal") the workqueue is cancelled but then if we bail out before the workqueue is setup we get this: |INFO: trying to register non-static key. |the code is fine but needs lockdep annotation. |turning off the locking correctness validator. |CPU: 0 PID: 708 Comm: modprobe Not tainted 3.12.0+ #435 |[] (lock_acquire+0xf0/0x108) from [] (flush_work+0x38/0x2ec) |[] (flush_work+0x38/0x2ec) from [] (__cancel_work_timer+0xa0/0x134) |[] (__cancel_work_timer+0xa0/0x134) from [] (musb_free+0x40/0x60 [musb_hdrc]) |[] (musb_free+0x40/0x60 [musb_hdrc]) from [] (musb_probe+0x678/0xb78 [musb_hdrc]) |[] (musb_probe+0x678/0xb78 [musb_hdrc]) from [] (platform_drv_probe+0x1c/0x24) |[] (platform_drv_probe+0x1c/0x24) from [] (driver_probe_device+0x90/0x224) |[] (driver_probe_device+0x90/0x224) from [] (bus_for_each_drv+0x60/0x8c) |[] (bus_for_each_drv+0x60/0x8c) from [] (device_attach+0x80/0xa4) |[] (device_attach+0x80/0xa4) from [] (bus_probe_device+0x88/0xac) |[] (bus_probe_device+0x88/0xac) from [] (device_add+0x388/0x6c8) |[] (device_add+0x388/0x6c8) from [] (platform_device_add+0x188/0x22c) |[] (platform_device_add+0x188/0x22c) from [] (dsps_probe+0x294/0x394 [musb_dsps]) |[] (dsps_probe+0x294/0x394 [musb_dsps]) from [] (platform_drv_probe+0x1c/0x24) |platform musb-hdrc.1.auto: Driver musb-hdrc requests probe deferral |musb-hdrc musb-hdrc.1.auto: musb_init_controller failed with status -517 This patch moves the init part to earlier part so it can be cleaned as part of the fail3 label because now it is surrounded by the fail4 label. Step two is to remove it from musb_free() and add it to the two cleanup paths (error path and device removal) separately. Cc: sta...@vger.kernel.org Signed-off-by: Sebastian Andrzej Siewior --- drivers/usb/musb/musb_core.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 3f0d2b1..7bf0031 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1809,7 +1809,6 @@ static void musb_free(struct musb *musb) disable_irq_wake(musb->nIrq); free_irq(musb->nIrq, musb); } - cancel_work_sync(&musb->irq_work); musb_host_free(musb); } @@ -1896,6 +1895,9 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) musb_platform_disable(musb); musb_generic_disable(musb); + /* Init IRQ workqueue before request_irq */ + INIT_WORK(&musb->irq_work, musb_irq_work); + /* setup musb parts of the core (especially endpoints) */ status = musb_core_init(plat->config->multipoint ? MUSB_CONTROLLER_MHDRC @@ -1905,9 +1907,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) setup_timer(&musb->otg_timer, musb_otg_timer_func, (unsigned long) musb); - /* Init IRQ workqueue before request_irq */ - INIT_WORK(&musb->irq_work, musb_irq_work); - /* attach to the IRQ */ if (request_irq(nIrq, musb->isr, 0, dev_name(dev), musb)) { dev_err(dev, "request_irq %d failed!\n", nIrq); @@ -1981,6 +1980,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) musb_host_cleanup(musb); fail3: + cancel_work_sync(&musb->irq_work); if (musb->dma_controller) dma_controller_destroy(musb->dma_controller); fail2_5: @@ -2043,6 +2043,7 @@ static int musb_remove(struct platform_device *pdev) if (musb->dma_controller) dma_controller_destroy(musb->dma_controller); + cancel_work_sync(&musb->irq_work); musb_free(musb); device_init_wakeup(dev, 0); return 0; -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
XHCI: Ring expansion failure
Hi By performing iterative port suspend and resume (which results in function suspend and resume), ring expansion failure is observed. Attached device has multiple interfaces for which interface host drivers are unlinking the urbs during function suspend and submitting urbs during resume. For the cases when dequeue ptr is close to the end of deq segment, value of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes high when function suspend is over. At the time of resume this high value is causing to trigger ring expansion in the middle of submitting urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the interface host drivers only request one TRB per urb (num_trb =1). Ring expansion logic is doubling the num of previously allocated segments every time. This is causing the num of segments to grow at fast pace (at the time of failure, one of the ep rings num_seg was 128), even though it is also increasing num_trbs_free. Even for the cases where we observe data transfer as well as suspend & resume (which are normal scenarios) ring expansion failure is easily observed due to large number of segments being allocated for ever. This means triggering of ring expansion will result in failure of further expansion at some point of time. I was trying to think of some options to reduce ring expansion with respect to suspend resume scenario: - 1) Can we reset enq and deq ptr to point to ep_ring first seg once all the urbs are unlinked (function suspend)? 2) Can we free the segments at the time of suspend and allocate segments back when resume happens? In general, as per the spec can we also implement shrinking of transfer ring (4.9.2.4). Please help to provide your comments on the options above (which can easily fit in to the current design) also feel free to add better options to consider to avoid ring expansion failures which can be added easily to the current implementation. Currently I am using 3.10 kernel and back ported recent fixes on xhci. Thanks, Hemant -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: xhci: Optimise setup of isochronous transfers
> -Original Message- > From: Andiry Xu [mailto:and...@gmail.com] > Sent: 05 November 2013 17:40 > To: David Laight > Cc: Linux-USB; Sarah Sharp > Subject: Re: [PATCH] usb: xhci: Optimise setup of isochronous transfers > > On Tue, Nov 5, 2013 at 6:21 AM, David Laight wrote: > > > > Close inspection shows that xhci_queue_isoc_tx() can only fail if > > prepare_transfer() gets an error from usb_hcd_link_urb_to_ep() for > > the first TD. The call to prepare_ring() cannot fail because an initial > > check is made to ensure the ring has enough space for all the TD. ... > > - ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index, > > - urb->stream_id, trbs_per_td, urb, i, > > mem_flags); > > - if (ret < 0) { > > - if (i == 0) > > - return ret; > > - goto cleanup; > > - } > > + /* prepare_transfer() was called earlier for the first TRB. > > +* It cannot fail for later TRBs. > > +*/ > > + if (i != 0) > > + prepare_transfer(xhci, xhci->devs[slot_id], > > ep_index, > > + urb->stream_id, trbs_per_td, urb, i, > > + mem_flags); ... > > -cleanup: > > - /* Clean up a partially enqueued isoc transfer. */ > > - > > - for (i--; i >= 0; i--) > > - list_del_init(&urb_priv->td[i].td_list); > > - > > - /* Use the first TD as a temporary variable to turn the TDs we've > > queued > > -* into No-ops with a software-owned cycle bit. That way the > > hardware > > -* won't accidentally start executing bogus TDs when we partially > > -* overwrite them. td->first_trb and td->start_seg are already set. > > -*/ > > - urb_priv->td[0].last_trb = ep_ring->enqueue; > > - /* Every TRB except the first & last will have its cycle bit > > flipped. */ > > - td_to_noop(xhci, ep_ring, &urb_priv->td[0], true); > > - > > - /* Reset the ring enqueue back to the first TRB and its cycle bit. > > */ > > - ep_ring->enqueue = urb_priv->td[0].first_trb; > > - ep_ring->enq_seg = urb_priv->td[0].start_seg; > > - /* The cycle bit in the first TRB won't be modified, get its > > inverse. */ > > - ep_ring->cycle_state = (ep_ring->enqueue->generic.field[3] & > > - TRB_CYCLE) ^ TRB_CYCLE; > > - ep_ring->num_trbs_free = ep_ring->num_trbs_free_temp; > > - usb_hcd_unlink_urb_from_ep(bus_to_hcd(urb->dev->bus), urb); > > - return ret; > > } > > This code is there for a reason. See commit > 522989a27c7badb608155b1f1dea3487ed431f74 for details. I presume you means this bit: When an isochronous transfer is enqueued, xhci_queue_isoc_tx_prepare() will ensure that there is enough room on the transfer rings for all of the isochronous TDs for that URB. However, when xhci_queue_isoc_tx() is enqueueing individual isoc TDs, the prepare_transfer() function can fail if the endpoint state has changed to disabled, error, or some other unknown state. The check for the endpoint state is inside prepare_ring() when called from prepare_transfer(). This is now only called for the first TD so cannot fail for subsequent ones. So if there is an error it will be handled after the ring (etc) contains all of the TD and they will be tidied up together. > Also, do you remove the return 0 sentence on purpose? Yes, function return type is now 'void'. David -- 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: XHCI: Ring expansion failure
> By performing iterative port suspend and resume (which results in function > suspend and resume), ring expansion failure is observed. Attached device > has multiple interfaces for which interface host drivers are unlinking the > urbs during function suspend and submitting urbs during resume. > > For the cases when dequeue ptr is close to the end of deq segment, value > of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes > high when function suspend is over. At the time of resume this high value > is causing to trigger ring expansion in the middle of submitting > urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the > interface host drivers only request one TRB per urb (num_trb =1). Ring > expansion logic is doubling the num of previously allocated segments every > time. This is causing the num of segments to grow at fast pace (at the > time of failure, one of the ep rings num_seg was 128), even though it is > also increasing num_trbs_free. Something must be miscalculating num_trbs_free during suspend/resume. ... > I was trying to think of some options to reduce ring expansion with > respect to suspend resume scenario: - > 1)Can we reset enq and deq ptr to point to ep_ring first seg once all the > urbs are unlinked (function suspend)? > 2)Can we free the segments at the time of suspend and allocate segments > back when resume happens? > > In general, as per the spec can we also implement shrinking of transfer > ring (4.9.2.4). I think the bulk tx code should be willing to queue urb when there aren't enough ring entries. Then all the code to handle multiple ring segments could be ripped out. (possibly allocating 128 entries for some of the rings - only 2k). This would simplify a lot of code. David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver
Hi, > From: Jingoo Han [mailto:jg1@samsung.com] > Sent: Wednesday, November 06, 2013 2:03 AM > > On Wednesday, November 06, 2013 1:13 AM, Kamil Debski wrote: > > > > Add a new driver for the Exynos USB PHY. The new driver uses the > > generic PHY framework. The driver includes support for the Exynos > 4x10 > > and 4x12 SoC families. > > > > Signed-off-by: Kamil Debski > > Signed-off-by: Kyungmin Park > > --- > > .../devicetree/bindings/phy/samsung-usbphy.txt | 52 > > drivers/phy/Kconfig| 23 +- > > drivers/phy/Makefile |4 + > > drivers/phy/phy-exynos-usb2.c | 234 > ++ > > drivers/phy/phy-exynos-usb2.h | 87 ++ > > drivers/phy/phy-exynos4210-usb2.c | 272 > > > drivers/phy/phy-exynos4212-usb2.c | 324 > > > 7 files changed, 995 insertions(+), 1 deletion(-) create mode > 100644 > > Documentation/devicetree/bindings/phy/samsung-usbphy.txt > > create mode 100644 drivers/phy/phy-exynos-usb2.c create mode 100644 > > drivers/phy/phy-exynos-usb2.h create mode 100644 > > drivers/phy/phy-exynos4210-usb2.c create mode 100644 > > drivers/phy/phy-exynos4212-usb2.c > > [] > > > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index > > d0caae9..c87bc65 100644 > > --- a/drivers/phy/Makefile > > +++ b/drivers/phy/Makefile > > @@ -7,3 +7,7 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp- > video.o > > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o > > obj-$(CONFIG_OMAP_USB2)+= phy-omap-usb2.o > > obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o > > +obj-$(CONFIG_PHY_EXYNOS5250_USB) += phy-exynos5250-usb.o > > Hi Kamil, > > Would you add 'phy-exynos5250-usb.c' file? :-) > > Now, I am testing Exynos5250 USB HOST with your patchset. > However, it makes error because there is no PHY driver for Exynos5250. > > WARNING: CPU: 0 PID: 18 at drivers/phy/phy-core.c:366 > phy_get+0x1e8/0x224() > Device: exynos-ehci > missing string > . I have to rewrite the power_on/power_off function in Exynos 5250 and I will include it in the next version. Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- 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] xhci-ring: process bulk set actual_length only when not COMP_STOP_INVAL
When suspend, some urb is put into cancelled_td_list. process_bulk_intr_td may process the last trb as below: xhci_transfer_event.transfer_len = 0 xhci_transfer_event.trb_comp_code = 27 urb.transfer_buffer_length = 1024 trb_comp_code is COMP_STOP_INVAL, it should be taken as invalid urb and discarded, but urb actual_length still is calculated as transfer_buffer_length. When handling Stop Endpoint Command completion, the urb in cancelled_td_list is transferred to upper layer(cdc-acm for example). The content of urb transfer_buffer is the overlay of previous two read transfer_buffer. It makes upper layer process wrong buffer. This patch is to set actual_length as transfer_buffer_length only if trb_comp_code is not COMP_STOP_INVAL when process_bulk_intr_td. Signed-off-by: xiao jin --- drivers/usb/host/xhci-ring.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 6bfbd80..c9a8863 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2319,8 +2319,11 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_td *td, *status = 0; } } else { - td->urb->actual_length = - td->urb->transfer_buffer_length; + if (trb_comp_code != COMP_STOP_INVAL) + td->urb->actual_length = + td->urb->transfer_buffer_length; + else + td->urb->actual_length = 0; /* Ignore a short packet completion if the * untransferred length was zero. */ -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver
Hi Kishon On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote: > Hi, > > On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote: > > Add a new driver for the Exynos USB PHY. The new driver uses the generic > > PHY framework. The driver includes support for the Exynos 4x10 and 4x12 > > SoC families. > > > > Signed-off-by: Kamil Debski > > Signed-off-by: Kyungmin Park > > --- > > .../devicetree/bindings/phy/samsung-usbphy.txt | 52 > > drivers/phy/Kconfig| 23 +- > > drivers/phy/Makefile |4 + > > drivers/phy/phy-exynos-usb2.c | 234 ++ > > drivers/phy/phy-exynos-usb2.h | 87 ++ > > drivers/phy/phy-exynos4210-usb2.c | 272 > > drivers/phy/phy-exynos4212-usb2.c | 324 > > > > 7 files changed, 995 insertions(+), 1 deletion(-) > > create mode 100644 > > Documentation/devicetree/bindings/phy/samsung-usbphy.txt > > create mode 100644 drivers/phy/phy-exynos-usb2.c > > create mode 100644 drivers/phy/phy-exynos-usb2.h > > create mode 100644 drivers/phy/phy-exynos4210-usb2.c > > create mode 100644 drivers/phy/phy-exynos4212-usb2.c [snip] > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > > index a344f3d..bdf0fab 100644 > > --- a/drivers/phy/Kconfig > > +++ b/drivers/phy/Kconfig [snip] > > @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO > > help > > Support for Display Port PHY found on Samsung EXYNOS SoCs. > > > > +config PHY_EXYNOS_USB2 > > + tristate "Samsung USB 2.0 PHY driver" > > + help > > + Enable this to support Samsung USB phy helper driver for Samsung SoCs. > > + This driver provides common interface to interact, for Samsung > > + USB 2.0 PHY driver. > > I still think we can get rid of this helper driver and have a single > driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2. This helper driver is a really nice way to avoid code duplication, while still leaving the code clean and readable. All the Samsung USB 2.0 PHYs require exactly the same semantics (isolation, reference rate configuration, power up, power on), but each one has completely different layout of registers and bits inside registers. Making a big single driver would end up being identical to the old Exynos USB2PHY driver with a lot of if and switch statements inside most of functions, which is not only ugly but makes any further extension hard. In addition, this approach makes it possible to disable support for SoCs that are not needed in particular use cases, allowing smaller kernel images. > > + > > +config PHY_EXYNOS4210_USB2 > > + bool "Support for Exynos 4210" > > + depends on PHY_EXYNOS_USB2 > > + depends on CPU_EXYNOS4210 > > + help > > + Enable USB PHY support for Exynos 4210 > > + > > +config PHY_EXYNOS4212_USB2 > > + bool "Support for Exynos 4212" > > + depends on PHY_EXYNOS_USB2 > > + depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) > > + help > > + Enable USB PHY support for Exynos 4212 > > + > > endmenu [snip] > > +extern const struct usb2_phy_config exynos4210_usb2_phy_config; > > +extern const struct usb2_phy_config exynos4212_usb2_phy_config; > > + > > +static const struct of_device_id exynos_usb2_phy_of_match[] = { > > +#ifdef CONFIG_PHY_EXYNOS4210_USB2 > > I don't think you'll need #ifdef here. Anyways the driver data can be > obtained using the appropriate compatible value in dt data no? Huh? This is not about driver data, but rather about the ability to match the driver only to devices that are actually supported with selected Kconfig options. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver
Hi, On Wednesday 06 November 2013 05:08 PM, Tomasz Figa wrote: Hi Kishon On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote: Hi, On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote: Add a new driver for the Exynos USB PHY. The new driver uses the generic PHY framework. The driver includes support for the Exynos 4x10 and 4x12 SoC families. Signed-off-by: Kamil Debski Signed-off-by: Kyungmin Park --- .../devicetree/bindings/phy/samsung-usbphy.txt | 52 drivers/phy/Kconfig| 23 +- drivers/phy/Makefile |4 + drivers/phy/phy-exynos-usb2.c | 234 ++ drivers/phy/phy-exynos-usb2.h | 87 ++ drivers/phy/phy-exynos4210-usb2.c | 272 drivers/phy/phy-exynos4212-usb2.c | 324 7 files changed, 995 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/phy/samsung-usbphy.txt create mode 100644 drivers/phy/phy-exynos-usb2.c create mode 100644 drivers/phy/phy-exynos-usb2.h create mode 100644 drivers/phy/phy-exynos4210-usb2.c create mode 100644 drivers/phy/phy-exynos4212-usb2.c [snip] diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index a344f3d..bdf0fab 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig [snip] @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO help Support for Display Port PHY found on Samsung EXYNOS SoCs. +config PHY_EXYNOS_USB2 + tristate "Samsung USB 2.0 PHY driver" + help + Enable this to support Samsung USB phy helper driver for Samsung SoCs. + This driver provides common interface to interact, for Samsung + USB 2.0 PHY driver. I still think we can get rid of this helper driver and have a single driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2. This helper driver is a really nice way to avoid code duplication, while still leaving the code clean and readable. All the Samsung USB 2.0 PHYs require exactly the same semantics (isolation, reference rate configuration, power up, power on), but each one has completely different layout of registers and bits inside registers. I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] and couldn't find that big a difference in register layout. Of course there are a few changes in HSIC bit fields and PHYFSEL but that's only minimal and could well be handled in a single driver. [1] -> http://diffchecker.com/py3tp68m Making a big single driver would end up being identical to the old Exynos USB2PHY driver with a lot of if and switch statements inside most of functions, which is not only ugly but makes any further extension hard. Probably we shouldn't try to over design and just convert the old exynos usb2 phy driver to the generic phy framework to begin with? Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver
On Wednesday 06 of November 2013 18:20:36 Kishon Vijay Abraham I wrote: > Hi, > > On Wednesday 06 November 2013 05:08 PM, Tomasz Figa wrote: > > Hi Kishon > > > > On Wednesday 06 of November 2013 13:48:13 Kishon Vijay Abraham I wrote: > >> Hi, > >> > >> On Tuesday 05 November 2013 09:43 PM, Kamil Debski wrote: > >>> Add a new driver for the Exynos USB PHY. The new driver uses the generic > >>> PHY framework. The driver includes support for the Exynos 4x10 and 4x12 > >>> SoC families. > >>> > >>> Signed-off-by: Kamil Debski > >>> Signed-off-by: Kyungmin Park > >>> --- > >>>.../devicetree/bindings/phy/samsung-usbphy.txt | 52 > >>>drivers/phy/Kconfig| 23 +- > >>>drivers/phy/Makefile |4 + > >>>drivers/phy/phy-exynos-usb2.c | 234 > >>> ++ > >>>drivers/phy/phy-exynos-usb2.h | 87 ++ > >>>drivers/phy/phy-exynos4210-usb2.c | 272 > >>> > >>>drivers/phy/phy-exynos4212-usb2.c | 324 > >>> > >>>7 files changed, 995 insertions(+), 1 deletion(-) > >>>create mode 100644 > >>> Documentation/devicetree/bindings/phy/samsung-usbphy.txt > >>>create mode 100644 drivers/phy/phy-exynos-usb2.c > >>>create mode 100644 drivers/phy/phy-exynos-usb2.h > >>>create mode 100644 drivers/phy/phy-exynos4210-usb2.c > >>>create mode 100644 drivers/phy/phy-exynos4212-usb2.c > > [snip] > >>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > >>> index a344f3d..bdf0fab 100644 > >>> --- a/drivers/phy/Kconfig > >>> +++ b/drivers/phy/Kconfig > > [snip] > >>> @@ -51,4 +51,25 @@ config PHY_EXYNOS_DP_VIDEO > >>> help > >>> Support for Display Port PHY found on Samsung EXYNOS SoCs. > >>> > >>> +config PHY_EXYNOS_USB2 > >>> + tristate "Samsung USB 2.0 PHY driver" > >>> + help > >>> + Enable this to support Samsung USB phy helper driver for Samsung SoCs. > >>> + This driver provides common interface to interact, for Samsung > >>> + USB 2.0 PHY driver. > >> > >> I still think we can get rid of this helper driver and have a single > >> driver for both PHY_EXYNOS4210_USB2 and PHY_EXYNOS4212_USB2. > > > > This helper driver is a really nice way to avoid code duplication, while > > still leaving the code clean and readable. > > > > All the Samsung USB 2.0 PHYs require exactly the same semantics > > (isolation, reference rate configuration, power up, power on), but each > > one has completely different layout of registers and bits inside > > registers. > > I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] > and couldn't find that big a difference in register layout. Of course > there are a few changes in HSIC bit fields and PHYFSEL but that's only > minimal and could well be handled in a single driver. > > [1] -> http://diffchecker.com/py3tp68m This is quite a lot of differences, especially including shifted bitfields... In addition there is another set of available PHYs and inter-dependencies between them. > > > > Making a big single driver would end up being identical to the old Exynos > > USB2PHY driver with a lot of if and switch statements inside most of > > functions, which is not only ugly but makes any further extension hard. > > Probably we shouldn't try to over design and just convert the old exynos > usb2 phy driver to the generic phy framework to begin with? The old exynos USB2 PHY driver is incomplete and has very limited functionality. It needs a complete redesign to support remaining features and this is the reason we decided to develop a new driver from scratch. I believe the way Kamil's driver is designed is definitely the way to go with Samsung's USB2 PHY drivers, especially considering that support for more SoCs using the same framework will be added - S3C64xx, S5PV210, and Exynos5250. Best regards. Tomasz -- 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 v4 0/2] libusbg: clean up and error handling
In reference to the message sent by Andrzej Pietrasiewicz (about libusbg (formerly libgadget)) I would like to propose some changes to libusbg. Creating directories is now performed after successful memory allocation and gadget function creation. Hard coded values are replaced with constants. Error handling added to functions that operates on strings. Changes since v3: - changes are now in four separate files - fixed code indentation Changes since v2: - fixed code indentation - removed unused variable ret Changes since v1: - fixed typos in MAX_LENGTH throughout Stanislaw Wadas (2): libusbg: Moved mkdir functions, added MAX_LENGTH & MAX_PATH_LENGTH libusbg: added fputs()/fgets() error handling include/gadget/gadget.h | 27 -- src/gadget.c| 95 ++- 2 files changed, 67 insertions(+), 55 deletions(-) -- 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 v4 1/4] libusbg: Path length replaced with MAX_LENGTH & MAX_PATH_LENGTH
256 hard coded value has been replaced by two defined constants MAX_LENGTH and MAX_PATH_LENGTH Signed-off-by: Stanislaw Wadas --- Changes since v1: - fixed typos in MAX_LENGTH throughout include/gadget/gadget.h | 27 +++ src/gadget.c| 46 +++--- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/include/gadget/gadget.h b/include/gadget/gadget.h index 6a32c39..9bca97e 100644 --- a/include/gadget/gadget.h +++ b/include/gadget/gadget.h @@ -33,13 +33,16 @@ #define DEFAULT_UDCNULL #define LANG_US_ENG0x0409 +#define MAX_LENGTH 256 +#define MAX_PATH_LENGTH 256 + /** * @struct state * @brief State of the gadget devices in the system */ struct state { - char path[256]; + char path[MAX_PATH_LENGTH]; TAILQ_HEAD(ghead, gadget) gadgets; }; @@ -51,8 +54,8 @@ struct state struct gadget { char name[40]; - char path[256]; - char udc[256]; + char path[MAX_PATH_LENGTH]; + char udc[MAX_LENGTH]; int dclass; int dsubclass; int dproto; @@ -61,9 +64,9 @@ struct gadget int bcdusb; int product; int vendor; - char str_ser[256]; - char str_mnf[256]; - char str_prd[256]; + char str_ser[MAX_LENGTH]; + char str_mnf[MAX_LENGTH]; + char str_prd[MAX_LENGTH]; TAILQ_ENTRY(gadget) gnode; TAILQ_HEAD(chead, config) configs; TAILQ_HEAD(fhead, function) functions; @@ -81,10 +84,10 @@ struct config struct gadget *parent; char name[40]; - char path[256]; + char path[MAX_PATH_LENGTH]; int maxpower; int bmattrs; - char str_cfg[256]; + char str_cfg[MAX_LENGTH]; }; /** @@ -136,7 +139,7 @@ struct serial_attrs { struct net_attrs { struct ether_addr dev_addr; struct ether_addr host_addr; - char ifname[256]; + char ifname[MAX_LENGTH]; int qmult; }; @@ -145,7 +148,7 @@ struct net_attrs { * @brief Attributes for the phonet USB function */ struct phonet_attrs { - char ifname[256]; + char ifname[MAX_LENGTH]; }; /** @@ -168,7 +171,7 @@ struct function struct gadget *parent; char name[40]; - char path[256]; + char path[MAX_PATH_LENGTH]; enum function_type type; union attrs attr; @@ -187,7 +190,7 @@ struct binding struct function *target; char name[40]; - char path[256]; + char path[MAX_PATH_LENGTH]; }; /* Library init and cleanup */ diff --git a/src/gadget.c b/src/gadget.c index 83b8b62..fb86e7e 100644 --- a/src/gadget.c +++ b/src/gadget.c @@ -82,7 +82,7 @@ static int file_select(const struct dirent *dent) static char *gadget_read_buf(char *path, char *name, char *file, char *buf) { - char p[256]; + char p[MAX_LENGTH]; FILE *fp; char *ret = NULL; @@ -92,7 +92,7 @@ static char *gadget_read_buf(char *path, char *name, char *file, char *buf) if (!fp) goto out; - ret = fgets(buf, 256, fp); + ret = fgets(buf, MAX_LENGTH, fp); fclose(fp); @@ -102,7 +102,7 @@ out: static int gadget_read_int(char *path, char *name, char *file, int base) { - char buf[256]; + char buf[MAX_LENGTH]; if (gadget_read_buf(path, name, file, buf)) return strtol(buf, NULL, base); @@ -125,7 +125,7 @@ static void gadget_read_string(char *path, char *name, char *file, char *buf) static void gadget_write_buf(char *path, char *name, char *file, char *buf) { - char p[256]; + char p[MAX_LENGTH]; FILE *fp; sprintf(p, "%s/%s/%s", path, name, file); @@ -143,7 +143,7 @@ static void gadget_write_buf(char *path, char *name, char *file, char *buf) static void gadget_write_int(char *path, char *name, char *file, int value, char *str) { - char buf[256]; + char buf[MAX_LENGTH]; sprintf(buf, str, value); gadget_write_buf(path, name, file, buf); @@ -196,7 +196,7 @@ static int gadget_parse_functions(char *path, struct gadget *g) struct function *f; int i, n; struct dirent **dent; - char fpath[256]; + char fpath[MAX_PATH_LENGTH]; sprintf(fpath, "%s/%s/functions", path, g->name); @@ -227,7 +227,7 @@ static void gadget_parse_config_bindings(struct config *c) { int i, n; struct dirent **dent; - char bpath[256]; + char bpath[MAX_PATH_LENGTH]; struct gadget *g = c->parent; struct binding *b; struct function *f; @@ -240,12 +240,12 @@ static void gadget_parse_config_bindings(struct config *c) for (i=0; i < n; i++) { TAILQ_FOREACH(f, &g->functions, fnode) { int n; - char contents[256]; - char cpath[256]; + char cont
[PATCH v4 2/4] libusbg: Added fputs()/fgets() error handling
Error handling was added to fputs()/fgets() functions. Signed-off-by: Stanislaw Wadas --- Changes since v3: - fixed code indentation Changes since v2: - fixed code indentation - removed unused variable ret Changes since v1: - fixed typos in MAX_LENGTH throughout src/gadget.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/gadget.c b/src/gadget.c index fb86e7e..d226d32 100644 --- a/src/gadget.c +++ b/src/gadget.c @@ -93,6 +93,11 @@ static char *gadget_read_buf(char *path, char *name, char *file, char *buf) goto out; ret = fgets(buf, MAX_LENGTH, fp); + if (ret == NULL) { + ERROR("read error"); + fclose(fp); + return ret; + } fclose(fp); @@ -136,7 +141,11 @@ static void gadget_write_buf(char *path, char *name, char *file, char *buf) return; } - fputs(buf, fp); + if (fputs(buf, fp) == EOF) { + ERROR("write error"); + fclose(fp); + return; + } fclose(fp); } -- 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 v4 3/4] libusbg: Moved mkdir() functions
mkdir() function is now performed after successful memory allocation and gadget function creation. Signed-off-by: Stanislaw Wadas --- src/gadget.c | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/gadget.c b/src/gadget.c index d226d32..79a1698 100644 --- a/src/gadget.c +++ b/src/gadget.c @@ -492,12 +492,6 @@ struct gadget *gadget_create_gadget(struct state *s, char *name, sprintf(gpath, "%s/%s", s->path, name); - ret = mkdir(gpath, S_IRWXU|S_IRWXG|S_IRWXO); - if (ret < 0) { - ERRORNO("%s\n", gpath); - return NULL; - } - g = malloc(sizeof(struct gadget)); if (!g) { ERRORNO("allocating gadget\n"); @@ -515,6 +509,12 @@ struct gadget *gadget_create_gadget(struct state *s, char *name, gadget_write_hex16(s->path, name, "idVendor", vendor); gadget_write_hex16(s->path, name, "idProduct", product); + ret = mkdir(gpath, S_IRWXU|S_IRWXG|S_IRWXO); + if (ret < 0) { + ERRORNO("%s\n", gpath); + return NULL; + } + /* Insert in string order */ if (TAILQ_EMPTY(&s->gadgets) || (strcmp(name, TAILQ_FIRST(&s->gadgets)->name) < 0)) @@ -628,12 +628,6 @@ struct function *gadget_create_function(struct gadget *g, enum function_type typ sprintf(fpath, "%s/%s/functions/%s", g->path, g->name, name); - ret = mkdir(fpath, S_IRWXU|S_IRWXG|S_IRWXO); - if (ret < 0) { - ERRORNO("%s\n", fpath); - return NULL; - } - f = malloc(sizeof(struct function)); if (!f) { ERRORNO("allocating function\n"); @@ -646,6 +640,12 @@ struct function *gadget_create_function(struct gadget *g, enum function_type typ gadget_parse_function_attrs(f); + ret = mkdir(fpath, S_IRWXU|S_IRWXG|S_IRWXO); + if (ret < 0) { + ERRORNO("%s\n", fpath); + return NULL; + } + /* Insert in string order */ if (TAILQ_EMPTY(&g->functions) || (strcmp(name, TAILQ_FIRST(&g->functions)->name) < 0)) @@ -682,12 +682,6 @@ struct config *gadget_create_config(struct gadget *g, char *name) sprintf(cpath, "%s/%s/configs/%s", g->path, g->name, name); - ret = mkdir(cpath, S_IRWXU|S_IRWXG|S_IRWXO); - if (ret < 0) { - ERRORNO("%s\n", cpath); - return NULL; - } - c = malloc(sizeof(struct config)); if (!c) { ERRORNO("allocating configuration\n"); @@ -698,6 +692,12 @@ struct config *gadget_create_config(struct gadget *g, char *name) strcpy(c->name, name); sprintf(c->path, "%s/%s/%s/%s", g->path, g->name, "configs", name); + ret = mkdir(cpath, S_IRWXU|S_IRWXG|S_IRWXO); + if (ret < 0) { + ERRORNO("%s\n", cpath); + return NULL; + } + /* Insert in string order */ if (TAILQ_EMPTY(&g->configs) || (strcmp(name, TAILQ_FIRST(&g->configs)->name) < 0)) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] libusbg: Added inline to gadget_write_string()
Added inline to gadget_write_string(). Signed-off-by: Stanislaw Wadas --- src/gadget.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gadget.c b/src/gadget.c index 79a1698..8ca68b3 100644 --- a/src/gadget.c +++ b/src/gadget.c @@ -162,7 +162,7 @@ static void gadget_write_int(char *path, char *name, char *file, int value, char #define gadget_write_hex16(p, n, f, v) gadget_write_int(p, n, f, v, "0x%04x\n") #define gadget_write_hex8(p, n, f, v) gadget_write_int(p, n, f, v, "0x%02x\n") -static void gadget_write_string(char *path, char *name, char *file, char *buf) +static inline void gadget_write_string(char *path, char *name, char *file, char *buf) { gadget_write_buf(path, name, file, buf); } -- 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 v3 1/3] phy: Add new Exynos USB PHY driver
> > I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] > > and couldn't find that big a difference in register layout. Of course > > there are a few changes in HSIC bit fields and PHYFSEL but that's only > > minimal and could well be handled in a single driver. > > > > [1] -> http://diffchecker.com/py3tp68m > > This is quite a lot of differences, especially including shifted > bitfields... In addition there is another set of available PHYs > and inter-dependencies between them. Might be worth feeding both files through "sed -e 's/_421[02]_/_421x_/'" prior to doing the diff. And maybe changing the order of some definitions so they match. Then the actual differences will be more obvious. David -- 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 v4 1/4] libusbg: Path length replaced with MAX_LENGTH & MAX_PATH_LENGTH
> 256 hard coded value has been replaced by two defined > constants MAX_LENGTH and MAX_PATH_LENGTH Think of better names. David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] phy: Add new Exynos USB PHY driver
Hi David, On Wednesday 06 of November 2013 13:03:45 David Laight wrote: > > > I just did a diff of registers in exynos 4210 and 4212 PHY drivers [1] > > > and couldn't find that big a difference in register layout. Of course > > > there are a few changes in HSIC bit fields and PHYFSEL but that's only > > > minimal and could well be handled in a single driver. > > > > > > [1] -> http://diffchecker.com/py3tp68m > > > > This is quite a lot of differences, especially including shifted > > bitfields... In addition there is another set of available PHYs > > and inter-dependencies between them. > > Might be worth feeding both files through "sed -e 's/_421[02]_/_421x_/'" > prior to doing the diff. > And maybe changing the order of some definitions so they match. > Then the actual differences will be more obvious. I have fed it already through my built-in sed when reading. Still despite many similarities, I think there is enough difference to justify having different callback functions for both, especially based on my experience with the old Exynos USB2 PHY driver in drivers/usb/phy, when trying to make it more complete. Best regards, Tomasz -- 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] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
"Yann E. MORIN" writes: > Dirk, All, > > On 2013-11-01 00:39 +0100, Dirk Gouders spake thusly: >> If choices consist of choice_values that depend on symbols set to 'm', >> those choice_values are not set to 'n' if the choice is changed from >> 'm' to 'y' (in which case only one active choice_value is allowed). >> Those values are also written to the config file causing modules to be >> built when they should not. >> >> The following config can be used to reproduce and examine the problem: >> >> config modules >> boolean modules >> default y >> option modules >> >> config dependency >> tristate "Dependency" >> default m >> >> choice >> prompt "Tristate Choice" >> default choice0 >> >> config choice0 >> tristate "Choice 0" >> >> config choice1 >> tristate "Choice 1" >> depends on dependency >> >> endchoice >> >> This patch sets choice_values' visibility that depend on symbols set >> to 'm' to 'n' if the corresponding choice is set to 'y'. This makes >> them disappear from the choice list and will also cause the >> choice_values' value set to 'n' in sym_calc_value() and as a result >> they are written as "not set" to the resulting .config file. > > It seems I'm missing something here. > > I just copy-pasted your example (test.in. below) and used it with > current master (cset #be408cd) without your patch, and then ran: > $ git clean -dX # clean the tree > $ make menuconfig # Generate the frontend > --> exit without saving > $ ./scripts/kconfig/mconf test.in > --> change the choice to 'y' > --> do not change anything else > --> exit and save > > $ cat .config > CONFIG_modules=y > CONFIG_dependency=m > CONFIG_c0=y > # CONFIG_c1 is not set > > (.config header elided on purpose) > This looks like the expected output to me. > > So I did further tests (still without your patch): > $ git clean -dX # clean the tree > $ make menuconfig # Generate the frontend > --> exit without saving > $ ./scripts/kconfig/mconf test.in > --> change nothing > --> exit and save > > $ cat .config > CONFIG_modules=y > CONFIG_dependency=m > # CONFIG_c0 is not set > # CONFIG_c1 is not set This, by the way, is the other problem I mentioned earlier. There is a default value defined for "Tristate Choice" and the way I understand the kconfig language, CONFIG_c0 should be 'm' here. But that is another issue it is just a nice example for what I tried to describe earlier. > $ ./scripts/kconfig/mconf test.in > --> change the choice to 'y' > --> do not change anything else > --> exit and save > > $ cat .config > CONFIG_modules=y > CONFIG_dependency=m > CONFIG_c0=y > # CONFIG_c1 is not set > > Still the expected output, as far as I can see. > > I can observe the exact same result with your patch applied. Ditto with > kbuild/for-next from Michal's tree, with or without your patch. > > So while I understand and can reproduce the original issue, and this > patch indeed solves this original issue, the test-case above does not > seem to completely illustrate the issue. > > Are you sure this test-case exhibits the problem for you? Yes, but obviously, I did not describe it very clearly. The steps to reproduce the problem are: $ ./scripts/kconfig/mconf test.in --> change c0 and c1 to 'm'# This is the missing part! --> change the choice to 'y' --> do not change anything else --> exit and save I spontaneously planned to answer with a modified config file with default values 'm' specified for 'c0' and 'c1' (complete file below) but I noticed that my latest patch does not help in that case. The first patch that modifies sym_calc_value() would handle it nicely but the latter one that modifies sym_calc_visibility() does not. The combination also does not work, because sym_calc_visibility() influences sym_calc_value(). So, I have to say that I am no longer really satisfied with the patch. It fixes the reported problem but I think it should fix related obvious problems as well (see config below). I'd prefer I take some more time and try to find a more sensible fix. Thanks for your review and testing, Yann. Dirk PS: Sebastian, I also want to say thank you to you for the testing so far! - Sample Kconfig --- config modules boolean modules default y option modules config dependency tristate "Dependency" default m choice tristate "Tristate Choice" default choice0 config choice0 tristate "Choice 0" default m config choice1 tristate "Choice 1" depends on dependency default m endchoice > Anyway, I'm taking that in my tree locally, but that won't be part of > for
Re: [RFC PATCH 1/2] usb: hcd: Remove USB phy if needed
Hi, On Wed, Nov 06, 2013 at 12:33:26AM +0400, Valentine Barshak wrote: > This adds remove_phy flag to the HCD structure. If the flag is > set and if hcd->phy is valid, the phy is shutdown and released > whenever usb_add_hcd fails or usb_hcd_remove is called. > This can be used by the HCD drivers to auto-remove > the external USB phy when it is no longer needed. > > Signed-off-by: Valentine Barshak > --- > drivers/usb/core/hcd.c | 14 +- > include/linux/usb/hcd.h | 1 + > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index d6a8d23..d939521 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -43,6 +43,7 @@ > > #include > #include > +#include > > #include "usb.h" > > @@ -2611,7 +2612,7 @@ int usb_add_hcd(struct usb_hcd *hcd, >*/ > if ((retval = hcd_buffer_create(hcd)) != 0) { > dev_dbg(hcd->self.controller, "pool alloc failed\n"); > - return retval; > + goto err_remove_phy; > } > > if ((retval = usb_register_bus(&hcd->self)) < 0) > @@ -2742,6 +2743,12 @@ err_allocate_root_hub: > usb_deregister_bus(&hcd->self); > err_register_bus: > hcd_buffer_destroy(hcd); > +err_remove_phy: > + if (hcd->remove_phy && hcd->phy) { > + usb_phy_shutdown(hcd->phy); > + usb_put_phy(hcd->phy); > + hcd->phy = NULL; > + } why do you need the flag at all ? If hcd->phy is valid, just casll usb_phy_shutdown() followed by usb_put_phy(). Did you find any issues with that ? -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 2/2] usb: hcd: Introduce CONFIG_USB_HCD_EXTERNAL_PHY option
Hi, On Wed, Nov 06, 2013 at 12:33:27AM +0400, Valentine Barshak wrote: > This adds external USB phy support to USB HCD driver that > allows to find and initialize external USB phy, bound to > the HCD when the HCD is added. > The usb_add_hcd function returns -EPROBE_DEFER if the USB > phy, bound to the HCD, is not ready. > If no USB phy is bound, the HCD is initialized as usual. > > Signed-off-by: Valentine Barshak > --- > drivers/usb/core/hcd.c | 20 > drivers/usb/host/Kconfig | 11 +++ > 2 files changed, 31 insertions(+) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index d939521..da9c4ba 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -2597,6 +2597,26 @@ int usb_add_hcd(struct usb_hcd *hcd, > int retval; > struct usb_device *rhdev; > > +#ifdef CONFIG_USB_HCD_EXTERNAL_PHY I think here would be a nicer location for a flag: if (hcd->has_external_phy) { phy = usb_get_phy_dev(); } that flag would get set by the glue driver (ehci-omap, ehci-msm, ohci-omap, etc), where necessary. -- balbi signature.asc Description: Digital signature
Re: Large USB HID transfers
On Tue, 5 Nov 2013, Cliff Brake wrote: > On Tue, Nov 5, 2013 at 9:57 AM, Alan Stern wrote: > > On Mon, 4 Nov 2013, Cliff Brake wrote: > > > > >> > >> I assume interrupts are disabled during both ohci_irq() and ehci_irq(). > > > > Currently they are. This will change somewhat in the not-too-distant > > future. > > Are there any patches available yet for this work? There is commit c04ee4b1136e (Revert "Revert "USB: EHCI: support running URB giveback in tasklet context"") in Greg KH's usb-next tree. Even with that commit, however, interrupts will remain disabled while the completion handler is called. Changing that behavior is not so easy, as quite a few drivers depend on it. There has been some work done to change this, too. See the series of patches beginning here: http://marc.info/?l=linux-usb&m=137675675628650&w=2 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 v4.1 2/4] usb: gadget: add quirk_ep_out_aligned_size field to struct usb_gadget
On Tue, 5 Nov 2013, David Cohen wrote: > Due to USB controllers may have different restrictions, usb gadget layer > needs to provide a generic way to inform gadget functions to complain > with non-standard requirements. > > This patch adds 'quirk_ep_out_aligned_size' field to struct usb_gadget > to inform when controller's epout requires buffer size to be aligned to > MaxPacketSize. A helper is also provided to align buffer size when > necessary. > > Signed-off-by: David Cohen > --- > > Changes from v4 to v4.1: > - Simplified usb_ep_align_maxpacketsize() macro as per Alen Stern's request > > include/linux/usb/gadget.h | 16 > 1 file changed, 16 insertions(+) > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index 23b3bfd0a842..41e8834af57d 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -441,6 +441,19 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep) > ep->ops->fifo_flush(ep); > } > > +/** > + * usb_ep_align_maxpacketsize - returns @len aligned to ep's maxpacketsize > + * @ep: the endpoint whose maxpacketsize is used to align @len > + * @len: buffer size's length to align to @ep's maxpacketsize > + * > + * This helper is used in case it's required for any reason to align buffer's > + * size to an ep's maxpacketsize. > + */ > +static inline size_t usb_ep_align_maxpacketsize(struct usb_ep *ep, size_t > len) > +{ > + return round_up(len, (size_t)ep->desc->wMaxPacketSize); > +} > + > > /*-*/ > > @@ -502,6 +515,8 @@ struct usb_gadget_ops { > * only supports HNP on a different root port. > * @b_hnp_enable: OTG device feature flag, indicating that the A-Host > * enabled HNP support. > + * @quirk_ep_out_aligned_size: epout requires buffer size to be aligned to > + * MaxPacketSize. > * > * Gadgets have a mostly-portable "gadget driver" implementing device > * functions, handling all usb configurations and interfaces. Gadget > @@ -541,6 +556,7 @@ struct usb_gadget { > unsignedb_hnp_enable:1; > unsigneda_hnp_support:1; > unsigneda_alt_hnp_support:1; > + unsignedquirk_ep_out_aligned_size:1; > }; > #define work_to_gadget(w)(container_of((w), struct usb_gadget, work)) This looks good now. Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] ehci-platform: Merge ppc-of EHCI driver into the ehci-platform driver
On Wed, 6 Nov 2013, Alistair Popple wrote: > Currently the ppc-of driver uses the compatibility string > "usb-ehci". This means platforms that use device-tree and implement an > EHCI compatible interface have to either use the ppc-of driver or add > a compatible line to the ehci-platform driver. It would be more > appropriate for the platform driver to be compatible with "usb-ehci" > as non-powerpc platforms are also beginning to utilise device-tree. > > This patch merges the device tree property parsing from ehci-ppc-of > into the platform driver and adds a "usb-ehci" compatibility > string. The existing ehci-ppc-of driver is renamed to ehci-440epx as > it contains platform specific work arounds for the 440EPX SoC. > > Signed-off-by: Alistair Popple > --- > > So I could submit something like this that essentially merges the ppc-of > driver into the platform driver instead of adding the "ibm,akebono-ehci" > compatible line to the platform driver. > > However I'm still fairly new to device-tree so I'm not sure what (if any) the > broader impact would be. A quick grep for "usb-ehci" turned up a couple of > ARM > device tree's using it however they all provided their own drivers and don't > select CONFIG_USB_EHCI_HCD_PLATFORM so I'm guess they shouldn't be impacted. > > I have attempted to fix up any PowerPC device trees/configs, although I > wasn't > sure if "usb-ehci" should remain in sequoia.dts or not given that it needs to > use the 440EPX specific driver. > > Also this hasn't been tested (beyond compilation) yet. > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci- > platform.c > index f6b790c..027f368 100644 > --- a/drivers/usb/host/ehci-platform.c > +++ b/drivers/usb/host/ehci-platform.c > @@ -77,6 +77,7 @@ static int ehci_platform_probe(struct platform_device *dev) > struct usb_hcd *hcd; > struct resource *res_mem; > struct usb_ehci_pdata *pdata; > + struct device_node *dn = dev->dev.of_node; > int irq; > int err = -ENOMEM; > > @@ -96,6 +97,18 @@ static int ehci_platform_probe(struct platform_device *dev) > > pdata = dev_get_platdata(&dev->dev); > > + /* Initialise platform data from device tree if available. */ > + if (!dn) { Shouldn't this be "if (dn)"? > + if (of_get_property(dn, "big-endian", NULL)) { > + pdata->big_endian_mmio = 1; > + pdata->big_endian_desc = 1; > + } > + if (of_get_property(dn, "big-endian-regs", NULL)) > + pdata->big_endian_mmio = 1; > + if (of_get_property(dn, "big-endian-desc", NULL)) > + pdata->big_endian_desc = 1; > + } > + This isn't good if there is more than one EHCI controller using ehci-platform. To accomodate such cases, it would be necessary to allocate a separate copy of ehci_platform_defaults for each controller. 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: USB: EHCI: create per-TT bandwidth tables
On Tue, 5 Nov 2013, Dan Carpenter wrote: > Hello Alan Stern, > > The patch b35c5009bbf6: "USB: EHCI: create per-TT bandwidth tables" > from Oct 11, 2013, leads to the following > static checker warning: "drivers/usb/host/ehci-sched.c:1377 > reserve_release_iso_bandwidth() >error: 'tt' dereferencing possible ERR_PTR()" > > drivers/usb/host/ehci-sched.c > 1375 tt = find_tt(stream->ps.udev); > > This can return ERR_PTR(-ENOMEM). > > 1376 if (sign > 0) > 1377 list_add_tail(&stream->ps.ps_list, > &tt->ps_list); > 1378 else > 1379 list_del(&stream->ps.ps_list); > 1380 > 1381 for (i = uframe >> 3; i < EHCI_BANDWIDTH_FRAMES; > 1382 i += stream->ps.bw_period) > 1383 tt->bandwidth[i] += tt_usecs; > > Also: > > drivers/usb/host/ehci-sched.c >257 /* FS/LS bus bandwidth */ >258 if (tt_usecs) { >259 tt = find_tt(qh->ps.udev); > >260 if (sign > 0) >261 list_add_tail(&qh->ps.ps_list, &tt->ps_list); >262 else >263 list_del(&qh->ps.ps_list); >264 >265 for (i = start_uf >> 3; i < EHCI_BANDWIDTH_FRAMES; >266 i += qh->ps.bw_period) >267 tt->bandwidth[i] += tt_usecs; Ah, yes. I sort of wondered if a static checker would be able to pick up on this. The overall pattern is not too uncommon: f(x) returns x->ptr, if x->ptr is non-NULL. Otherwise it attempts to allocate a new structure and makes x->ptr point to it. If memory isn't available, f(x) returns ERR_PTR(-ENOMEM). g() calls f(x) twice, but checks the result with IS_ERR() only the first time. It is known that nothing will change or deallocate x->ptr between the two calls. Short of adding redundant tests, how would you suggest this be handled? Do nothing and live with a false positive report from the checker? 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: [RFC PATCH 1/2] usb: hcd: Remove USB phy if needed
On Wed, 6 Nov 2013, Felipe Balbi wrote: > Hi, > > On Wed, Nov 06, 2013 at 12:33:26AM +0400, Valentine Barshak wrote: > > This adds remove_phy flag to the HCD structure. If the flag is > > set and if hcd->phy is valid, the phy is shutdown and released > > whenever usb_add_hcd fails or usb_hcd_remove is called. > > This can be used by the HCD drivers to auto-remove > > the external USB phy when it is no longer needed. > > > > Signed-off-by: Valentine Barshak > > --- > > drivers/usb/core/hcd.c | 14 +- > > include/linux/usb/hcd.h | 1 + > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > > index d6a8d23..d939521 100644 > > --- a/drivers/usb/core/hcd.c > > +++ b/drivers/usb/core/hcd.c > > @@ -43,6 +43,7 @@ > > > > #include > > #include > > +#include > > > > #include "usb.h" > > > > @@ -2611,7 +2612,7 @@ int usb_add_hcd(struct usb_hcd *hcd, > > */ > > if ((retval = hcd_buffer_create(hcd)) != 0) { > > dev_dbg(hcd->self.controller, "pool alloc failed\n"); > > - return retval; > > + goto err_remove_phy; > > } > > > > if ((retval = usb_register_bus(&hcd->self)) < 0) > > @@ -2742,6 +2743,12 @@ err_allocate_root_hub: > > usb_deregister_bus(&hcd->self); > > err_register_bus: > > hcd_buffer_destroy(hcd); > > +err_remove_phy: > > + if (hcd->remove_phy && hcd->phy) { > > + usb_phy_shutdown(hcd->phy); > > + usb_put_phy(hcd->phy); > > + hcd->phy = NULL; > > + } > > why do you need the flag at all ? If hcd->phy is valid, just casll > usb_phy_shutdown() followed by usb_put_phy(). Did you find any issues > with that ? That's a reasonable thing to do, but it means that a few other HC drivers would have to be updated (they shouldn't call usb_phy_shutdown or usb_put_phy any more). 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: [RFC PATCH 2/2] usb: hcd: Introduce CONFIG_USB_HCD_EXTERNAL_PHY option
On Wed, 6 Nov 2013, Felipe Balbi wrote: > Hi, > > On Wed, Nov 06, 2013 at 12:33:27AM +0400, Valentine Barshak wrote: > > This adds external USB phy support to USB HCD driver that > > allows to find and initialize external USB phy, bound to > > the HCD when the HCD is added. > > The usb_add_hcd function returns -EPROBE_DEFER if the USB > > phy, bound to the HCD, is not ready. > > If no USB phy is bound, the HCD is initialized as usual. > > > > Signed-off-by: Valentine Barshak > > --- > > drivers/usb/core/hcd.c | 20 > > drivers/usb/host/Kconfig | 11 +++ > > 2 files changed, 31 insertions(+) > > > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > > index d939521..da9c4ba 100644 > > --- a/drivers/usb/core/hcd.c > > +++ b/drivers/usb/core/hcd.c > > @@ -2597,6 +2597,26 @@ int usb_add_hcd(struct usb_hcd *hcd, > > int retval; > > struct usb_device *rhdev; > > > > +#ifdef CONFIG_USB_HCD_EXTERNAL_PHY I don't see any reason to add a new Kconfig symbol. Just use "#ifdef USB_PHY" instead. > I think here would be a nicer location for a flag: > > if (hcd->has_external_phy) { > phy = usb_get_phy_dev(); > > > } > > that flag would get set by the glue driver (ehci-omap, ehci-msm, > ohci-omap, etc), where necessary. The problem Valentine is facing is that the glue driver doesn't know whether or not to set the flag. The way he set it up, the decision is pushed down into usb_get_phy_dev, which ought to have enough information. 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: [RFC PATCH 1/2] usb: hcd: Remove USB phy if needed
On 11/06/2013 07:45 PM, Felipe Balbi wrote: Hi, On Wed, Nov 06, 2013 at 12:33:26AM +0400, Valentine Barshak wrote: This adds remove_phy flag to the HCD structure. If the flag is set and if hcd->phy is valid, the phy is shutdown and released whenever usb_add_hcd fails or usb_hcd_remove is called. This can be used by the HCD drivers to auto-remove the external USB phy when it is no longer needed. Signed-off-by: Valentine Barshak --- drivers/usb/core/hcd.c | 14 +- include/linux/usb/hcd.h | 1 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index d6a8d23..d939521 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -43,6 +43,7 @@ #include #include +#include #include "usb.h" @@ -2611,7 +2612,7 @@ int usb_add_hcd(struct usb_hcd *hcd, */ if ((retval = hcd_buffer_create(hcd)) != 0) { dev_dbg(hcd->self.controller, "pool alloc failed\n"); - return retval; + goto err_remove_phy; } if ((retval = usb_register_bus(&hcd->self)) < 0) @@ -2742,6 +2743,12 @@ err_allocate_root_hub: usb_deregister_bus(&hcd->self); err_register_bus: hcd_buffer_destroy(hcd); +err_remove_phy: + if (hcd->remove_phy && hcd->phy) { + usb_phy_shutdown(hcd->phy); + usb_put_phy(hcd->phy); + hcd->phy = NULL; + } why do you need the flag at all ? If hcd->phy is valid, just casll usb_phy_shutdown() followed by usb_put_phy(). Did you find any issues with that ? I haven't been able to test it with other platforms. However, some drivers use devm_usb_get_phy_dev() and we may face refcounter issues if we call usb_put_phy unconditionally. Adding this flag seems safe enough and we doesn't affect other drivers. Thanks, Val. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] usb: hcd: Introduce CONFIG_USB_HCD_EXTERNAL_PHY option
On 11/06/2013 08:39 PM, Alan Stern wrote: On Wed, 6 Nov 2013, Felipe Balbi wrote: Hi, On Wed, Nov 06, 2013 at 12:33:27AM +0400, Valentine Barshak wrote: This adds external USB phy support to USB HCD driver that allows to find and initialize external USB phy, bound to the HCD when the HCD is added. The usb_add_hcd function returns -EPROBE_DEFER if the USB phy, bound to the HCD, is not ready. If no USB phy is bound, the HCD is initialized as usual. Signed-off-by: Valentine Barshak --- drivers/usb/core/hcd.c | 20 drivers/usb/host/Kconfig | 11 +++ 2 files changed, 31 insertions(+) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index d939521..da9c4ba 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2597,6 +2597,26 @@ int usb_add_hcd(struct usb_hcd *hcd, int retval; struct usb_device *rhdev; +#ifdef CONFIG_USB_HCD_EXTERNAL_PHY I don't see any reason to add a new Kconfig symbol. Just use "#ifdef USB_PHY" instead. I just thought that most of the drivers would not need this code, so I added a config option which can be enabled only if necessary. I'll remove and use USB_PHY instead. Thanks. I think here would be a nicer location for a flag: if (hcd->has_external_phy) { phy = usb_get_phy_dev(); } that flag would get set by the glue driver (ehci-omap, ehci-msm, ohci-omap, etc), where necessary. The problem Valentine is facing is that the glue driver doesn't know whether or not to set the flag. The way he set it up, the decision is pushed down into usb_get_phy_dev, which ought to have enough information. Exactly. Alan Stern Thanks, Val. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] usb: hcd: Remove USB phy if needed
Hi, On Wed, Nov 06, 2013 at 08:43:36PM +0400, Valentine wrote: > On 11/06/2013 07:45 PM, Felipe Balbi wrote: > >Hi, > > > >On Wed, Nov 06, 2013 at 12:33:26AM +0400, Valentine Barshak wrote: > >>This adds remove_phy flag to the HCD structure. If the flag is > >>set and if hcd->phy is valid, the phy is shutdown and released > >>whenever usb_add_hcd fails or usb_hcd_remove is called. > >>This can be used by the HCD drivers to auto-remove > >>the external USB phy when it is no longer needed. > >> > >>Signed-off-by: Valentine Barshak > >>--- > >> drivers/usb/core/hcd.c | 14 +- > >> include/linux/usb/hcd.h | 1 + > >> 2 files changed, 14 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > >>index d6a8d23..d939521 100644 > >>--- a/drivers/usb/core/hcd.c > >>+++ b/drivers/usb/core/hcd.c > >>@@ -43,6 +43,7 @@ > >> > >> #include > >> #include > >>+#include > >> > >> #include "usb.h" > >> > >>@@ -2611,7 +2612,7 @@ int usb_add_hcd(struct usb_hcd *hcd, > >> */ > >>if ((retval = hcd_buffer_create(hcd)) != 0) { > >>dev_dbg(hcd->self.controller, "pool alloc failed\n"); > >>- return retval; > >>+ goto err_remove_phy; > >>} > >> > >>if ((retval = usb_register_bus(&hcd->self)) < 0) > >>@@ -2742,6 +2743,12 @@ err_allocate_root_hub: > >>usb_deregister_bus(&hcd->self); > >> err_register_bus: > >>hcd_buffer_destroy(hcd); > >>+err_remove_phy: > >>+ if (hcd->remove_phy && hcd->phy) { > >>+ usb_phy_shutdown(hcd->phy); > >>+ usb_put_phy(hcd->phy); > >>+ hcd->phy = NULL; > >>+ } > > > >why do you need the flag at all ? If hcd->phy is valid, just casll > >usb_phy_shutdown() followed by usb_put_phy(). Did you find any issues > >with that ? > > > > I haven't been able to test it with other platforms. > However, some drivers use devm_usb_get_phy_dev() and we may face refcounter > issues > if we call usb_put_phy unconditionally. > Adding this flag seems safe enough and we doesn't affect other drivers. then use devm_usb_get_phy_dev() here and remove the call from all other drivers ;-) -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 2/2] usb: hcd: Introduce CONFIG_USB_HCD_EXTERNAL_PHY option
On Wed, Nov 06, 2013 at 08:47:36PM +0400, Valentine wrote: > On 11/06/2013 08:39 PM, Alan Stern wrote: > >On Wed, 6 Nov 2013, Felipe Balbi wrote: > > > >>Hi, > >> > >>On Wed, Nov 06, 2013 at 12:33:27AM +0400, Valentine Barshak wrote: > >>>This adds external USB phy support to USB HCD driver that > >>>allows to find and initialize external USB phy, bound to > >>>the HCD when the HCD is added. > >>>The usb_add_hcd function returns -EPROBE_DEFER if the USB > >>>phy, bound to the HCD, is not ready. > >>>If no USB phy is bound, the HCD is initialized as usual. > >>> > >>>Signed-off-by: Valentine Barshak > >>>--- > >>> drivers/usb/core/hcd.c | 20 > >>> drivers/usb/host/Kconfig | 11 +++ > >>> 2 files changed, 31 insertions(+) > >>> > >>>diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > >>>index d939521..da9c4ba 100644 > >>>--- a/drivers/usb/core/hcd.c > >>>+++ b/drivers/usb/core/hcd.c > >>>@@ -2597,6 +2597,26 @@ int usb_add_hcd(struct usb_hcd *hcd, > >>> int retval; > >>> struct usb_device *rhdev; > >>> > >>>+#ifdef CONFIG_USB_HCD_EXTERNAL_PHY > > > >I don't see any reason to add a new Kconfig symbol. Just use "#ifdef > >USB_PHY" instead. > > I just thought that most of the drivers would not need this code, > so I added a config option which can be enabled only if necessary. > I'll remove and use USB_PHY instead. Thanks. > > > > >>I think here would be a nicer location for a flag: > >> > >>if (hcd->has_external_phy) { > >>phy = usb_get_phy_dev(); > >> > >> > >>} > >> > >>that flag would get set by the glue driver (ehci-omap, ehci-msm, > >>ohci-omap, etc), where necessary. > > > >The problem Valentine is facing is that the glue driver doesn't know > >whether or not to set the flag. The way he set it up, the decision is > >pushed down into usb_get_phy_dev, which ought to have enough > >information. > > Exactly. got it ;-) -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 1/2] usb: hcd: Remove USB phy if needed
On 11/06/2013 09:04 PM, Felipe Balbi wrote: Hi, On Wed, Nov 06, 2013 at 08:43:36PM +0400, Valentine wrote: On 11/06/2013 07:45 PM, Felipe Balbi wrote: Hi, On Wed, Nov 06, 2013 at 12:33:26AM +0400, Valentine Barshak wrote: This adds remove_phy flag to the HCD structure. If the flag is set and if hcd->phy is valid, the phy is shutdown and released whenever usb_add_hcd fails or usb_hcd_remove is called. This can be used by the HCD drivers to auto-remove the external USB phy when it is no longer needed. Signed-off-by: Valentine Barshak --- drivers/usb/core/hcd.c | 14 +- include/linux/usb/hcd.h | 1 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index d6a8d23..d939521 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -43,6 +43,7 @@ #include #include +#include #include "usb.h" @@ -2611,7 +2612,7 @@ int usb_add_hcd(struct usb_hcd *hcd, */ if ((retval = hcd_buffer_create(hcd)) != 0) { dev_dbg(hcd->self.controller, "pool alloc failed\n"); - return retval; + goto err_remove_phy; } if ((retval = usb_register_bus(&hcd->self)) < 0) @@ -2742,6 +2743,12 @@ err_allocate_root_hub: usb_deregister_bus(&hcd->self); err_register_bus: hcd_buffer_destroy(hcd); +err_remove_phy: + if (hcd->remove_phy && hcd->phy) { + usb_phy_shutdown(hcd->phy); + usb_put_phy(hcd->phy); + hcd->phy = NULL; + } why do you need the flag at all ? If hcd->phy is valid, just casll usb_phy_shutdown() followed by usb_put_phy(). Did you find any issues with that ? I haven't been able to test it with other platforms. However, some drivers use devm_usb_get_phy_dev() and we may face refcounter issues if we call usb_put_phy unconditionally. Adding this flag seems safe enough and we doesn't affect other drivers. then use devm_usb_get_phy_dev() here and remove the call from all other drivers ;-) The glue drivers may use different ways of getting the USB phy: usb_get_phy; usb_get_phy_dev; devm_usb_get_phy_dev. We can't consolidate phy getting here since usb_get_phy and usb_get_phy_dev are not equivalent. So we let the glue drivers decide which way of getting/putting USB phy to use. Thanks, Val. -- 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: XHCI: Ring expansion failure
On Wed, Nov 6, 2013 at 12:53 AM, wrote: > Hi > > By performing iterative port suspend and resume (which results in function > suspend and resume), ring expansion failure is observed. Attached device > has multiple interfaces for which interface host drivers are unlinking the > urbs during function suspend and submitting urbs during resume. > > For the cases when dequeue ptr is close to the end of deq segment, value > of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes > high when function suspend is over. At the time of resume this high value > is causing to trigger ring expansion in the middle of submitting > urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the > interface host drivers only request one TRB per urb (num_trb =1). Ring > expansion logic is doubling the num of previously allocated segments every > time. This is causing the num of segments to grow at fast pace (at the > time of failure, one of the ep rings num_seg was 128), even though it is > also increasing num_trbs_free. > Sounds like a bug. Why num_trbs_in_deq_seg becomes high? How many urbs are submitted during resume? Normally only isoc transfer triggers ring expansion with multiple trbs per urb. > Even for the cases where we observe data transfer as well as suspend & > resume (which are normal scenarios) ring expansion failure is easily > observed due to large number of segments being allocated for ever. This > means triggering of ring expansion will result in failure of further > expansion at some point of time. > > I was trying to think of some options to reduce ring expansion with > respect to suspend resume scenario: - > 1) Can we reset enq and deq ptr to point to ep_ring first seg once all > the > urbs are unlinked (function suspend)? > 2) Can we free the segments at the time of suspend and allocate segments > back when resume happens? > These does not point to the root cause of ring expansion failure. Thanks, Andiry > In general, as per the spec can we also implement shrinking of transfer > ring (4.9.2.4). > > Please help to provide your comments on the options above (which can > easily fit in to the current design) also feel free to add better options > to consider to avoid ring expansion failures which can be added easily to > the current implementation. > > Currently I am using 3.10 kernel and back ported recent fixes on xhci. > > Thanks, > Hemant > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
xhci message rate control needed
fedora kernel 3.9.10-100.fc17.x86_64 xhci_hcd :12:00.0: ERROR Transfer event TRB DMA ptr not part of current TD i got this message about every 200 MICROseconds after a warm boot. unplugging the usb3 cables silenced it. plugging back in, everything came up fine. grepping messages for :12:00.0 shows everything on the usb3 hub; hubs, cameras and network adapters (fyi none of which are really usb3 - all usb2 except the first hub). i can check whatever you like if you want more info but i'm really only reporting the message flood. cheers andrew-- 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 v4 3/4] usb: ffs: check quirk to pad epout buf size when not aligned to maxpacketsize
On Tue, Nov 05 2013, Alan Stern wrote: > Maybe Michal can enlighten us. Sorry for late response, this thread fell under my radar for some reason. So here's how it works: epfile represents an end point file on the fuctionfs file system, i.e. what user space is seeing. It's numbering is independent of which USB configuration is chosen. A FunctionFS user space daemon may read/write to those files regardless of whether the function is enabled. If it is not, the operation will block until host enables the function. epfile->ep represents an actual USB end point, and it's number does not have to correspond to the epfile file name, and may be different in different configuration. FunctionFS hides all that from the user space daemon. epfile->ep is set when host changes configuration (i.e. function's set_alt or disable callbacks). IIRC this implies that epfile->ep cannot be protected by a mutex, and therefore is protected by a spinlock. Since it is protected by a spinlock, the ffs_epfile_io function cannot lock it and then proceed to allocating memory and copying data from user space. That's why there is the need for the loop since there is no way to guarantee that while the memory was allocated and data was read from user space (if it is a write), the function has not been disabled and re-enabled. However, I'm not sure whether maxpacketsize can change. It is part of endpoint's descriptor and even though the endpoint number can change while ffs_epfile_io is running, all the other descriptor fields should stay the same. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
Dirk, All, On 2013-11-06 15:43 +0100, Dirk Gouders spake thusly: > "Yann E. MORIN" writes: [--SNIP--] > > It seems I'm missing something here. [--SNIP--] > Yes, but obviously, I did not describe it very clearly. The steps to > reproduce the problem are: > > $ ./scripts/kconfig/mconf test.in >--> change c0 and c1 to 'm'# This is the missing part! Aha! Gotcha. Thanks. > I spontaneously planned to answer with a modified config file with > default values 'm' specified for 'c0' and 'c1' (complete file below) but > I noticed that my latest patch does not help in that case. The first > patch that modifies sym_calc_value() would handle it nicely but the > latter one that modifies sym_calc_visibility() does not. The > combination also does not work, because sym_calc_visibility() influences > sym_calc_value(). > > So, I have to say that I am no longer really satisfied with the patch. > It fixes the reported problem but I think it should fix related > obvious problems as well (see config below). I'd prefer I take some > more time and try to find a more sensible fix. Please, one patch to fix one bug. It does not matter if you need to touch the same part of the code, but please keep fixes for different bugs, separate (unless of course, the bugs are just different manifestations of the same deficiency in the code). Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' -- 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: EHCI: create per-TT bandwidth tables
On Wed, Nov 06, 2013 at 11:24:37AM -0500, Alan Stern wrote: > Short of adding redundant tests, how would you suggest this be handled? Yeah. I discourage people from making changes to please the checker. It often hurts readability. Also the checker is changing all the time and later we might want to complain about the redundant tests. > Do nothing and live with a false positive report from the checker? I'm fine with false positives. These emails are a one time thing. 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: [RFC PATCH] ehci-platform: Merge ppc-of EHCI driver into the ehci-platform driver
On Wed, 2013-11-06 at 18:39 +1100, Alistair Popple wrote: > diff --git a/arch/powerpc/boot/dts/sequoia.dts > b/arch/powerpc/boot/dts/sequoia.dts > index b1d3292..e28371e 100644 > --- a/arch/powerpc/boot/dts/sequoia.dts > +++ b/arch/powerpc/boot/dts/sequoia.dts > @@ -153,7 +153,7 @@ > }; > > USB0: ehci@e300 { > - compatible = "ibm,usb-ehci-440epx", "usb-ehci"; > + compatible = "ibm,usb-ehci-440epx"; Compatible properties are string lists so it *should* be ok to have all of them in the list. However I do remember Russell mentioning a problem with the current matching code so adding him on CC for comments. > +/* > + * 440EPx Errata USBH_3 > + * Fix: Enable Break Memory Transfer (BMT) in INSNREG3 > + */ > +#define PPC440EPX_EHCI0_INSREG_BMT (0x1 << 0) > +static int > +ppc44x_enable_bmt(struct device_node *dn) > +{ > + __iomem u32 *insreg_virt; > + > + insreg_virt = of_iomap(dn, 1); > + if (!insreg_virt) > + return -EINVAL; > + > + out_be32(insreg_virt + 3, PPC440EPX_EHCI0_INSREG_BMT); > + > + iounmap(insreg_virt); > + return 0; > +} > + I would go even further and add the 44x workarounds to the normal platform device, with a compatible check in there. That isn't the first time we add quirks to an existing driver. > + /* Initialise platform data from device tree if available. */ > + if (!dn) { That was supposed to be if (dn) no ? > + if (of_get_property(dn, "big-endian", NULL)) { > + pdata->big_endian_mmio = 1; > + pdata->big_endian_desc = 1; > + } > + if (of_get_property(dn, "big-endian-regs", NULL)) > + pdata->big_endian_mmio = 1; > + if (of_get_property(dn, "big-endian-desc", NULL)) > + pdata->big_endian_desc = 1; > + } > + > irq = platform_get_irq(dev, 0); > if (irq < 0) { > dev_err(&dev->dev, "no irq provided"); > @@ -203,9 +216,10 @@ static int ehci_platform_resume(struct device *dev) > #define ehci_platform_resume NULL > #endif /* CONFIG_PM */ > > -static const struct of_device_id vt8500_ehci_ids[] = { > +static const struct of_device_id ehci_platform_ids[] = { > { .compatible = "via,vt8500-ehci", }, > { .compatible = "wm,prizm-ehci", }, > + { .compatible = "usb-ehci", }, > {} > }; Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: hub: Use correct reset for wedged USB3 devices that are NOTATTACHED
This patch adds a check for USB_STATE_NOTATTACHED to the hub_port_warm_reset_required() workaround for ports that end up in Compliance Mode in hub_events() when trying to decide which reset function to use. Trying to call usb_reset_device() with a NOTATTACHED device will just fail and leave the port broken. Also bumped the messages about this kind of reset failure from dev_dbg() to dev_warn() to make it easier to notice, since calling that function with a NOTATTACHED device would almost always be a bug Signed-off-by: Julius Werner --- drivers/usb/core/hub.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index e6b682c..0188056 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4799,8 +4799,9 @@ static void hub_events(void) hub->ports[i - 1]->child; dev_dbg(hub_dev, "warm reset port %d\n", i); - if (!udev || !(portstatus & - USB_PORT_STAT_CONNECTION)) { + if (!udev || + !(portstatus & USB_PORT_STAT_CONNECTION) || + udev->state == USB_STATE_NOTATTACHED) { status = hub_port_reset(hub, i, NULL, HUB_BH_RESET_TIME, true); @@ -5074,7 +5075,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) if (udev->state == USB_STATE_NOTATTACHED || udev->state == USB_STATE_SUSPENDED) { - dev_dbg(&udev->dev, "device reset not allowed in state %d\n", + dev_warn(&udev->dev, "device reset not allowed in state %d\n", udev->state); return -EINVAL; } @@ -5237,7 +5238,7 @@ int usb_reset_device(struct usb_device *udev) if (udev->state == USB_STATE_NOTATTACHED || udev->state == USB_STATE_SUSPENDED) { - dev_dbg(&udev->dev, "device reset not allowed in state %d\n", + dev_warn(&udev->dev, "device reset not allowed in state %d\n", udev->state); return -EINVAL; } -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci-plat: Enable USB 2.0 hardware LPM support for platform xHCs
*bump* Hi Sarah, is there anything else that needs to be resolved to pick this patch up? Looks like Matthias' recent LPM fixes are all in now so there should be no way this could cause any trouble? -- 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: XHCI: Ring expansion failure
> On Wed, Nov 6, 2013 at 12:53 AM, wrote: >> Hi >> >> By performing iterative port suspend and resume (which results in >> function >> suspend and resume), ring expansion failure is observed. Attached device >> has multiple interfaces for which interface host drivers are unlinking >> the >> urbs during function suspend and submitting urbs during resume. >> >> For the cases when dequeue ptr is close to the end of deq segment, value >> of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes >> high when function suspend is over. At the time of resume this high >> value >> is causing to trigger ring expansion in the middle of submitting >> urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the >> interface host drivers only request one TRB per urb (num_trb =1). Ring >> expansion logic is doubling the num of previously allocated segments >> every >> time. This is causing the num of segments to grow at fast pace (at the >> time of failure, one of the ep rings num_seg was 128), even though it is >> also increasing num_trbs_free. >> > > Sounds like a bug. Why num_trbs_in_deq_seg becomes high? How many urbs > are submitted during resume? Normally only isoc transfer triggers ring > expansion with multiple trbs per urb. Let me explain it little further what is being observed in terms of enq , deq ptr:- For example 1) Lets say First seg of the ring starts with 0xed0d3000 which means enq ptr and deq ptr are pointing to this address. 2) Interface driver queuing 10 urbs (one urb represents one trb) enq ptr goes to ed0d30a0. 3) Interface suspend happened. All 10 urbs are going to get unlinked. For the first urb to be unlinked, it is added to ep->cancelled_td_list and Stop ep cmd was called (rest of the urbs added to ep->cancelled_td_list before stop ep cmd completed). 4) When stop ep cmd is completed, for 9 urbs td_to_noop() is called. For the first urb (for which stop ep cmd was called), xhci_find_new_dequeue_state() gets called which updates deq ptr to next trb (0x ed0d3010) so set tr deq ptr cmd is issued to xhci to set its deq ptr to next trb(0xed0d3010). 5) In set tr deq cmd completion handler update_ring_for_set_deq_completion() gets called which will just increments num_trbs_free to only one, but we unlinked all the urbs. So when interface suspend finished we have enq ptr pointing to 0x ed0d30a0 and dep ptr pointing to 0x ed0d3010. With num_trbs_free only incremented by one. 6) At the time of interface resume we re-queue 10 urbs back with num_trbs_free (just incremented 1 during suspend). This means num_trbs_free is incrementing very slowly every time interface suspend happens. And it is very easy to get into a situation where room_on_ring() returning 0 as deq ptr will be reaching to the end of the seg and causing num_trbs_in_deq_seg to go high enough that ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg condition becomes true. Thanks, Hemant >> Even for the cases where we observe data transfer as well as suspend & >> resume (which are normal scenarios) ring expansion failure is easily >> observed due to large number of segments being allocated for ever. This >> means triggering of ring expansion will result in failure of further >> expansion at some point of time. >> >> I was trying to think of some options to reduce ring expansion with >> respect to suspend resume scenario: - >> 1) Can we reset enq and deq ptr to point to ep_ring first seg once >> all the >> urbs are unlinked (function suspend)? >> 2) Can we free the segments at the time of suspend and allocate >> segments >> back when resume happens? >> > > These does not point to the root cause of ring expansion failure. > > Thanks, > Andiry > >> In general, as per the spec can we also implement shrinking of transfer >> ring (4.9.2.4). >> >> Please help to provide your comments on the options above (which can >> easily fit in to the current design) also feel free to add better >> options >> to consider to avoid ring expansion failures which can be added easily >> to >> the current implementation. >> >> Currently I am using 3.10 kernel and back ported recent fixes on xhci. >> >> Thanks, >> Hemant >> >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Use correct reset for wedged USB3 devices that are NOTATTACHED
On Wed, Nov 06, 2013 at 12:27:21PM -0800, Julius Werner wrote: > This patch adds a check for USB_STATE_NOTATTACHED to the > hub_port_warm_reset_required() workaround for ports that end up in > Compliance Mode in hub_events() when trying to decide which reset > function to use. Trying to call usb_reset_device() with a NOTATTACHED > device will just fail and leave the port broken. Who makes those calls, drivers? Any specific ones that you know need to be fixed? > Also bumped the messages about this kind of reset failure from dev_dbg() > to dev_warn() to make it easier to notice, since calling that function > with a NOTATTACHED device would almost always be a bug But what can a user do if those messages show up? 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] usb: hub: Use correct reset for wedged USB3 devices that are NOTATTACHED
On Wed, 6 Nov 2013, Julius Werner wrote: > This patch adds a check for USB_STATE_NOTATTACHED to the > hub_port_warm_reset_required() workaround for ports that end up in > Compliance Mode in hub_events() when trying to decide which reset > function to use. Trying to call usb_reset_device() with a NOTATTACHED > device will just fail and leave the port broken. What if the device is in USB_STATE_SUSPENDED? > > Also bumped the messages about this kind of reset failure from dev_dbg() > to dev_warn() to make it easier to notice, since calling that function > with a NOTATTACHED device would almost always be a bug Not at all. If a device is unplugged, its state changes to NOTATTACHED before the driver is unbound. During that time, the driver will see all its URBs failing, so it may very well try to reset the device. (For example, usbhid behaves like this.) That isn't a bug. 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: XHCI: Ring expansion failure
On Wed, Nov 6, 2013 at 12:33 PM, wrote: >> On Wed, Nov 6, 2013 at 12:53 AM, wrote: >>> Hi >>> >>> By performing iterative port suspend and resume (which results in >>> function >>> suspend and resume), ring expansion failure is observed. Attached device >>> has multiple interfaces for which interface host drivers are unlinking >>> the >>> urbs during function suspend and submitting urbs during resume. >>> >>> For the cases when dequeue ptr is close to the end of deq segment, value >>> of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes >>> high when function suspend is over. At the time of resume this high >>> value >>> is causing to trigger ring expansion in the middle of submitting >>> urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the >>> interface host drivers only request one TRB per urb (num_trb =1). Ring >>> expansion logic is doubling the num of previously allocated segments >>> every >>> time. This is causing the num of segments to grow at fast pace (at the >>> time of failure, one of the ep rings num_seg was 128), even though it is >>> also increasing num_trbs_free. >>> >> >> Sounds like a bug. Why num_trbs_in_deq_seg becomes high? How many urbs >> are submitted during resume? Normally only isoc transfer triggers ring >> expansion with multiple trbs per urb. > > Let me explain it little further what is being observed in terms of enq , > deq ptr:- > > For example > 1) Let’s say First seg of the ring starts with 0xed0d3000 which means enq > ptr and deq ptr are pointing to this address. > 2) Interface driver queuing 10 urbs (one urb represents one trb) enq ptr > goes to ed0d30a0. > 3) Interface suspend happened. All 10 urbs are going to get unlinked. For > the first urb to be unlinked, it is added to ep->cancelled_td_list and > Stop ep cmd was called (rest of the urbs added to ep->cancelled_td_list > before stop ep cmd completed). > 4) When stop ep cmd is completed, for 9 urbs td_to_noop() is called. For > the first urb (for which stop ep cmd was called), > xhci_find_new_dequeue_state() gets called which updates deq ptr to next > trb (0x ed0d3010) so set tr deq ptr cmd is issued to xhci to set its deq > ptr to next trb(0xed0d3010). > 5) In set tr deq cmd completion handler > update_ring_for_set_deq_completion() gets called which will just > increments num_trbs_free to only one, but we unlinked all the urbs. So > when interface suspend finished we have enq ptr pointing to 0x ed0d30a0 > and dep ptr pointing to 0x ed0d3010. With num_trbs_free only incremented > by one. > 6) At the time of interface resume we re-queue 10 urbs back with > num_trbs_free (just incremented 1 during suspend). This means > num_trbs_free is incrementing very slowly every time interface suspend > happens. And it is very easy to get into a situation where room_on_ring() > returning 0 as deq ptr will be reaching to the end of the seg and causing > num_trbs_in_deq_seg to go high enough that ring->num_trbs_free < num_trbs > + num_trbs_in_deq_seg condition becomes true. > So it's because the suspend/resume loops and urbs are enqueued and unlinked, deq_ptr does not follow enq_ptr. This is expected since deq_ptr and num_trbs_free increase in inc_deq(), which is called by irq handler. As there is no transfer on this ring, deq_ptr will not get updated. I'm still curious about the failure of ring expansion. 128 segments occupy 128KB, which should not be a big deal. Ring expansion is expected to run until there is no memory to allocate. Does it fail because of allocation failure? Thanks, Andiry > >>> Even for the cases where we observe data transfer as well as suspend & >>> resume (which are normal scenarios) ring expansion failure is easily >>> observed due to large number of segments being allocated for ever. This >>> means triggering of ring expansion will result in failure of further >>> expansion at some point of time. >>> >>> I was trying to think of some options to reduce ring expansion with >>> respect to suspend resume scenario: - >>> 1) Can we reset enq and deq ptr to point to ep_ring first seg once >>> all the >>> urbs are unlinked (function suspend)? >>> 2) Can we free the segments at the time of suspend and allocate >>> segments >>> back when resume happens? >>> >> >> These does not point to the root cause of ring expansion failure. >> >> Thanks, >> Andiry >> >>> In general, as per the spec can we also implement shrinking of transfer >>> ring (4.9.2.4). >>> >>> Please help to provide your comments on the options above (which can >>> easily fit in to the current design) also feel free to add better >>> options >>> to consider to avoid ring expansion failures which can be added easily >>> to >>> the current implementation. >>> >>> Currently I am using 3.10 kernel and back ported recent fixes on xhci. >>> >>> Thanks, >>> Hemant >>> >>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >>> the body of a mess
Re: XHCI: Ring expansion failure
> On Wed, Nov 6, 2013 at 12:33 PM, wrote: >>> On Wed, Nov 6, 2013 at 12:53 AM, wrote: Hi By performing iterative port suspend and resume (which results in function suspend and resume), ring expansion failure is observed. Attached device has multiple interfaces for which interface host drivers are unlinking the urbs during function suspend and submitting urbs during resume. For the cases when dequeue ptr is close to the end of deq segment, value of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes high when function suspend is over. At the time of resume this high value is causing to trigger ring expansion in the middle of submitting urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the interface host drivers only request one TRB per urb (num_trb =1). Ring expansion logic is doubling the num of previously allocated segments every time. This is causing the num of segments to grow at fast pace (at the time of failure, one of the ep rings num_seg was 128), even though it is also increasing num_trbs_free. >>> >>> Sounds like a bug. Why num_trbs_in_deq_seg becomes high? How many urbs >>> are submitted during resume? Normally only isoc transfer triggers ring >>> expansion with multiple trbs per urb. >> >> Let me explain it little further what is being observed in terms of enq >> , >> deq ptr:- >> >> For example >> 1) Lets say First seg of the ring starts with 0xed0d3000 which >> means enq >> ptr and deq ptr are pointing to this address. >> 2) Interface driver queuing 10 urbs (one urb represents one trb) >> enq ptr >> goes to ed0d30a0. >> 3) Interface suspend happened. All 10 urbs are going to get >> unlinked. For >> the first urb to be unlinked, it is added to ep->cancelled_td_list and >> Stop ep cmd was called (rest of the urbs added to ep->cancelled_td_list >> before stop ep cmd completed). >> 4) When stop ep cmd is completed, for 9 urbs td_to_noop() is >> called. For >> the first urb (for which stop ep cmd was called), >> xhci_find_new_dequeue_state() gets called which updates deq ptr to next >> trb (0x ed0d3010) so set tr deq ptr cmd is issued to xhci to set its deq >> ptr to next trb(0xed0d3010). >> 5) In set tr deq cmd completion handler >> update_ring_for_set_deq_completion() gets called which will just >> increments num_trbs_free to only one, but we unlinked all the urbs. So >> when interface suspend finished we have enq ptr pointing to 0x ed0d30a0 >> and dep ptr pointing to 0x ed0d3010. With num_trbs_free only incremented >> by one. >> 6) At the time of interface resume we re-queue 10 urbs back with >> num_trbs_free (just incremented 1 during suspend). This means >> num_trbs_free is incrementing very slowly every time interface suspend >> happens. And it is very easy to get into a situation where >> room_on_ring() >> returning 0 as deq ptr will be reaching to the end of the seg and >> causing >> num_trbs_in_deq_seg to go high enough that ring->num_trbs_free < >> num_trbs >> + num_trbs_in_deq_seg condition becomes true. >> > > So it's because the suspend/resume loops and urbs are enqueued and > unlinked, deq_ptr does not follow enq_ptr. This is expected since > deq_ptr and num_trbs_free increase in inc_deq(), which is called by > irq handler. As there is no transfer on this ring, deq_ptr will not > get updated. Considering the suspend resume scenario, with this behavior don't you think ring expansion will be triggered very often and for no reason (as we are just doing suspend and resume). In handle_stopped_endpoint() when traversing to ep->cancelled_td_list is it better to call xhci_find_new_dequeue_state() for last td and call td_to_noop() for all previous tds. Doing that we will update deq ptr to set to enq ptr with correct value of num_trbs_free when interface suspends finishes. > > I'm still curious about the failure of ring expansion. 128 segments > occupy 128KB, which should not be a big deal. Ring expansion is > expected to run until there is no memory to allocate. Does it fail > because of allocation failure? yes, xhci_ring_expansion() returns -ENOMEM. There are 19 rings allocated. > > Thanks, > Andiry > >> Even for the cases where we observe data transfer as well as suspend & resume (which are normal scenarios) ring expansion failure is easily observed due to large number of segments being allocated for ever. This means triggering of ring expansion will result in failure of further expansion at some point of time. I was trying to think of some options to reduce ring expansion with respect to suspend resume scenario: - 1) Can we reset enq and deq ptr to point to ep_ring first seg once all the urbs are unlinked (function suspend)? 2) Can we free the segments at the time of suspend and allocate segments back when resum
Re: [PATCH] usb: hub: Use correct reset for wedged USB3 devices that are NOTATTACHED
> Who makes those calls, drivers? Any specific ones that you know need to be fixed? Well, the one I'm worried about is the one this patch is fixing, in hub_events(). I have seen this happen when having certain USB3 devices plugged into a host controller that always looses power on suspend-to-ram (dwc3-exynos). Since the host controller resets itself the port no longer has PORT_STAT_ENABLE set and that causes hub_activate() to mark the device as USB_STATE_NOTATTACHED. The next time khubd runs hub_events(), the port may be in Compliance Mode (because the unexpected HC reset can confuse the link state machines on both sides) and the kernel correctly tries to reset it, but doesn't take the fact into account that usb_reset_device() doesn't work on NOTATTACHED devices. > But what can a user do if those messages show up? Nothing. I was just thinking this might help developers follow the kernel decisions better and understand a potential problem faster (e.g. if the user posted his log in a bug report somewhere). > What if the device is in USB_STATE_SUSPENDED? I'm not sure that is possible at that point in hub_events(), I don't know of a way that could lead to this situation. I could still add the check just to be sure if you want it, though. > Not at all. If a device is unplugged, its state changes to NOTATTACHED > before the driver is unbound. During that time, the driver will see > all its URBs failing, so it may very well try to reset the device. > (For example, usbhid behaves like this.) That isn't a bug. Oh, okay, I wasn't quite sure how that plays together. Would you think it's still valuable to print it out (maybe as dev_info() instead of dev_warn()) instead of just silently ignoring the reset request? It would have certainly been useful for me to find this problem faster, but I can take it out again if you think it would result in too much noise. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCH] USB patches for 3.13-rc1
The following changes since commit 31d141e3a666269a3b6fcccddb0351caf7454240: Linux 3.12-rc6 (2013-10-19 12:28:15 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-3.13-rc1 for you to fetch changes up to 7d49f0bac41ee9b012af1efe2f725d91a87a8fe9: USB: Maintainers change for usb serial drivers (2013-10-31 08:53:52 -0700) USB driver update for 3.13-rc1 Here's the big USB driver update for 3.13-rc1. It includes the usual xhci changes, EHCI updates to get the scheduling of USB transactions working better, and a raft of gadget and musb updates as well. All of this has been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Alan Stern (21): USB: see if URB comes from a completion handler USB: EHCI: code rearrangement in iso_stream_schedule() USB: EHCI: handle isochronous underruns with tasklets USB: fix substandard locking for the sysfs files USB: NS_TO_US should round up USB: EHCI: check the right uframes for CSPLIT USB: EHCI: compute full-speed bandwidth usage correctly USB: EHCI: No SSPLIT allowed in uframe 7 USB: EHCI: change toggle only upon successful reset USB: EHCI: use consistent NO_FRAME value USB: EHCI: create a "periodic schedule info" struct USB: EHCI: use a bandwidth-allocation table USB: add a private-data pointer to struct usb_tt USB: EHCI: create per-TT bandwidth tables USB: EHCI: start new isochronous streams ASAP USB: EHCI: fix type mismatch in check_intr_schedule USB: EHCI: fix sparse errors USB: OHCI: fix and explain sparse errors USB: change dev_warn about missing reset-resume to dev_dbg USB: EHCI: add check for wakeup/suspend race USB: UHCI: check for wakeup/suspend race Alexey Khoroshilov (1): USB: wusbcore: fix usb_dev leaks Andreas Larsson (1): usb: gadget: Make VERBOSE_DEBUG enableable via Kconfig Andrzej Pietrasiewicz (29): usb: gadget: configfs: add a method to unregister the gadget usb: gadget: create a utility module for mass_storage usb: gadget: f_mass_storage: factor out a header file usb: gadget: f_mass_storage: add a level of indirection for luns storage usb: gadget: f_mass_storage: use usb_gstrings_attach usb: gadget: f_mass_storage: create _fsg_common_free_buffers usb: gadget: f_mass_storage: make sysfs interface optional usb: gadget: f_mass_storage: create fsg_common_setup for use in fsg_common_init usb: gadget: f_mass_storage: create fsg_common_set_num_buffers for use in fsg_common_init usb: gadget: f_mass_storage: create lun handling helpers for use in fsg_common_init usb: gadget: f_mass_storage: create fsg_common_set_cdev for use in fsg_common_init usb: gadget: f_mass_storage: create lun creation helpers for use in fsg_common_init usb: gadget: f_mass_storage: create fsg_common_set_inquiry_string for use in fsg_common_init usb: gadget: f_mass_storage: create fsg_common_run_thread for use in fsg_common_init usb: gadget: f_mass_storage: convert to new function interface with backward compatibility usb: gadget: mass_storage: convert to new interface of f_mass_storage usb: gadget: storage_common: make attribute operations more generic usb: gadget: storage_common: add methods to show/store 'cdrom' and 'removable' usb: gadget: f_mass_storage: add configfs support usb: gadget: acm_ms: convert to new interface of f_mass_storage usb: gadget: multi: convert to new interface of f_ecm usb: gadget: multi: convert to new interface of f_rndis usb: gadget: multi: convert to new interface of f_mass_storage usb: gadget: f_mass_storage: remove compatibility layer usb: gadget: mass_storage: merge usb_f_mass_storage module with u_ms module usb: gadget: storage_common: use strtobool instead of kstrtouint usb: gadget: storage_common: pass filesem to fsg_store_cdrom usb: gadget: f_mass_storage: style corrections, cleanup & simplification usb/gadget: f_mass_storage: use string literal as format in dev_set_name Anton Tikhomirov (10): usb: phy: Pass OTG FSM pointer to callback functions usb: phy: Check OTG FSM callback existance in helper functions usb: phy: Add and use missed helper functions usb: phy: Fix OTG FSM timer handling usb: phy: Add and use missed OTG FSM timers usb: phy: Rename OTG FSM informative variables usb: phy: Rename "B-device session end SRP" OTG FSM input usb: phy: Add and use missed OTG FSM inputs/outputs usb: phy: Reordering of OTG FSM variables USB: phy: samsung: Support multiple PHYs of same type Bjorn Helgaas (1): USB: correct the usb_disconnec
Re: XHCI: Ring expansion failure
>> On Wed, Nov 6, 2013 at 12:33 PM, wrote: On Wed, Nov 6, 2013 at 12:53 AM, wrote: > Hi > > By performing iterative port suspend and resume (which results in > function > suspend and resume), ring expansion failure is observed. Attached > device > has multiple interfaces for which interface host drivers are > unlinking > the > urbs during function suspend and submitting urbs during resume. > > For the cases when dequeue ptr is close to the end of deq segment, > value > of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) > becomes > high when function suspend is over. At the time of resume this high > value > is causing to trigger ring expansion in the middle of submitting > urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the > interface host drivers only request one TRB per urb (num_trb =1). > Ring > expansion logic is doubling the num of previously allocated segments > every > time. This is causing the num of segments to grow at fast pace (at > the > time of failure, one of the ep rings num_seg was 128), even though it > is > also increasing num_trbs_free. > Sounds like a bug. Why num_trbs_in_deq_seg becomes high? How many urbs are submitted during resume? Normally only isoc transfer triggers ring expansion with multiple trbs per urb. >>> >>> Let me explain it little further what is being observed in terms of enq >>> , >>> deq ptr:- >>> >>> For example >>> 1) Lets say First seg of the ring starts with 0xed0d3000 which >>> means enq >>> ptr and deq ptr are pointing to this address. >>> 2) Interface driver queuing 10 urbs (one urb represents one trb) >>> enq ptr >>> goes to ed0d30a0. >>> 3) Interface suspend happened. All 10 urbs are going to get >>> unlinked. For >>> the first urb to be unlinked, it is added to ep->cancelled_td_list and >>> Stop ep cmd was called (rest of the urbs added to ep->cancelled_td_list >>> before stop ep cmd completed). >>> 4) When stop ep cmd is completed, for 9 urbs td_to_noop() is >>> called. For >>> the first urb (for which stop ep cmd was called), >>> xhci_find_new_dequeue_state() gets called which updates deq ptr to next >>> trb (0x ed0d3010) so set tr deq ptr cmd is issued to xhci to set its >>> deq >>> ptr to next trb(0xed0d3010). >>> 5) In set tr deq cmd completion handler >>> update_ring_for_set_deq_completion() gets called which will just >>> increments num_trbs_free to only one, but we unlinked all the urbs. So >>> when interface suspend finished we have enq ptr pointing to 0x ed0d30a0 >>> and dep ptr pointing to 0x ed0d3010. With num_trbs_free only >>> incremented >>> by one. >>> 6) At the time of interface resume we re-queue 10 urbs back with >>> num_trbs_free (just incremented 1 during suspend). This means >>> num_trbs_free is incrementing very slowly every time interface suspend >>> happens. And it is very easy to get into a situation where >>> room_on_ring() >>> returning 0 as deq ptr will be reaching to the end of the seg and >>> causing >>> num_trbs_in_deq_seg to go high enough that ring->num_trbs_free < >>> num_trbs >>> + num_trbs_in_deq_seg condition becomes true. >>> >> >> So it's because the suspend/resume loops and urbs are enqueued and >> unlinked, deq_ptr does not follow enq_ptr. This is expected since >> deq_ptr and num_trbs_free increase in inc_deq(), which is called by >> irq handler. As there is no transfer on this ring, deq_ptr will not >> get updated. > Considering the suspend resume scenario, with this behavior don't you > think ring expansion will be triggered very often and for no reason (as we > are just doing suspend and resume). > > In handle_stopped_endpoint() when traversing to ep->cancelled_td_list is > it better to call xhci_find_new_dequeue_state() for last td and call > td_to_noop() for all previous tds. Doing that we will update deq ptr to > set to enq ptr with correct value of num_trbs_free when interface suspends > finishes. i think instead of what suggested above, we need to somehow update num_trbs_free to the correct value any time before interface driver is going to submit the first urb as a result of interface resume. As after resume xhci is going to skip no op trbs and starts fetching the transfer trbs on the ring. >> >> I'm still curious about the failure of ring expansion. 128 segments >> occupy 128KB, which should not be a big deal. Ring expansion is >> expected to run until there is no memory to allocate. Does it fail >> because of allocation failure? > yes, xhci_ring_expansion() returns -ENOMEM. There are 19 rings allocated. >> >> Thanks, >> Andiry >> >>> > Even for the cases where we observe data transfer as well as suspend > & > resume (which are normal scenarios) ring expansion failure is easily > observed due to large number of segments being allocated for ever. > This > means triggering of rin
[PATCH] usb: phy: phy-mxs-usb: set the correct platform drvdata
We need to set mxs_phy rather as the platform drvdata so that we can get the correct mxs_phy in mxs_phy_remove(). Change-Id: I6e1753cc978e2ed3fbb3d1865be0c54031d337d2 Signed-off-by: Jisheng Zhang --- drivers/usb/phy/phy-mxs-usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index fdd33b4..545844b 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -164,7 +164,7 @@ static int mxs_phy_probe(struct platform_device *pdev) mxs_phy->clk = clk; - platform_set_drvdata(pdev, &mxs_phy->phy); + platform_set_drvdata(pdev, mxs_phy); ret = usb_add_phy_dev(&mxs_phy->phy); if (ret) -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: phy-mxs-usb: set the correct platform drvdata
On Thu, Nov 07, 2013 at 10:26:20AM +0800, Jisheng Zhang wrote: > We need to set mxs_phy rather as the platform drvdata so that we can get > the correct mxs_phy in mxs_phy_remove(). > > Change-Id: I6e1753cc978e2ed3fbb3d1865be0c54031d337d2 > Signed-off-by: Jisheng Zhang Acked-by: Shawn Guo Change-Id should be dropped though. Shawn > --- > drivers/usb/phy/phy-mxs-usb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > index fdd33b4..545844b 100644 > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -164,7 +164,7 @@ static int mxs_phy_probe(struct platform_device *pdev) > > mxs_phy->clk = clk; > > - platform_set_drvdata(pdev, &mxs_phy->phy); > + platform_set_drvdata(pdev, mxs_phy); > > ret = usb_add_phy_dev(&mxs_phy->phy); > if (ret) > -- > 1.8.4.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] ehci-platform: Merge ppc-of EHCI driver into the ehci-platform driver
On Wed, 6 Nov 2013 11:14:44 Alan Stern wrote: > On Wed, 6 Nov 2013, Alistair Popple wrote: [snip] > > + /* Initialise platform data from device tree if available. */ > > + if (!dn) { > > Shouldn't this be "if (dn)"? Yep. Thanks. > > + if (of_get_property(dn, "big-endian", NULL)) { > > + pdata->big_endian_mmio = 1; > > + pdata->big_endian_desc = 1; > > + } > > + if (of_get_property(dn, "big-endian-regs", NULL)) > > + pdata->big_endian_mmio = 1; > > + if (of_get_property(dn, "big-endian-desc", NULL)) > > + pdata->big_endian_desc = 1; > > + } > > + > > This isn't good if there is more than one EHCI controller using > ehci-platform. To accomodate such cases, it would be necessary to > allocate a separate copy of ehci_platform_defaults for each controller. OK, that's a problem. Rather than allocating platform data for each controller I will move the device tree parsing into ehci_platform_reset(). > 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: [RFC PATCH] ehci-platform: Merge ppc-of EHCI driver into the ehci-platform driver
On Thu, 7 Nov 2013 06:57:00 Benjamin Herrenschmidt wrote: > On Wed, 2013-11-06 at 18:39 +1100, Alistair Popple wrote: [snip] > > I would go even further and add the 44x workarounds to the normal > platform device, with a compatible check in there. That isn't the > first time we add quirks to an existing driver. Ok, easily done. I guess I was just cautious of adding a bunch of platform specific code into an otherwise generic driver as there seems to be a number of other platforms with their own quirks and it would be easy to end up with a driver full of platform specific quirks. That said it is probably better than the current situation in which each platform has its own copy/variation of the generic code in ehci-platform. Unless anyone is against this I will merge the 440EPX specific quirks into the ehci-platform driver and submit it as part of the next version of the Akebono patch series. > > + /* Initialise platform data from device tree if available. */ > > + if (!dn) { > > That was supposed to be if (dn) no ? It sure was, thanks. > > > + if (of_get_property(dn, "big-endian", NULL)) { > > + pdata->big_endian_mmio = 1; > > + pdata->big_endian_desc = 1; > > + } > > + if (of_get_property(dn, "big-endian-regs", NULL)) > > + pdata->big_endian_mmio = 1; > > + if (of_get_property(dn, "big-endian-desc", NULL)) > > + pdata->big_endian_desc = 1; > > + } > > + > > > > irq = platform_get_irq(dev, 0); > > if (irq < 0) { > > > > dev_err(&dev->dev, "no irq provided"); > > > > @@ -203,9 +216,10 @@ static int ehci_platform_resume(struct device *dev) > > > > #define ehci_platform_resume NULL > > #endif /* CONFIG_PM */ > > > > -static const struct of_device_id vt8500_ehci_ids[] = { > > +static const struct of_device_id ehci_platform_ids[] = { > > > > { .compatible = "via,vt8500-ehci", }, > > { .compatible = "wm,prizm-ehci", }, > > > > + { .compatible = "usb-ehci", }, > > > > {} > > > > }; > > Cheers, > Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: phy-mxs-usb: set the correct platform drvdata
On Wed, 6 Nov 2013 18:34:47 -0800 Shawn Guo wrote: > On Thu, Nov 07, 2013 at 10:26:20AM +0800, Jisheng Zhang wrote: > > We need to set mxs_phy rather as the platform drvdata so that we can get > > the correct mxs_phy in mxs_phy_remove(). > > > > Change-Id: I6e1753cc978e2ed3fbb3d1865be0c54031d337d2 > > Signed-off-by: Jisheng Zhang > > Acked-by: Shawn Guo > > Change-Id should be dropped though. OOPs, sorry, I should take care of that. I'll remind myself every time. Thank you for your pointing out Jisheng > > Shawn > > > --- > > drivers/usb/phy/phy-mxs-usb.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > > index fdd33b4..545844b 100644 > > --- a/drivers/usb/phy/phy-mxs-usb.c > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > @@ -164,7 +164,7 @@ static int mxs_phy_probe(struct platform_device *pdev) > > > > mxs_phy->clk = clk; > > > > - platform_set_drvdata(pdev, &mxs_phy->phy); > > + platform_set_drvdata(pdev, mxs_phy); > > > > ret = usb_add_phy_dev(&mxs_phy->phy); > > if (ret) > > -- > > 1.8.4.2 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: phy-mxs-usb: set the correct platform drvdata
On Thu, Nov 07, 2013 at 10:26:20AM +0800, Jisheng Zhang wrote: > We need to set mxs_phy rather as the platform drvdata so that we can get > the correct mxs_phy in mxs_phy_remove(). > > Change-Id: I6e1753cc978e2ed3fbb3d1865be0c54031d337d2 please strip Gerrit-specific tags from commit log. -- balbi signature.asc Description: Digital signature
[PATCH v2] usb: phy: phy-mxs-usb: set the correct platform drvdata
We need to set mxs_phy rather as the platform drvdata so that we can get the correct mxs_phy in mxs_phy_remove(). Signed-off-by: Jisheng Zhang --- drivers/usb/phy/phy-mxs-usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index fdd33b4..545844b 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -164,7 +164,7 @@ static int mxs_phy_probe(struct platform_device *pdev) mxs_phy->clk = clk; - platform_set_drvdata(pdev, &mxs_phy->phy); + platform_set_drvdata(pdev, mxs_phy); ret = usb_add_phy_dev(&mxs_phy->phy); if (ret) -- 1.8.4.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] IBM Akebono: Add support to the OHCI platform driver for Akebono
On Tue, 5 Nov 2013 10:04:02 Alan Stern wrote: [snip] > > > > + /* Platforms using DT don't always provide platform data. > > +* This should provide reasonable defaults. */ > > /* >* The accepted format for multi-line >* comments is like this. >*/ > Ok, I'll fix that for the next revision. > > + if (!pdata) > > + dev->dev.platform_data = pdata = &ohci_platform_defaults; > > + > > > > irq = platform_get_irq(dev, 0); > > if (irq < 0) { > > > > dev_err(&dev->dev, "no irq provided"); > > > > @@ -171,6 +175,11 @@ static int ohci_platform_resume(struct device *dev) > > > > #define ohci_platform_resume NULL > > #endif /* CONFIG_PM */ > > > > +static const struct of_device_id ohci_of_match[] = { > > + { .compatible = "ibm,akebono-ohci", }, > > + {}, > > +}; > > + > > > > static const struct platform_device_id ohci_platform_table[] = { > > > > { "ohci-platform", 0 }, > > { } > > > > @@ -191,6 +200,7 @@ static struct platform_driver ohci_platform_driver = { > > > > .owner = THIS_MODULE, > > .name = "ohci-platform", > > .pm = &ohci_platform_pm_ops, > > > > + .of_match_table = ohci_of_match, > > > > } > > > > }; > > Update the comment formatting, and then you can resubmit with > > Acked-by: Alan Stern Thanks. Based on the discussion for the EHCI driver I would like to change the compatibility string to "usb-ochi" (instead of "ibm,akebono-ohci"). Are you still happy for me to add the Acked-by with the alternate compatibility (and of course the formatting fix)? No other drivers currently use "usb-ochi" so it shouldn't require any merging of drivers. Regards, Alistair -- 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/7] IBM Akebono: Add support to the OHCI platform driver for Akebono
On Thu, 2013-11-07 at 14:34 +1100, Alistair Popple wrote: > Thanks. Based on the discussion for the EHCI driver I would like to change > the > compatibility string to "usb-ochi" (instead of "ibm,akebono-ohci"). Are you > still happy for me to add the Acked-by with the alternate compatibility (and > of course the formatting fix)? No other drivers currently use "usb-ochi" so > it > shouldn't require any merging of drivers. s/ochi/ohci/ :-) Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] usb: phy: phy-mxs-usb: set the correct platform drvdata
> > Signed-off-by: Jisheng Zhang > --- > drivers/usb/phy/phy-mxs-usb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs- > usb.c > index fdd33b4..545844b 100644 > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -164,7 +164,7 @@ static int mxs_phy_probe(struct platform_device *pdev) > > mxs_phy->clk = clk; > > - platform_set_drvdata(pdev, &mxs_phy->phy); > + platform_set_drvdata(pdev, mxs_phy); > > ret = usb_add_phy_dev(&mxs_phy->phy); > if (ret) > -- It is so unlucky the struct usb_phy phy is the first element of struct mxs_phy, so we haven't run any problems. Jisheng, thanks for fixing it. Acked-by: Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/16] usb/gadget: g_ffs: convert to new interface of f_ecm
There is a new funtion interface and g_ffs is the last gadget to use the old. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 93 +--- 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index f66d96a..960f8a6 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -883,6 +883,7 @@ config USB_FUNCTIONFS_ETH bool "Include configuration with CDC ECM (Ethernet)" depends on USB_FUNCTIONFS && NET select USB_U_ETHER + select USB_F_ECM help Include a configuration with CDC ECM function (Ethernet) and the Function Filesystem. diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index e954082..cda4c5d 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -28,8 +28,7 @@ #define USB_ETH_RNDIS y # endif -#define USBF_ECM_INCLUDED -# include "f_ecm.c" +# include "u_ecm.h" #define USB_FSUBSET_INCLUDED # include "f_subset.c" # ifdef USB_ETH_RNDIS @@ -39,11 +38,15 @@ # endif # include "u_ether.h" +USB_ETHERNET_MODULE_PARAMETERS(); + static u8 gfs_host_mac[ETH_ALEN]; static struct eth_dev *the_dev; # ifdef CONFIG_USB_FUNCTIONFS_ETH static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], struct eth_dev *dev); +static struct usb_function_instance *fi_ecm; +static struct usb_function *f_ecm; # endif #else # define the_dev NULL @@ -76,10 +79,6 @@ struct gfs_ffs_obj { USB_GADGET_COMPOSITE_OPTIONS(); -#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS -USB_ETHERNET_MODULE_PARAMETERS(); -#endif - static struct usb_device_descriptor gfs_dev_desc = { .bLength= sizeof gfs_dev_desc, .bDescriptorType= USB_DT_DEVICE, @@ -355,14 +354,50 @@ static int gfs_bind(struct usb_composite_dev *cdev) if (missing_funcs) return -ENODEV; -#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS +#if defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) { + struct f_ecm_opts *ecm_opts; + + fi_ecm = usb_get_function_instance("ecm"); + if (IS_ERR(fi_ecm)) + return PTR_ERR(fi_ecm); + ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); + + gether_set_qmult(ecm_opts->net, qmult); + + if (!gether_set_host_addr(ecm_opts->net, host_addr)) + pr_info("using host ethernet address: %s", host_addr); + if (!gether_set_dev_addr(ecm_opts->net, dev_addr)) + pr_info("using self ethernet address: %s", dev_addr); + + the_dev = netdev_priv(ecm_opts->net); + } else { + the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, + gfs_host_mac, qmult); + } + +#elif defined CONFIG_USB_FUNCTIONFS_RNDIS + the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, gfs_host_mac, qmult); #endif - if (IS_ERR(the_dev)) { - ret = PTR_ERR(the_dev); - goto error_quick; + if (IS_ERR(the_dev)) + return PTR_ERR(the_dev); + +#if defined CONFIG_USB_FUNCTIONFS_RNDIS && defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) { + struct f_ecm_opts *ecm_opts; + + ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); + + gether_set_gadget(ecm_opts->net, cdev->gadget); + ret = gether_register_netdev(ecm_opts->net); + if (ret) + goto error; + ecm_opts->bound = true; + gether_get_host_addr_u8(ecm_opts->net, gfs_host_mac); } +#endif ret = usb_string_ids_tab(cdev, gfs_strings); if (unlikely(ret < 0)) @@ -398,9 +433,16 @@ error_unbind: for (i = 0; i < func_num; i++) functionfs_unbind(ffs_tab[i].ffs_data); error: +#if defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) + usb_put_function_instance(fi_ecm); + else + gether_cleanup(the_dev); + the_dev = NULL; +#elif defined CONFIG_USB_FUNCTIONFS_RNDIS gether_cleanup(the_dev); the_dev = NULL; -error_quick: +#endif return ret; } @@ -413,8 +455,19 @@ static int gfs_unbind(struct usb_composite_dev *cdev) ENTER(); + +#if defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) { + usb_put_function(f_ecm); + usb_put_function_instance(fi_ecm); + } else { + gether_cleanup(the_dev); + } + the_de
[PATCH v2 06/16] usb/gadget: f_subset: remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_subset.c | 60 + drivers/usb/gadget/u_ether.h |3 -- 2 files changed, 1 insertions(+), 62 deletions(-) diff --git a/drivers/usb/gadget/f_subset.c b/drivers/usb/gadget/f_subset.c index 7c8674f..f1a5919 100644 --- a/drivers/usb/gadget/f_subset.c +++ b/drivers/usb/gadget/f_subset.c @@ -301,7 +301,6 @@ geth_bind(struct usb_configuration *c, struct usb_function *f) int status; struct usb_ep *ep; -#ifndef USB_FSUBSET_INCLUDED struct f_gether_opts*gether_opts; gether_opts = container_of(f->fi, struct f_gether_opts, func_inst); @@ -322,7 +321,7 @@ geth_bind(struct usb_configuration *c, struct usb_function *f) return status; gether_opts->bound = true; } -#endif + us = usb_gstrings_attach(cdev, geth_strings, ARRAY_SIZE(geth_string_defs)); if (IS_ERR(us)) @@ -393,61 +392,6 @@ fail: return status; } -#ifdef USB_FSUBSET_INCLUDED - -static void -geth_old_unbind(struct usb_configuration *c, struct usb_function *f) -{ - geth_string_defs[0].id = 0; - usb_free_all_descriptors(f); - kfree(func_to_geth(f)); -} - -/** - * geth_bind_config - add CDC Subset network link to a configuration - * @c: the configuration to support the network link - * @ethaddr: a buffer in which the ethernet address of the host side - * side of the link was recorded - * @dev: eth_dev structure - * Context: single threaded during gadget setup - * - * Returns zero on success, else negative errno. - * - * Caller must have called @gether_setup(). Caller is also responsible - * for calling @gether_cleanup() before module unload. - */ -int geth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev) -{ - struct f_gether *geth; - int status; - - /* allocate and initialize one new instance */ - geth = kzalloc(sizeof *geth, GFP_KERNEL); - if (!geth) - return -ENOMEM; - - /* export host's Ethernet address in CDC format */ - snprintf(geth->ethaddr, sizeof geth->ethaddr, "%pm", ethaddr); - geth_string_defs[1].s = geth->ethaddr; - - geth->port.ioport = dev; - geth->port.cdc_filter = DEFAULT_FILTER; - - geth->port.func.name = "cdc_subset"; - geth->port.func.bind = geth_bind; - geth->port.func.unbind = geth_old_unbind; - geth->port.func.set_alt = geth_set_alt; - geth->port.func.disable = geth_disable; - - status = usb_add_function(c, &geth->port.func); - if (status) - kfree(geth); - return status; -} - -#else - static inline struct f_gether_opts *to_f_gether_opts(struct config_item *item) { return container_of(to_config_group(item), struct f_gether_opts, @@ -573,5 +517,3 @@ static struct usb_function *geth_alloc(struct usb_function_instance *fi) DECLARE_USB_FUNCTION_INIT(geth, geth_alloc_inst, geth_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); - -#endif diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h index 64125a5..f310e6f 100644 --- a/drivers/usb/gadget/u_ether.h +++ b/drivers/usb/gadget/u_ether.h @@ -268,9 +268,6 @@ static inline bool can_support_ecm(struct usb_gadget *gadget) } /* each configuration may bind one instance of an ethernet link */ -int geth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev); - #ifdef USB_ETH_RNDIS int rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/16] usb/gadget: g_ffs: convert to new interface of f_subset
There is a new function interface of f_subset and g_ffs is the last to use the old one. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 69 ++-- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 960f8a6..77411d7 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -884,6 +884,7 @@ config USB_FUNCTIONFS_ETH depends on USB_FUNCTIONFS && NET select USB_U_ETHER select USB_F_ECM + select USB_F_SUBSET help Include a configuration with CDC ECM function (Ethernet) and the Function Filesystem. diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index cda4c5d..99ae8ec 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -21,6 +21,8 @@ * a "gcc --combine ... part1.c part2.c part3.c ... " build would. */ #if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS +#include + # if defined USB_ETH_RNDIS #undef USB_ETH_RNDIS # endif @@ -29,8 +31,7 @@ # endif # include "u_ecm.h" -#define USB_FSUBSET_INCLUDED -# include "f_subset.c" +# include "u_gether.h" # ifdef USB_ETH_RNDIS #define USB_FRNDIS_INCLUDED #include "f_rndis.c" @@ -47,6 +48,8 @@ static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], struct eth_dev *dev); static struct usb_function_instance *fi_ecm; static struct usb_function *f_ecm; +static struct usb_function_instance *fi_geth; +static struct usb_function *f_geth; # endif #else # define the_dev NULL @@ -348,6 +351,9 @@ static void functionfs_release_dev_callback(struct ffs_data *ffs_data) */ static int gfs_bind(struct usb_composite_dev *cdev) { +#if defined CONFIG_USB_FUNCTIONFS_ETH + struct net_device *net; +#endif int ret, i; ENTER(); @@ -362,19 +368,25 @@ static int gfs_bind(struct usb_composite_dev *cdev) if (IS_ERR(fi_ecm)) return PTR_ERR(fi_ecm); ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); - - gether_set_qmult(ecm_opts->net, qmult); - - if (!gether_set_host_addr(ecm_opts->net, host_addr)) - pr_info("using host ethernet address: %s", host_addr); - if (!gether_set_dev_addr(ecm_opts->net, dev_addr)) - pr_info("using self ethernet address: %s", dev_addr); - - the_dev = netdev_priv(ecm_opts->net); + net = ecm_opts->net; } else { - the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, - gfs_host_mac, qmult); + struct f_gether_opts *geth_opts; + + fi_geth = usb_get_function_instance("geth"); + if (IS_ERR(fi_geth)) + return PTR_ERR(fi_geth); + geth_opts = container_of(fi_geth, struct f_gether_opts, +func_inst); + net = geth_opts->net; } + gether_set_qmult(net, qmult); + + if (!gether_set_host_addr(net, host_addr)) + pr_info("using host ethernet address: %s", host_addr); + if (!gether_set_dev_addr(net, dev_addr)) + pr_info("using self ethernet address: %s", dev_addr); + + the_dev = netdev_priv(net); #elif defined CONFIG_USB_FUNCTIONFS_RNDIS @@ -385,18 +397,24 @@ static int gfs_bind(struct usb_composite_dev *cdev) return PTR_ERR(the_dev); #if defined CONFIG_USB_FUNCTIONFS_RNDIS && defined CONFIG_USB_FUNCTIONFS_ETH + gether_set_gadget(net, cdev->gadget); + ret = gether_register_netdev(net); + if (ret) + goto error; + if (can_support_ecm(cdev->gadget)) { struct f_ecm_opts *ecm_opts; ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); - - gether_set_gadget(ecm_opts->net, cdev->gadget); - ret = gether_register_netdev(ecm_opts->net); - if (ret) - goto error; ecm_opts->bound = true; - gether_get_host_addr_u8(ecm_opts->net, gfs_host_mac); + } else { + struct f_gether_opts *geth_opts; + + geth_opts = container_of(fi_geth, struct f_gether_opts, +func_inst); + geth_opts->bound = true; } + gether_get_host_addr_u8(net, gfs_host_mac); #endif ret = usb_string_ids_tab(cdev, gfs_strings); @@ -437,7 +455,7 @@ error: if (can_support_ecm(cdev->gadget)) usb_put_function_instance(fi_ecm); else - gether_cleanup(the_dev); + usb_put_function_inst
[PATCH v2 01/16] usb/gadget: configfs: allow setting function instance's name
USB function's configfs config group is created in a generic way in usb/gadget/configfs.c:function_make(), which in turn delegates actual allocation and setup of the USB function instance to a particular implementation, e.g. in f_acm.c. The said implementation does its job in a parameter-less function e.g. acm_alloc_instance(), which results in creating an unnamed config group, whose name is set later in function_make(). function_make() creates the name by parsing a string of the form: . which comes from userspace as a parameter to mkdir invocation. Up to now only has been used, while has been ignored. This patch adds a set_inst_name() operation to struct usb_function_instance which allows passing the from function_make() so that it is not ignored. It is entirely up to the implementor of set_inst_name() what to do with the . In a typical case, the struct usb_function_instance is embedded in a larger struct which is retrieved in set_inst_name() with container_of(), and the larger struct contains a field to store the . Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/configfs.c |7 +++ include/linux/usb/composite.h |2 ++ 2 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 2588511..d6c8ab4 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -564,6 +564,13 @@ static struct config_group *function_make( usb_put_function_instance(fi); return ERR_PTR(ret); } + if (fi->set_inst_name) { + ret = fi->set_inst_name(fi, instance_name); + if (ret) { + usb_put_function_instance(fi); + return ERR_PTR(ret); + } + } gi = container_of(group, struct gadget_info, functions_group); diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h index 5e61589..dba63f5 100644 --- a/include/linux/usb/composite.h +++ b/include/linux/usb/composite.h @@ -468,6 +468,8 @@ struct usb_function_instance { struct config_group group; struct list_head cfs_list; struct usb_function_driver *fd; + int (*set_inst_name)(struct usb_function_instance *inst, + const char *name); void (*free_func_inst)(struct usb_function_instance *inst); }; -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/16] usb/gadget: f_ecm: remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmim Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_ecm.c | 73 +- drivers/usb/gadget/u_ether.h |2 - 2 files changed, 1 insertions(+), 74 deletions(-) diff --git a/drivers/usb/gadget/f_ecm.c b/drivers/usb/gadget/f_ecm.c index 8d9e6f7..798760f 100644 --- a/drivers/usb/gadget/f_ecm.c +++ b/drivers/usb/gadget/f_ecm.c @@ -691,7 +691,6 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) int status; struct usb_ep *ep; -#ifndef USBF_ECM_INCLUDED struct f_ecm_opts *ecm_opts; if (!can_support_ecm(cdev->gadget)) @@ -715,7 +714,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) return status; ecm_opts->bound = true; } -#endif + us = usb_gstrings_attach(cdev, ecm_strings, ARRAY_SIZE(ecm_string_defs)); if (IS_ERR(us)) @@ -834,74 +833,6 @@ fail: return status; } -#ifdef USBF_ECM_INCLUDED - -static void -ecm_old_unbind(struct usb_configuration *c, struct usb_function *f) -{ - struct f_ecm*ecm = func_to_ecm(f); - - DBG(c->cdev, "ecm unbind\n"); - - usb_free_all_descriptors(f); - - kfree(ecm->notify_req->buf); - usb_ep_free_request(ecm->notify, ecm->notify_req); - kfree(ecm); -} - -/** - * ecm_bind_config - add CDC Ethernet network link to a configuration - * @c: the configuration to support the network link - * @ethaddr: a buffer in which the ethernet address of the host side - * side of the link was recorded - * @dev: eth_dev structure - * Context: single threaded during gadget setup - * - * Returns zero on success, else negative errno. - * - * Caller must have called @gether_setup(). Caller is also responsible - * for calling @gether_cleanup() before module unload. - */ -int -ecm_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev) -{ - struct f_ecm*ecm; - int status; - - if (!can_support_ecm(c->cdev->gadget) || !ethaddr) - return -EINVAL; - - /* allocate and initialize one new instance */ - ecm = kzalloc(sizeof *ecm, GFP_KERNEL); - if (!ecm) - return -ENOMEM; - - /* export host's Ethernet address in CDC format */ - snprintf(ecm->ethaddr, sizeof ecm->ethaddr, "%pm", ethaddr); - ecm_string_defs[1].s = ecm->ethaddr; - - ecm->port.ioport = dev; - ecm->port.cdc_filter = DEFAULT_FILTER; - - ecm->port.func.name = "cdc_ethernet"; - /* descriptors are per-instance copies */ - ecm->port.func.bind = ecm_bind; - ecm->port.func.unbind = ecm_old_unbind; - ecm->port.func.set_alt = ecm_set_alt; - ecm->port.func.get_alt = ecm_get_alt; - ecm->port.func.setup = ecm_setup; - ecm->port.func.disable = ecm_disable; - - status = usb_add_function(c, &ecm->port.func); - if (status) - kfree(ecm); - return status; -} - -#else - static inline struct f_ecm_opts *to_f_ecm_opts(struct config_item *item) { return container_of(to_config_group(item), struct f_ecm_opts, @@ -1040,5 +971,3 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi) DECLARE_USB_FUNCTION_INIT(ecm, ecm_alloc_inst, ecm_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); - -#endif diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h index fb23d1f..64125a5 100644 --- a/drivers/usb/gadget/u_ether.h +++ b/drivers/usb/gadget/u_ether.h @@ -270,8 +270,6 @@ static inline bool can_support_ecm(struct usb_gadget *gadget) /* each configuration may bind one instance of an ethernet link */ int geth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], struct eth_dev *dev); -int ecm_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev); #ifdef USB_ETH_RNDIS -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/16] Equivalent of g_ffs with configfs
This series aims at integrating configfs into FunctionFS, the way it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet and mass_storage. It contains everything that is required to provide the equivalent of g_ffs.ko with configfs. Configfs support in FunctionFS has been awaited by the Android folks as discussed during the LPC 2013. Apart from that a VLAIS (variable length array in a struct) patch is added. It is not strictly necessary, but it won't hurt and it will make the Clang people happy. FunctionFS is more complicated than most other USB functions, so the resulting patch series is a bit long. Howerver, it is supposed to do things in steps. 1) preliminary work: extend generic configfs support in USB gadget, convert g_ffs to new function interfaces of f_ecm, f_subset, f_rndis; g_ffs is the last user of the old interfaces so remove compatibility layers 2) factoring out a u_fs.h header and finally making f_fs a separate module with both old and new function interface 3) converting all users of f_fs (in fact only g_ffs) to the new interface and remove compatiblilty layer 4) add configfs support v1..v2: - fixes after Michal's review, thank you, Michal! - removal of unnecessary hunks, simplified VLAIS replacement code, used proper pointer dereferencing, simplified ffs_do_functionfs_bind(), ffs_alloc_inst() and ffs_free(), style corrections, allocated usb_function structures in one chunk for all configs in g_ffs Rebased onto Felipe's master. At the moment I don't have any public git repo at hand, so a branch 'usb-gadget-configfs' will be available later at git://git.linaro.org/people/mszyprowski/linux-srpol.git BACKWARD COMPATIBILITY == Please note that the old g_ffs.ko is still available and works. USING THE NEW "GADGET" == Please refer to this post: http://www.spinics.net/lists/linux-usb/msg76388.html for general information from Sebastian on how to use configfs-based gadgets (*). With configfs the procedure is as follows, compared to the information mentioned above (*): instead of mkdir functions/acm.ttyS1 do mkdir functions/ffs. e.g. mkdir functions/ffs.ptp In functions/. there will be no attribute files at all, because all parameters are passed with FunctionFS. An example gadget with adb and ptp functions: (I assume there are ptpd and adbd daemons using FunctionFS available, a proof-of-concept adbd can be found here: https://android-review.googlesource.com/#/c/31640/, an early version of ptp which I used is here: http://www.spinics.net/lists/linux-usb/msg41963.html) $ modprobe libcomposite $ mount none cfg -t configfs $ mkdir cfg/usb_gadget/g1 $ cd cfg/usb_gadget/g1 $ mkdir configs/c.1 $ mkdir functions/ffs.adb $ mkdir functions/ffs.ptp $ mkdir strings/0x409 $ mkdir configs/c.1/strings/0x409 $ echo 0x2d01 > idProduct $ echo 0x04e8 > idVendor $ echo my-serial-num > strings/0x409/serialnumber $ echo my-manufacturer > strings/0x409/manufacturer $ echo "FunctionFS gadget (ptp, adb)" > strings/0x409/product $ echo "Conf 1" > configs/c.1/strings/0x409/configuration $ echo 120 > configs/c.1/MaxPower $ ln -s functions/ffs.ptp configs/c.1 $ ln -s functions/ffs.adb configs/c.1 $ mkdir /dev/usbffs $ mount ptp /dev/usbffs -t functionfs $ ptpd $ mkdir -p /dev/usbgadget/adb $ mount -t functionfs adb /dev/usbgadget/adb -o uid=2000,gid=2000 $ adbd $ echo s3c-hsotg > cfg/usb_gadget/g1/UDC After unbinding the gadget with echo "" > UDC the symbolic links in the configuration directory can be removed, the strings/* subdirectories in the configuration directory can be removed, the strings/* subdirectories at the gadget level can be removed and the configs/* subdirectories can be removed. The functions/* subdirectories can be removed. After that the gadget directory can be removed. FunctionFS instances should be unmounted, daemons need to be closed and then the respective modules can be unloaded. TESTING THE FUNCTIONS (actually there is only one) = FunctionFS) device: connect the gadget, enable it host: mount the ptp device, adb shell Andrzej Pietrasiewicz (16): usb/gadget: configfs: allow setting function instance's name usb/gadget: g_ffs: remove a reduntant gfs_ether_setup variable usb/gadget: g_ffs: convert to new interface of f_ecm usb/gadget: f_ecm: remove compatibility layer usb/gadget: g_ffs: convert to new interface of f_subset usb/gadget: f_subset: remove compatibility layer usb/gadget: g_ffs: convert to new interface of f_rndis usb/gadget: f_rndis: remove compatibility layer usb/gadget: rndis: merge u_rndis.ko with usb_f_rndis.ko usb/gadget: FunctionFS: Remove VLAIS usage from gadget code usb/gadget: FunctionFS: create utility file usb/gadget: FunctionFS: add devices management code usb/gadget: FunctionFS: convert to new function interface with backward compatibility usb/gadget: g_ffs: convert to new interface of f_fs usb/gadget: FunctionFS: Remove compatibili
[PATCH v2 07/16] usb/gadget: g_ffs: convert to new interface of f_rndis
There is a new interface of f_rndis and g_ffs is the last to use the old one. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 105 +++- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 77411d7..54b9856 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -894,6 +894,7 @@ config USB_FUNCTIONFS_RNDIS depends on USB_FUNCTIONFS && NET select USB_U_ETHER select USB_U_RNDIS + select USB_F_RNDIS help Include a configuration with RNDIS function (Ethernet) and the Filesystem. diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 99ae8ec..7099a11 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -33,29 +33,25 @@ # include "u_ecm.h" # include "u_gether.h" # ifdef USB_ETH_RNDIS -#define USB_FRNDIS_INCLUDED -#include "f_rndis.c" +#include "u_rndis.h" #include "rndis.h" # endif # include "u_ether.h" USB_ETHERNET_MODULE_PARAMETERS(); -static u8 gfs_host_mac[ETH_ALEN]; -static struct eth_dev *the_dev; # ifdef CONFIG_USB_FUNCTIONFS_ETH -static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev); +static int eth_bind_config(struct usb_configuration *c); static struct usb_function_instance *fi_ecm; static struct usb_function *f_ecm; static struct usb_function_instance *fi_geth; static struct usb_function *f_geth; # endif -#else -# define the_dev NULL -# define gether_cleanup(dev) do { } while (0) -# define gfs_host_mac NULL -struct eth_dev; +# ifdef CONFIG_USB_FUNCTIONFS_RNDIS +static int bind_rndis_config(struct usb_configuration *c); +static struct usb_function_instance *fi_rndis; +static struct usb_function *f_rndis; +# endif #endif #include "f_fs.c" @@ -148,12 +144,11 @@ static struct usb_gadget_strings *gfs_dev_strings[] = { struct gfs_configuration { struct usb_configuration c; - int (*eth)(struct usb_configuration *c, u8 *ethaddr, - struct eth_dev *dev); + int (*eth)(struct usb_configuration *c); } gfs_configurations[] = { #ifdef CONFIG_USB_FUNCTIONFS_RNDIS { - .eth= rndis_bind_config, + .eth= bind_rndis_config, }, #endif @@ -351,7 +346,7 @@ static void functionfs_release_dev_callback(struct ffs_data *ffs_data) */ static int gfs_bind(struct usb_composite_dev *cdev) { -#if defined CONFIG_USB_FUNCTIONFS_ETH +#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS struct net_device *net; #endif int ret, i; @@ -379,28 +374,38 @@ static int gfs_bind(struct usb_composite_dev *cdev) func_inst); net = geth_opts->net; } - gether_set_qmult(net, qmult); +#endif + +#ifdef CONFIG_USB_FUNCTIONFS_RNDIS + { + struct f_rndis_opts *rndis_opts; + + fi_rndis = usb_get_function_instance("rndis"); + if (IS_ERR(fi_rndis)) { + ret = PTR_ERR(fi_rndis); + goto error; + } + rndis_opts = container_of(fi_rndis, struct f_rndis_opts, + func_inst); +#ifndef CONFIG_USB_FUNCTIONFS_ETH + net = rndis_opts->net; +#endif + } +#endif +#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS + gether_set_qmult(net, qmult); if (!gether_set_host_addr(net, host_addr)) pr_info("using host ethernet address: %s", host_addr); if (!gether_set_dev_addr(net, dev_addr)) pr_info("using self ethernet address: %s", dev_addr); - - the_dev = netdev_priv(net); - -#elif defined CONFIG_USB_FUNCTIONFS_RNDIS - - the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, gfs_host_mac, - qmult); #endif - if (IS_ERR(the_dev)) - return PTR_ERR(the_dev); #if defined CONFIG_USB_FUNCTIONFS_RNDIS && defined CONFIG_USB_FUNCTIONFS_ETH gether_set_gadget(net, cdev->gadget); ret = gether_register_netdev(net); if (ret) - goto error; + goto error_rndis; if (can_support_ecm(cdev->gadget)) { struct f_ecm_opts *ecm_opts; @@ -414,12 +419,13 @@ static int gfs_bind(struct usb_composite_dev *cdev) func_inst); geth_opts->bound = true; } - gether_get_host_addr_u8(net, gfs_host_mac); + + rndis_borrow_net(fi_rndis, net); #endif ret = usb_string_ids_tab(cdev, gfs_strings); if (unlikely(ret < 0)) - goto error; +
[PATCH v2 02/16] usb/gadget: g_ffs: remove a reduntant gfs_ether_setup variable
Since d6a0143985489e470a118605352f4b18df0ce142 usb: gadget: move the global the_dev variable to their users "the_dev" variable can be used as a "setup done" flag; non-NULL meaning "setup done", NULL meaning "setup not done". Moreover, gether_cleanup() can be safely called with a NULL argument. Corrected a comment to be consistent with the code. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/g_ffs.c | 13 + 1 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 2344efe..e954082 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -182,7 +182,6 @@ static __refdata struct usb_composite_driver gfs_driver = { static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; -static bool gfs_ether_setup; static bool gfs_registered; static bool gfs_single_func; static struct gfs_ffs_obj *ffs_tab; @@ -364,7 +363,6 @@ static int gfs_bind(struct usb_composite_dev *cdev) ret = PTR_ERR(the_dev); goto error_quick; } - gfs_ether_setup = true; ret = usb_string_ids_tab(cdev, gfs_strings); if (unlikely(ret < 0)) @@ -401,8 +399,8 @@ error_unbind: functionfs_unbind(ffs_tab[i].ffs_data); error: gether_cleanup(the_dev); + the_dev = NULL; error_quick: - gfs_ether_setup = false; return ret; } @@ -415,18 +413,17 @@ static int gfs_unbind(struct usb_composite_dev *cdev) ENTER(); + gether_cleanup(the_dev); + the_dev = NULL; + /* * We may have been called in an error recovery from * composite_bind() after gfs_unbind() failure so we need to -* check if gfs_ffs_data is not NULL since gfs_bind() handles +* check if instance's ffs_data is not NULL since gfs_bind() handles * all error recovery itself. I'd rather we werent called * from composite on orror recovery, but what you're gonna * do...? */ - if (gfs_ether_setup) - gether_cleanup(the_dev); - gfs_ether_setup = false; - for (i = func_num; i--; ) if (ffs_tab[i].ffs_data) functionfs_unbind(ffs_tab[i].ffs_data); -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/16] usb/gadget: rndis: merge u_rndis.ko with usb_f_rndis.ko
The rndis function's users use only the new interface, so the two modules can be merged. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |7 --- drivers/usb/gadget/Makefile |4 +--- drivers/usb/gadget/f_rndis.c | 22 +- drivers/usb/gadget/rndis.c |7 ++- drivers/usb/gadget/u_rndis.h |2 ++ 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 54b9856..0d117e2 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -512,9 +512,6 @@ config USB_U_SERIAL config USB_U_ETHER tristate -config USB_U_RNDIS - tristate - config USB_F_SERIAL tristate @@ -642,7 +639,6 @@ config USB_CONFIGFS_RNDIS depends on USB_CONFIGFS depends on NET select USB_U_ETHER - select USB_U_RNDIS select USB_F_RNDIS help Microsoft Windows XP bundles the "Remote NDIS" (RNDIS) protocol, @@ -760,7 +756,6 @@ config USB_ETH depends on NET select USB_LIBCOMPOSITE select USB_U_ETHER - select USB_U_RNDIS select USB_F_ECM select USB_F_SUBSET select CRC32 @@ -893,7 +888,6 @@ config USB_FUNCTIONFS_RNDIS bool "Include configuration with RNDIS (Ethernet)" depends on USB_FUNCTIONFS && NET select USB_U_ETHER - select USB_U_RNDIS select USB_F_RNDIS help Include a configuration with RNDIS function (Ethernet) and the Filesystem. @@ -1068,7 +1062,6 @@ config USB_G_MULTI config USB_G_MULTI_RNDIS bool "RNDIS + CDC Serial + Storage configuration" depends on USB_G_MULTI - select USB_U_RNDIS select USB_F_RNDIS default y help diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index f1af396..fd9fe1f 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -47,8 +47,6 @@ obj-$(CONFIG_USB_F_SERIAL)+= usb_f_serial.o usb_f_obex-y := f_obex.o obj-$(CONFIG_USB_F_OBEX) += usb_f_obex.o obj-$(CONFIG_USB_U_ETHER) += u_ether.o -u_rndis-y := rndis.o -obj-$(CONFIG_USB_U_RNDIS) += u_rndis.o usb_f_ncm-y:= f_ncm.o obj-$(CONFIG_USB_F_NCM)+= usb_f_ncm.o usb_f_ecm-y:= f_ecm.o @@ -59,7 +57,7 @@ usb_f_eem-y := f_eem.o obj-$(CONFIG_USB_F_EEM)+= usb_f_eem.o usb_f_ecm_subset-y := f_subset.o obj-$(CONFIG_USB_F_SUBSET) += usb_f_ecm_subset.o -usb_f_rndis-y := f_rndis.o +usb_f_rndis-y := f_rndis.o rndis.o obj-$(CONFIG_USB_F_RNDIS) += usb_f_rndis.o usb_f_mass_storage-y := f_mass_storage.o storage_common.o obj-$(CONFIG_USB_F_MASS_STORAGE)+= usb_f_mass_storage.o diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c index 9d7c995..c11761c 100644 --- a/drivers/usb/gadget/f_rndis.c +++ b/drivers/usb/gadget/f_rndis.c @@ -979,6 +979,26 @@ static struct usb_function *rndis_alloc(struct usb_function_instance *fi) return &rndis->port.func; } -DECLARE_USB_FUNCTION_INIT(rndis, rndis_alloc_inst, rndis_alloc); +DECLARE_USB_FUNCTION(rndis, rndis_alloc_inst, rndis_alloc); + +static int __init rndis_mod_init(void) +{ + int ret; + + ret = rndis_init(); + if (ret) + return ret; + + return usb_function_register(&rndisusb_func); +} +module_init(rndis_mod_init); + +static void __exit rndis_mod_exit(void) +{ + usb_function_unregister(&rndisusb_func); + rndis_exit(); +} +module_exit(rndis_mod_exit); + MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c index 9575085..3184a13 100644 --- a/drivers/usb/gadget/rndis.c +++ b/drivers/usb/gadget/rndis.c @@ -1142,7 +1142,7 @@ static struct proc_dir_entry *rndis_connect_state [RNDIS_MAX_CONFIGS]; #endif /* CONFIG_USB_GADGET_DEBUG_FILES */ -static int rndis_init(void) +int rndis_init(void) { u8 i; @@ -1174,9 +1174,8 @@ static int rndis_init(void) return 0; } -module_init(rndis_init); -static void rndis_exit(void) +void rndis_exit(void) { #ifdef CONFIG_USB_GADGET_DEBUG_FILES u8 i; @@ -1188,6 +1187,4 @@ static void rndis_exit(void) } #endif } -module_exit(rndis_exit); -MODULE_LICENSE("GPL"); diff --git a/drivers/usb/gadget/u_rndis.h b/drivers/usb/gadget/u_rndis.h index c62ba82..7291b15 100644 --- a/drivers/usb/gadget/u_rndis.h +++ b/drivers/usb/gadget/u_rndis.h @@ -36,6 +36,8 @@ struct f_rndis_opts { int refcnt; }; +int rndis_init(void); +void rndis_exit(void); void rndis_borrow_net(struct usb_function_instance *f, struct net_device *net); #endif /* U_RNDIS_H */ -- 1.7.0.4 -- To unsubscribe from this list: send
[PATCH v2 10/16] usb/gadget: FunctionFS: Remove VLAIS usage from gadget code
The use of variable length arrays in structs (VLAIS) in the Linux Kernel code precludes the use of compilers which don't implement VLAIS (for instance the Clang compiler). This alternate patch calculates offsets into the kmalloc-ed memory buffer using macros. The previous patch required multiple kmalloc and kfree calls. This version uses "group" vs "struct" since it really is not a struct and is essentially a group of VLA in a common allocated block. This version also fixes the issues pointed out by Andrzej Pietrasiewicz and Michal Nazarewicz. Signed-off-by: Mark Charlebois Signed-off-by: Behan Webster [elimination of miexed declaration and code, checkpatch cleanup] [fixes after Michal's review] Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- drivers/usb/gadget/f_fs.c | 116 +--- 1 files changed, 76 insertions(+), 40 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 44cf775..b2fd90e 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -30,6 +30,31 @@ #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */ +/* Variable Length Array Macros **/ +#define vla_group(groupname) size_t groupname##__next = 0 +#define vla_group_size(groupname) groupname##__next + +#define vla_item(groupname, type, name, n) \ + size_t groupname##_##name##__offset = ({ \ + size_t align_mask = __alignof__(type) - 1; \ + size_t offset = (groupname##__next + align_mask) & ~align_mask;\ + size_t size = (n) * sizeof(type); \ + groupname##__next = offset + size; \ + offset;\ + }) + +#define vla_item_with_sz(groupname, type, name, n) \ + size_t groupname##_##name##__sz = (n) * sizeof(type); \ + size_t groupname##_##name##__offset = ({ \ + size_t align_mask = __alignof__(type) - 1; \ + size_t offset = (groupname##__next + align_mask) & ~align_mask;\ + size_t size = groupname##_##name##__sz;\ + groupname##__next = offset + size; \ + offset;\ + }) + +#define vla_ptr(ptr, groupname, name) \ + ((void *) ((char *)ptr + groupname##_##name##__offset)) /* Debugging / @@ -1901,30 +1926,34 @@ static int __ffs_data_got_strings(struct ffs_data *ffs, /* Allocate everything in one chunk so there's less maintenance. */ { - struct { - struct usb_gadget_strings *stringtabs[lang_count + 1]; - struct usb_gadget_strings stringtab[lang_count]; - struct usb_string strings[lang_count*(needed_count+1)]; - } *d; unsigned i = 0; + vla_group(d); + vla_item(d, struct usb_gadget_strings *, stringtabs, + lang_count + 1); + vla_item(d, struct usb_gadget_strings, stringtab, lang_count); + vla_item(d, struct usb_string, strings, + lang_count*(needed_count+1)); - d = kmalloc(sizeof *d, GFP_KERNEL); - if (unlikely(!d)) { + char *vlabuf = kmalloc(vla_group_size(d), GFP_KERNEL); + + if (unlikely(!vlabuf)) { kfree(_data); return -ENOMEM; } - stringtabs = d->stringtabs; - t = d->stringtab; + /* Initialize the VLA pointers */ + stringtabs = vla_ptr(vlabuf, d, stringtabs); + t = vla_ptr(vlabuf, d, stringtab); i = lang_count; do { *stringtabs++ = t++; } while (--i); *stringtabs = NULL; - stringtabs = d->stringtabs; - t = d->stringtab; - s = d->strings; + /* stringtabs = vlabuf = d_stringtabs for later kfree */ + stringtabs = vla_ptr(vlabuf, d, stringtabs); + t = vla_ptr(vlabuf, d, stringtab); + s = vla_ptr(vlabuf, d, strings); strings = s; } @@ -2200,16 +2229,16 @@ static int ffs_func_bind(struct usb_configuration *c, int ret; /* Make it a single chunk, less management later on */ - struct { - struct ffs_ep eps[ffs->eps_count]; - struct usb_descriptor_header - *fs_descs[full ? ffs->fs_descs_count + 1 : 0]; -
[PATCH v2 11/16] usb/gadget: FunctionFS: create utility file
A header file to be used by f_fs.c and g_ffs.c will be required when f_fs.c is converted into a module. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_fs.c |1 + drivers/usb/gadget/g_ffs.c | 19 ++- drivers/usb/gadget/u_fs.h | 28 3 files changed, 35 insertions(+), 13 deletions(-) create mode 100644 drivers/usb/gadget/u_fs.h diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index b2fd90e..277d193 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -27,6 +27,7 @@ #include #include +#include "u_fs.h" #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */ diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 7099a11..1aaa103 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -69,13 +69,6 @@ MODULE_LICENSE("GPL"); #define GFS_MAX_DEVS 10 -struct gfs_ffs_obj { - const char *name; - bool mounted; - bool desc_ready; - struct ffs_data *ffs_data; -}; - USB_GADGET_COMPOSITE_OPTIONS(); static struct usb_device_descriptor gfs_dev_desc = { @@ -181,7 +174,7 @@ static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; static bool gfs_registered; static bool gfs_single_func; -static struct gfs_ffs_obj *ffs_tab; +static struct ffs_dev *ffs_tab; static int __init gfs_init(void) { @@ -224,7 +217,7 @@ static void __exit gfs_exit(void) } module_exit(gfs_exit); -static struct gfs_ffs_obj *gfs_find_dev(const char *dev_name) +static struct ffs_dev *gfs_find_dev(const char *dev_name) { int i; @@ -242,7 +235,7 @@ static struct gfs_ffs_obj *gfs_find_dev(const char *dev_name) static int functionfs_ready_callback(struct ffs_data *ffs) { - struct gfs_ffs_obj *ffs_obj; + struct ffs_dev *ffs_obj; int ret; ENTER(); @@ -283,7 +276,7 @@ done: static void functionfs_closed_callback(struct ffs_data *ffs) { - struct gfs_ffs_obj *ffs_obj; + struct ffs_dev *ffs_obj; ENTER(); mutex_lock(&gfs_lock); @@ -305,7 +298,7 @@ done: static void *functionfs_acquire_dev_callback(const char *dev_name) { - struct gfs_ffs_obj *ffs_dev; + struct ffs_dev *ffs_dev; ENTER(); mutex_lock(&gfs_lock); @@ -329,7 +322,7 @@ done: static void functionfs_release_dev_callback(struct ffs_data *ffs_data) { - struct gfs_ffs_obj *ffs_dev; + struct ffs_dev *ffs_dev; ENTER(); mutex_lock(&gfs_lock); diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h new file mode 100644 index 000..5d9229a --- /dev/null +++ b/drivers/usb/gadget/u_fs.h @@ -0,0 +1,28 @@ +/* + * u_fs.h + * + * Utility definitions for the FunctionFS + * + * Copyright (c) 2013 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Author: Andrzej Pietrasiewicz + * + * 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 U_FFS_H +#define U_FFS_H + +#include + +struct ffs_dev { + const char *name; + bool mounted; + bool desc_ready; + struct ffs_data *ffs_data; +}; + +#endif /* U_FFS_H */ -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/16] usb/gadget: FunctionFS: add devices management code
This will be required in order to use the new function interface (usb_get_function_instance/usb_put_function_instance) Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyunmgin Park --- drivers/usb/gadget/f_fs.c | 41 drivers/usb/gadget/g_ffs.c | 50 ++- drivers/usb/gadget/u_fs.h |2 + 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 277d193..bfbb580 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -361,6 +361,11 @@ ffs_sb_create_file(struct super_block *sb, const char *name, void *data, const struct file_operations *fops, struct dentry **dentry_p); +/* Devices management ***/ + +static struct ffs_dev *ffs_alloc_dev(void); +static void ffs_free_dev(struct ffs_dev *dev); +static struct ffs_dev *ffs_find_dev(const char *name); /* Misc helper functions / @@ -2465,6 +2470,42 @@ static int ffs_func_revmap_intf(struct ffs_function *func, u8 intf) } +/* Devices management ***/ + +static LIST_HEAD(ffs_devices); + +static struct ffs_dev *ffs_alloc_dev(void) +{ + struct ffs_dev *dev; + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) + return ERR_PTR(-ENOMEM); + + list_add(&dev->entry, &ffs_devices); + + return dev; +} + +static void ffs_free_dev(struct ffs_dev *dev) +{ + list_del(&dev->entry); + + kfree(dev); +} + +static struct ffs_dev *ffs_find_dev(const char *name) +{ + struct ffs_dev *dev; + + list_for_each_entry(dev, &ffs_devices, entry) + if (strcmp(dev->name, name) == 0) + return dev; + + return NULL; +} + + /* Misc helper functions / static int ffs_mutex_lock(struct mutex *mutex, unsigned nonblock) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 1aaa103..48b0940 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -174,35 +174,49 @@ static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; static bool gfs_registered; static bool gfs_single_func; -static struct ffs_dev *ffs_tab; +static struct ffs_dev **ffs_tab; static int __init gfs_init(void) { int i; + int ret = 0; ENTER(); - if (!func_num) { + if (func_num < 2) { gfs_single_func = true; func_num = 1; } - ffs_tab = kcalloc(func_num, sizeof *ffs_tab, GFP_KERNEL); + ffs_tab = kcalloc(func_num, sizeof(*ffs_tab), GFP_KERNEL); if (!ffs_tab) return -ENOMEM; - if (!gfs_single_func) - for (i = 0; i < func_num; i++) - ffs_tab[i].name = func_names[i]; + for (i = 0; i < func_num; i++) { + ffs_tab[i] = ffs_alloc_dev(); + if (IS_ERR(ffs_tab[i])) { + ret = PTR_ERR(ffs_tab[i]); + goto no_dev; + } + if (!gfs_single_func) + ffs_tab[i]->name = func_names[i]; + } missing_funcs = func_num; return functionfs_init(); +no_dev: + while (--i >= 0) + ffs_free_dev(ffs_tab[i]); + kfree(ffs_tab); + return ret; } module_init(gfs_init); static void __exit gfs_exit(void) { + int i; + ENTER(); mutex_lock(&gfs_lock); @@ -213,24 +227,20 @@ static void __exit gfs_exit(void) functionfs_cleanup(); mutex_unlock(&gfs_lock); + for (i = 0; i < func_num; i++) + ffs_free_dev(ffs_tab[i]); kfree(ffs_tab); } module_exit(gfs_exit); static struct ffs_dev *gfs_find_dev(const char *dev_name) { - int i; - ENTER(); if (gfs_single_func) - return &ffs_tab[0]; - - for (i = 0; i < func_num; i++) - if (strcmp(ffs_tab[i].name, dev_name) == 0) - return &ffs_tab[i]; + return ffs_tab[0]; - return NULL; + return ffs_find_dev(dev_name); } static int functionfs_ready_callback(struct ffs_data *ffs) @@ -422,10 +432,10 @@ static int gfs_bind(struct usb_composite_dev *cdev) gfs_dev_desc.iProduct = gfs_strings[USB_GADGET_PRODUCT_IDX].id; for (i = func_num; i--; ) { - ret = functionfs_bind(ffs_tab[i].ffs_data, cdev); + ret = functionfs_bind(ffs_tab[i]->ffs_data, cdev); if (unlikely(ret < 0)) { while (++i < func_num) - functionfs_unbind(ffs_tab[i].ffs_data); + functionfs_unbind(ffs_tab[i]->ffs_data); g
[PATCH v2 08/16] usb/gadget: f_rndis: remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_rndis.c | 72 +- drivers/usb/gadget/u_ether.h | 36 - 2 files changed, 1 insertions(+), 107 deletions(-) diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c index 717ed7f..9d7c995 100644 --- a/drivers/usb/gadget/f_rndis.c +++ b/drivers/usb/gadget/f_rndis.c @@ -675,7 +675,6 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f) int status; struct usb_ep *ep; -#ifndef USB_FRNDIS_INCLUDED struct f_rndis_opts *rndis_opts; if (!can_support_rndis(c)) @@ -697,7 +696,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f) return status; rndis_opts->bound = true; } -#endif + us = usb_gstrings_attach(cdev, rndis_strings, ARRAY_SIZE(rndis_string_defs)); if (IS_ERR(us)) @@ -782,13 +781,6 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f) rndis->port.open = rndis_open; rndis->port.close = rndis_close; -#ifdef USB_FRNDIS_INCLUDED - status = rndis_register(rndis_response_available, rndis); - if (status < 0) - goto fail; - rndis->config = status; -#endif - rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3, 0); rndis_set_host_mac(rndis->config, rndis->ethaddr); @@ -830,66 +822,6 @@ fail: return status; } -#ifdef USB_FRNDIS_INCLUDED - -static void -rndis_old_unbind(struct usb_configuration *c, struct usb_function *f) -{ - struct f_rndis *rndis = func_to_rndis(f); - - rndis_deregister(rndis->config); - - usb_free_all_descriptors(f); - - kfree(rndis->notify_req->buf); - usb_ep_free_request(rndis->notify, rndis->notify_req); - - kfree(rndis); -} - -int -rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - u32 vendorID, const char *manufacturer, struct eth_dev *dev) -{ - struct f_rndis *rndis; - int status; - - /* allocate and initialize one new instance */ - status = -ENOMEM; - rndis = kzalloc(sizeof *rndis, GFP_KERNEL); - if (!rndis) - goto fail; - - memcpy(rndis->ethaddr, ethaddr, ETH_ALEN); - rndis->vendorID = vendorID; - rndis->manufacturer = manufacturer; - - rndis->port.ioport = dev; - /* RNDIS activates when the host changes this filter */ - rndis->port.cdc_filter = 0; - - /* RNDIS has special (and complex) framing */ - rndis->port.header_len = sizeof(struct rndis_packet_msg_type); - rndis->port.wrap = rndis_add_header; - rndis->port.unwrap = rndis_rm_hdr; - - rndis->port.func.name = "rndis"; - /* descriptors are per-instance copies */ - rndis->port.func.bind = rndis_bind; - rndis->port.func.unbind = rndis_old_unbind; - rndis->port.func.set_alt = rndis_set_alt; - rndis->port.func.setup = rndis_setup; - rndis->port.func.disable = rndis_disable; - - status = usb_add_function(c, &rndis->port.func); - if (status) - kfree(rndis); -fail: - return status; -} - -#else - void rndis_borrow_net(struct usb_function_instance *f, struct net_device *net) { struct f_rndis_opts *opts; @@ -1050,5 +982,3 @@ static struct usb_function *rndis_alloc(struct usb_function_instance *fi) DECLARE_USB_FUNCTION_INIT(rndis, rndis_alloc_inst, rndis_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); - -#endif diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h index f310e6f..f1e0cbe 100644 --- a/drivers/usb/gadget/u_ether.h +++ b/drivers/usb/gadget/u_ether.h @@ -267,40 +267,4 @@ static inline bool can_support_ecm(struct usb_gadget *gadget) return true; } -/* each configuration may bind one instance of an ethernet link */ -#ifdef USB_ETH_RNDIS - -int rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - u32 vendorID, const char *manufacturer, struct eth_dev *dev); - -#else - -static inline int -rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - u32 vendorID, const char *manufacturer, struct eth_dev *dev) -{ - return 0; -} - -#endif - -/** - * rndis_bind_config - add RNDIS network link to a configuration - * @c: the configuration to support the network link - * @ethaddr: a buffer in which the ethernet address of the host side - * side of the link was recorded - * Context: single threaded during gadget setup - * - * Returns zero on success, else negative errno. - * - * Caller must have called @gether_setup(). Caller is also responsible - * for calling @get
[PATCH RESEND v2 00/16] Equivalent of g_ffs with configfs
This series aims at integrating configfs into FunctionFS, the way it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet and mass_storage. It contains everything that is required to provide the equivalent of g_ffs.ko with configfs. Configfs support in FunctionFS has been awaited by the Android folks as discussed during the LPC 2013. Apart from that a VLAIS (variable length array in a struct) patch is added. It is not strictly necessary, but it won't hurt and it will make the Clang people happy. FunctionFS is more complicated than most other USB functions, so the resulting patch series is a bit long. Howerver, it is supposed to do things in steps. 1) preliminary work: extend generic configfs support in USB gadget, convert g_ffs to new function interfaces of f_ecm, f_subset, f_rndis; g_ffs is the last user of the old interfaces so remove compatibility layers 2) factoring out a u_fs.h header and finally making f_fs a separate module with both old and new function interface 3) converting all users of f_fs (in fact only g_ffs) to the new interface and remove compatiblilty layer 4) add configfs support v1..v2: - fixes after Michal's review, thank you, Michal! - removal of unnecessary hunks, simplified VLAIS replacement code, used proper pointer dereferencing, simplified ffs_do_functionfs_bind(), ffs_alloc_inst() and ffs_free(), style corrections, allocated usb_function structures in one chunk for all configs in g_ffs Rebased onto Felipe's master. At the moment I don't have any public git repo at hand, so a branch 'usb-gadget-configfs' will be available later at git://git.linaro.org/people/mszyprowski/linux-srpol.git BACKWARD COMPATIBILITY == Please note that the old g_ffs.ko is still available and works. USING THE NEW "GADGET" == Please refer to this post: http://www.spinics.net/lists/linux-usb/msg76388.html for general information from Sebastian on how to use configfs-based gadgets (*). With configfs the procedure is as follows, compared to the information mentioned above (*): instead of mkdir functions/acm.ttyS1 do mkdir functions/ffs. e.g. mkdir functions/ffs.ptp In functions/. there will be no attribute files at all, because all parameters are passed with FunctionFS. An example gadget with adb and ptp functions: (I assume there are ptpd and adbd daemons using FunctionFS available, a proof-of-concept adbd can be found here: https://android-review.googlesource.com/#/c/31640/, an early version of ptp which I used is here: http://www.spinics.net/lists/linux-usb/msg41963.html) $ modprobe libcomposite $ mount none cfg -t configfs $ mkdir cfg/usb_gadget/g1 $ cd cfg/usb_gadget/g1 $ mkdir configs/c.1 $ mkdir functions/ffs.adb $ mkdir functions/ffs.ptp $ mkdir strings/0x409 $ mkdir configs/c.1/strings/0x409 $ echo 0x2d01 > idProduct $ echo 0x04e8 > idVendor $ echo my-serial-num > strings/0x409/serialnumber $ echo my-manufacturer > strings/0x409/manufacturer $ echo "FunctionFS gadget (ptp, adb)" > strings/0x409/product $ echo "Conf 1" > configs/c.1/strings/0x409/configuration $ echo 120 > configs/c.1/MaxPower $ ln -s functions/ffs.ptp configs/c.1 $ ln -s functions/ffs.adb configs/c.1 $ mkdir /dev/usbffs $ mount ptp /dev/usbffs -t functionfs $ ptpd $ mkdir -p /dev/usbgadget/adb $ mount -t functionfs adb /dev/usbgadget/adb -o uid=2000,gid=2000 $ adbd $ echo s3c-hsotg > cfg/usb_gadget/g1/UDC After unbinding the gadget with echo "" > UDC the symbolic links in the configuration directory can be removed, the strings/* subdirectories in the configuration directory can be removed, the strings/* subdirectories at the gadget level can be removed and the configs/* subdirectories can be removed. The functions/* subdirectories can be removed. After that the gadget directory can be removed. FunctionFS instances should be unmounted, daemons need to be closed and then the respective modules can be unloaded. TESTING THE FUNCTIONS (actually there is only one) = FunctionFS) device: connect the gadget, enable it host: mount the ptp device, adb shell Andrzej Pietrasiewicz (16): usb/gadget: configfs: allow setting function instance's name usb/gadget: g_ffs: remove a reduntant gfs_ether_setup variable usb/gadget: g_ffs: convert to new interface of f_ecm usb/gadget: f_ecm: remove compatibility layer usb/gadget: g_ffs: convert to new interface of f_subset usb/gadget: f_subset: remove compatibility layer usb/gadget: g_ffs: convert to new interface of f_rndis usb/gadget: f_rndis: remove compatibility layer usb/gadget: rndis: merge u_rndis.ko with usb_f_rndis.ko usb/gadget: FunctionFS: Remove VLAIS usage from gadget code usb/gadget: FunctionFS: create utility file usb/gadget: FunctionFS: add devices management code usb/gadget: FunctionFS: convert to new function interface with backward compatibility usb/gadget: g_ffs: convert to new interface of f_fs usb/gadget: FunctionFS: Remove compatibili
[PATCH RESEND v2 02/16] usb/gadget: g_ffs: remove a reduntant gfs_ether_setup variable
Since d6a0143985489e470a118605352f4b18df0ce142 usb: gadget: move the global the_dev variable to their users "the_dev" variable can be used as a "setup done" flag; non-NULL meaning "setup done", NULL meaning "setup not done". Moreover, gether_cleanup() can be safely called with a NULL argument. Corrected a comment to be consistent with the code. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/g_ffs.c | 13 + 1 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 2344efe..e954082 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -182,7 +182,6 @@ static __refdata struct usb_composite_driver gfs_driver = { static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; -static bool gfs_ether_setup; static bool gfs_registered; static bool gfs_single_func; static struct gfs_ffs_obj *ffs_tab; @@ -364,7 +363,6 @@ static int gfs_bind(struct usb_composite_dev *cdev) ret = PTR_ERR(the_dev); goto error_quick; } - gfs_ether_setup = true; ret = usb_string_ids_tab(cdev, gfs_strings); if (unlikely(ret < 0)) @@ -401,8 +399,8 @@ error_unbind: functionfs_unbind(ffs_tab[i].ffs_data); error: gether_cleanup(the_dev); + the_dev = NULL; error_quick: - gfs_ether_setup = false; return ret; } @@ -415,18 +413,17 @@ static int gfs_unbind(struct usb_composite_dev *cdev) ENTER(); + gether_cleanup(the_dev); + the_dev = NULL; + /* * We may have been called in an error recovery from * composite_bind() after gfs_unbind() failure so we need to -* check if gfs_ffs_data is not NULL since gfs_bind() handles +* check if instance's ffs_data is not NULL since gfs_bind() handles * all error recovery itself. I'd rather we werent called * from composite on orror recovery, but what you're gonna * do...? */ - if (gfs_ether_setup) - gether_cleanup(the_dev); - gfs_ether_setup = false; - for (i = func_num; i--; ) if (ffs_tab[i].ffs_data) functionfs_unbind(ffs_tab[i].ffs_data); -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND v2 05/16] usb/gadget: g_ffs: convert to new interface of f_subset
There is a new function interface of f_subset and g_ffs is the last to use the old one. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 69 ++-- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 960f8a6..77411d7 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -884,6 +884,7 @@ config USB_FUNCTIONFS_ETH depends on USB_FUNCTIONFS && NET select USB_U_ETHER select USB_F_ECM + select USB_F_SUBSET help Include a configuration with CDC ECM function (Ethernet) and the Function Filesystem. diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index cda4c5d..99ae8ec 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -21,6 +21,8 @@ * a "gcc --combine ... part1.c part2.c part3.c ... " build would. */ #if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS +#include + # if defined USB_ETH_RNDIS #undef USB_ETH_RNDIS # endif @@ -29,8 +31,7 @@ # endif # include "u_ecm.h" -#define USB_FSUBSET_INCLUDED -# include "f_subset.c" +# include "u_gether.h" # ifdef USB_ETH_RNDIS #define USB_FRNDIS_INCLUDED #include "f_rndis.c" @@ -47,6 +48,8 @@ static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], struct eth_dev *dev); static struct usb_function_instance *fi_ecm; static struct usb_function *f_ecm; +static struct usb_function_instance *fi_geth; +static struct usb_function *f_geth; # endif #else # define the_dev NULL @@ -348,6 +351,9 @@ static void functionfs_release_dev_callback(struct ffs_data *ffs_data) */ static int gfs_bind(struct usb_composite_dev *cdev) { +#if defined CONFIG_USB_FUNCTIONFS_ETH + struct net_device *net; +#endif int ret, i; ENTER(); @@ -362,19 +368,25 @@ static int gfs_bind(struct usb_composite_dev *cdev) if (IS_ERR(fi_ecm)) return PTR_ERR(fi_ecm); ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); - - gether_set_qmult(ecm_opts->net, qmult); - - if (!gether_set_host_addr(ecm_opts->net, host_addr)) - pr_info("using host ethernet address: %s", host_addr); - if (!gether_set_dev_addr(ecm_opts->net, dev_addr)) - pr_info("using self ethernet address: %s", dev_addr); - - the_dev = netdev_priv(ecm_opts->net); + net = ecm_opts->net; } else { - the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, - gfs_host_mac, qmult); + struct f_gether_opts *geth_opts; + + fi_geth = usb_get_function_instance("geth"); + if (IS_ERR(fi_geth)) + return PTR_ERR(fi_geth); + geth_opts = container_of(fi_geth, struct f_gether_opts, +func_inst); + net = geth_opts->net; } + gether_set_qmult(net, qmult); + + if (!gether_set_host_addr(net, host_addr)) + pr_info("using host ethernet address: %s", host_addr); + if (!gether_set_dev_addr(net, dev_addr)) + pr_info("using self ethernet address: %s", dev_addr); + + the_dev = netdev_priv(net); #elif defined CONFIG_USB_FUNCTIONFS_RNDIS @@ -385,18 +397,24 @@ static int gfs_bind(struct usb_composite_dev *cdev) return PTR_ERR(the_dev); #if defined CONFIG_USB_FUNCTIONFS_RNDIS && defined CONFIG_USB_FUNCTIONFS_ETH + gether_set_gadget(net, cdev->gadget); + ret = gether_register_netdev(net); + if (ret) + goto error; + if (can_support_ecm(cdev->gadget)) { struct f_ecm_opts *ecm_opts; ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); - - gether_set_gadget(ecm_opts->net, cdev->gadget); - ret = gether_register_netdev(ecm_opts->net); - if (ret) - goto error; ecm_opts->bound = true; - gether_get_host_addr_u8(ecm_opts->net, gfs_host_mac); + } else { + struct f_gether_opts *geth_opts; + + geth_opts = container_of(fi_geth, struct f_gether_opts, +func_inst); + geth_opts->bound = true; } + gether_get_host_addr_u8(net, gfs_host_mac); #endif ret = usb_string_ids_tab(cdev, gfs_strings); @@ -437,7 +455,7 @@ error: if (can_support_ecm(cdev->gadget)) usb_put_function_instance(fi_ecm); else - gether_cleanup(the_dev); + usb_put_function_inst
[PATCH RESEND v2 03/16] usb/gadget: g_ffs: convert to new interface of f_ecm
There is a new funtion interface and g_ffs is the last gadget to use the old. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 93 +--- 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index f66d96a..960f8a6 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -883,6 +883,7 @@ config USB_FUNCTIONFS_ETH bool "Include configuration with CDC ECM (Ethernet)" depends on USB_FUNCTIONFS && NET select USB_U_ETHER + select USB_F_ECM help Include a configuration with CDC ECM function (Ethernet) and the Function Filesystem. diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index e954082..cda4c5d 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -28,8 +28,7 @@ #define USB_ETH_RNDIS y # endif -#define USBF_ECM_INCLUDED -# include "f_ecm.c" +# include "u_ecm.h" #define USB_FSUBSET_INCLUDED # include "f_subset.c" # ifdef USB_ETH_RNDIS @@ -39,11 +38,15 @@ # endif # include "u_ether.h" +USB_ETHERNET_MODULE_PARAMETERS(); + static u8 gfs_host_mac[ETH_ALEN]; static struct eth_dev *the_dev; # ifdef CONFIG_USB_FUNCTIONFS_ETH static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], struct eth_dev *dev); +static struct usb_function_instance *fi_ecm; +static struct usb_function *f_ecm; # endif #else # define the_dev NULL @@ -76,10 +79,6 @@ struct gfs_ffs_obj { USB_GADGET_COMPOSITE_OPTIONS(); -#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS -USB_ETHERNET_MODULE_PARAMETERS(); -#endif - static struct usb_device_descriptor gfs_dev_desc = { .bLength= sizeof gfs_dev_desc, .bDescriptorType= USB_DT_DEVICE, @@ -355,14 +354,50 @@ static int gfs_bind(struct usb_composite_dev *cdev) if (missing_funcs) return -ENODEV; -#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS +#if defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) { + struct f_ecm_opts *ecm_opts; + + fi_ecm = usb_get_function_instance("ecm"); + if (IS_ERR(fi_ecm)) + return PTR_ERR(fi_ecm); + ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); + + gether_set_qmult(ecm_opts->net, qmult); + + if (!gether_set_host_addr(ecm_opts->net, host_addr)) + pr_info("using host ethernet address: %s", host_addr); + if (!gether_set_dev_addr(ecm_opts->net, dev_addr)) + pr_info("using self ethernet address: %s", dev_addr); + + the_dev = netdev_priv(ecm_opts->net); + } else { + the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, + gfs_host_mac, qmult); + } + +#elif defined CONFIG_USB_FUNCTIONFS_RNDIS + the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, gfs_host_mac, qmult); #endif - if (IS_ERR(the_dev)) { - ret = PTR_ERR(the_dev); - goto error_quick; + if (IS_ERR(the_dev)) + return PTR_ERR(the_dev); + +#if defined CONFIG_USB_FUNCTIONFS_RNDIS && defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) { + struct f_ecm_opts *ecm_opts; + + ecm_opts = container_of(fi_ecm, struct f_ecm_opts, func_inst); + + gether_set_gadget(ecm_opts->net, cdev->gadget); + ret = gether_register_netdev(ecm_opts->net); + if (ret) + goto error; + ecm_opts->bound = true; + gether_get_host_addr_u8(ecm_opts->net, gfs_host_mac); } +#endif ret = usb_string_ids_tab(cdev, gfs_strings); if (unlikely(ret < 0)) @@ -398,9 +433,16 @@ error_unbind: for (i = 0; i < func_num; i++) functionfs_unbind(ffs_tab[i].ffs_data); error: +#if defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) + usb_put_function_instance(fi_ecm); + else + gether_cleanup(the_dev); + the_dev = NULL; +#elif defined CONFIG_USB_FUNCTIONFS_RNDIS gether_cleanup(the_dev); the_dev = NULL; -error_quick: +#endif return ret; } @@ -413,8 +455,19 @@ static int gfs_unbind(struct usb_composite_dev *cdev) ENTER(); + +#if defined CONFIG_USB_FUNCTIONFS_ETH + if (can_support_ecm(cdev->gadget)) { + usb_put_function(f_ecm); + usb_put_function_instance(fi_ecm); + } else { + gether_cleanup(the_dev); + } + the_de
[PATCH RESEND v2 16/16] usb/gadget: FunctionFS: add configfs support
Add support for using FunctionFS in configfs-based USB gadgets. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- Documentation/ABI/testing/configfs-usb-gadget-ffs |9 ++ drivers/usb/gadget/Kconfig| 12 ++ drivers/usb/gadget/f_fs.c | 150 - drivers/usb/gadget/u_fs.h |5 + 4 files changed, 175 insertions(+), 1 deletions(-) create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-ffs diff --git a/Documentation/ABI/testing/configfs-usb-gadget-ffs b/Documentation/ABI/testing/configfs-usb-gadget-ffs new file mode 100644 index 000..14343e2 --- /dev/null +++ b/Documentation/ABI/testing/configfs-usb-gadget-ffs @@ -0,0 +1,9 @@ +What: /config/usb-gadget/gadget/functions/ffs.name +Date: Nov 2013 +KenelVersion: 3.13 +Description: The purpose of this directory is to create and remove it. + + A corresponding USB function instance is created/removed. + There are no attributes here. + + All parameters are set through FunctionFS. diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 2479644..8da2b1d 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -689,6 +689,18 @@ config USB_CONFIGFS_MASS_STORAGE device (in much the same way as the "loop" device driver), specified as a module parameter or sysfs option. +config USB_CONFIGFS_F_FS + boolean "Function filesystem (FunctionFS)" + depends on USB_CONFIGFS + select USB_F_FS + help + The Function Filesystem (FunctionFS) lets one create USB + composite functions in user space in the same way GadgetFS + lets one create USB gadgets in user space. This allows creation + of composite gadgets such that some of the functions are + implemented in kernel space (for instance Ethernet, serial or + mass storage) and other are implemented in user space. + config USB_ZERO tristate "Gadget Zero (DEVELOPMENT)" select USB_LIBCOMPOSITE diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index b24d754..f5c9b5c 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -29,6 +29,7 @@ #include #include "u_fs.h" +#include "configfs.h" #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */ @@ -161,6 +162,7 @@ static struct ffs_dev *ffs_alloc_dev(void); static void ffs_free_dev(struct ffs_dev *dev); static struct mutex *ffs_dev_lock; +static DEFINE_MUTEX(_ffs_dev_lock); static void *(*ffs_acquire_dev_cb)(const char *name); static void (*ffs_release_dev_cb)(struct ffs_data *ffs_data); @@ -2362,6 +2364,117 @@ void ffs_set_closed_cb(void (*cb)(struct ffs_data *ffs_data)) EXPORT_SYMBOL(ffs_set_closed_cb); +/* Configfs support */ + +static int ffs_ready_callback(struct ffs_data *ffs) +{ + struct ffs_dev *ffs_dev; + int ret = 0; + + ENTER(); + mutex_lock(ffs_dev_lock); + + ffs_dev = ffs->private_data; + if (!ffs_dev) { + ret = -EINVAL; + goto done; + } + + if (WARN_ON(ffs_dev->desc_ready)) { + ret = -EBUSY; + goto done; + } + ffs_dev->desc_ready = true; + ffs_dev->ffs_data = ffs; + +done: + mutex_unlock(ffs_dev_lock); + return ret; +} + +static void ffs_closed_callback(struct ffs_data *ffs) +{ + struct ffs_dev *ffs_dev; + + ENTER(); + mutex_lock(ffs_dev_lock); + + ffs_dev = ffs->private_data; + if (!ffs_dev) + goto done; + + ffs_dev->desc_ready = false; + if (!ffs_dev->opts || !ffs_dev->opts->func_inst.group.cg_item.ci_parent) + goto done; + + unregister_gadget_item(ffs_dev->opts-> + func_inst.group.cg_item.ci_parent->ci_parent); + +done: + mutex_unlock(ffs_dev_lock); +} + +static void *ffs_acquire_dev_callback(const char *dev_name) +{ + struct ffs_dev *ffs_dev; + + ENTER(); + mutex_lock(ffs_dev_lock); + + ffs_dev = ffs_find_dev(dev_name); + if (!ffs_dev) { + ffs_dev = ERR_PTR(-ENODEV); + goto done; + } + + if (ffs_dev->mounted) { + ffs_dev = ERR_PTR(-EBUSY); + goto done; + } + ffs_dev->mounted = true; + +done: + mutex_unlock(ffs_dev_lock); + return ffs_dev; +} + +static inline struct f_fs_opts *to_ffs_opts(struct config_item *item) +{ + return container_of(to_config_group(item), struct f_fs_opts, + func_inst.group); +} + +static void ffs_release_dev_callback(struct ffs_data *ffs_data) +{ + struct ffs_dev *ffs_dev; + + ENTER(); + mutex_lock(ffs_dev_lock); + + ffs_dev = ffs_data-
[PATCH RESEND v2 13/16] usb/gadget: FunctionFS: convert to new function interface with backward compatibility
This is required in order to integrate configfs support. f_fs needs to be a separately compiled module and so it needs to use the new interface. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |3 + drivers/usb/gadget/Makefile|2 + drivers/usb/gadget/f_fs.c | 568 +--- drivers/usb/gadget/g_ffs.c |3 +- drivers/usb/gadget/u_fs.h | 221 include/linux/usb/functionfs.h |5 +- 6 files changed, 586 insertions(+), 216 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 0d117e2..66444d0 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -539,6 +539,9 @@ config USB_F_RNDIS config USB_F_MASS_STORAGE tristate +config USB_F_FS + tristate + choice tristate "USB Gadget Drivers" default USB_ETH diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index fd9fe1f..6bb1155 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -61,6 +61,8 @@ usb_f_rndis-y := f_rndis.o rndis.o obj-$(CONFIG_USB_F_RNDIS) += usb_f_rndis.o usb_f_mass_storage-y := f_mass_storage.o storage_common.o obj-$(CONFIG_USB_F_MASS_STORAGE)+= usb_f_mass_storage.o +usb_f_fs-y := f_fs.o +obj-$(CONFIG_USB_F_FS) += usb_f_fs.o # # USB gadget drivers diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index bfbb580..469cb86 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -57,210 +58,6 @@ #define vla_ptr(ptr, groupname, name) \ ((void *) ((char *)ptr + groupname##_##name##__offset)) -/* Debugging / - -#ifdef VERBOSE_DEBUG -#ifndef pr_vdebug -# define pr_vdebug pr_debug -#endif /* pr_vdebug */ -# define ffs_dump_mem(prefix, ptr, len) \ - print_hex_dump_bytes(pr_fmt(prefix ": "), DUMP_PREFIX_NONE, ptr, len) -#else -#ifndef pr_vdebug -# define pr_vdebug(...) do { } while (0) -#endif /* pr_vdebug */ -# define ffs_dump_mem(prefix, ptr, len) do { } while (0) -#endif /* VERBOSE_DEBUG */ - -#define ENTER()pr_vdebug("%s()\n", __func__) - - -/* The data structure and setup file / - -enum ffs_state { - /* -* Waiting for descriptors and strings. -* -* In this state no open(2), read(2) or write(2) on epfiles -* may succeed (which should not be the problem as there -* should be no such files opened in the first place). -*/ - FFS_READ_DESCRIPTORS, - FFS_READ_STRINGS, - - /* -* We've got descriptors and strings. We are or have called -* functionfs_ready_callback(). functionfs_bind() may have -* been called but we don't know. -* -* This is the only state in which operations on epfiles may -* succeed. -*/ - FFS_ACTIVE, - - /* -* All endpoints have been closed. This state is also set if -* we encounter an unrecoverable error. The only -* unrecoverable error is situation when after reading strings -* from user space we fail to initialise epfiles or -* functionfs_ready_callback() returns with error (<0). -* -* In this state no open(2), read(2) or write(2) (both on ep0 -* as well as epfile) may succeed (at this point epfiles are -* unlinked and all closed so this is not a problem; ep0 is -* also closed but ep0 file exists and so open(2) on ep0 must -* fail). -*/ - FFS_CLOSING -}; - - -enum ffs_setup_state { - /* There is no setup request pending. */ - FFS_NO_SETUP, - /* -* User has read events and there was a setup request event -* there. The next read/write on ep0 will handle the -* request. -*/ - FFS_SETUP_PENDING, - /* -* There was event pending but before user space handled it -* some other event was introduced which canceled existing -* setup. If this state is set read/write on ep0 return -* -EIDRM. This state is only set when adding event. -*/ - FFS_SETUP_CANCELED -}; - - - -struct ffs_epfile; -struct ffs_function; - -struct ffs_data { - struct usb_gadget *gadget; - - /* -* Protect access read/write operations, only one read/write -* at a time. As a consequence protects ep0req and company. -* While setup request is being processed (queued) this is -* held. -*/ - struct mutexmutex; - - /* -* Protect access to endpoint related structures (basically -* usb_ep_queue(), usb_ep_dequeue(
[PATCH RESEND v2 04/16] usb/gadget: f_ecm: remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmim Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_ecm.c | 73 +- drivers/usb/gadget/u_ether.h |2 - 2 files changed, 1 insertions(+), 74 deletions(-) diff --git a/drivers/usb/gadget/f_ecm.c b/drivers/usb/gadget/f_ecm.c index 8d9e6f7..798760f 100644 --- a/drivers/usb/gadget/f_ecm.c +++ b/drivers/usb/gadget/f_ecm.c @@ -691,7 +691,6 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) int status; struct usb_ep *ep; -#ifndef USBF_ECM_INCLUDED struct f_ecm_opts *ecm_opts; if (!can_support_ecm(cdev->gadget)) @@ -715,7 +714,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f) return status; ecm_opts->bound = true; } -#endif + us = usb_gstrings_attach(cdev, ecm_strings, ARRAY_SIZE(ecm_string_defs)); if (IS_ERR(us)) @@ -834,74 +833,6 @@ fail: return status; } -#ifdef USBF_ECM_INCLUDED - -static void -ecm_old_unbind(struct usb_configuration *c, struct usb_function *f) -{ - struct f_ecm*ecm = func_to_ecm(f); - - DBG(c->cdev, "ecm unbind\n"); - - usb_free_all_descriptors(f); - - kfree(ecm->notify_req->buf); - usb_ep_free_request(ecm->notify, ecm->notify_req); - kfree(ecm); -} - -/** - * ecm_bind_config - add CDC Ethernet network link to a configuration - * @c: the configuration to support the network link - * @ethaddr: a buffer in which the ethernet address of the host side - * side of the link was recorded - * @dev: eth_dev structure - * Context: single threaded during gadget setup - * - * Returns zero on success, else negative errno. - * - * Caller must have called @gether_setup(). Caller is also responsible - * for calling @gether_cleanup() before module unload. - */ -int -ecm_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev) -{ - struct f_ecm*ecm; - int status; - - if (!can_support_ecm(c->cdev->gadget) || !ethaddr) - return -EINVAL; - - /* allocate and initialize one new instance */ - ecm = kzalloc(sizeof *ecm, GFP_KERNEL); - if (!ecm) - return -ENOMEM; - - /* export host's Ethernet address in CDC format */ - snprintf(ecm->ethaddr, sizeof ecm->ethaddr, "%pm", ethaddr); - ecm_string_defs[1].s = ecm->ethaddr; - - ecm->port.ioport = dev; - ecm->port.cdc_filter = DEFAULT_FILTER; - - ecm->port.func.name = "cdc_ethernet"; - /* descriptors are per-instance copies */ - ecm->port.func.bind = ecm_bind; - ecm->port.func.unbind = ecm_old_unbind; - ecm->port.func.set_alt = ecm_set_alt; - ecm->port.func.get_alt = ecm_get_alt; - ecm->port.func.setup = ecm_setup; - ecm->port.func.disable = ecm_disable; - - status = usb_add_function(c, &ecm->port.func); - if (status) - kfree(ecm); - return status; -} - -#else - static inline struct f_ecm_opts *to_f_ecm_opts(struct config_item *item) { return container_of(to_config_group(item), struct f_ecm_opts, @@ -1040,5 +971,3 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi) DECLARE_USB_FUNCTION_INIT(ecm, ecm_alloc_inst, ecm_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); - -#endif diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h index fb23d1f..64125a5 100644 --- a/drivers/usb/gadget/u_ether.h +++ b/drivers/usb/gadget/u_ether.h @@ -270,8 +270,6 @@ static inline bool can_support_ecm(struct usb_gadget *gadget) /* each configuration may bind one instance of an ethernet link */ int geth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], struct eth_dev *dev); -int ecm_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev); #ifdef USB_ETH_RNDIS -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND v2 01/16] usb/gadget: configfs: allow setting function instance's name
USB function's configfs config group is created in a generic way in usb/gadget/configfs.c:function_make(), which in turn delegates actual allocation and setup of the USB function instance to a particular implementation, e.g. in f_acm.c. The said implementation does its job in a parameter-less function e.g. acm_alloc_instance(), which results in creating an unnamed config group, whose name is set later in function_make(). function_make() creates the name by parsing a string of the form: . which comes from userspace as a parameter to mkdir invocation. Up to now only has been used, while has been ignored. This patch adds a set_inst_name() operation to struct usb_function_instance which allows passing the from function_make() so that it is not ignored. It is entirely up to the implementor of set_inst_name() what to do with the . In a typical case, the struct usb_function_instance is embedded in a larger struct which is retrieved in set_inst_name() with container_of(), and the larger struct contains a field to store the . Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/configfs.c |7 +++ include/linux/usb/composite.h |2 ++ 2 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 2588511..d6c8ab4 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -564,6 +564,13 @@ static struct config_group *function_make( usb_put_function_instance(fi); return ERR_PTR(ret); } + if (fi->set_inst_name) { + ret = fi->set_inst_name(fi, instance_name); + if (ret) { + usb_put_function_instance(fi); + return ERR_PTR(ret); + } + } gi = container_of(group, struct gadget_info, functions_group); diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h index 5e61589..dba63f5 100644 --- a/include/linux/usb/composite.h +++ b/include/linux/usb/composite.h @@ -468,6 +468,8 @@ struct usb_function_instance { struct config_group group; struct list_head cfs_list; struct usb_function_driver *fd; + int (*set_inst_name)(struct usb_function_instance *inst, + const char *name); void (*free_func_inst)(struct usb_function_instance *inst); }; -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND v2 10/16] usb/gadget: FunctionFS: Remove VLAIS usage from gadget code
The use of variable length arrays in structs (VLAIS) in the Linux Kernel code precludes the use of compilers which don't implement VLAIS (for instance the Clang compiler). This alternate patch calculates offsets into the kmalloc-ed memory buffer using macros. The previous patch required multiple kmalloc and kfree calls. This version uses "group" vs "struct" since it really is not a struct and is essentially a group of VLA in a common allocated block. This version also fixes the issues pointed out by Andrzej Pietrasiewicz and Michal Nazarewicz. Signed-off-by: Mark Charlebois Signed-off-by: Behan Webster [elimination of miexed declaration and code, checkpatch cleanup] [fixes after Michal's review] Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- drivers/usb/gadget/f_fs.c | 116 +--- 1 files changed, 76 insertions(+), 40 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 44cf775..b2fd90e 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -30,6 +30,31 @@ #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */ +/* Variable Length Array Macros **/ +#define vla_group(groupname) size_t groupname##__next = 0 +#define vla_group_size(groupname) groupname##__next + +#define vla_item(groupname, type, name, n) \ + size_t groupname##_##name##__offset = ({ \ + size_t align_mask = __alignof__(type) - 1; \ + size_t offset = (groupname##__next + align_mask) & ~align_mask;\ + size_t size = (n) * sizeof(type); \ + groupname##__next = offset + size; \ + offset;\ + }) + +#define vla_item_with_sz(groupname, type, name, n) \ + size_t groupname##_##name##__sz = (n) * sizeof(type); \ + size_t groupname##_##name##__offset = ({ \ + size_t align_mask = __alignof__(type) - 1; \ + size_t offset = (groupname##__next + align_mask) & ~align_mask;\ + size_t size = groupname##_##name##__sz;\ + groupname##__next = offset + size; \ + offset;\ + }) + +#define vla_ptr(ptr, groupname, name) \ + ((void *) ((char *)ptr + groupname##_##name##__offset)) /* Debugging / @@ -1901,30 +1926,34 @@ static int __ffs_data_got_strings(struct ffs_data *ffs, /* Allocate everything in one chunk so there's less maintenance. */ { - struct { - struct usb_gadget_strings *stringtabs[lang_count + 1]; - struct usb_gadget_strings stringtab[lang_count]; - struct usb_string strings[lang_count*(needed_count+1)]; - } *d; unsigned i = 0; + vla_group(d); + vla_item(d, struct usb_gadget_strings *, stringtabs, + lang_count + 1); + vla_item(d, struct usb_gadget_strings, stringtab, lang_count); + vla_item(d, struct usb_string, strings, + lang_count*(needed_count+1)); - d = kmalloc(sizeof *d, GFP_KERNEL); - if (unlikely(!d)) { + char *vlabuf = kmalloc(vla_group_size(d), GFP_KERNEL); + + if (unlikely(!vlabuf)) { kfree(_data); return -ENOMEM; } - stringtabs = d->stringtabs; - t = d->stringtab; + /* Initialize the VLA pointers */ + stringtabs = vla_ptr(vlabuf, d, stringtabs); + t = vla_ptr(vlabuf, d, stringtab); i = lang_count; do { *stringtabs++ = t++; } while (--i); *stringtabs = NULL; - stringtabs = d->stringtabs; - t = d->stringtab; - s = d->strings; + /* stringtabs = vlabuf = d_stringtabs for later kfree */ + stringtabs = vla_ptr(vlabuf, d, stringtabs); + t = vla_ptr(vlabuf, d, stringtab); + s = vla_ptr(vlabuf, d, strings); strings = s; } @@ -2200,16 +2229,16 @@ static int ffs_func_bind(struct usb_configuration *c, int ret; /* Make it a single chunk, less management later on */ - struct { - struct ffs_ep eps[ffs->eps_count]; - struct usb_descriptor_header - *fs_descs[full ? ffs->fs_descs_count + 1 : 0]; -
[PATCH RESEND v2 12/16] usb/gadget: FunctionFS: add devices management code
This will be required in order to use the new function interface (usb_get_function_instance/usb_put_function_instance) Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- drivers/usb/gadget/f_fs.c | 41 drivers/usb/gadget/g_ffs.c | 50 ++- drivers/usb/gadget/u_fs.h |2 + 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 277d193..bfbb580 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -361,6 +361,11 @@ ffs_sb_create_file(struct super_block *sb, const char *name, void *data, const struct file_operations *fops, struct dentry **dentry_p); +/* Devices management ***/ + +static struct ffs_dev *ffs_alloc_dev(void); +static void ffs_free_dev(struct ffs_dev *dev); +static struct ffs_dev *ffs_find_dev(const char *name); /* Misc helper functions / @@ -2465,6 +2470,42 @@ static int ffs_func_revmap_intf(struct ffs_function *func, u8 intf) } +/* Devices management ***/ + +static LIST_HEAD(ffs_devices); + +static struct ffs_dev *ffs_alloc_dev(void) +{ + struct ffs_dev *dev; + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) + return ERR_PTR(-ENOMEM); + + list_add(&dev->entry, &ffs_devices); + + return dev; +} + +static void ffs_free_dev(struct ffs_dev *dev) +{ + list_del(&dev->entry); + + kfree(dev); +} + +static struct ffs_dev *ffs_find_dev(const char *name) +{ + struct ffs_dev *dev; + + list_for_each_entry(dev, &ffs_devices, entry) + if (strcmp(dev->name, name) == 0) + return dev; + + return NULL; +} + + /* Misc helper functions / static int ffs_mutex_lock(struct mutex *mutex, unsigned nonblock) diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 1aaa103..48b0940 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -174,35 +174,49 @@ static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; static bool gfs_registered; static bool gfs_single_func; -static struct ffs_dev *ffs_tab; +static struct ffs_dev **ffs_tab; static int __init gfs_init(void) { int i; + int ret = 0; ENTER(); - if (!func_num) { + if (func_num < 2) { gfs_single_func = true; func_num = 1; } - ffs_tab = kcalloc(func_num, sizeof *ffs_tab, GFP_KERNEL); + ffs_tab = kcalloc(func_num, sizeof(*ffs_tab), GFP_KERNEL); if (!ffs_tab) return -ENOMEM; - if (!gfs_single_func) - for (i = 0; i < func_num; i++) - ffs_tab[i].name = func_names[i]; + for (i = 0; i < func_num; i++) { + ffs_tab[i] = ffs_alloc_dev(); + if (IS_ERR(ffs_tab[i])) { + ret = PTR_ERR(ffs_tab[i]); + goto no_dev; + } + if (!gfs_single_func) + ffs_tab[i]->name = func_names[i]; + } missing_funcs = func_num; return functionfs_init(); +no_dev: + while (--i >= 0) + ffs_free_dev(ffs_tab[i]); + kfree(ffs_tab); + return ret; } module_init(gfs_init); static void __exit gfs_exit(void) { + int i; + ENTER(); mutex_lock(&gfs_lock); @@ -213,24 +227,20 @@ static void __exit gfs_exit(void) functionfs_cleanup(); mutex_unlock(&gfs_lock); + for (i = 0; i < func_num; i++) + ffs_free_dev(ffs_tab[i]); kfree(ffs_tab); } module_exit(gfs_exit); static struct ffs_dev *gfs_find_dev(const char *dev_name) { - int i; - ENTER(); if (gfs_single_func) - return &ffs_tab[0]; - - for (i = 0; i < func_num; i++) - if (strcmp(ffs_tab[i].name, dev_name) == 0) - return &ffs_tab[i]; + return ffs_tab[0]; - return NULL; + return ffs_find_dev(dev_name); } static int functionfs_ready_callback(struct ffs_data *ffs) @@ -422,10 +432,10 @@ static int gfs_bind(struct usb_composite_dev *cdev) gfs_dev_desc.iProduct = gfs_strings[USB_GADGET_PRODUCT_IDX].id; for (i = func_num; i--; ) { - ret = functionfs_bind(ffs_tab[i].ffs_data, cdev); + ret = functionfs_bind(ffs_tab[i]->ffs_data, cdev); if (unlikely(ret < 0)) { while (++i < func_num) - functionfs_unbind(ffs_tab[i].ffs_data); + functionfs_unbind(ffs_tab[i]->ffs_data); g
[PATCH RESEND v2 14/16] usb/gadget: g_ffs: convert to new interface of f_fs
Prepare for configfs integration. Use the new interface so that f_fs can be made a module. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 135 2 files changed, 87 insertions(+), 49 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 66444d0..2479644 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -862,6 +862,7 @@ config USB_GADGETFS config USB_FUNCTIONFS tristate "Function Filesystem" select USB_LIBCOMPOSITE + select USB_F_FS select USB_FUNCTIONFS_GENERIC if !(USB_FUNCTIONFS_ETH || USB_FUNCTIONFS_RNDIS) help The Function Filesystem (FunctionFS) lets one create USB diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index caf0035..6a06a61 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -13,13 +13,7 @@ #define pr_fmt(fmt) "g_ffs: " fmt #include -/* - * kbuild is not very cooperative with respect to linking separately - * compiled library objects into one module. So for now we won't use - * separate compilation ... ensuring init/exit sections work to shrink - * the runtime footprint, and giving us at least some parts of what - * a "gcc --combine ... part1.c part2.c part3.c ... " build would. - */ + #if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS #include @@ -54,8 +48,7 @@ static struct usb_function *f_rndis; # endif #endif -#define USB_FFS_INCLUDED -#include "f_fs.c" +#include "u_fs.h" #define DRIVER_NAME"g_ffs" #define DRIVER_DESC"USB Function Filesystem" @@ -139,6 +132,7 @@ static struct usb_gadget_strings *gfs_dev_strings[] = { struct gfs_configuration { struct usb_configuration c; int (*eth)(struct usb_configuration *c); + int num; } gfs_configurations[] = { #ifdef CONFIG_USB_FUNCTIONFS_RNDIS { @@ -161,6 +155,11 @@ struct gfs_configuration { static int gfs_bind(struct usb_composite_dev *cdev); static int gfs_unbind(struct usb_composite_dev *cdev); static int gfs_do_config(struct usb_configuration *c); +static int functionfs_ready_callback(struct ffs_data *ffs); +static void functionfs_closed_callback(struct ffs_data *ffs); +static void *functionfs_acquire_dev_callback(const char *dev_name); +static void functionfs_release_dev_callback(struct ffs_data *ffs_data); + static __refdata struct usb_composite_driver gfs_driver = { .name = DRIVER_NAME, @@ -175,7 +174,22 @@ static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; static bool gfs_registered; static bool gfs_single_func; -static struct ffs_dev **ffs_tab; +static struct usb_function_instance **fi_ffs; +static struct usb_function **f_ffs[] = { +#ifdef CONFIG_USB_FUNCTIONFS_RNDIS + NULL, +#endif + +#ifdef CONFIG_USB_FUNCTIONFS_ETH + NULL, +#endif + +#ifdef CONFIG_USB_FUNCTIONFS_GENERIC + NULL, +#endif +}; + +#define N_CONF ARRAY_SIZE(f_ffs) static int __init gfs_init(void) { @@ -184,32 +198,54 @@ static int __init gfs_init(void) ENTER(); + ffs_set_auto_init(false); + if (func_num < 2) { gfs_single_func = true; func_num = 1; } - ffs_tab = kcalloc(func_num, sizeof(*ffs_tab), GFP_KERNEL); - if (!ffs_tab) - return -ENOMEM; + /* +* Allocate in one chunk for easier maintenance +*/ + f_ffs[0] = kcalloc(func_num * N_CONF, sizeof(*f_ffs), GFP_KERNEL); + if (!f_ffs[0]) { + ret = -ENOMEM; + goto no_func; + } + for (i = 1; i < N_CONF; ++i) + f_ffs[i] = f_ffs[0] + i * func_num; + + fi_ffs = kcalloc(func_num, sizeof(*fi_ffs), GFP_KERNEL); + if (!fi_ffs) { + ret = -ENOMEM; + goto no_func; + } for (i = 0; i < func_num; i++) { - ffs_tab[i] = ffs_alloc_dev(); - if (IS_ERR(ffs_tab[i])) { - ret = PTR_ERR(ffs_tab[i]); + fi_ffs[i] = usb_get_function_instance("ffs"); + if (IS_ERR(fi_ffs[i])) { + ret = PTR_ERR(fi_ffs[i]); goto no_dev; } if (!gfs_single_func) - ffs_tab[i]->name = func_names[i]; + to_f_fs_opts(fi_ffs[i])->dev->name = func_names[i]; } missing_funcs = func_num; - return functionfs_init(NULL); + ffs_set_acquire_dev_cb(functionfs_acquire_dev_callback); + ffs_set_release_dev_cb(functionfs_release_dev_callback); + ffs_set_ready_cb(functionfs_ready_callback); + ffs_set_closed_cb(functionfs_closed_callback); + return functionfs_init(THIS_MODULE); no_dev: while (--i >= 0) - ffs_free_dev(ffs_tab[i]); - kfree(ffs_tab); +
[PATCH RESEND v2 15/16] usb/gadget: FunctionFS: Remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_fs.c | 148 +--- drivers/usb/gadget/u_fs.h |2 - include/linux/usb/functionfs.h | 33 - 3 files changed, 2 insertions(+), 181 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 469cb86..b24d754 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -97,19 +97,12 @@ static struct ffs_function *ffs_func_from_usb(struct usb_function *f) return container_of(f, struct ffs_function, function); } -#ifdef USB_FFS_INCLUDED -static void ffs_func_free(struct ffs_function *func); -#endif static void ffs_func_eps_disable(struct ffs_function *func); static int __must_check ffs_func_eps_enable(struct ffs_function *func); static int ffs_func_bind(struct usb_configuration *, struct usb_function *); -#ifdef USB_FFS_INCLUDED -static void old_ffs_func_unbind(struct usb_configuration *, - struct usb_function *); -#endif static int ffs_func_set_alt(struct usb_function *, unsigned, unsigned); static void ffs_func_disable(struct usb_function *); static int ffs_func_setup(struct usb_function *, @@ -167,7 +160,6 @@ ffs_sb_create_file(struct super_block *sb, const char *name, void *data, static struct ffs_dev *ffs_alloc_dev(void); static void ffs_free_dev(struct ffs_dev *dev); -#ifndef USB_FFS_INCLUDED static struct mutex *ffs_dev_lock; static void *(*ffs_acquire_dev_cb)(const char *name); @@ -182,7 +174,6 @@ static struct kref auto_init_ref; static DEFINE_MUTEX(auto_init_lock); static struct module *ffs_owner; -#endif /* Misc helper functions / @@ -308,11 +299,8 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf, ffs->state = FFS_ACTIVE; mutex_unlock(&ffs->mutex); -#ifdef USB_FFS_INCLUDED - ret = functionfs_ready_callback(ffs); -#else ret = ffs_ready_cb(ffs); -#endif + if (unlikely(ret < 0)) { ffs->state = FFS_CLOSING; return ret; @@ -1043,13 +1031,6 @@ ffs_fs_mount(struct file_system_type *t, int flags, return ERR_PTR(-ENOMEM); } -#ifdef USB_FFS_INCLUDED - ffs_dev = functionfs_acquire_dev_callback(dev_name); - if (IS_ERR(ffs_dev)) { - ffs_data_put(ffs); - return ERR_CAST(ffs_dev); - } -#else if (ffs_owner && !try_module_get(ffs_owner)) { ffs_data_put(ffs); return ERR_PTR(-ENODEV); @@ -1062,21 +1043,16 @@ ffs_fs_mount(struct file_system_type *t, int flags, return ERR_CAST(ffs_dev); } -#endif + ffs->private_data = ffs_dev; data.ffs_data = ffs; rv = mount_nodev(t, flags, &data, ffs_sb_fill); if (IS_ERR(rv) && data.ffs_data) { -#ifdef USB_FFS_INCLUDED - functionfs_release_dev_callback(data.ffs_data); - ffs_data_put(data.ffs_data); -#else ffs_release_dev_cb(data.ffs_data); ffs_data_put(data.ffs_data); if (ffs_owner) module_put(ffs_owner); -#endif } return rv; } @@ -1088,15 +1064,10 @@ ffs_fs_kill_sb(struct super_block *sb) kill_litter_super(sb); if (sb->s_fs_info) { -#ifdef USB_FFS_INCLUDED - functionfs_release_dev_callback(sb->s_fs_info); - ffs_data_put(sb->s_fs_info); -#else ffs_release_dev_cb(sb->s_fs_info); ffs_data_put(sb->s_fs_info); if (ffs_owner) module_put(ffs_owner); -#endif } } @@ -1117,7 +1088,6 @@ int functionfs_init(struct module *owner) ENTER(); -#ifndef USB_FFS_INCLUDED if (!ffs_acquire_dev_cb || !ffs_release_dev_cb || !ffs_ready_cb || !ffs_closed_cb) { ret = -ENODEV; @@ -1125,7 +1095,6 @@ int functionfs_init(struct module *owner) return ret; } ffs_owner = owner; -#endif ret = register_filesystem(&ffs_fs_type); if (likely(!ret)) @@ -1135,17 +1104,13 @@ int functionfs_init(struct module *owner) return ret; } -#ifndef USB_FFS_INCLUDED EXPORT_SYMBOL(functionfs_init); -#endif void functionfs_cleanup(void) { -#ifndef USB_FFS_INCLUDED ffs_owner = NULL; do_auto_init = true; auto_init_active = true; -#endif ENTER(); @@ -1153,9 +1118,7 @@ void functionfs_cleanup(void) unregister_filesystem(&ffs_fs_type); } -#ifndef USB_FFS_INCLUDED EXPORT_SYMBOL(functionfs_cleanup); -#endif /* ffs_data and ff
[PATCH RESEND v2 09/16] usb/gadget: rndis: merge u_rndis.ko with usb_f_rndis.ko
The rndis function's users use only the new interface, so the two modules can be merged. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |7 --- drivers/usb/gadget/Makefile |4 +--- drivers/usb/gadget/f_rndis.c | 22 +- drivers/usb/gadget/rndis.c |7 ++- drivers/usb/gadget/u_rndis.h |2 ++ 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 54b9856..0d117e2 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -512,9 +512,6 @@ config USB_U_SERIAL config USB_U_ETHER tristate -config USB_U_RNDIS - tristate - config USB_F_SERIAL tristate @@ -642,7 +639,6 @@ config USB_CONFIGFS_RNDIS depends on USB_CONFIGFS depends on NET select USB_U_ETHER - select USB_U_RNDIS select USB_F_RNDIS help Microsoft Windows XP bundles the "Remote NDIS" (RNDIS) protocol, @@ -760,7 +756,6 @@ config USB_ETH depends on NET select USB_LIBCOMPOSITE select USB_U_ETHER - select USB_U_RNDIS select USB_F_ECM select USB_F_SUBSET select CRC32 @@ -893,7 +888,6 @@ config USB_FUNCTIONFS_RNDIS bool "Include configuration with RNDIS (Ethernet)" depends on USB_FUNCTIONFS && NET select USB_U_ETHER - select USB_U_RNDIS select USB_F_RNDIS help Include a configuration with RNDIS function (Ethernet) and the Filesystem. @@ -1068,7 +1062,6 @@ config USB_G_MULTI config USB_G_MULTI_RNDIS bool "RNDIS + CDC Serial + Storage configuration" depends on USB_G_MULTI - select USB_U_RNDIS select USB_F_RNDIS default y help diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index f1af396..fd9fe1f 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -47,8 +47,6 @@ obj-$(CONFIG_USB_F_SERIAL)+= usb_f_serial.o usb_f_obex-y := f_obex.o obj-$(CONFIG_USB_F_OBEX) += usb_f_obex.o obj-$(CONFIG_USB_U_ETHER) += u_ether.o -u_rndis-y := rndis.o -obj-$(CONFIG_USB_U_RNDIS) += u_rndis.o usb_f_ncm-y:= f_ncm.o obj-$(CONFIG_USB_F_NCM)+= usb_f_ncm.o usb_f_ecm-y:= f_ecm.o @@ -59,7 +57,7 @@ usb_f_eem-y := f_eem.o obj-$(CONFIG_USB_F_EEM)+= usb_f_eem.o usb_f_ecm_subset-y := f_subset.o obj-$(CONFIG_USB_F_SUBSET) += usb_f_ecm_subset.o -usb_f_rndis-y := f_rndis.o +usb_f_rndis-y := f_rndis.o rndis.o obj-$(CONFIG_USB_F_RNDIS) += usb_f_rndis.o usb_f_mass_storage-y := f_mass_storage.o storage_common.o obj-$(CONFIG_USB_F_MASS_STORAGE)+= usb_f_mass_storage.o diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c index 9d7c995..c11761c 100644 --- a/drivers/usb/gadget/f_rndis.c +++ b/drivers/usb/gadget/f_rndis.c @@ -979,6 +979,26 @@ static struct usb_function *rndis_alloc(struct usb_function_instance *fi) return &rndis->port.func; } -DECLARE_USB_FUNCTION_INIT(rndis, rndis_alloc_inst, rndis_alloc); +DECLARE_USB_FUNCTION(rndis, rndis_alloc_inst, rndis_alloc); + +static int __init rndis_mod_init(void) +{ + int ret; + + ret = rndis_init(); + if (ret) + return ret; + + return usb_function_register(&rndisusb_func); +} +module_init(rndis_mod_init); + +static void __exit rndis_mod_exit(void) +{ + usb_function_unregister(&rndisusb_func); + rndis_exit(); +} +module_exit(rndis_mod_exit); + MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c index 9575085..3184a13 100644 --- a/drivers/usb/gadget/rndis.c +++ b/drivers/usb/gadget/rndis.c @@ -1142,7 +1142,7 @@ static struct proc_dir_entry *rndis_connect_state [RNDIS_MAX_CONFIGS]; #endif /* CONFIG_USB_GADGET_DEBUG_FILES */ -static int rndis_init(void) +int rndis_init(void) { u8 i; @@ -1174,9 +1174,8 @@ static int rndis_init(void) return 0; } -module_init(rndis_init); -static void rndis_exit(void) +void rndis_exit(void) { #ifdef CONFIG_USB_GADGET_DEBUG_FILES u8 i; @@ -1188,6 +1187,4 @@ static void rndis_exit(void) } #endif } -module_exit(rndis_exit); -MODULE_LICENSE("GPL"); diff --git a/drivers/usb/gadget/u_rndis.h b/drivers/usb/gadget/u_rndis.h index c62ba82..7291b15 100644 --- a/drivers/usb/gadget/u_rndis.h +++ b/drivers/usb/gadget/u_rndis.h @@ -36,6 +36,8 @@ struct f_rndis_opts { int refcnt; }; +int rndis_init(void); +void rndis_exit(void); void rndis_borrow_net(struct usb_function_instance *f, struct net_device *net); #endif /* U_RNDIS_H */ -- 1.7.0.4 -- To unsubscribe from this list: send
[PATCH RESEND v2 11/16] usb/gadget: FunctionFS: create utility file
A header file to be used by f_fs.c and g_ffs.c will be required when f_fs.c is converted into a module. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_fs.c |1 + drivers/usb/gadget/g_ffs.c | 19 ++- drivers/usb/gadget/u_fs.h | 28 3 files changed, 35 insertions(+), 13 deletions(-) create mode 100644 drivers/usb/gadget/u_fs.h diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index b2fd90e..277d193 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -27,6 +27,7 @@ #include #include +#include "u_fs.h" #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */ diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 7099a11..1aaa103 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -69,13 +69,6 @@ MODULE_LICENSE("GPL"); #define GFS_MAX_DEVS 10 -struct gfs_ffs_obj { - const char *name; - bool mounted; - bool desc_ready; - struct ffs_data *ffs_data; -}; - USB_GADGET_COMPOSITE_OPTIONS(); static struct usb_device_descriptor gfs_dev_desc = { @@ -181,7 +174,7 @@ static DEFINE_MUTEX(gfs_lock); static unsigned int missing_funcs; static bool gfs_registered; static bool gfs_single_func; -static struct gfs_ffs_obj *ffs_tab; +static struct ffs_dev *ffs_tab; static int __init gfs_init(void) { @@ -224,7 +217,7 @@ static void __exit gfs_exit(void) } module_exit(gfs_exit); -static struct gfs_ffs_obj *gfs_find_dev(const char *dev_name) +static struct ffs_dev *gfs_find_dev(const char *dev_name) { int i; @@ -242,7 +235,7 @@ static struct gfs_ffs_obj *gfs_find_dev(const char *dev_name) static int functionfs_ready_callback(struct ffs_data *ffs) { - struct gfs_ffs_obj *ffs_obj; + struct ffs_dev *ffs_obj; int ret; ENTER(); @@ -283,7 +276,7 @@ done: static void functionfs_closed_callback(struct ffs_data *ffs) { - struct gfs_ffs_obj *ffs_obj; + struct ffs_dev *ffs_obj; ENTER(); mutex_lock(&gfs_lock); @@ -305,7 +298,7 @@ done: static void *functionfs_acquire_dev_callback(const char *dev_name) { - struct gfs_ffs_obj *ffs_dev; + struct ffs_dev *ffs_dev; ENTER(); mutex_lock(&gfs_lock); @@ -329,7 +322,7 @@ done: static void functionfs_release_dev_callback(struct ffs_data *ffs_data) { - struct gfs_ffs_obj *ffs_dev; + struct ffs_dev *ffs_dev; ENTER(); mutex_lock(&gfs_lock); diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h new file mode 100644 index 000..5d9229a --- /dev/null +++ b/drivers/usb/gadget/u_fs.h @@ -0,0 +1,28 @@ +/* + * u_fs.h + * + * Utility definitions for the FunctionFS + * + * Copyright (c) 2013 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Author: Andrzej Pietrasiewicz + * + * 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 U_FFS_H +#define U_FFS_H + +#include + +struct ffs_dev { + const char *name; + bool mounted; + bool desc_ready; + struct ffs_data *ffs_data; +}; + +#endif /* U_FFS_H */ -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND v2 06/16] usb/gadget: f_subset: remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_subset.c | 60 + drivers/usb/gadget/u_ether.h |3 -- 2 files changed, 1 insertions(+), 62 deletions(-) diff --git a/drivers/usb/gadget/f_subset.c b/drivers/usb/gadget/f_subset.c index 7c8674f..f1a5919 100644 --- a/drivers/usb/gadget/f_subset.c +++ b/drivers/usb/gadget/f_subset.c @@ -301,7 +301,6 @@ geth_bind(struct usb_configuration *c, struct usb_function *f) int status; struct usb_ep *ep; -#ifndef USB_FSUBSET_INCLUDED struct f_gether_opts*gether_opts; gether_opts = container_of(f->fi, struct f_gether_opts, func_inst); @@ -322,7 +321,7 @@ geth_bind(struct usb_configuration *c, struct usb_function *f) return status; gether_opts->bound = true; } -#endif + us = usb_gstrings_attach(cdev, geth_strings, ARRAY_SIZE(geth_string_defs)); if (IS_ERR(us)) @@ -393,61 +392,6 @@ fail: return status; } -#ifdef USB_FSUBSET_INCLUDED - -static void -geth_old_unbind(struct usb_configuration *c, struct usb_function *f) -{ - geth_string_defs[0].id = 0; - usb_free_all_descriptors(f); - kfree(func_to_geth(f)); -} - -/** - * geth_bind_config - add CDC Subset network link to a configuration - * @c: the configuration to support the network link - * @ethaddr: a buffer in which the ethernet address of the host side - * side of the link was recorded - * @dev: eth_dev structure - * Context: single threaded during gadget setup - * - * Returns zero on success, else negative errno. - * - * Caller must have called @gether_setup(). Caller is also responsible - * for calling @gether_cleanup() before module unload. - */ -int geth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev) -{ - struct f_gether *geth; - int status; - - /* allocate and initialize one new instance */ - geth = kzalloc(sizeof *geth, GFP_KERNEL); - if (!geth) - return -ENOMEM; - - /* export host's Ethernet address in CDC format */ - snprintf(geth->ethaddr, sizeof geth->ethaddr, "%pm", ethaddr); - geth_string_defs[1].s = geth->ethaddr; - - geth->port.ioport = dev; - geth->port.cdc_filter = DEFAULT_FILTER; - - geth->port.func.name = "cdc_subset"; - geth->port.func.bind = geth_bind; - geth->port.func.unbind = geth_old_unbind; - geth->port.func.set_alt = geth_set_alt; - geth->port.func.disable = geth_disable; - - status = usb_add_function(c, &geth->port.func); - if (status) - kfree(geth); - return status; -} - -#else - static inline struct f_gether_opts *to_f_gether_opts(struct config_item *item) { return container_of(to_config_group(item), struct f_gether_opts, @@ -573,5 +517,3 @@ static struct usb_function *geth_alloc(struct usb_function_instance *fi) DECLARE_USB_FUNCTION_INIT(geth, geth_alloc_inst, geth_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); - -#endif diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h index 64125a5..f310e6f 100644 --- a/drivers/usb/gadget/u_ether.h +++ b/drivers/usb/gadget/u_ether.h @@ -268,9 +268,6 @@ static inline bool can_support_ecm(struct usb_gadget *gadget) } /* each configuration may bind one instance of an ethernet link */ -int geth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev); - #ifdef USB_ETH_RNDIS int rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND v2 08/16] usb/gadget: f_rndis: remove compatibility layer
There are no old function interface users left, so the old interface can be removed. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/f_rndis.c | 72 +- drivers/usb/gadget/u_ether.h | 36 - 2 files changed, 1 insertions(+), 107 deletions(-) diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c index 717ed7f..9d7c995 100644 --- a/drivers/usb/gadget/f_rndis.c +++ b/drivers/usb/gadget/f_rndis.c @@ -675,7 +675,6 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f) int status; struct usb_ep *ep; -#ifndef USB_FRNDIS_INCLUDED struct f_rndis_opts *rndis_opts; if (!can_support_rndis(c)) @@ -697,7 +696,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f) return status; rndis_opts->bound = true; } -#endif + us = usb_gstrings_attach(cdev, rndis_strings, ARRAY_SIZE(rndis_string_defs)); if (IS_ERR(us)) @@ -782,13 +781,6 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f) rndis->port.open = rndis_open; rndis->port.close = rndis_close; -#ifdef USB_FRNDIS_INCLUDED - status = rndis_register(rndis_response_available, rndis); - if (status < 0) - goto fail; - rndis->config = status; -#endif - rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3, 0); rndis_set_host_mac(rndis->config, rndis->ethaddr); @@ -830,66 +822,6 @@ fail: return status; } -#ifdef USB_FRNDIS_INCLUDED - -static void -rndis_old_unbind(struct usb_configuration *c, struct usb_function *f) -{ - struct f_rndis *rndis = func_to_rndis(f); - - rndis_deregister(rndis->config); - - usb_free_all_descriptors(f); - - kfree(rndis->notify_req->buf); - usb_ep_free_request(rndis->notify, rndis->notify_req); - - kfree(rndis); -} - -int -rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - u32 vendorID, const char *manufacturer, struct eth_dev *dev) -{ - struct f_rndis *rndis; - int status; - - /* allocate and initialize one new instance */ - status = -ENOMEM; - rndis = kzalloc(sizeof *rndis, GFP_KERNEL); - if (!rndis) - goto fail; - - memcpy(rndis->ethaddr, ethaddr, ETH_ALEN); - rndis->vendorID = vendorID; - rndis->manufacturer = manufacturer; - - rndis->port.ioport = dev; - /* RNDIS activates when the host changes this filter */ - rndis->port.cdc_filter = 0; - - /* RNDIS has special (and complex) framing */ - rndis->port.header_len = sizeof(struct rndis_packet_msg_type); - rndis->port.wrap = rndis_add_header; - rndis->port.unwrap = rndis_rm_hdr; - - rndis->port.func.name = "rndis"; - /* descriptors are per-instance copies */ - rndis->port.func.bind = rndis_bind; - rndis->port.func.unbind = rndis_old_unbind; - rndis->port.func.set_alt = rndis_set_alt; - rndis->port.func.setup = rndis_setup; - rndis->port.func.disable = rndis_disable; - - status = usb_add_function(c, &rndis->port.func); - if (status) - kfree(rndis); -fail: - return status; -} - -#else - void rndis_borrow_net(struct usb_function_instance *f, struct net_device *net) { struct f_rndis_opts *opts; @@ -1050,5 +982,3 @@ static struct usb_function *rndis_alloc(struct usb_function_instance *fi) DECLARE_USB_FUNCTION_INIT(rndis, rndis_alloc_inst, rndis_alloc); MODULE_LICENSE("GPL"); MODULE_AUTHOR("David Brownell"); - -#endif diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h index f310e6f..f1e0cbe 100644 --- a/drivers/usb/gadget/u_ether.h +++ b/drivers/usb/gadget/u_ether.h @@ -267,40 +267,4 @@ static inline bool can_support_ecm(struct usb_gadget *gadget) return true; } -/* each configuration may bind one instance of an ethernet link */ -#ifdef USB_ETH_RNDIS - -int rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - u32 vendorID, const char *manufacturer, struct eth_dev *dev); - -#else - -static inline int -rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - u32 vendorID, const char *manufacturer, struct eth_dev *dev) -{ - return 0; -} - -#endif - -/** - * rndis_bind_config - add RNDIS network link to a configuration - * @c: the configuration to support the network link - * @ethaddr: a buffer in which the ethernet address of the host side - * side of the link was recorded - * Context: single threaded during gadget setup - * - * Returns zero on success, else negative errno. - * - * Caller must have called @gether_setup(). Caller is also responsible - * for calling @get
[PATCH RESEND v2 07/16] usb/gadget: g_ffs: convert to new interface of f_rndis
There is a new interface of f_rndis and g_ffs is the last to use the old one. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Kconfig |1 + drivers/usb/gadget/g_ffs.c | 105 +++- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 77411d7..54b9856 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -894,6 +894,7 @@ config USB_FUNCTIONFS_RNDIS depends on USB_FUNCTIONFS && NET select USB_U_ETHER select USB_U_RNDIS + select USB_F_RNDIS help Include a configuration with RNDIS function (Ethernet) and the Filesystem. diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c index 99ae8ec..7099a11 100644 --- a/drivers/usb/gadget/g_ffs.c +++ b/drivers/usb/gadget/g_ffs.c @@ -33,29 +33,25 @@ # include "u_ecm.h" # include "u_gether.h" # ifdef USB_ETH_RNDIS -#define USB_FRNDIS_INCLUDED -#include "f_rndis.c" +#include "u_rndis.h" #include "rndis.h" # endif # include "u_ether.h" USB_ETHERNET_MODULE_PARAMETERS(); -static u8 gfs_host_mac[ETH_ALEN]; -static struct eth_dev *the_dev; # ifdef CONFIG_USB_FUNCTIONFS_ETH -static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], - struct eth_dev *dev); +static int eth_bind_config(struct usb_configuration *c); static struct usb_function_instance *fi_ecm; static struct usb_function *f_ecm; static struct usb_function_instance *fi_geth; static struct usb_function *f_geth; # endif -#else -# define the_dev NULL -# define gether_cleanup(dev) do { } while (0) -# define gfs_host_mac NULL -struct eth_dev; +# ifdef CONFIG_USB_FUNCTIONFS_RNDIS +static int bind_rndis_config(struct usb_configuration *c); +static struct usb_function_instance *fi_rndis; +static struct usb_function *f_rndis; +# endif #endif #include "f_fs.c" @@ -148,12 +144,11 @@ static struct usb_gadget_strings *gfs_dev_strings[] = { struct gfs_configuration { struct usb_configuration c; - int (*eth)(struct usb_configuration *c, u8 *ethaddr, - struct eth_dev *dev); + int (*eth)(struct usb_configuration *c); } gfs_configurations[] = { #ifdef CONFIG_USB_FUNCTIONFS_RNDIS { - .eth= rndis_bind_config, + .eth= bind_rndis_config, }, #endif @@ -351,7 +346,7 @@ static void functionfs_release_dev_callback(struct ffs_data *ffs_data) */ static int gfs_bind(struct usb_composite_dev *cdev) { -#if defined CONFIG_USB_FUNCTIONFS_ETH +#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS struct net_device *net; #endif int ret, i; @@ -379,28 +374,38 @@ static int gfs_bind(struct usb_composite_dev *cdev) func_inst); net = geth_opts->net; } - gether_set_qmult(net, qmult); +#endif + +#ifdef CONFIG_USB_FUNCTIONFS_RNDIS + { + struct f_rndis_opts *rndis_opts; + + fi_rndis = usb_get_function_instance("rndis"); + if (IS_ERR(fi_rndis)) { + ret = PTR_ERR(fi_rndis); + goto error; + } + rndis_opts = container_of(fi_rndis, struct f_rndis_opts, + func_inst); +#ifndef CONFIG_USB_FUNCTIONFS_ETH + net = rndis_opts->net; +#endif + } +#endif +#if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS + gether_set_qmult(net, qmult); if (!gether_set_host_addr(net, host_addr)) pr_info("using host ethernet address: %s", host_addr); if (!gether_set_dev_addr(net, dev_addr)) pr_info("using self ethernet address: %s", dev_addr); - - the_dev = netdev_priv(net); - -#elif defined CONFIG_USB_FUNCTIONFS_RNDIS - - the_dev = gether_setup(cdev->gadget, dev_addr, host_addr, gfs_host_mac, - qmult); #endif - if (IS_ERR(the_dev)) - return PTR_ERR(the_dev); #if defined CONFIG_USB_FUNCTIONFS_RNDIS && defined CONFIG_USB_FUNCTIONFS_ETH gether_set_gadget(net, cdev->gadget); ret = gether_register_netdev(net); if (ret) - goto error; + goto error_rndis; if (can_support_ecm(cdev->gadget)) { struct f_ecm_opts *ecm_opts; @@ -414,12 +419,13 @@ static int gfs_bind(struct usb_composite_dev *cdev) func_inst); geth_opts->bound = true; } - gether_get_host_addr_u8(net, gfs_host_mac); + + rndis_borrow_net(fi_rndis, net); #endif ret = usb_string_ids_tab(cdev, gfs_strings); if (unlikely(ret < 0)) - goto error; +
[PATCH v2 2/4] usb/gadget: factor out alloc_ep_req
alloc_ep_req() is a function repeated in several modules. Make a common implementation and use it. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park Acked-by: Michal Nazarewicz --- drivers/usb/gadget/Makefile |2 +- drivers/usb/gadget/f_hid.c| 18 +- drivers/usb/gadget/f_loopback.c |8 +++- drivers/usb/gadget/f_midi.c | 22 +++--- drivers/usb/gadget/f_sourcesink.c | 23 +-- drivers/usb/gadget/g_zero.h |1 - drivers/usb/gadget/u_f.c | 32 drivers/usb/gadget/u_f.h | 26 ++ 8 files changed, 83 insertions(+), 49 deletions(-) create mode 100644 drivers/usb/gadget/u_f.c create mode 100644 drivers/usb/gadget/u_f.h diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index 6bb1155..6cccdfe 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -7,7 +7,7 @@ ccflags-$(CONFIG_USB_GADGET_VERBOSE)+= -DVERBOSE_DEBUG obj-$(CONFIG_USB_GADGET) += udc-core.o obj-$(CONFIG_USB_LIBCOMPOSITE) += libcomposite.o libcomposite-y := usbstring.o config.o epautoconf.o -libcomposite-y += composite.o functions.o configfs.o +libcomposite-y += composite.o functions.o configfs.o u_f.o obj-$(CONFIG_USB_DUMMY_HCD)+= dummy_hcd.o obj-$(CONFIG_USB_NET2272) += net2272.o obj-$(CONFIG_USB_NET2280) += net2280.o diff --git a/drivers/usb/gadget/f_hid.c b/drivers/usb/gadget/f_hid.c index 6e69a8e..a95290a 100644 --- a/drivers/usb/gadget/f_hid.c +++ b/drivers/usb/gadget/f_hid.c @@ -20,6 +20,8 @@ #include #include +#include "u_f.h" + static int major, minors; static struct class *hidg_class; @@ -334,20 +336,10 @@ static int f_hidg_open(struct inode *inode, struct file *fd) /*-*/ /*usb_function */ -static struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep, unsigned length) +static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep, + unsigned length) { - struct usb_request *req; - - req = usb_ep_alloc_request(ep, GFP_ATOMIC); - if (req) { - req->length = length; - req->buf = kmalloc(length, GFP_ATOMIC); - if (!req->buf) { - usb_ep_free_request(ep, req); - req = NULL; - } - } - return req; + return alloc_ep_req(ep, length, length); } static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request *req) diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c index 4a3873a..b790653 100644 --- a/drivers/usb/gadget/f_loopback.c +++ b/drivers/usb/gadget/f_loopback.c @@ -20,6 +20,7 @@ #include #include "g_zero.h" +#include "u_f.h" /* * LOOPBACK FUNCTION ... a testing vehicle for USB peripherals, @@ -293,6 +294,11 @@ static void disable_loopback(struct f_loopback *loop) VDBG(cdev, "%s disabled\n", loop->function.name); } +static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len) +{ + return alloc_ep_req(ep, len, buflen); +} + static int enable_loopback(struct usb_composite_dev *cdev, struct f_loopback *loop) { @@ -332,7 +338,7 @@ fail0: * than 'buflen' bytes each. */ for (i = 0; i < qlen && result == 0; i++) { - req = alloc_ep_req(ep, 0); + req = lb_alloc_ep_req(ep, 0); if (req) { req->complete = loopback_complete; result = usb_ep_queue(ep, req, GFP_ATOMIC); diff --git a/drivers/usb/gadget/f_midi.c b/drivers/usb/gadget/f_midi.c index 263e721..36d4bb2 100644 --- a/drivers/usb/gadget/f_midi.c +++ b/drivers/usb/gadget/f_midi.c @@ -32,6 +32,8 @@ #include #include +#include "u_f.h" + MODULE_AUTHOR("Ben Williamson"); MODULE_LICENSE("GPL v2"); @@ -191,20 +193,10 @@ static struct usb_gadget_strings *midi_strings[] = { NULL, }; -static struct usb_request *alloc_ep_req(struct usb_ep *ep, unsigned length) +static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep, + unsigned length) { - struct usb_request *req; - - req = usb_ep_alloc_request(ep, GFP_ATOMIC); - if (req) { - req->length = length; - req->buf = kmalloc(length, GFP_ATOMIC); - if (!req->buf) { - usb_ep_free_request(ep, req); - req = NULL; - } - } - return req; + return alloc_ep_req(ep, length, length); } static void free_ep_req(struct usb_ep *ep, struct usb_request *req) @@ -365,7 +357,7 @@ static int f_midi_set_alt(struct usb_
Re: CDC-ACM device issue
On Tue, 2013-11-05 at 21:16 +, Luke-Jr wrote: > I am trying to interface with the "HEX" devices sold by http://technobit.eu/ > which appear as CDC-ACM devices, but give an I/O error whenever I try to open > them, with some rather unclear error description in dmesg: > > [10526714.860052] usb 5-2: new full-speed USB device number 4 using uhci_hcd > [10526715.055017] usb 5-2: New USB device found, idVendor=04d8, idProduct=000a > [10526715.055026] usb 5-2: New USB device strings: Mfr=1, Product=2, > SerialNumber=0 > [10526715.055032] usb 5-2: Product: CDC RS-232 Emulation Demo > [10526715.055037] usb 5-2: Manufacturer: Microchip Technology Inc. > [10526715.058200] cdc_acm 5-2:1.0: This device cannot do calls on its own. It > is not a modem. > [10526715.061222] cdc_acm 5-2:1.0: ttyACM0: USB ACM device > [10526756.939797] tty_port_close_start: tty->count = 1 port count = 0. > > The manufacturer reports that it works fine on Windows. > Any ideas how I can get this to work on Linux, or at least get more > meaningful > error information so it can be debugged further? What does strace say? The kernel message is a warning, not a hard error. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] usb/gadget: f_loopback: add configfs support
Add support for using the loopback USB function in gadgets composed with configfs. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- .../ABI/testing/configfs-usb-gadget-loopback |8 ++ drivers/usb/gadget/Kconfig | 12 ++ drivers/usb/gadget/f_loopback.c| 132 drivers/usb/gadget/g_zero.h| 12 ++ drivers/usb/gadget/zero.c |4 +- 5 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-loopback diff --git a/Documentation/ABI/testing/configfs-usb-gadget-loopback b/Documentation/ABI/testing/configfs-usb-gadget-loopback new file mode 100644 index 000..852b236 --- /dev/null +++ b/Documentation/ABI/testing/configfs-usb-gadget-loopback @@ -0,0 +1,8 @@ +What: /config/usb-gadget/gadget/functions/Loopback.name +Date: Nov 2013 +KenelVersion: 3.13 +Description: + The attributes: + + qlen- depth of loopback queue + bulk_buflen - buffer length diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 8da2b1d..d6e00af 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -701,6 +701,18 @@ config USB_CONFIGFS_F_FS implemented in kernel space (for instance Ethernet, serial or mass storage) and other are implemented in user space. +config USB_CONFIGFS_F_LB + boolean "Loopback function (for testing)" + depends on USB_CONFIGFS + select USB_F_SS_LB + help + It loops back a configurable number of transfers. + It also implements control requests, for "chapter 9" conformance. + Make this be the first driver you try using on top of any new + USB peripheral controller driver. Then you can use host-side + test software, like the "usbtest" driver, to put your hardware + and its driver through a basic set of functional tests. + config USB_ZERO tristate "Gadget Zero (DEVELOPMENT)" select USB_LIBCOMPOSITE diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c index b790653..c35bb40 100644 --- a/drivers/usb/gadget/f_loopback.c +++ b/drivers/usb/gadget/f_loopback.c @@ -231,6 +231,14 @@ autoconf_fail: static void lb_free_func(struct usb_function *f) { + struct f_lb_opts *opts; + + opts = container_of(f->fi, struct f_lb_opts, func_inst); + + mutex_lock(&opts->lock); + opts->refcnt--; + mutex_unlock(&opts->lock); + usb_free_all_descriptors(f); kfree(func_to_loop(f)); } @@ -386,6 +394,11 @@ static struct usb_function *loopback_alloc(struct usb_function_instance *fi) return ERR_PTR(-ENOMEM); lb_opts = container_of(fi, struct f_lb_opts, func_inst); + + mutex_lock(&lb_opts->lock); + lb_opts->refcnt++; + mutex_unlock(&lb_opts->lock); + buflen = lb_opts->bulk_buflen; qlen = lb_opts->qlen; if (!qlen) @@ -402,6 +415,118 @@ static struct usb_function *loopback_alloc(struct usb_function_instance *fi) return &loop->function; } +static inline struct f_lb_opts *to_f_lb_opts(struct config_item *item) +{ + return container_of(to_config_group(item), struct f_lb_opts, + func_inst.group); +} + +CONFIGFS_ATTR_STRUCT(f_lb_opts); +CONFIGFS_ATTR_OPS(f_lb_opts); + +static void lb_attr_release(struct config_item *item) +{ + struct f_lb_opts *lb_opts = to_f_lb_opts(item); + + usb_put_function_instance(&lb_opts->func_inst); +} + +static struct configfs_item_operations lb_item_ops = { + .release= lb_attr_release, + .show_attribute = f_lb_opts_attr_show, + .store_attribute= f_lb_opts_attr_store, +}; + +static ssize_t f_lb_opts_qlen_show(struct f_lb_opts *opts, char *page) +{ + int result; + + mutex_lock(&opts->lock); + result = sprintf(page, "%d", opts->qlen); + mutex_unlock(&opts->lock); + + return result; +} + +static ssize_t f_lb_opts_qlen_store(struct f_lb_opts *opts, + const char *page, size_t len) +{ + int ret; + u32 num; + + mutex_lock(&opts->lock); + if (opts->refcnt) { + ret = -EBUSY; + goto end; + } + + ret = kstrtou32(page, 0, &num); + if (ret) + goto end; + + opts->qlen = num; + ret = len; +end: + mutex_unlock(&opts->lock); + return ret; +} + +static struct f_lb_opts_attribute f_lb_opts_qlen = + __CONFIGFS_ATTR(qlen, S_IRUGO | S_IWUSR, + f_lb_opts_qlen_show, + f_lb_opts_qlen_store); + +static ssize_t f_lb_opts_bulk_buflen_show(struct f_lb_opts *opts, char *page) +{ + int result; + + mutex_lock(&opts->lock); + result = sprintf(page, "%d",
[PATCH v2 1/4] usb/gadget: composite: redirect setup requests
If there are setup requests not directed to an endpont or an interface, current config's setup() has been attempted so far. This patch, in case the above fails, adds code which tries the setup() of configuration's function if there is only one function in the configuration. This behavior is required to provide equivalent of gadget zero with configfs. The gadget zero has a "config driver" for sourcesink, but all it does is delegating the request to the function proper. So when the equivalent gadget is set up with configfs it needs to handle requests directed to "config driver", but with configfs it is not possible to specify "config drivers". Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- drivers/usb/gadget/composite.c | 16 +++- 1 files changed, 15 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index d4f0f33..6c0d25d 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -1451,8 +1451,22 @@ unknown: struct usb_configuration*c; c = cdev->config; - if (c && c->setup) + if (!c) + goto done; + + /* try current config's setup */ + if (c->setup) { value = c->setup(c, ctrl); + goto done; + } + + /* try the only function in the current config */ + if (!list_is_singular(&c->functions)) + goto done; + f = list_first_entry(&c->functions, struct usb_function, +list); + if (f->setup) + value = f->setup(f, ctrl); } goto done; -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] usb/gadget: f_sourcesink: add configfs support
Add support for using the sourcesink function in gadgets composed with configfs. Signed-off-by: Andrzej Pietrasiewicz Signed-off-by: Kyungmin Park --- .../ABI/testing/configfs-usb-gadget-sourcesink | 12 + drivers/usb/gadget/Kconfig |7 +- drivers/usb/gadget/f_sourcesink.c | 318 drivers/usb/gadget/g_zero.h| 11 + drivers/usb/gadget/zero.c |4 +- 5 files changed, 347 insertions(+), 5 deletions(-) create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-sourcesink diff --git a/Documentation/ABI/testing/configfs-usb-gadget-sourcesink b/Documentation/ABI/testing/configfs-usb-gadget-sourcesink new file mode 100644 index 000..a30f309 --- /dev/null +++ b/Documentation/ABI/testing/configfs-usb-gadget-sourcesink @@ -0,0 +1,12 @@ +What: /config/usb-gadget/gadget/functions/SourceSink.name +Date: Nov 2013 +KenelVersion: 3.13 +Description: + The attributes: + + pattern - 0 (all zeros), 1 (mod63), 2 (none) + isoc_interval - 1..16 + isoc_maxpacket - 0 - 1023 (fs), 0 - 1024 (hs/ss) + isoc_mult - 0..2 (hs/ss only) + isoc_maxburst - 0..15 (ss only) + qlen- buffer length diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index d6e00af..597189d 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -701,12 +701,13 @@ config USB_CONFIGFS_F_FS implemented in kernel space (for instance Ethernet, serial or mass storage) and other are implemented in user space. -config USB_CONFIGFS_F_LB - boolean "Loopback function (for testing)" +config USB_CONFIGFS_F_LB_SS + boolean "Loopback and sourcesink function (for testing)" depends on USB_CONFIGFS select USB_F_SS_LB help - It loops back a configurable number of transfers. + Loopback function loops back a configurable number of transfers. + Sourcesink function either sinks and sources bulk data. It also implements control requests, for "chapter 9" conformance. Make this be the first driver you try using on top of any new USB peripheral controller driver. Then you can use host-side diff --git a/drivers/usb/gadget/f_sourcesink.c b/drivers/usb/gadget/f_sourcesink.c index c5ad4a1..5d4251e 100644 --- a/drivers/usb/gadget/f_sourcesink.c +++ b/drivers/usb/gadget/f_sourcesink.c @@ -477,6 +477,14 @@ no_iso: static void sourcesink_free_func(struct usb_function *f) { + struct f_ss_opts *opts; + + opts = container_of(f->fi, struct f_ss_opts, func_inst); + + mutex_lock(&opts->lock); + opts->refcnt--; + mutex_unlock(&opts->lock); + usb_free_all_descriptors(f); kfree(func_to_ss(f)); } @@ -865,6 +873,11 @@ static struct usb_function *source_sink_alloc_func( return NULL; ss_opts = container_of(fi, struct f_ss_opts, func_inst); + + mutex_lock(&ss_opts->lock); + ss_opts->refcnt++; + mutex_unlock(&ss_opts->lock); + pattern = ss_opts->pattern; isoc_interval = ss_opts->isoc_interval; isoc_maxpacket = ss_opts->isoc_maxpacket; @@ -885,6 +898,303 @@ static struct usb_function *source_sink_alloc_func( return &ss->function; } +static inline struct f_ss_opts *to_f_ss_opts(struct config_item *item) +{ + return container_of(to_config_group(item), struct f_ss_opts, + func_inst.group); +} + +CONFIGFS_ATTR_STRUCT(f_ss_opts); +CONFIGFS_ATTR_OPS(f_ss_opts); + +static void ss_attr_release(struct config_item *item) +{ + struct f_ss_opts *ss_opts = to_f_ss_opts(item); + + usb_put_function_instance(&ss_opts->func_inst); +} + +static struct configfs_item_operations ss_item_ops = { + .release= ss_attr_release, + .show_attribute = f_ss_opts_attr_show, + .store_attribute= f_ss_opts_attr_store, +}; + +static ssize_t f_ss_opts_pattern_show(struct f_ss_opts *opts, char *page) +{ + int result; + + mutex_lock(&opts->lock); + result = sprintf(page, "%d", opts->pattern); + mutex_unlock(&opts->lock); + + return result; +} + +static ssize_t f_ss_opts_pattern_store(struct f_ss_opts *opts, + const char *page, size_t len) +{ + int ret; + u8 num; + + mutex_lock(&opts->lock); + if (opts->refcnt) { + ret = -EBUSY; + goto end; + } + + ret = kstrtou8(page, 0, &num); + if (ret) + goto end; + + if (num != 0 && num != 1 && num != 2) { + ret = -EINVAL; + goto end; + } + + opts->pattern = num; + ret = len; +end: + mutex_unlock(&opts->lock); + return ret; +} + +static struct f_ss_opt
[PATCH v2 0/4] Equivalent of g_zero with configfs
This series aims at integrating configfs into gadget zero, the way it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet, mass_storage and FunctionFS. It contains everything that is required to provide the equivalent of g_zero.ko with configfs. v1..v2: - fixes after Michal's review, thank you, Michal! - simplified redirecting the setup request Rebased onto Felipe's master. BACKWARD COMPATIBILITY == Please note that the old g_zero.ko is still available and works. USING THE NEW "GADGET" == Please refer to this post: http://www.spinics.net/lists/linux-usb/msg76388.html for general information from Sebastian on how to use configfs-based gadgets (*). With configfs the procedure is as follows, compared to the information mentioned above (*): instead of mkdir functions/acm.ttyS1 do mkdir functions/Loopback.0 mkdir functions/SourceSink.0 In the Loopback.0 directory there will be two attributes: qlen - depth of loopback queue bulk_buflen - buffer length In the SourceSink.0 directory there will be six attributes: pattern - 0 (all zeros), 1 (mod63), 2 (none) isoc_interval - 1..16 isoc_maxpacket - 0 - 1023 (fs), 0 - 1024 (hs/ss) isoc_mult - 0..2 (hs/ss only) isoc_maxburst - 0..15 (ss only) bulk_buflen - buffer length An example gadget with Loopback and SourceSink functions: $ modprobe libcomposite $ mount none cfg -t configfs $ mkdir cfg/usb_gadget/g1 $ cd cfg/usb_gadget/g1 $ mkdir configs/c.1 $ mkdir configs/c.2 $ mkdir functions/Loopback.0 $ mkdir functions/SourceSink.0 $ mkdir strings/0x409 $ mkdir configs/c.1/strings/0x409 $ mkdir configs/c.2/strings/0x409 $ echo 0x2d01 > idProduct $ echo 0x04e8 > idVendor $ echo my-serial-num > strings/0x409/serialnumber $ echo my-manufacturer > strings/0x409/manufacturer $ echo "Test gadget" > strings/0x409/product $ echo "Conf 1" > configs/c.1/strings/0x409/configuration $ echo "Conf 2" > configs/c.2/strings/0x409/configuration $ echo 120 > configs/c.1/MaxPower $ ln -s functions/Loopback.0 configs/c.1 $ ln -s functions/SourceSink.0 configs/c.2 $ echo s3c-hsotg > cfg/usb_gadget/g1/UDC After unbinding the gadget with echo "" > UDC the symbolic links in the configuration directory can be removed, the strings/* subdirectories in the configuration directory can be removed, the strings/* subdirectories at the gadget level can be removed and the configs/* subdirectories can be removed. The functions/* subdirectories can be removed. After that the gadget directory can be removed. Daemons need to be closed and then the respective modules can be unloaded. TESTING THE FUNCTIONS (actually there is only one) = loopback, souresink) device: run the gadget host: test-usb http://www.linux-usb.org/usbtest/testusb.c The results should be the same as with the g_zero.ko. Andrzej Pietrasiewicz (4): usb/gadget: composite: redirect setup requests usb/gadget: factor out alloc_ep_req usb/gadget: f_loopback: add configfs support usb/gadget: f_sourcesink: add configfs support .../ABI/testing/configfs-usb-gadget-loopback |8 + .../ABI/testing/configfs-usb-gadget-sourcesink | 12 + drivers/usb/gadget/Kconfig | 13 + drivers/usb/gadget/Makefile|2 +- drivers/usb/gadget/composite.c | 16 +- drivers/usb/gadget/f_hid.c | 18 +- drivers/usb/gadget/f_loopback.c| 140 - drivers/usb/gadget/f_midi.c| 22 +- drivers/usb/gadget/f_sourcesink.c | 341 ++- drivers/usb/gadget/g_zero.h| 24 ++- drivers/usb/gadget/u_f.c | 32 ++ drivers/usb/gadget/u_f.h | 26 ++ drivers/usb/gadget/zero.c |8 +- 13 files changed, 608 insertions(+), 54 deletions(-) create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-loopback create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-sourcesink create mode 100644 drivers/usb/gadget/u_f.c create mode 100644 drivers/usb/gadget/u_f.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] USB: make hcd->irq signed for error handling
There is a bug in ehci_msm_probe() where we do: hcd->irq = platform_get_irq(pdev, 0); if (hcd->irq < 0) { The problem is that hcd->irq is unsigned so the error handling doesn't work. I have made it signed. Signed-off-by: Dan Carpenter diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index b8aba19..95be4a0 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -143,7 +143,7 @@ struct usb_hcd { unsignedhas_tt:1; /* Integrated TT in root hub */ unsignedamd_resume_bug:1; /* AMD remote wakeup quirk */ - unsigned intirq;/* irq allocated */ + int irq;/* irq allocated */ void __iomem*regs; /* device memory/io */ resource_size_t rsrc_start; /* memory/io resource start */ resource_size_t rsrc_len; /* memory/io resource length */ -- 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