Re: [PATCH 1/1] usb: phy: Kconfig: change FSL_USB2_OTG as tristate
On Wed, Jan 30, 2019 at 1:30 AM Peter Chen wrote: > > It depends on USB_OTG_FSM, but USB_OTG_FSM can be module, so > FSL_USB2_OTG should be tristate. It fixes below build warning: > > drivers/usb/phy/phy-fsl-usb.o: In function `fsl_otg_ioctl': > phy-fsl-usb.c:(.text+0x5e4): undefined reference to `otg_statemachine' > drivers/usb/phy/phy-fsl-usb.o: In function `fsl_otg_start_srp': > phy-fsl-usb.c:(.text+0x680): undefined reference to `otg_statemachine' > drivers/usb/phy/phy-fsl-usb.o: In function `fsl_otg_set_host': > phy-fsl-usb.c:(.text+0x800): undefined reference to `otg_statemachine' > drivers/usb/phy/phy-fsl-usb.o: In function `fsl_otg_start_hnp': > phy-fsl-usb.c:(.text+0x88c): undefined reference to `otg_statemachine' > drivers/usb/phy/phy-fsl-usb.o: In function `show_fsl_usb2_otg_state': > phy-fsl-usb.c:(.text+0xa44): undefined reference to `usb_otg_state_string' > drivers/usb/phy/phy-fsl-usb.o: In function `a_wait_enum': > phy-fsl-usb.c:(.text+0x1718): undefined reference to `otg_statemachine' > drivers/usb/phy/phy-fsl-usb.o: In function `fsl_otg_set_peripheral': > phy-fsl-usb.c:(.text+0x1f20): undefined reference to > `usb_gadget_vbus_disconnect' > phy-fsl-usb.c:(.text+0x1f40): undefined reference to `otg_statemachine' > > Cc: #v4.1+ > Cc: Li Yang > Reported-by: Mark Brown > Signed-off-by: Peter Chen Acked-by: Li Yang > --- > drivers/usb/phy/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > index d7312eed6088..5444d2437475 100644 > --- a/drivers/usb/phy/Kconfig > +++ b/drivers/usb/phy/Kconfig > @@ -20,7 +20,7 @@ config AB8500_USB > in host mode, low speed. > > config FSL_USB2_OTG > - bool "Freescale USB OTG Transceiver Driver" > + tristate "Freescale USB OTG Transceiver Driver" > depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM > depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be > 'y' > select USB_PHY > -- > 2.14.1 >
Re: [PATCH v1] arm64: dts: dwc3: Add description of 'configure-gfladj'
On Wed, Aug 29, 2018 at 2:38 AM Yinbo Zhu wrote: > > This patch is to add description of 'configure-gfladj' to binding > so that configuring devicetree > > Signed-off-by: Yinbo Zhu This patch should be sent to the DWC3 driver maintainer to review and probably merged through usb tree. You shouldn't use the arm64: dts: prefix for this patch either. > --- > Documentation/devicetree/bindings/usb/dwc3.txt |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt > b/Documentation/devicetree/bindings/usb/dwc3.txt > index 3e4c38b..40c6568 100644 > --- a/Documentation/devicetree/bindings/usb/dwc3.txt > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > @@ -101,6 +101,7 @@ Optional properties: > When just one value, which means INCRX burst mode > enabled. When > more than one value, which means undefined length > INCR burst type > enabled. The values can be 1, 4, 8, 16, 32, 64, 128 > and 256. > + - configure-gfladj: determine whether frame length adjustment is required > or not. > > - in addition all properties from usb-xhci.txt from the current directory > are > supported as well > -- > 1.7.1 >
Re: [PATCH 22/24] USB: gadget: udc: Remove redundant license text
On Mon, Nov 6, 2017 at 8:37 AM, Greg Kroah-Hartman wrote: > Now that the SPDX tag is in all USB files, that identifies the license > in a specific and legally-defined manner. So the extra GPL text wording > can be removed as it is no longer needed at all. > > This is done on a quest to remove the 700+ different ways that files in > the kernel describe the GPL license text. And there's unneeded stuff > like the address (sometimes incorrect) for the FSF which is never > needed. > > No copyright headers or other non-license-description text was removed. > > Cc: Felipe Balbi > Cc: Nicolas Ferre > Cc: Kevin Cernekee > Cc: Florian Fainelli > Cc: Li Yang > Cc: Vladimir Zapolskiy > Cc: Sylvain Lemieux > Cc: Daniel Mack > Cc: Haojian Zhuang > Cc: Robert Jarzmik > Cc: Michal Simek > Cc: "Sören Brinkmann" > Cc: Raviteja Garimella > Cc: Romain Perier > Cc: Johan Hovold > Cc: Al Cooper > Cc: Srinath Mannam > Cc: Roger Quadros > Cc: Krzysztof Opasiak > Cc: Stefan Agner > Cc: Alan Stern > Cc: "Felix Hädicke" > Cc: Peter Chen > Cc: Allen Pais > Cc: Yuyang Du > Signed-off-by: Greg Kroah-Hartman > --- > drivers/usb/gadget/udc/fsl_mxc_udc.c| 5 - > drivers/usb/gadget/udc/fsl_qe_udc.c | 5 - > drivers/usb/gadget/udc/fsl_qe_udc.h | 5 - > drivers/usb/gadget/udc/fsl_udc_core.c | 5 - > drivers/usb/gadget/udc/fsl_usb2_udc.h | 5 - Acked-by: Li Yang Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe 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: chipidea: Configure DMA properties and ops from DT
On Tue, Mar 8, 2016 at 9:40 PM, Bjorn Andersson wrote: > On Tue, Mar 8, 2016 at 11:52 AM, Li Yang wrote: >> On Wed, Mar 2, 2016 at 4:59 PM, Li Yang wrote: >>> On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson >>> wrote: >>>> On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote: >>>> >>>>> >>>>> >>>>> On 22/02/16 05:32, Bjorn Andersson wrote: >>>>> >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set >>>>> >to be able to do DMA allocations, so use the of_dma_configure() helper >>>>> >to populate the dma properties and assign an appropriate dma_ops. > [..] >>>>> None of the drivers call of_dma_configure() explicitly, which makes me >>>>> feel >>>>> that we are doing something wrong. TBH, this should be handled in more >>>>> generic way rather than driver like this having an explicit call to >>>>> of_dma_configure(). >>>>> >>>> >>>> I agree, trying to figure out if it should be inherited or something. >>> >>> I also agree. We need address it in a more generic way. I did a >>> search for platform_device_add()/platform_device_register() in the >>> kernel source code. I found a lot of them and many could be also >>> doing DMA. Looks like it is still too early to assume every device is >>> already getting dma_ops set through bus probe. Otherwise, many >>> drivers are potentially broken by this assumption. >> >> Any further comment on this topic? I added the linux-arm mailing list >> which was missing from previous discussion. >> > > I had the chance to go through this with Arnd and the verdict is that > devices not described in DT should not do DMA (or allocate buffers for > doing DMA). > > So I believe the solution is to fall back on Peter's description; the > chipidea driver is the core driver and the Qualcomm code should just > be a platform layer. > > My suggestion is that we turn the chipidea core into a set of APIs > that can called by the platform specific pieces. That way we will have > the chipidea core be the device described in the DT. But like I said, this problem is not just existing for chipidea driver. We already found that the dwc3 driver is also suffering from the same issue. I don't know how many other drivers are impacted by this change, but I suspect there will be some. A grep of platform_device_add() in driver/ directory returns many possible drivers to be impacted. As far as I know, the drivers/net/ethernet/freescale/fman/mac.c is registering child ethernet devices that definitely will do dma. If you want to do this kind of rework to all these drivers, it will be a really big effort. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe 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: chipidea: Configure DMA properties and ops from DT
On Wed, Mar 2, 2016 at 4:59 PM, Li Yang wrote:
> On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson
> wrote:
>> On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote:
>>
>>>
>>>
>>> On 22/02/16 05:32, Bjorn Andersson wrote:
>>> >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set
>>> >to be able to do DMA allocations, so use the of_dma_configure() helper
>>> >to populate the dma properties and assign an appropriate dma_ops.
>
> We also hit the same issue with the dwc3 driver.
>
>>> >
>>> >Signed-off-by: Bjorn Andersson
>>> >---
>>> > drivers/usb/chipidea/core.c | 4
>>> > 1 file changed, 4 insertions(+)
>>> >
>>> >diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
>>> >index 7404064b9bbc..047b9d4e67aa 100644
>>> >--- a/drivers/usb/chipidea/core.c
>>> >+++ b/drivers/usb/chipidea/core.c
>>> >@@ -62,6 +62,7 @@
>>> > #include
>>> > #include
>>> > #include
>>> >+#include
>>> > #include
>>> > #include
>>> > #include
>>> >@@ -834,6 +835,9 @@ struct platform_device *ci_hdrc_add_device(struct
>>> >device *dev,
>>> > pdev->dev.dma_parms = dev->dma_parms;
>>> > dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
>>> >
>>> >+if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>>> >+of_dma_configure(&pdev->dev, dev->of_node);
>>> >+
>>> Would we hit the same issue if we are on non Device tree platforms like ACPI
>>> or platform device style itself?
>>>
>>
>> As far as I can see, yes.
>>
>>>
>>> > ret = platform_device_add_resources(pdev, res, nres);
>>> > if (ret)
>>> > goto err;
>>> >
>>>
>>> I think this is the side effect of commit
>>> 1dccb598df549d892b6450c261da54cdd7af44b4(arm64: simplify dma_get_ops)
>>>
>>
>> I agree, before that we would have hit:
>>
>> __generic_dma_ops() {
>> ..
>>else if (acpi_disabled)
>>return dma_ops;
>> ...
>> }
>>
>> with dma_ops being swiotlb_dma_ops from arm64_dma_init().
>>
>>
>> But this would not have saved us in the ACPI case, i.e. the result would
>> have been as with my suggested patch. Poking Arnd here to see if he has
>> any input.
>>
>>> None of the drivers call of_dma_configure() explicitly, which makes me feel
>>> that we are doing something wrong. TBH, this should be handled in more
>>> generic way rather than driver like this having an explicit call to
>>> of_dma_configure().
>>>
>>
>> I agree, trying to figure out if it should be inherited or something.
>
> I also agree. We need address it in a more generic way. I did a
> search for platform_device_add()/platform_device_register() in the
> kernel source code. I found a lot of them and many could be also
> doing DMA. Looks like it is still too early to assume every device is
> already getting dma_ops set through bus probe. Otherwise, many
> drivers are potentially broken by this assumption.
Any further comment on this topic? I added the linux-arm mailing list
which was missing from previous discussion.
Regards,
Leo
--
To unsubscribe from this list: send the line "unsubscribe 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: chipidea: Configure DMA properties and ops from DT
On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson
wrote:
> On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote:
>
>>
>>
>> On 22/02/16 05:32, Bjorn Andersson wrote:
>> >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set
>> >to be able to do DMA allocations, so use the of_dma_configure() helper
>> >to populate the dma properties and assign an appropriate dma_ops.
We also hit the same issue with the dwc3 driver.
>> >
>> >Signed-off-by: Bjorn Andersson
>> >---
>> > drivers/usb/chipidea/core.c | 4
>> > 1 file changed, 4 insertions(+)
>> >
>> >diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
>> >index 7404064b9bbc..047b9d4e67aa 100644
>> >--- a/drivers/usb/chipidea/core.c
>> >+++ b/drivers/usb/chipidea/core.c
>> >@@ -62,6 +62,7 @@
>> > #include
>> > #include
>> > #include
>> >+#include
>> > #include
>> > #include
>> > #include
>> >@@ -834,6 +835,9 @@ struct platform_device *ci_hdrc_add_device(struct
>> >device *dev,
>> > pdev->dev.dma_parms = dev->dma_parms;
>> > dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
>> >
>> >+if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>> >+of_dma_configure(&pdev->dev, dev->of_node);
>> >+
>> Would we hit the same issue if we are on non Device tree platforms like ACPI
>> or platform device style itself?
>>
>
> As far as I can see, yes.
>
>>
>> > ret = platform_device_add_resources(pdev, res, nres);
>> > if (ret)
>> > goto err;
>> >
>>
>> I think this is the side effect of commit
>> 1dccb598df549d892b6450c261da54cdd7af44b4(arm64: simplify dma_get_ops)
>>
>
> I agree, before that we would have hit:
>
> __generic_dma_ops() {
> ..
>else if (acpi_disabled)
>return dma_ops;
> ...
> }
>
> with dma_ops being swiotlb_dma_ops from arm64_dma_init().
>
>
> But this would not have saved us in the ACPI case, i.e. the result would
> have been as with my suggested patch. Poking Arnd here to see if he has
> any input.
>
>> None of the drivers call of_dma_configure() explicitly, which makes me feel
>> that we are doing something wrong. TBH, this should be handled in more
>> generic way rather than driver like this having an explicit call to
>> of_dma_configure().
>>
>
> I agree, trying to figure out if it should be inherited or something.
I also agree. We need address it in a more generic way. I did a
search for platform_device_add()/platform_device_register() in the
kernel source code. I found a lot of them and many could be also
doing DMA. Looks like it is still too early to assume every device is
already getting dma_ops set through bus probe. Otherwise, many
drivers are potentially broken by this assumption.
Regards,
Leo
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7][v5]Add OTG support for FSL socs
On Mon, Feb 8, 2016 at 3:18 AM, Ramneek Mehresh wrote: > Hi Balbi, > > A kind reminder for the below request. Please let me know if any changes are > required on my side. You are still using the obsolete email address of Balbi trying to get his attention. commit a55f6286575863ebfa5577d4d3bb3b6f1dbd45ec Author: Felipe Balbi Date: Wed Feb 3 20:23:01 2016 +0200 MAINTAINERS: fix my email address As I'm not working for Texas Instruments anymore, ba...@ti.com isn't a valid address. I'll be using ba...@kernel.org at least for the time being. Acked-by: Greg Kroah-Hartman Signed-off-by: Felipe Balbi - Leo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] usb: gadget: fsl_udc_core: Use module_platform_driver_probe macro
> Subject: [PATCH 1/1] usb: gadget: fsl_udc_core: Use
> module_platform_driver_probe macro
>
> module_platform_driver_probe() eliminates the boilerplate and simplifies
> the code.
>
> Signed-off-by: Sachin Kamat
> Cc: Li Yang
Acked-by: Li Yang
> ---
> drivers/usb/gadget/fsl_udc_core.c | 16 +---
> 1 files changed, 1 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/gadget/fsl_udc_core.c
> b/drivers/usb/gadget/fsl_udc_core.c
> index 04d5fef..f523897 100644
> --- a/drivers/usb/gadget/fsl_udc_core.c
> +++ b/drivers/usb/gadget/fsl_udc_core.c
> @@ -2747,21 +2747,7 @@ static struct platform_driver udc_driver = {
> },
> };
>
> -static int __init udc_init(void)
> -{
> - printk(KERN_INFO "%s (%s)\n", driver_desc, DRIVER_VERSION);
> - return platform_driver_probe(&udc_driver, fsl_udc_probe);
> -}
> -
> -module_init(udc_init);
> -
> -static void __exit udc_exit(void)
> -{
> - platform_driver_unregister(&udc_driver);
> - printk(KERN_WARNING "%s unregistered\n", driver_desc);
> -}
> -
> -module_exit(udc_exit);
> +module_platform_driver_probe(udc_driver, fsl_udc_probe);
>
> MODULE_DESCRIPTION(DRIVER_DESC);
> MODULE_AUTHOR(DRIVER_AUTHOR);
> --
> 1.7.4.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [BUILD BREAK] usb: gadget: fsl_mxc_udc can't compile on current v3.8-rc3
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Friday, January 11, 2013 4:41 PM > To: Chen Peter-B29397 > Cc: ba...@ti.com; ker...@pengutronix.de; Li Yang-R58472; Greg KH; linux- > u...@vger.kernel.org; linuxppc-...@lists.ozlabs.org > Subject: Re: [BUILD BREAK] usb: gadget: fsl_mxc_udc can't compile on > current v3.8-rc3 > > Hi, > > On Fri, Jan 11, 2013 at 08:35:29AM +, Chen Peter-B29397 wrote: > > > > I am working on it, but there are two versions, this one and > chipidea's. > > > > > > > > Anyway, I will send a patch to fix this problem. > > > > > > if you're already using chipidea, then send me a patch removing this > > > driver and focus your effort on chipidea. > > > > > Added Sascha > > > > Now, not all of FSL i.mx USB can move to use chipidea due to some > > platform and USB PHY problem. > > then we need to target fixing those problems and moving to chipidea > completely at some point. There's no reason to duplicate efforts if we > already have a re-usable driver in tree, right ? > > Let's fix this build break and focus on making sure all i.MX platforms > can use chipidea so we can drop fsl udc on next merge window. That would > be a great patchset to see. I do agree that we need move to use the chipidea driver and eventually remove the fsl udc driver, but there were many users of the current driver such as PowerPC and Coldfire platforms besides the i.MX platforms. The support for them with chipidea driver could also be broken for now. I would suggest to have a transitional period that both drivers are kept while new development be based on the new driver. Added Ramneek. What do you think of the current status for chipidea driver on PowerPC platforms? Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe 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: [BUILD BREAK] usb: gadget: fsl_mxc_udc can't compile on current v3.8-rc3
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Thursday, January 10, 2013 10:30 PM > To: Felipe Balbi > Cc: Li Yang-R58472; linux-usb@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [BUILD BREAK] usb: gadget: fsl_mxc_udc can't compile on > current v3.8-rc3 > > On Thu, Jan 10, 2013 at 12:08:35PM +0200, Felipe Balbi wrote: > > Hi, > > > > Some recent patch has caused fsl_mxc_udc.c driver to fail compilation > > because it can't find anymore. > > > > I would like this to be fixed still during this -rc cycle. > > Me too, who's sending a patch? :) Hi Peter, Who is currently working on the i.mx USB? Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe 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 2/3] usb: gadget: fsl_qe_udc: do not use tasklet_disable before tasklet_kill
On Wed, Oct 31, 2012 at 9:26 PM, Felipe Balbi wrote: > On Wed, Oct 31, 2012 at 04:06:00PM +0800, Xiaotian Feng wrote: >> If tasklet_disable() is called before related tasklet handled, >> tasklet_kill will never be finished. tasklet_kill is enough. > > how did you test this ? Why changing FSL driver instead of switching > over to chipidea which is supposed to be shared by every licensee of the > chipidea core ? The QE UDC is an private controller that is not compatible with the Chipidea core. - Leo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Saturday, October 20, 2012 1:37 AM > To: Simon Haggett > Cc: Li Yang-R58472; Felipe Balbi; Greg Kroah-Hartman; linux- > u...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > ker...@vger.kernel.org; Laurent Pinchart > Subject: Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests > for a disabled USB endpoint on Freescale hardware > > Hi, > > On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > > Some gadget drivers may attempt to dequeue requests for an endpoint > > that has already been disabled. For example, in the UVC gadget driver, > > uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 > > is selected. When the userspace application subsequently issues the > > VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to > ensure that all requests have been cancelled. > > bug is on uvc gadget, then. Laurent ? > > Also, fsl should be removed from the tree, I'm trying to persuade iMX > folks to use drivers/usb/chipidea instead. Besides the iMX usage, the driver is also being used by many Freescale PowerPC/Coldfire SoCs. I agree that it's ideal to move to a common driver. But it is a large task to make the chipidea driver works for all the hardware that fsl_udc had supported and been tested on. > > > For the Freescale High Speed Dual-Role USB controller, > > fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If > > this is called for a disabled endpoint, a kernel oops will occur when > the ep->ep.desc field is dereferenced (by ep_index()). > > fsl_ep_disable() sets this field to NULL, as well as deleting all > > pending requests for the endpoint. > > > > This patch adds an additional check to fsl_ep_dequeue() to ensure that > > the endpoint has not already been disabled before attempting to dequeue > requests. > > > > Signed-off-by: Simon Haggett > > --- > > drivers/usb/gadget/fsl_udc_core.c |5 - > > 1 files changed, 4 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/usb/gadget/fsl_udc_core.c > > b/drivers/usb/gadget/fsl_udc_core.c > > index 6ae70cb..acd513b 100644 > > --- a/drivers/usb/gadget/fsl_udc_core.c > > +++ b/drivers/usb/gadget/fsl_udc_core.c > > @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, > struct usb_request *_req) > > int ep_num, stopped, ret = 0; > > u32 epctrl; > > > > - if (!_ep || !_req) > > + /* Ensure that the ep and request are valid, and the ep is not > > +* disabled > > +*/ > > + if (!_ep || !_req || !ep->ep.desc) > > return -EINVAL; > > > > spin_lock_irqsave(&ep->udc->lock, flags); > > -- > > 1.7.4.1 > > > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: gadget: fsl_udc_core: do not immediatly prime STATUS for IN xfer
> -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Thursday, September 06, 2012 10:28 PM > To: Enrico Scholz > Cc: ba...@ti.com; Chen Peter-B29397; linux-usb@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org; Li Yang-R58472; gre...@linuxfoundation.org > Subject: Re: [PATCH] usb: gadget: fsl_udc_core: do not immediatly prime > STATUS for IN xfer > > Hi, > > On Thu, Sep 06, 2012 at 04:27:12PM +0200, Enrico Scholz wrote: > > Felipe Balbi writes: > > > > >> > Because the fsl_udc_core driver shares one 'status_req' object > > >> > for the complete ep0 control transfer, it is not possible to > > >> > prime the final STATUS phase immediately after the IN > > >> > transaction. E.g. ch9getstatus() > > >> > executed: > > >> > > > >> > | req = udc->status_req; > > >> > | ... > > >> > | list_add_tail(&req->queue, &ep->queue); if > > >> > | (ep0_prime_status(udc, EP_DIR_OUT)) > > >> > | > > >> > | struct fsl_req *req = udc->status_req; > > >> > | list_add_tail(&req->queue, &ep->queue); > > >> > > > >> > which corrupts the ep->queue list by inserting 'status_req' > > >> > twice. This causes a kernel oops e.g. when 'lsusb -v' is executed > on the host. > > >> > > > >> > Patch delays the final 'ep0_prime_status(udc, EP_DIR_OUT))' by > > >> > moving it into the ep0 completion handler. > > >> > > > >> Enrico, thanks for pointing this problem. > > >> > > >> As "prime STATUS phase immediately after the IN transaction" is > > >> followed USB 2.0 spec, to fix this problem, it is better to add > data_req for ep0. > > >> In fact, it is already at FSL i.mx internal code, just still not > mainlined. > > > > > > so, do I get an Acked-by to this patch ? Does it need to go on > > > v3.6-rc or can it wait until v3.7 merge window ? > > > > Without this (or the mentioned data_req patch), I can crash a g_multi > > gadget by executing 'lsusb -v' as root on the host. Should not be > > exploitable (only a BUG_ON() is triggered) but issue should be fixed > > asap. > > cool, so I'll apply to my fixes branch as soon as I get Acked-by or > Tested-by from someone. This seems to revert the error handling for USB2.0 spec 8.5.3.3. But the problem is a serious one to be fixed right away. So Acked-by: Li Yang We need to revisit the error handling issue later and find a proper way to address it. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
