[PATCH 1/1] usb: option: add support for Telit LE910

2014-10-13 Thread Daniele Palmas
option driver, added VID/PID for Telit LE910 modem. Interfaces description
is almost the same than LE920, except that the qmi interface is number 2
(instead than 5).

Signed-off-by: Daniele Palmas 
---
 drivers/usb/serial/option.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index d1a3f60..0ed7626 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -269,6 +269,7 @@ static void option_instat_callback(struct urb *urb);
 #define TELIT_PRODUCT_DE910_DUAL   0x1010
 #define TELIT_PRODUCT_UE910_V2 0x1012
 #define TELIT_PRODUCT_LE9200x1200
+#define TELIT_PRODUCT_LE9100x1201
 
 /* ZTE PRODUCTS */
 #define ZTE_VENDOR_ID  0x19d2
@@ -594,6 +595,11 @@ static const struct option_blacklist_info 
telit_le920_blacklist = {
.reserved = BIT(1) | BIT(5),
 };
 
+static const struct option_blacklist_info telit_le910_blacklist = {
+   .sendsetup = BIT(0),
+   .reserved = BIT(1) | BIT(2),
+};
+
 static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(OPTION_VENDOR_ID, OPTION_PRODUCT_COLT) },
{ USB_DEVICE(OPTION_VENDOR_ID, OPTION_PRODUCT_RICOLA) },
@@ -1140,6 +1146,8 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_UE910_V2) },
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920),
.driver_info = (kernel_ulong_t)&telit_le920_blacklist },
+   { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE910),
+   .driver_info = (kernel_ulong_t)&telit_le910_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MF622, 0xff, 
0xff, 0xff) }, /* ZTE WCDMA products */
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0002, 0xff, 0xff, 
0xff),
.driver_info = (kernel_ulong_t)&net_intf1_blacklist },
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7

2014-10-13 Thread Vivek Gautam
Hi Felipe,


On Tue, Oct 14, 2014 at 4:14 AM, Felipe Balbi  wrote:
> Hi,
>
> On Mon, Oct 13, 2014 at 01:54:59PM +0900, Anton Tikhomirov wrote:
>> Hi Vivek,
>>
>> > Exynos7 also has a separate special gate clock going to the IP
>> > apart from the usual AHB clock. So add support for the same.
>>
>> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
>> by the driver. Adding only sclk is not enough.
>>
>> >
>> > Signed-off-by: Vivek Gautam 
>> > ---
>> >  drivers/usb/dwc3/dwc3-exynos.c |   16 
>> >  1 file changed, 16 insertions(+)
>> >
>> > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
>> > exynos.c
>> > index 3951a65..7dc6a98 100644
>> > --- a/drivers/usb/dwc3/dwc3-exynos.c
>> > +++ b/drivers/usb/dwc3/dwc3-exynos.c
>> > @@ -35,6 +35,7 @@ struct dwc3_exynos {
>> > struct device   *dev;
>> >
>> > struct clk  *clk;
>>
>> The clock "clk" in Exynos5 just gated all that above 7 clocks, which
>> we should control separately now in Exynos7.
>>
>
> should I drop this patch for now ?

Yes, better to hold this for some time till we get more clarity
from our h/w team.


-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7

2014-10-13 Thread Vivek Gautam
Hi Tomasz,


On Tue, Oct 14, 2014 at 6:56 AM, Anton Tikhomirov
 wrote:
> Hello,
>
>> Hi Anton,
>>
>> On 13.10.2014 06:54, Anton Tikhomirov wrote:
>> > Hi Vivek,
>> >
>> >> Exynos7 also has a separate special gate clock going to the IP
>> >> apart from the usual AHB clock. So add support for the same.
>> >
>> > As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
>> > by the driver. Adding only sclk is not enough.
>> >
>>
>> I'm quite interested in this discussion. Has it happened on mailing
>> lists?
>
> No, we used company messenger for the discussion.

Yea, we head a round of discussion at our end regarding this, and we are
going to get more clarity on this from our H/W team too, this week.

>
>>
>> In general, previous SoCs also gave the possibility of controlling all
>> the bus clocks separately, in addition to bulk gates, but there was no
>
> correct
>
>> real advantage in using those, while burdening the clock tree with
>> numerous clocks. Isn't Exynos7 similar in this aspect?
>
> Exynos7 doesn't have "Gating all clocks for USBDRD30" bit. The clocks
> should be controlled separately.

true, on Exynos7 we have separate gates for the available clocks going to
USB-DRD block. So we will have to add these basic required number of
clocks.





-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe 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: Fwd: [Bug 86201] ch341-uart ttyUSB0: usb_serial_generic_write_bulk_callback - nonzero urb status: -71

2014-10-13 Thread Cristian
Hello!

The message appears when I'm printing and disconnect USB-Serial cable

Regards,
--
Cristian



2014-10-13 23:38 GMT-03:00 Greg Kroah-Hartman :
> On Mon, Oct 13, 2014 at 11:32:22PM -0300, Cristian wrote:
>> Hello!
>>
>> Bug tracking
>
> What do you mean by this?
>
> Please send the information that was in the bug to this list so that
> people can see it and try to help.
>
> 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: Fwd: [Bug 86201] ch341-uart ttyUSB0: usb_serial_generic_write_bulk_callback - nonzero urb status: -71

2014-10-13 Thread Greg Kroah-Hartman
On Mon, Oct 13, 2014 at 11:32:22PM -0300, Cristian wrote:
> Hello!
> 
> Bug tracking

What do you mean by this?

Please send the information that was in the bug to this list so that
people can see it and try to help.

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


Fwd: [Bug 86201] ch341-uart ttyUSB0: usb_serial_generic_write_bulk_callback - nonzero urb status: -71

2014-10-13 Thread Cristian
Hello!

Bug tracking

Regards,
--
Cristian




-- Forwarded message --
From:  
Date: 2014-10-13 23:20 GMT-03:00
Subject: [Bug 86201] ch341-uart ttyUSB0:
usb_serial_generic_write_bulk_callback - nonzero urb status: -71
To: carav...@gmail.com


https://bugzilla.kernel.org/show_bug.cgi?id=86201

--- Comment #1 from Greg Kroah-Hartman  ---
On Mon, Oct 13, 2014 at 09:15:49PM +, bugzilla-dae...@bugzilla.kernel.org
wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=86201
>
> Bug ID: 86201
>Summary: ch341-uart ttyUSB0:
> usb_serial_generic_write_bulk_callback - nonzero urb
> status: -71

Please send this to the linux-usb@vger.kernel.org mailing list.

--
You are receiving this mail because:
You reported the bug.


-- 
Cristian
--
To unsubscribe from this list: send the line "unsubscribe 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: RCU bug with v3.17-rc3 ?

2014-10-13 Thread Greg KH
On Mon, Oct 13, 2014 at 12:43:07PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 13, 2014 at 09:11:34AM +, David Laight wrote:
> > From: Nathan Lynch
> > > On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
> > > >
> > > > Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
> > > > it seems that this has been known about for some time.)
> > > 
> > > Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x < 3
> > > are affected, as well as 4.9.0.
> > > 
> > > > We can blacklist these GCC versions quite easily.  We already have GCC
> > > > 3.3 blacklisted, and it's trivial to add others.  I would want to 
> > > > include
> > > > some proper details about the bug, just like the other existing entries
> > > > we already have in asm-offsets.c, where we name the functions that the
> > > > compiler is known to break where appropriate.
> > > 
> > > Before blacklisting anything, it's worth considering that simple version
> > > checks would break existing pre-4.8.3 compilers that have been patched
> > > for PR58854.  It looks like Yocto and Buildroot issued releases with
> > > patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
> > > the most we can reasonably do without breaking some correctly-behaving
> > > toolchains is to emit a warning.
> > 
> > Is it possible to compile a small code fragment and check the generated
> > code for the bug?
> > Possibly predicated on the broken version number to avoid false positives.
> 
> I don't see how - it looks like it requires an interrupt to occur at an
> opportune moment to provoke the function to fail.  The alternative would
> be to parse the assembly generated by the compiler to determine how it
> is dealing with the stack.
> 
> I think the only viable solution here is that:
> 
> 1. We blacklist the bad compiler versions outright in the kernel.

Yes, please do this, it's what we have done for other buggy compiler
versions, no need to do something different here.

> Remember, it's the distro's choice to fix these buggy compilers, so the
> onus is on _them_ to deal with the mess they've created by doing so.

I totally agree.

Is someone going to send this patch, or do I have to write it myself?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7

2014-10-13 Thread Anton Tikhomirov
Hello,

> Hi Anton,
> 
> On 13.10.2014 06:54, Anton Tikhomirov wrote:
> > Hi Vivek,
> >
> >> Exynos7 also has a separate special gate clock going to the IP
> >> apart from the usual AHB clock. So add support for the same.
> >
> > As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> > by the driver. Adding only sclk is not enough.
> >
> 
> I'm quite interested in this discussion. Has it happened on mailing
> lists?

No, we used company messenger for the discussion.

> 
> In general, previous SoCs also gave the possibility of controlling all
> the bus clocks separately, in addition to bulk gates, but there was no

correct

> real advantage in using those, while burdening the clock tree with
> numerous clocks. Isn't Exynos7 similar in this aspect?

Exynos7 doesn't have "Gating all clocks for USBDRD30" bit. The clocks
should be controlled separately.

> 
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] usb: chipidea: ci_hdrc_imx.c: Remove unneeded OOM message

2014-10-13 Thread Peter Chen
On Mon, Oct 13, 2014 at 08:58:30AM -0300, Fabio Estevam wrote:
> MM core code already complains when devm_kzalloc() fails, so no need to print
> the error locally.
> 
> Signed-off-by: Fabio Estevam 
> ---
> Changes since v1:
> - Add usb in the Subject
> 
>  drivers/usb/chipidea/ci_hdrc_imx.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index a7ab0f1..74b5b09 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -115,10 +115,8 @@ static int ci_hdrc_imx_probe(struct platform_device 
> *pdev)
>   const struct ci_hdrc_imx_platform_flag *imx_platform_flag = of_id->data;
>  
>   data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data) {
> - dev_err(&pdev->dev, "Failed to allocate ci_hdrc-imx data!\n");
> + if (!data)
>   return -ENOMEM;
> - }
>  
>   data->usbmisc_data = usbmisc_get_init_data(&pdev->dev);
>   if (IS_ERR(data->usbmisc_data))
> -- 
> 1.9.1
> 

Both two have applied, thanks.

-- 
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7

2014-10-13 Thread Tomasz Figa
Hi Anton,

On 13.10.2014 06:54, Anton Tikhomirov wrote:
> Hi Vivek,
> 
>> Exynos7 also has a separate special gate clock going to the IP
>> apart from the usual AHB clock. So add support for the same.
> 
> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> by the driver. Adding only sclk is not enough. 
> 

I'm quite interested in this discussion. Has it happened on mailing lists?

In general, previous SoCs also gave the possibility of controlling all
the bus clocks separately, in addition to bulk gates, but there was no
real advantage in using those, while burdening the clock tree with
numerous clocks. Isn't Exynos7 similar in this aspect?

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 v2 1/4] dwc3: exynos: Add support for SCLK present on Exynos7

2014-10-13 Thread Felipe Balbi
Hi,

On Mon, Oct 13, 2014 at 01:54:59PM +0900, Anton Tikhomirov wrote:
> Hi Vivek,
> 
> > Exynos7 also has a separate special gate clock going to the IP
> > apart from the usual AHB clock. So add support for the same.
> 
> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> by the driver. Adding only sclk is not enough. 
> 
> > 
> > Signed-off-by: Vivek Gautam 
> > ---
> >  drivers/usb/dwc3/dwc3-exynos.c |   16 
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> > exynos.c
> > index 3951a65..7dc6a98 100644
> > --- a/drivers/usb/dwc3/dwc3-exynos.c
> > +++ b/drivers/usb/dwc3/dwc3-exynos.c
> > @@ -35,6 +35,7 @@ struct dwc3_exynos {
> > struct device   *dev;
> > 
> > struct clk  *clk;
> 
> The clock "clk" in Exynos5 just gated all that above 7 clocks, which
> we should control separately now in Exynos7.
> 

should I drop this patch for now ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] usb: musb: dsps: kill OTG timer on suspend

2014-10-13 Thread Felipe Balbi
On Mon, Oct 13, 2014 at 11:13:40AM +0200, Sebastian Andrzej Siewior wrote:
> On 2014-09-15 09:39:09 [-0500], Felipe Balbi wrote:
> > if we don't make sure to kill the timer, it could
> > expire after we have already gated our clocks.
> > 
> > That will trigger a Data Abort exception because
> > we would try to access register while clock is gated.
> 
> That timer deserves to be killed because nobody wants it to wakeup the
> system from suspend. However the Data Abort wouldn't happen if the timer
> would use pm_runtime_get_sync() right?

correct, but we want to suspend ;-) there is a race here:

mod_timer();

/* before mod_timer() expires */
"echo mem > /sys/power/suspend"

->runtime_suspend()
/* clocks are now gated */

/* mod_timer() expires and BOOM! */

> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -870,6 +870,7 @@ static int dsps_suspend(struct device *dev)
> > struct musb *musb = platform_get_drvdata(glue->musb);
> > void __iomem *mbase = musb->ctrl_base;
> >  
> > +   del_timer_sync(&glue->timer);
> > glue->context.control = dsps_readl(mbase, wrp->control);
> > glue->context.epintr = dsps_readl(mbase, wrp->epintr_set);
> > glue->context.coreintr = dsps_readl(mbase, wrp->coreintr_set);
> > @@ -895,6 +896,7 @@ static int dsps_resume(struct device *dev)
> > dsps_writel(mbase, wrp->mode, glue->context.mode);
> > dsps_writel(mbase, wrp->tx_mode, glue->context.tx_mode);
> > dsps_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
> > +   setup_timer(&glue->timer, otg_timer, (unsigned long) musb);
> 
> You need a mod_timer() here instead. I will send a patch in a few.

yeah, I was confused if it should be setup_timer() or mod_timer(). Since
I deleted the timer on suspend, I thought setup_timer() was more
appropriate, no ?

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH][RFC] storage: Reject bogus max LUN values

2014-10-13 Thread Mark Knibbs
On Mon, 13 Oct 2014 07:14:39 -0700
Matthew Dharm  wrote:

> Is there a constant we can pull from a SCSI header, instead of having
> a "magic number" in the code?

I don't know, but it would probably be in a USB-related header; isn't the
4-bits-for-LUN a USB mass storage bulk only thing?

I guess you could add a couple of new defines, something like
#define BOT_LUNbits 4
#define BOT_LUNmask ((1 << BOT_LUNbits) - 1)

In the patch I posted the condition could be
<= BOT_LUNmask
or
& ~BOT_LUNmask

They could also be used in usb_Stor_Bulk_transport() like this:
bcb->Lun = srb->device->lun;
if (us->fflags & US_FL_SCM_MULT_TARG)
bcb->Lun |= srb->device->id << BOT_LUNbits;
bcb->Length = srb->cmd_len;
...
usb_stor_dbg(us, "Bulk Command S 0x%x T 0x%x L %d F %d Trg %d LUN %d CL 
%d\n",
 le32_to_cpu(bcb->Signature), bcb->Tag,
 le32_to_cpu(bcb->DataTransferLength), bcb->Flags,
 (bcb->Lun >> BOT_LUNbits), (bcb->Lun & BOT_LUNmask),
 bcb->Length);

Though IMO it would be clearer in the call to usb_stor_dbg to use
srb->device->id   instead of  (bcb->Lun >> BOT_LUNbits)
and
srb->device->lun  instead of  (bcb->Lun & BOT_LUNmask)


Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

2014-10-13 Thread David Cohen
The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
quirk implemented to align buffer size to maxpacketsize on out endpoint.
As result, functionfs does not work on Intel platforms using dwc3 driver
(i.e. Bay Trail and Merrifield). This patch fixes the issue.

This code is based on a previous Qiuxu's patch.

Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
Cc:  # v3.16+
Signed-off-by: David Cohen 
Signed-off-by: Qiuxu Zhuo 
Acked-by: Michal Nazarewicz 
---

Hi,

Since this is a feature that worked in past, this is meant for stable
versions >= 3.16 too.

v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.
v2 to v3: fixed compiler warning about data_len being used unitialized

Br, David Cohen

---
 drivers/usb/gadget/function/f_fs.c | 40 --
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 0dc3552d1360..9b6bc4d30352 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -648,15 +648,26 @@ static void ffs_user_copy_worker(struct work_struct *work)
if (io_data->read && ret > 0) {
int i;
size_t pos = 0;
+
+   /*
+* Since req->length may be bigger than io_data->len (after
+* being rounded up to maxpacketsize), we may end up with more
+* data then user space has space for.
+*/
+   ret = min_t(int, ret, io_data->len);
+
use_mm(io_data->mm);
for (i = 0; i < io_data->nr_segs; i++) {
+   size_t len = min_t(size_t, ret - pos,
+   io_data->iovec[i].iov_len);
+   if (!len)
+   break;
if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
-&io_data->buf[pos],
-io_data->iovec[i].iov_len))) {
+&io_data->buf[pos], len))) {
ret = -EFAULT;
break;
}
-   pos += io_data->iovec[i].iov_len;
+   pos += len;
}
unuse_mm(io_data->mm);
}
@@ -688,7 +699,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
struct ffs_epfile *epfile = file->private_data;
struct ffs_ep *ep;
char *data = NULL;
-   ssize_t ret, data_len;
+   ssize_t ret, data_len = -EINVAL;
int halt;
 
/* Are we still active? */
@@ -788,13 +799,30 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
/* Fire the request */
struct usb_request *req;
 
+   /*
+* Sanity Check: even though data_len can't be used
+* uninitialized at the time I write this comment, some
+* compilers complain about this situation.
+* In order to keep the code clean from warnings, data_len is
+* being initialized to -EINVAL during its declaration, which
+* means we can't rely on compiler anymore to warn no future
+* changes won't result in data_len being used uninitialized.
+* For such reason, we're adding this redundant sanity check
+* here.
+*/
+   if (unlikely(data_len == -EINVAL)) {
+   WARN(1, "%s: data_len == -EINVAL\n", __func__);
+   ret = -EINVAL;
+   goto error_lock;
+   }
+
if (io_data->aio) {
req = usb_ep_alloc_request(ep->ep, GFP_KERNEL);
if (unlikely(!req))
goto error_lock;
 
req->buf  = data;
-   req->length   = io_data->len;
+   req->length   = data_len;
 
io_data->buf = data;
io_data->ep = ep->ep;
@@ -816,7 +844,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
 
req = ep->req;
req->buf  = data;
-   req->length   = io_data->len;
+   req->length   = data_len;
 
req->context  = &done;
req->complete = ffs_epfile_io_complete;
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

2014-10-13 Thread Felipe Balbi
On Mon, Oct 13, 2014 at 09:55:56AM -0700, David Cohen wrote:
> On Mon, Oct 13, 2014 at 10:32:12AM -0500, Felipe Balbi wrote:
> > On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> > > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> > > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > > As result, functionfs does not work on Intel platforms using dwc3 driver
> > > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> > > 
> > > This code is based on a previous Qiuxu's patch.
> > > 
> > > Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> > > Cc:  # v3.16+
> > > Signed-off-by: David Cohen 
> > > Signed-off-by: Qiuxu Zhuo 
> > > Acked-by: Michal Nazarewicz 
> > > ---
> > > 
> > > Hi,
> > > 
> > > Since this is a feature that worked in past, this is meant for stable
> > > versions >= 3.16 too.
> > > 
> > > v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.
> > 
> > this adds a build warning for use of maybe unitialized data_len. Plese
> > fix.
> 
> It's a false-positive warning. data_len is only initialized if halt != 0
> and it's only used if halt != 0 too.
> 
> Do you prefer to initialize it to 0 during the declaration to silent the
> compiler?

yup.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

2014-10-13 Thread David Cohen
On Mon, Oct 13, 2014 at 10:32:12AM -0500, Felipe Balbi wrote:
> On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > As result, functionfs does not work on Intel platforms using dwc3 driver
> > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> > 
> > This code is based on a previous Qiuxu's patch.
> > 
> > Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> > Cc:  # v3.16+
> > Signed-off-by: David Cohen 
> > Signed-off-by: Qiuxu Zhuo 
> > Acked-by: Michal Nazarewicz 
> > ---
> > 
> > Hi,
> > 
> > Since this is a feature that worked in past, this is meant for stable
> > versions >= 3.16 too.
> > 
> > v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.
> 
> this adds a build warning for use of maybe unitialized data_len. Plese
> fix.

It's a false-positive warning. data_len is only initialized if halt != 0
and it's only used if halt != 0 too.

Do you prefer to initialize it to 0 during the declaration to silent the
compiler?

BR, David

> 
> -- 
> 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 v2] usb: atm: fix style issues

2014-10-13 Thread Jonas Brunsgaard
I will try again. This is my first commit to the kernel and the
motivation for me is to learn the flow regarding patch submissions.

So I am glad for the feedback, just remember to comment/criticize the
code so I know what to improve ;)

A new patch will be in your mailbox soon.

Good day
--
To unsubscribe from this list: send the line "unsubscribe 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: ffs: fix regression when quirk_ep_out_aligned_size flag is set

2014-10-13 Thread Felipe Balbi
On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> quirk implemented to align buffer size to maxpacketsize on out endpoint.
> As result, functionfs does not work on Intel platforms using dwc3 driver
> (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> 
> This code is based on a previous Qiuxu's patch.
> 
> Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> Cc:  # v3.16+
> Signed-off-by: David Cohen 
> Signed-off-by: Qiuxu Zhuo 
> Acked-by: Michal Nazarewicz 
> ---
> 
> Hi,
> 
> Since this is a feature that worked in past, this is meant for stable
> versions >= 3.16 too.
> 
> v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.

this adds a build warning for use of maybe unitialized data_len. Plese
fix.

-- 
balbi


signature.asc
Description: Digital signature


Re: XHCI host dies with LPM + webcam gadget

2014-10-13 Thread Mathias Nyman
On 25.09.2014 14:08, Amit Virdi wrote:
> Hello Mathias,
> 
> I'm trying to connect a webcam gadget on USB3.0 interface with xHCI host when 
> LPM is enabled. My xHCI host is TI's TUSB7340EVM. xHCI host stops responding 
> as soon as the webcam gadget module is inserted on the device side (The 
> device is also running linux)
> 
> Host machine kernel version is 3.16. I have enabled quirk for LPM enable for 
> my XHCI card.
> 
> The detailed description and a host kernel logs can be found from bugzila:
> https://bugzilla.kernel.org/show_bug.cgi?id=85141
> 
> Could you please help in rectifying this bug? Please let me know if there's 
> any more information required.
> 
> Thanks
> Amit Virdi
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi

I'm not really sure what's going on, but there are some things that look a bit 
suspicious in the log.

After setting maximum_exit_latency successfully with "evaluate context 
commands" according to U1 and U2 parameters it then wants to disable link state
and set maximum_exit_latency back to 0.
This last "evaluate context command" times out, xhci driver tries to abort the 
command, but also aborting the command fails.

Could you try clearing the slot_context dev_state bits, I think that those 
should be set to zero in the input context
We currently in xhci_change_max_exit_latency() copy it blindly from the output 
context, and there seems to be some reserved vendor bits set there as well.

A patch like this should do it:

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2a5d45b..19c9518 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4004,6 +4004,7 @@ static int __maybe_unused 
xhci_change_max_exit_latency(struct xhci_hcd *xhci,
slot_ctx = xhci_get_slot_ctx(xhci, command->in_ctx);
slot_ctx->dev_info2 &= cpu_to_le32(~((u32) MAX_EXIT));
slot_ctx->dev_info2 |= cpu_to_le32(max_exit_latency);
+   slot_ctx->dev_state = 0;
 
xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
"Set up evaluate context for LPM MEL change.");


If that doesn't help, you could try easing up the busyloop that is checking 
xhci state, this could help in successfully aborting the command,
A patch like this should help:

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2a5d45b..0cc451c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -70,8 +70,8 @@ int xhci_handshake(struct xhci_hcd *xhci, void __iomem *ptr,
result &= mask;
if (result == done)
return 0;
-   udelay(1);
-   usec--;
+   udelay(10);
+   usec -= 10;
} while (usec > 0);
return -ETIMEDOUT;
 }


Otherwise It might just be that using LPM timeout values known to work with 
Intel hosts don't really work with others.
(That's why we got the Intel specific timeout u1 and u2 timeout calculation)

-Mathias

--
To unsubscribe from this list: send the line "unsubscribe 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 3/4] usb: dwc3: add quirk to be compatible for AMD NL

2014-10-13 Thread Felipe Balbi
Hi,

On Sat, Oct 11, 2014 at 01:22:58PM +0800, Huang Rui wrote:
> On Sat, Oct 11, 2014 at 01:14:44PM +0800, Huang Rui wrote:
> > On Fri, Oct 10, 2014 at 09:04:15AM -0500, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Fri, Oct 10, 2014 at 05:25:34PM +0800, Huang Rui wrote:
> > > > > > I enabled dwc3 and gadget debug/verbose configuration, the whole 
> > > > > > testing dmesg
> > > > > 
> > > > > oh, that's why it's so slow :-) I'm getting over 30MB/sec with a 
> > > > > Cortex
> > > > > A9 :-)
> > > > > 
> > > > 
> > > > Yes, maybe have two reasons:
> > > > 1) The input clock is much slower than SOC's.
> > > > 2) I used high speed mode.
> > > 
> > > right, i'm running at highspeed too.
> > > 
> > > > Because of the timing issue on FPGA, bulk write transfer would get
> > > > stuck when use more than 1MB (can pass on small file write) on super
> > > > speed mode. (Gadget Zero failed on 1/3/5/7 with 10s timeout)
> > > 
> > > These shouldn't fail. I'll leave testusb running tonight.
> > > 
> > > > > > Do you want to see the whole testing dmesg, with which debug level
> > > > > > enablement?
> > > > > 
> > > > > This is good for me, thank you.
> > > > 
> > > > The test log with booting is attached. Please review.
> > > 
> > > will do.
> > > 
> > > > > ps: FYI, I left my board running overnight the same test. It has been
> > > > > pretty stable so far.
> > > > > 
> > > > 
> > > > High speed mode is stable in my FPGA board, but super speed is not
> > > > at current.
> > > 
> > > weird. Got any logs ? If you want to share logs I can probably help you
> > > debugging that.
> > > 
> > 
> > Sure. Below is my controller as super speed mode on gadget zero test 1 (bulk
> > write). Test 9/10 can be passed and device is able to enumerated, so control
> > transfer should be OK.
> > 
> > Bus 007 Device 004: ID 0525:a4a0 Netchip Technology, Inc. Linux-USB "Gadget 
> > Zero"
> > 
> > root@hr-bak:/home/ray/usb# ./testusb.sh 1
> > unknown speed   /dev/bus/usb/007/0040
> > /dev/bus/usb/007/004 test 1 --> 110 (Connection timed out)
> > 
> > Host:
> > [ 8793.096303] usb 7-1: new SuperSpeed USB device number 4 using xhci_hcd
> > [ 8793.119876] usb 7-1: New USB device found, idVendor=0525, idProduct=a4a0
> > [ 8793.120109] usb 7-1: New USB device strings: Mfr=1, Product=2, 
> > SerialNumber=3
> > [ 8793.120352] usb 7-1: Product: Gadget Zero
> > [ 8793.120493] usb 7-1: Manufacturer: Linux 3.17.0-rc5-dwc3-upstream+ with 
> > dwc3-gadget
> > [ 8793.120751] usb 7-1: SerialNumber: 0123456789.0123456789.0123456789
> > [ 8793.489749] usbtest 7-1:3.0: Linux gadget zero
> > [ 8793.489933] usbtest 7-1:3.0: super-speed {control in/out bulk-in 
> > bulk-out} tests (+alt)
> > [ 8793.490246] usbcore: registered new interface driver usbtest
> > [ 8815.325781] usbcore: deregistering interface driver usbtest
> > [ 8819.760443] usbtest 7-1:3.0: Linux gadget zero
> > [ 8819.760621] usbtest 7-1:3.0: super-speed {control in/out bulk-in 
> > bulk-out} tests (+alt)
> > [ 8819.760921] usbcore: registered new interface driver usbtest
> > [ 8891.317350] usbtest 7-1:3.0: TEST 1:  write 512 bytes 20 times
> > [ 8901.316770] usb 7-1: test1 failed, iterations left 19, status -110 (not 
> > 0)

oh, ok. See this thread:

http://marc.info/?l=linux-usb&m=141296688426206

-- 
balbi


signature.asc
Description: Digital signature


[PATCH v2] usb: gadget: function: Remove redundant usb_free_all_descriptors

2014-10-13 Thread pavi1729 Sid
Removed the usb_free_all_descriptors call in *_bind functions
as this call is already present in usb_assign_descriptors.
usb_assign_descriptors is the only call where usb descriptor
allocation happens and in case of error, freeing of the
allocated memory takes place inside the same call. Hence the
call in the *_bind functions are redundant.
The presence of usb_free_all_descriptors in the *_bind
functions might result in double-free corruption of the
allocated memory.

Signed-off-by: Pavitrakumar Managutte 
---
 drivers/usb/gadget/function/f_eem.c|  1 -
 drivers/usb/gadget/function/f_hid.c|  5 +++--
 drivers/usb/gadget/function/f_ncm.c|  1 -
 drivers/usb/gadget/function/f_obex.c   |  1 -
 drivers/usb/gadget/function/f_phonet.c |  2 +-
 drivers/usb/gadget/function/f_rndis.c  |  5 +++--
 drivers/usb/gadget/function/f_subset.c |  1 -
 drivers/usb/gadget/function/f_uac2.c   | 10 ++
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/function/f_eem.c
b/drivers/usb/gadget/function/f_eem.c
index 4d8b236..c9e90de 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
struct usb_function *f)
 return 0;

 fail:
-usb_free_all_descriptors(f);
 if (eem->port.out_ep)
 eem->port.out_ep->driver_data = NULL;
 if (eem->port.in_ep)
diff --git a/drivers/usb/gadget/function/f_hid.c
b/drivers/usb/gadget/function/f_hid.c
index a95290a..59ab62c 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -621,12 +621,14 @@ static int __init hidg_bind(struct
usb_configuration *c, struct usb_function *f)
 dev = MKDEV(major, hidg->minor);
 status = cdev_add(&hidg->cdev, dev, 1);
 if (status)
-goto fail;
+goto fail_free_descs;

 device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);

 return 0;

+fail_free_descs:
+usb_free_all_descriptors(f);
 fail:
 ERROR(f->config->cdev, "hidg_bind FAILED\n");
 if (hidg->req != NULL) {
@@ -635,7 +637,6 @@ fail:
 usb_ep_free_request(hidg->in_ep, hidg->req);
 }

-usb_free_all_descriptors(f);
 return status;
 }

diff --git a/drivers/usb/gadget/function/f_ncm.c
b/drivers/usb/gadget/function/f_ncm.c
index bcdc882..38a9279 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
struct usb_function *f)
 return 0;

 fail:
-usb_free_all_descriptors(f);
 if (ncm->notify_req) {
 kfree(ncm->notify_req->buf);
 usb_ep_free_request(ncm->notify, ncm->notify_req);
diff --git a/drivers/usb/gadget/function/f_obex.c
b/drivers/usb/gadget/function/f_obex.c
index aebae18..9f7148d 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -391,7 +391,6 @@ static int obex_bind(struct usb_configuration *c,
struct usb_function *f)
 return 0;

 fail:
-usb_free_all_descriptors(f);
 /* we might as well release our claims on endpoints */
 if (obex->port.out)
 obex->port.out->driver_data = NULL;
diff --git a/drivers/usb/gadget/function/f_phonet.c
b/drivers/usb/gadget/function/f_phonet.c
index b9cfc15..1ec8b7f 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -570,8 +570,8 @@ static int pn_bind(struct usb_configuration *c,
struct usb_function *f)
 err_req:
 for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
 usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
-err:
 usb_free_all_descriptors(f);
+err:
 if (fp->out_ep)
 fp->out_ep->driver_data = NULL;
 if (fp->in_ep)
diff --git a/drivers/usb/gadget/function/f_rndis.c
b/drivers/usb/gadget/function/f_rndis.c
index ddb09dc..2f0517f 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
 if (rndis->manufacturer && rndis->vendorID &&
 rndis_set_param_vendor(rndis->config, rndis->vendorID,
rndis->manufacturer))
-goto fail;
+goto fail_free_descs;

 /* NOTE:  all that is done without knowing or caring about
  * the network link ... which is unavailable to this code
@@ -817,10 +817,11 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
 rndis->notify->name);
 return 0;

+fail_free_descs:
+usb_free_all_descriptors(f);
 fail:
 kfree(f->os_desc_table);
 f->os_desc_n = 0;
-usb_free_all_descriptors(f);

 if (rndis->notify_req) {
 kfree(rndis->notify_req->buf);
diff --git a/drivers/usb/gadget/function/f_subset.c
b/drivers/usb/gadget/function/f_subset.c
index 1ea8baf..e3dfa67 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -380

Re: xHCI bug

2014-10-13 Thread Felipe Balbi
On Mon, Oct 13, 2014 at 03:38:24PM +0300, Mathias Nyman wrote:
> Hi
> 
> On 10.10.2014 21:47, Felipe Balbi wrote:
> > Hi Mathias,
> > 
> > I seem to be able to rather easily kill xHCI by just running
> > test.sh/testusb with a USB2 device attached to it (I used my AM437x
> > Starter Kit for the test).
> > 
> > I noticed that after not too long, all tests started failing (see below)
> > but using ehci, it works just fine (afaicr).
> > 
> > Here's a snippet of the hang, but it has been running for a while and
> > before testusb I ran over 24 hours of MSC testing, also passed full
> > USB20CV and USB30CV with g_mass_storage and g_zero (and most other
> > gadget drivers too):
> > 
> 
> Ouch, I don't have a proper device in my office to run testusb with, I
> probably should set up one here.
> (If I understood correctly any device able to use g_zero should work?)

correct, anything that can run g_zero.

> Could you try with xhci debugging enabled? (will probably produce a
> lot of output)
> 
> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control

I'll try, sure.

> > [874685.176903] usbtest 1-7:3.0: Linux gadget zero
> > [874685.176906] usbtest 1-7:3.0: high-speed {control in/out bulk-in 
> > bulk-out} tests (+alt)
> > [874703.140258] usbtest 1-7:3.0: TEST 9:  ch9 (subset) control tests, 5000 
> > times
> > [874723.716857] usbtest 1-7:3.0: TEST 10:  queue 32 control calls, 5000 
> > times
> > [874743.773236] usbtest 1-7:3.0: TEST 14:  15000 ep0out, 1..256 vary 1
> > [874748.774291] usbtest 1-7:3.0: ctrl_out write failed, code -110, count 0
> > [874754.297205] usbtest 1-7:3.0: set altsetting to 0 failed, -110
> 
> Timeout? 

yeah, testusb won't wait forever :-)

> > [874790.680681] xhci_hcd :00:14.0: URB transfer length is wrong, xHC 
> > issue? req. len = 0, act. len = 4294967288
> 
> Hmm, "act. len" value looks awfully close to full 32 bit, driver might be 
> wrapping around something here. 

true.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH][RFC] storage: Reject bogus max LUN values

2014-10-13 Thread Matthew Dharm
Is there a constant we can pull from a SCSI header, instead of having
a "magic number" in the code?

Matt

On Sun, Oct 12, 2014 at 5:54 PM, Alan Stern  wrote:
> On Sun, 12 Oct 2014, Mark Knibbs wrote:
>
>> --- linux-3.17/drivers/usb/storage/transport.c.orig   2014-10-05 
>> 20:12:36.0 +0100
>> +++ linux-3.17/drivers/usb/storage/transport.c2014-10-12 
>> 13:11:38.0 +0100
>> @@ -1035,9 +1035,20 @@ int usb_stor_Bulk_max_lun(struct us_data
>>   usb_stor_dbg(us, "GetMaxLUN command result is %d, data is %d\n",
>>result, us->iobuf[0]);
>>
>> - /* if we have a successful request, return the result */
>> - if (result > 0)
>> - return us->iobuf[0];
>> + /*
>> +  * If we have a successful request, return the result if valid. The
>> +  * CBW LUN field is 4 bits wide, so the value reported by the device
>> +  * should fit into that.
>> +  */
>> + if (result > 0) {
>> + if (!(us->iobuf[0] & 0xf0)) {
>
> Since the Max-LUN value is just a plain old number (with no special
> interpretations for the individual bits), it is easier to understand if
> the code treats it that way.  Simply say:
>
> if (us->iobuf[0] < 16) {
>
>> + return us->iobuf[0];
>> + } else {
>> + dev_info(&us->pusb_intf->dev,
>> +  "Max LUN %d is not valid, using 0 instead",
>> +  us->iobuf[0]);
>> + }
>> + }
>>
>>   /*
>>* Some devices don't like GetMaxLUN.  They may STALL the control
>
> Apart from that minor issue,
>
> 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



-- 
Matthew Dharm
Maintainer, USB Mass Storage driver for Linux
--
To unsubscribe from this list: send the line "unsubscribe 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: Subject: [PATCH] usb: gadget: function: Remove redundant usb_free_all_descriptors

2014-10-13 Thread pavi1729 Sid
On Mon, Oct 13, 2014 at 6:55 PM, Robert Baldyga  wrote:
> Hi,
>
> "Subject: " at the beginning of your email subject looks unnecessary.
>
> When you're sending next version of your patch, you should add subject
> prefix "[PATCH vN]", when N is number of version (eg. [PATCH v2]). See
> Documentation/SubmittingPatches.
> ('git format-patch --subject-prefix' is helpful).
>
> On 10/13/2014 11:35 AM, pavi1729 Sid wrote:
>> Removed the usb_free_all_descriptors call in *_bind functions
>> as this call is already present in usb_assign_descriptors.
>> usb_assign_descriptors is the only call where usb descriptor
>> allocation happens, also in case of error freeing of the
>> allocated memory takes place in the same call. Hence the
>> call in the *_bind functions are redundant.
>> Also the presence of usb_free_all_descriptors in the *_bind
>> functions might result in double-free corruption of the
>> allocated mmeory.
>
> s/mmeory/memory
Will fix it.
>
>>
>> Signed-off-by: Pavitrakumar Managutte 
>> ---
>>  drivers/usb/gadget/function/f_eem.c|  1 -
>>  drivers/usb/gadget/function/f_hid.c|  7 +++
>>  drivers/usb/gadget/function/f_ncm.c|  1 -
>>  drivers/usb/gadget/function/f_phonet.c |  1 -
>>  drivers/usb/gadget/function/f_rndis.c  |  5 +++--
>>  drivers/usb/gadget/function/f_subset.c |  1 -
>>  drivers/usb/gadget/function/f_uac2.c   | 10 ++
>>  7 files changed, 12 insertions(+), 14 deletions(-)
>
> There is another one usb function which needs to be fixed this way:
> drivers/usb/gadget/function/f_obex.c
Will fix it.
>
>>
>> diff --git a/drivers/usb/gadget/function/f_eem.c
>> b/drivers/usb/gadget/function/f_eem.c
>> index 4d8b236..c9e90de 100644
>> --- a/drivers/usb/gadget/function/f_eem.c
>> +++ b/drivers/usb/gadget/function/f_eem.c
>> @@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
>> struct usb_function *f)
>>  return 0;
>>
>>  fail:
>> -usb_free_all_descriptors(f);
>>  if (eem->port.out_ep)
>>  eem->port.out_ep->driver_data = NULL;
>>  if (eem->port.in_ep)
>> diff --git a/drivers/usb/gadget/function/f_hid.c
>> b/drivers/usb/gadget/function/f_hid.c
>> index a95290a..c36de0c 100644
>> --- a/drivers/usb/gadget/function/f_hid.c
>> +++ b/drivers/usb/gadget/function/f_hid.c
>> @@ -621,12 +621,14 @@ static int __init hidg_bind(struct
>> usb_configuration *c, struct usb_function *f)
>>  dev = MKDEV(major, hidg->minor);
>>  status = cdev_add(&hidg->cdev, dev, 1);
>>  if (status)
>> -goto fail;
>> +goto fail_free_descs;
>>
>>  device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);
>>
>>  return 0;
>>
>> +fail_free_descs:
>> +usb_free_all_descriptors(f);
>>  fail:
>>  ERROR(f->config->cdev, "hidg_bind FAILED\n");
>>  if (hidg->req != NULL) {
>> @@ -635,7 +637,6 @@ fail:
>>  usb_ep_free_request(hidg->in_ep, hidg->req);
>>  }
>>
>> -usb_free_all_descriptors(f);
>>  return status;
>>  }
>>
>> @@ -652,8 +653,6 @@ static void hidg_unbind(struct usb_configuration
>> *c, struct usb_function *f)
>>  kfree(hidg->req->buf);
>>  usb_ep_free_request(hidg->in_ep, hidg->req);
>>
>> -usb_free_all_descriptors(f);
>> -
>
> Why usb_free_all_descriptors() removed from here?
My bad .. its not supposed to be in unbind function. Will fix it.
>
>>  kfree(hidg->report_desc);
>>  kfree(hidg);
>>  }
>> diff --git a/drivers/usb/gadget/function/f_ncm.c
>> b/drivers/usb/gadget/function/f_ncm.c
>> index bcdc882..38a9279 100644
>> --- a/drivers/usb/gadget/function/f_ncm.c
>> +++ b/drivers/usb/gadget/function/f_ncm.c
>> @@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
>> struct usb_function *f)
>>  return 0;
>>
>>  fail:
>> -usb_free_all_descriptors(f);
>>  if (ncm->notify_req) {
>>  kfree(ncm->notify_req->buf);
>>  usb_ep_free_request(ncm->notify, ncm->notify_req);
>> diff --git a/drivers/usb/gadget/function/f_phonet.c
>> b/drivers/usb/gadget/function/f_phonet.c
>> index b9cfc15..74ddcac 100644
>> --- a/drivers/usb/gadget/function/f_phonet.c
>> +++ b/drivers/usb/gadget/function/f_phonet.c
>> @@ -571,7 +571,6 @@ err_req:
>>  for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
>>  usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
>>  err:
>> -usb_free_all_descriptors(f);
>
> What if usb_ep_alloc_request() fails?
> Consider calling usb_free_all_descriptors() under label "err_req".
Agreed. Will fix it.
>
>>  if (fp->out_ep)
>>  fp->out_ep->driver_data = NULL;
>>  if (fp->in_ep)
>> diff --git a/drivers/usb/gadget/function/f_rndis.c
>> b/drivers/usb/gadget/function/f_rndis.c
>> index ddb09dc..2f0517f 100644
>> --- a/drivers/usb/gadget/function/f_rndis.c
>> +++ b/drivers/usb/gadget/function/f_rndis.c
>> @@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
>> usb_function *f)
>>  if (rndis->manufacturer && rndis->vendorID &&
>>  rndis_set_param_vendor(rndis->config, rndis-

[PATCH v2 1/2] usb: chipidea: core: Remove unneeded OOM message

2014-10-13 Thread Fabio Estevam
MM core code already complains when devm_kzalloc() fails, so no need to print
the error locally.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Add usb in the Subject

 drivers/usb/chipidea/core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 3df5005..c59929a 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -589,10 +589,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return PTR_ERR(base);
 
ci = devm_kzalloc(dev, sizeof(*ci), GFP_KERNEL);
-   if (!ci) {
-   dev_err(dev, "can't allocate device\n");
+   if (!ci)
return -ENOMEM;
-   }
 
ci->dev = dev;
ci->platdata = dev_get_platdata(dev);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Subject: [PATCH] usb: gadget: function: Remove redundant usb_free_all_descriptors

2014-10-13 Thread Robert Baldyga
Hi,

"Subject: " at the beginning of your email subject looks unnecessary.

When you're sending next version of your patch, you should add subject
prefix "[PATCH vN]", when N is number of version (eg. [PATCH v2]). See
Documentation/SubmittingPatches.
('git format-patch --subject-prefix' is helpful).

On 10/13/2014 11:35 AM, pavi1729 Sid wrote:
> Removed the usb_free_all_descriptors call in *_bind functions
> as this call is already present in usb_assign_descriptors.
> usb_assign_descriptors is the only call where usb descriptor
> allocation happens, also in case of error freeing of the
> allocated memory takes place in the same call. Hence the
> call in the *_bind functions are redundant.
> Also the presence of usb_free_all_descriptors in the *_bind
> functions might result in double-free corruption of the
> allocated mmeory.

s/mmeory/memory

> 
> Signed-off-by: Pavitrakumar Managutte 
> ---
>  drivers/usb/gadget/function/f_eem.c|  1 -
>  drivers/usb/gadget/function/f_hid.c|  7 +++
>  drivers/usb/gadget/function/f_ncm.c|  1 -
>  drivers/usb/gadget/function/f_phonet.c |  1 -
>  drivers/usb/gadget/function/f_rndis.c  |  5 +++--
>  drivers/usb/gadget/function/f_subset.c |  1 -
>  drivers/usb/gadget/function/f_uac2.c   | 10 ++
>  7 files changed, 12 insertions(+), 14 deletions(-)

There is another one usb function which needs to be fixed this way:
drivers/usb/gadget/function/f_obex.c

> 
> diff --git a/drivers/usb/gadget/function/f_eem.c
> b/drivers/usb/gadget/function/f_eem.c
> index 4d8b236..c9e90de 100644
> --- a/drivers/usb/gadget/function/f_eem.c
> +++ b/drivers/usb/gadget/function/f_eem.c
> @@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
> struct usb_function *f)
>  return 0;
> 
>  fail:
> -usb_free_all_descriptors(f);
>  if (eem->port.out_ep)
>  eem->port.out_ep->driver_data = NULL;
>  if (eem->port.in_ep)
> diff --git a/drivers/usb/gadget/function/f_hid.c
> b/drivers/usb/gadget/function/f_hid.c
> index a95290a..c36de0c 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -621,12 +621,14 @@ static int __init hidg_bind(struct
> usb_configuration *c, struct usb_function *f)
>  dev = MKDEV(major, hidg->minor);
>  status = cdev_add(&hidg->cdev, dev, 1);
>  if (status)
> -goto fail;
> +goto fail_free_descs;
> 
>  device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);
> 
>  return 0;
> 
> +fail_free_descs:
> +usb_free_all_descriptors(f);
>  fail:
>  ERROR(f->config->cdev, "hidg_bind FAILED\n");
>  if (hidg->req != NULL) {
> @@ -635,7 +637,6 @@ fail:
>  usb_ep_free_request(hidg->in_ep, hidg->req);
>  }
> 
> -usb_free_all_descriptors(f);
>  return status;
>  }
> 
> @@ -652,8 +653,6 @@ static void hidg_unbind(struct usb_configuration
> *c, struct usb_function *f)
>  kfree(hidg->req->buf);
>  usb_ep_free_request(hidg->in_ep, hidg->req);
> 
> -usb_free_all_descriptors(f);
> -

Why usb_free_all_descriptors() removed from here?

>  kfree(hidg->report_desc);
>  kfree(hidg);
>  }
> diff --git a/drivers/usb/gadget/function/f_ncm.c
> b/drivers/usb/gadget/function/f_ncm.c
> index bcdc882..38a9279 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
> struct usb_function *f)
>  return 0;
> 
>  fail:
> -usb_free_all_descriptors(f);
>  if (ncm->notify_req) {
>  kfree(ncm->notify_req->buf);
>  usb_ep_free_request(ncm->notify, ncm->notify_req);
> diff --git a/drivers/usb/gadget/function/f_phonet.c
> b/drivers/usb/gadget/function/f_phonet.c
> index b9cfc15..74ddcac 100644
> --- a/drivers/usb/gadget/function/f_phonet.c
> +++ b/drivers/usb/gadget/function/f_phonet.c
> @@ -571,7 +571,6 @@ err_req:
>  for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
>  usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
>  err:
> -usb_free_all_descriptors(f);

What if usb_ep_alloc_request() fails?
Consider calling usb_free_all_descriptors() under label "err_req".

>  if (fp->out_ep)
>  fp->out_ep->driver_data = NULL;
>  if (fp->in_ep)
> diff --git a/drivers/usb/gadget/function/f_rndis.c
> b/drivers/usb/gadget/function/f_rndis.c
> index ddb09dc..2f0517f 100644
> --- a/drivers/usb/gadget/function/f_rndis.c
> +++ b/drivers/usb/gadget/function/f_rndis.c
> @@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
> usb_function *f)
>  if (rndis->manufacturer && rndis->vendorID &&
>  rndis_set_param_vendor(rndis->config, rndis->vendorID,
> rndis->manufacturer))
> -goto fail;
> +goto fail_free_descs;
> 
>  /* NOTE:  all that is done without knowing or caring about
>   * the network link ... which is unavailable to this code
> @@ -817,10 +817,11 @@ rndis_bind(struct usb_configura

Re: xHCI bug

2014-10-13 Thread Mathias Nyman
On 10.10.2014 22:01, Tony Lindgren wrote:
> * Felipe Balbi  [141010 11:51]:
>> Hi again,
>>
>> On Fri, Oct 10, 2014 at 01:47:55PM -0500, Felipe Balbi wrote:
>>> I seem to be able to rather easily kill xHCI by just running
>>> test.sh/testusb with a USB2 device attached to it (I used my AM437x
>>> Starter Kit for the test).
>>
>> I think Tony mentioned taht it's also pretty easy to break xHCI by
>> trying to cold flash an old N900
> 
> Heh yeah. After upgrading my build box, I noticed few things.
> The new mobo came with xchi, and the following broke:
> 
> 1. My old printer MFC-7820N no longer worked for scanning
> 
> 2. Trying to cold flash any omap3 boards over USB stopped working
>for most of the time
> 
> 3. Back-ups to a USB drive occasionally started failing
> 
> 4. After disabling xhci in the BIOS, things were still flakey
>with ehci
> 
> 5. I ended up installing a usb2.0 ehci pcie card to fix things
> 

Hmm, sounds like it might not only be a xhci issue then.

If you for some reason want to enable xhci and try coldflashing n900/use scanner
could you enable xhci debugging and send me the output of a failing case?

echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control

-Mathias

--
To unsubscribe from this list: send the line "unsubscribe 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 bug

2014-10-13 Thread Mathias Nyman
Hi

On 10.10.2014 21:47, Felipe Balbi wrote:
> Hi Mathias,
> 
> I seem to be able to rather easily kill xHCI by just running
> test.sh/testusb with a USB2 device attached to it (I used my AM437x
> Starter Kit for the test).
> 
> I noticed that after not too long, all tests started failing (see below)
> but using ehci, it works just fine (afaicr).
> 
> Here's a snippet of the hang, but it has been running for a while and
> before testusb I ran over 24 hours of MSC testing, also passed full
> USB20CV and USB30CV with g_mass_storage and g_zero (and most other
> gadget drivers too):
> 

Ouch, I don't have a proper device in my office to run testusb with, I probably 
should set up one here.
(If I understood correctly any device able to use g_zero should work?)

Could you try with xhci debugging enabled? (will probably produce a lot of 
output)

echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control

> [874685.176903] usbtest 1-7:3.0: Linux gadget zero
> [874685.176906] usbtest 1-7:3.0: high-speed {control in/out bulk-in bulk-out} 
> tests (+alt)
> [874703.140258] usbtest 1-7:3.0: TEST 9:  ch9 (subset) control tests, 5000 
> times
> [874723.716857] usbtest 1-7:3.0: TEST 10:  queue 32 control calls, 5000 times
> [874743.773236] usbtest 1-7:3.0: TEST 14:  15000 ep0out, 1..256 vary 1
> [874748.774291] usbtest 1-7:3.0: ctrl_out write failed, code -110, count 0
> [874754.297205] usbtest 1-7:3.0: set altsetting to 0 failed, -110

Timeout? 
...


> [874790.680681] xhci_hcd :00:14.0: URB transfer length is wrong, xHC 
> issue? req. len = 0, act. len = 4294967288

Hmm, "act. len" value looks awfully close to full 32 bit, driver might be 
wrapping around something here. 

-Mathias

--
To unsubscribe from this list: send the line "unsubscribe 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 2/2] usb: chipidea: ci_hdrc_imx.c: Remove unneeded OOM message

2014-10-13 Thread Fabio Estevam
MM core code already complains when devm_kzalloc() fails, so no need to print
the error locally.

Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Add usb in the Subject

 drivers/usb/chipidea/ci_hdrc_imx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index a7ab0f1..74b5b09 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -115,10 +115,8 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
const struct ci_hdrc_imx_platform_flag *imx_platform_flag = of_id->data;
 
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-   if (!data) {
-   dev_err(&pdev->dev, "Failed to allocate ci_hdrc-imx data!\n");
+   if (!data)
return -ENOMEM;
-   }
 
data->usbmisc_data = usbmisc_get_init_data(&pdev->dev);
if (IS_ERR(data->usbmisc_data))
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] chipidea: core: Remove unneeded OOM message

2014-10-13 Thread Fabio Estevam
On Mon, Oct 13, 2014 at 2:53 AM, Peter Chen  wrote:
> On Sun, Oct 12, 2014 at 11:26:33PM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam 
>>
>> MM core code already complains when devm_kzalloc() fails, so no need to print
>> the error locally.
>>
>
> For the title: you may need to add "usb" first. "usb: chipdea: xxx".

Ok, will do it in v2.

> Besides, would you show me where the MM core code to do that?

Please check the following commit for a better explanation:

commit ebfdc40969f24fc0cdd1349835d36e8ebae05374
Author: Joe Perches 
Date:   Wed Aug 6 16:10:27 2014 -0700

checkpatch: attempt to find unnecessary 'out of memory' messages
--
To unsubscribe from this list: send the line "unsubscribe 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: RCU bug with v3.17-rc3 ?

2014-10-13 Thread Russell King - ARM Linux
On Mon, Oct 13, 2014 at 09:11:34AM +, David Laight wrote:
> From: Nathan Lynch
> > On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
> > >
> > > Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
> > > it seems that this has been known about for some time.)
> > 
> > Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x < 3
> > are affected, as well as 4.9.0.
> > 
> > > We can blacklist these GCC versions quite easily.  We already have GCC
> > > 3.3 blacklisted, and it's trivial to add others.  I would want to include
> > > some proper details about the bug, just like the other existing entries
> > > we already have in asm-offsets.c, where we name the functions that the
> > > compiler is known to break where appropriate.
> > 
> > Before blacklisting anything, it's worth considering that simple version
> > checks would break existing pre-4.8.3 compilers that have been patched
> > for PR58854.  It looks like Yocto and Buildroot issued releases with
> > patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
> > the most we can reasonably do without breaking some correctly-behaving
> > toolchains is to emit a warning.
> 
> Is it possible to compile a small code fragment and check the generated
> code for the bug?
> Possibly predicated on the broken version number to avoid false positives.

I don't see how - it looks like it requires an interrupt to occur at an
opportune moment to provoke the function to fail.  The alternative would
be to parse the assembly generated by the compiler to determine how it
is dealing with the stack.

I think the only viable solution here is that:

1. We blacklist the bad compiler versions outright in the kernel.
2. We /consider/ a testing a preprocessor symbol which when present
   indicates that these versions are fixed and should not be blacklisted.

The argument for (2) is that /if/ distros want to patch their compilers
to fix the problem, they /also/ have the ability to patch their compilers
to make them identifyable, and that is a far more reliable solution than
trying to parse the assembly output from multiple different GCC versions.

Remember, it's the distro's choice to fix these buggy compilers, so the
onus is on _them_ to deal with the mess they've created by doing so.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe 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: [gadget] Testing midi function

2014-10-13 Thread Andrzej Pietrasiewicz

Hi Clemens,

W dniu 13.10.2014 o 12:44, Clemens Ladisch pisze:

Andrzej Pietrasiewicz wrote:






I've never used it, but as far as I can see, the MIDI function ends up
as a normal sound card with MIDI port(s).  Your device firmware would
access them through the normal ALSA rawmidi or sequencer APIs.

As for testing:  To monitor received data, use amidi or aseqdump.  To
send data, use amidi or aplaymidi.  To make a loopback device, use
aconnect to connect the input to the output.

This requires the alsa-lib and alsa-utils packages.



Thank you for your response.

AP
--
To unsubscribe from this list: send the line "unsubscribe 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: [gadget] Testing midi function

2014-10-13 Thread Clemens Ladisch
Andrzej Pietrasiewicz wrote:
> I would like to try the midi function of usb gadget.
>
> Can anybody please tell me:
>
> 1) What userspace programs do I need at the gadget side?

I've never used it, but as far as I can see, the MIDI function ends up
as a normal sound card with MIDI port(s).  Your device firmware would
access them through the normal ALSA rawmidi or sequencer APIs.

As for testing:  To monitor received data, use amidi or aseqdump.  To
send data, use amidi or aplaymidi.  To make a loopback device, use
aconnect to connect the input to the output.

This requires the alsa-lib and alsa-utils packages.

> 2) What do I need at the host side?

USB MIDI is plug&play in every OS.  For Linux, see above.


Regards,
Clemens
--
To unsubscribe from this list: send the line "unsubscribe 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: musb: dsps: start OTG timer on resume again

2014-10-13 Thread Sebastian Andrzej Siewior
Commit 468bcc2a2ca ("usb: musb: dsps: kill OTG timer on suspend") stopped
the timer in suspend path but forgot the re-enable it in the resume
path. This patch fixes the behaviour.

Cc:  # v3.14+
Fixes 468bcc2a2ca "usb: musb: dsps: kill OTG timer on suspend"
Signed-off-by: Sebastian Andrzej Siewior 
---

This hasn't been tested except for compile time since I can't do suspend
to mem on vanila. Since setup_timer() does not start the timer again I
guess it is not working properly.

 drivers/usb/musb/musb_dsps.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 981756387d43..48bc09e7b83b 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -906,7 +906,9 @@ static int dsps_resume(struct device *dev)
dsps_writel(mbase, wrp->mode, glue->context.mode);
dsps_writel(mbase, wrp->tx_mode, glue->context.tx_mode);
dsps_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
-   setup_timer(&glue->timer, otg_timer, (unsigned long) musb);
+   if (musb->xceiv->state == OTG_STATE_B_IDLE &&
+   musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+   mod_timer(&glue->timer, jiffies + wrp->poll_seconds * HZ);
 
return 0;
 }
-- 
2.1.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


[gadget] Testing midi function

2014-10-13 Thread Andrzej Pietrasiewicz

Hi all,

I would like to try the midi function of usb gadget.

Can anybody please tell me:

1) What userspace programs do I need at the gadget side?
How to run them?

2) What do I need at the host side? How to tell whether my midi gadget
is working correctly?

AP
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Subject: [PATCH] usb: gadget: function: Remove redundant usb_free_all_descriptors

2014-10-13 Thread pavi1729 Sid
Removed the usb_free_all_descriptors call in *_bind functions
as this call is already present in usb_assign_descriptors.
usb_assign_descriptors is the only call where usb descriptor
allocation happens, also in case of error freeing of the
allocated memory takes place in the same call. Hence the
call in the *_bind functions are redundant.
Also the presence of usb_free_all_descriptors in the *_bind
functions might result in double-free corruption of the
allocated mmeory.

Signed-off-by: Pavitrakumar Managutte 
---
 drivers/usb/gadget/function/f_eem.c|  1 -
 drivers/usb/gadget/function/f_hid.c|  7 +++
 drivers/usb/gadget/function/f_ncm.c|  1 -
 drivers/usb/gadget/function/f_phonet.c |  1 -
 drivers/usb/gadget/function/f_rndis.c  |  5 +++--
 drivers/usb/gadget/function/f_subset.c |  1 -
 drivers/usb/gadget/function/f_uac2.c   | 10 ++
 7 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/function/f_eem.c
b/drivers/usb/gadget/function/f_eem.c
index 4d8b236..c9e90de 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
struct usb_function *f)
 return 0;

 fail:
-usb_free_all_descriptors(f);
 if (eem->port.out_ep)
 eem->port.out_ep->driver_data = NULL;
 if (eem->port.in_ep)
diff --git a/drivers/usb/gadget/function/f_hid.c
b/drivers/usb/gadget/function/f_hid.c
index a95290a..c36de0c 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -621,12 +621,14 @@ static int __init hidg_bind(struct
usb_configuration *c, struct usb_function *f)
 dev = MKDEV(major, hidg->minor);
 status = cdev_add(&hidg->cdev, dev, 1);
 if (status)
-goto fail;
+goto fail_free_descs;

 device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);

 return 0;

+fail_free_descs:
+usb_free_all_descriptors(f);
 fail:
 ERROR(f->config->cdev, "hidg_bind FAILED\n");
 if (hidg->req != NULL) {
@@ -635,7 +637,6 @@ fail:
 usb_ep_free_request(hidg->in_ep, hidg->req);
 }

-usb_free_all_descriptors(f);
 return status;
 }

@@ -652,8 +653,6 @@ static void hidg_unbind(struct usb_configuration
*c, struct usb_function *f)
 kfree(hidg->req->buf);
 usb_ep_free_request(hidg->in_ep, hidg->req);

-usb_free_all_descriptors(f);
-
 kfree(hidg->report_desc);
 kfree(hidg);
 }
diff --git a/drivers/usb/gadget/function/f_ncm.c
b/drivers/usb/gadget/function/f_ncm.c
index bcdc882..38a9279 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
struct usb_function *f)
 return 0;

 fail:
-usb_free_all_descriptors(f);
 if (ncm->notify_req) {
 kfree(ncm->notify_req->buf);
 usb_ep_free_request(ncm->notify, ncm->notify_req);
diff --git a/drivers/usb/gadget/function/f_phonet.c
b/drivers/usb/gadget/function/f_phonet.c
index b9cfc15..74ddcac 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -571,7 +571,6 @@ err_req:
 for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
 usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
 err:
-usb_free_all_descriptors(f);
 if (fp->out_ep)
 fp->out_ep->driver_data = NULL;
 if (fp->in_ep)
diff --git a/drivers/usb/gadget/function/f_rndis.c
b/drivers/usb/gadget/function/f_rndis.c
index ddb09dc..2f0517f 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
 if (rndis->manufacturer && rndis->vendorID &&
 rndis_set_param_vendor(rndis->config, rndis->vendorID,
rndis->manufacturer))
-goto fail;
+goto fail_free_descs;

 /* NOTE:  all that is done without knowing or caring about
  * the network link ... which is unavailable to this code
@@ -817,10 +817,11 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
 rndis->notify->name);
 return 0;

+fail_free_descs:
+usb_free_all_descriptors(f);
 fail:
 kfree(f->os_desc_table);
 f->os_desc_n = 0;
-usb_free_all_descriptors(f);

 if (rndis->notify_req) {
 kfree(rndis->notify_req->buf);
diff --git a/drivers/usb/gadget/function/f_subset.c
b/drivers/usb/gadget/function/f_subset.c
index 1ea8baf..e3dfa67 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -380,7 +380,6 @@ geth_bind(struct usb_configuration *c, struct
usb_function *f)
 return 0;

 fail:
-usb_free_all_descriptors(f);
 /* we might as well release our claims on endpoints */
 if (geth->port.out_ep)
 geth->port.out_ep->driver_data = NULL;
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/funct

Re: [PATCH] usb: musb: dsps: kill OTG timer on suspend

2014-10-13 Thread Sebastian Andrzej Siewior
On 2014-09-15 09:39:09 [-0500], Felipe Balbi wrote:
> if we don't make sure to kill the timer, it could
> expire after we have already gated our clocks.
> 
> That will trigger a Data Abort exception because
> we would try to access register while clock is gated.

That timer deserves to be killed because nobody wants it to wakeup the
system from suspend. However the Data Abort wouldn't happen if the timer
would use pm_runtime_get_sync() right?
 
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -870,6 +870,7 @@ static int dsps_suspend(struct device *dev)
>   struct musb *musb = platform_get_drvdata(glue->musb);
>   void __iomem *mbase = musb->ctrl_base;
>  
> + del_timer_sync(&glue->timer);
>   glue->context.control = dsps_readl(mbase, wrp->control);
>   glue->context.epintr = dsps_readl(mbase, wrp->epintr_set);
>   glue->context.coreintr = dsps_readl(mbase, wrp->coreintr_set);
> @@ -895,6 +896,7 @@ static int dsps_resume(struct device *dev)
>   dsps_writel(mbase, wrp->mode, glue->context.mode);
>   dsps_writel(mbase, wrp->tx_mode, glue->context.tx_mode);
>   dsps_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
> + setup_timer(&glue->timer, otg_timer, (unsigned long) musb);

You need a mod_timer() here instead. I will send a patch in a few.

>   return 0;
>  }

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: RCU bug with v3.17-rc3 ?

2014-10-13 Thread David Laight
From: Nathan Lynch
> On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
> >
> > Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
> > it seems that this has been known about for some time.)
> 
> Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x < 3
> are affected, as well as 4.9.0.
> 
> > We can blacklist these GCC versions quite easily.  We already have GCC
> > 3.3 blacklisted, and it's trivial to add others.  I would want to include
> > some proper details about the bug, just like the other existing entries
> > we already have in asm-offsets.c, where we name the functions that the
> > compiler is known to break where appropriate.
> 
> Before blacklisting anything, it's worth considering that simple version
> checks would break existing pre-4.8.3 compilers that have been patched
> for PR58854.  It looks like Yocto and Buildroot issued releases with
> patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
> the most we can reasonably do without breaking some correctly-behaving
> toolchains is to emit a warning.

Is it possible to compile a small code fragment and check the generated
code for the bug?
Possibly predicated on the broken version number to avoid false positives.

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


[PATCH v2] usb: musb: musb_dsps: fix NULL pointer in suspend

2014-10-13 Thread Sebastian Andrzej Siewior
So testing managed to configure musb in DMA mode but not load the
matching cppi41 driver for DMA. This results in

|musb-hdrc musb-hdrc.0.auto: Failed to request rx1.
|musb-hdrc musb-hdrc.0.auto: musb_init_controller failed with status -517
|platform musb-hdrc.0.auto: Driver musb-hdrc requests probe deferral

which is "okay". Once the driver is loaded we re-try probing and
everyone is happy. Until then if you try suspend say
echo mem > /sys/power/state
then you go boom

|Unable to handle kernel NULL pointer dereference at virtual address 03a4
|pgd = cf50c000
|[03a4] *pgd=8f6a3831, *pte=, *ppte=
|Internal error: Oops: 17 [#1] ARM
|PC is at dsps_suspend+0x18/0x9c [musb_dsps]
|LR is at dsps_suspend+0x18/0x9c [musb_dsps]
|pc : [] lr : [] psr: a013
|sp : cbd97e00 ip : c0af4394 fp : 
|r10: c0831d90 r9 : 0002 r8 : cf6da410
|r7 : c03ba4dc r6 : bf08f224 r5 :  r4 : cbc5fcd0
|r3 : bf08e250 r2 : bf08f264 r1 : cf6da410 r0 : 
|[] (dsps_suspend [musb_dsps]) from [] 
(platform_pm_suspend+0x2c/0x54)
|Code: e1a04000 e9900041 e2800010 eb4caa8e (e59053a4)

because platform_get_drvdata(glue->musb) returns a NULL pointer as long as the
device is not fully probed.

Tested-by: George Cherian 
Signed-off-by: Sebastian Andrzej Siewior 
---
v1…v2: rebase on top of testing/fixes

 drivers/usb/musb/musb_dsps.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 154bcf1b5dfa..981756387d43 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -868,9 +868,15 @@ static int dsps_suspend(struct device *dev)
struct dsps_glue *glue = dev_get_drvdata(dev);
const struct dsps_musb_wrapper *wrp = glue->wrp;
struct musb *musb = platform_get_drvdata(glue->musb);
-   void __iomem *mbase = musb->ctrl_base;
+   void __iomem *mbase;
 
del_timer_sync(&glue->timer);
+
+   if (!musb)
+   /* This can happen if the musb device is in -EPROBE_DEFER */
+   return 0;
+
+   mbase = musb->ctrl_base;
glue->context.control = dsps_readl(mbase, wrp->control);
glue->context.epintr = dsps_readl(mbase, wrp->epintr_set);
glue->context.coreintr = dsps_readl(mbase, wrp->coreintr_set);
@@ -887,8 +893,12 @@ static int dsps_resume(struct device *dev)
struct dsps_glue *glue = dev_get_drvdata(dev);
const struct dsps_musb_wrapper *wrp = glue->wrp;
struct musb *musb = platform_get_drvdata(glue->musb);
-   void __iomem *mbase = musb->ctrl_base;
+   void __iomem *mbase;
+
+   if (!musb)
+   return 0;
 
+   mbase = musb->ctrl_base;
dsps_writel(mbase, wrp->control, glue->context.control);
dsps_writel(mbase, wrp->epintr_set, glue->context.epintr);
dsps_writel(mbase, wrp->coreintr_set, glue->context.coreintr);
-- 
2.1.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: gadget: function: Remove redundant usb_free_all_descriptors

2014-10-13 Thread pavi1729 Sid
On Mon, Oct 13, 2014 at 2:02 PM, Robert Baldyga  wrote:
> Hi,
>
> On 10/10/2014 03:09 PM, pavi1729 ivap wrote:
>> From: Pavitra 
>> Date: Fri, 10 Oct 2014 16:05:30 +0530
>> Subject: [PATCH] usb: gadget: function: Remove redundant
>>  usb_free_all_descriptors
>>
>> Removed the usb_free_all_descriptors call in *_bind functions
>> as this call is already present in usb_assign_descriptors.
>> usb_assign_descriptors is the only call where usb descriptor
>> allocation happens, also in case of error freeing of the
>> allocated memory takes place in the same call. Hence the
>> call in the *_bind functions are redundant.
>> Also the presence of usb_free_all_descriptors in the *_bind
>> functions might result in double-free corruption of the
>> allocated mmeory.
>>
>> Signed-off-by: Pavitra 
>
> You should use your full real name in sign-off (see
> Documentation/SubmittingPatches).
will do that.
>
> I see there are no maintainers in your CC list. You should use
> scripts/get_maintainer.pl on your patch.
Will do that.
>
>> ---
>>  drivers/usb/gadget/function/f_eem.c|  1 -
>>  drivers/usb/gadget/function/f_hid.c|  7 +++
>>  drivers/usb/gadget/function/f_ncm.c|  1 -
>>  drivers/usb/gadget/function/f_phonet.c |  1 -
>>  drivers/usb/gadget/function/f_rndis.c  |  5 +++--
>>  drivers/usb/gadget/function/f_subset.c |  1 -
>>  drivers/usb/gadget/function/f_uac2.c   | 10 ++
>>  7 files changed, 12 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_eem.c
>> b/drivers/usb/gadget/function/f_eem.c
>> index 4d8b236..c9e90de 100644
>> --- a/drivers/usb/gadget/function/f_eem.c
>> +++ b/drivers/usb/gadget/function/f_eem.c
>> @@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
>> struct usb_function *f)
>>  return 0;
>>
>>  fail:
>> -usb_free_all_descriptors(f);
>>  if (eem->port.out_ep)
>>  eem->port.out_ep->driver_data = NULL;
>>  if (eem->port.in_ep)
>> diff --git a/drivers/usb/gadget/function/f_hid.c
>> b/drivers/usb/gadget/function/f_hid.c
>> index a95290a..df4f390 100644
>> --- a/drivers/usb/gadget/function/f_hid.c
>> +++ b/drivers/usb/gadget/function/f_hid.c
>> @@ -621,12 +621,14 @@ static int __init hidg_bind(struct
>> usb_configuration *c, struct usb_function *f)
>>  dev = MKDEV(major, hidg->minor);
>>  status = cdev_add(&hidg->cdev, dev, 1);
>>  if (status)
>> -goto fail;
>> +goto err;
>>
>>  device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);
>>
>>  return 0;
>>
>> +err:
>
> Maybe it would be better to use label name more consistent with the
> naming convention in modified file (for example when existing label name
> is "fail", you can use name "fail_free_descs" for your label).
Agreed.
>
>> +usb_free_all_descriptors(f);
>>  fail:
>>  ERROR(f->config->cdev, "hidg_bind FAILED\n");
>>  if (hidg->req != NULL) {
>> @@ -635,7 +637,6 @@ fail:
>>  usb_ep_free_request(hidg->in_ep, hidg->req);
>>  }
>>
>> -usb_free_all_descriptors(f);
>>  return status;
>>  }
>>
>> @@ -652,8 +653,6 @@ static void hidg_unbind(struct usb_configuration
>> *c, struct usb_function *f)
>>  kfree(hidg->req->buf);
>>  usb_ep_free_request(hidg->in_ep, hidg->req);
>>
>> -usb_free_all_descriptors(f);
>> -
>>  kfree(hidg->report_desc);
>>  kfree(hidg);
>>  }
>> diff --git a/drivers/usb/gadget/function/f_ncm.c
>> b/drivers/usb/gadget/function/f_ncm.c
>> index bcdc882..38a9279 100644
>> --- a/drivers/usb/gadget/function/f_ncm.c
>> +++ b/drivers/usb/gadget/function/f_ncm.c
>> @@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
>> struct usb_function *f)
>>  return 0;
>>
>>  fail:
>> -usb_free_all_descriptors(f);
>>  if (ncm->notify_req) {
>>  kfree(ncm->notify_req->buf);
>>  usb_ep_free_request(ncm->notify, ncm->notify_req);
>> diff --git a/drivers/usb/gadget/function/f_phonet.c
>> b/drivers/usb/gadget/function/f_phonet.c
>> index b9cfc15..74ddcac 100644
>> --- a/drivers/usb/gadget/function/f_phonet.c
>> +++ b/drivers/usb/gadget/function/f_phonet.c
>> @@ -571,7 +571,6 @@ err_req:
>>  for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
>>  usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
>>  err:
>> -usb_free_all_descriptors(f);
>>  if (fp->out_ep)
>>  fp->out_ep->driver_data = NULL;
>>  if (fp->in_ep)
>> diff --git a/drivers/usb/gadget/function/f_rndis.c
>> b/drivers/usb/gadget/function/f_rndis.c
>> index ddb09dc..dae8c4b 100644
>> --- a/drivers/usb/gadget/function/f_rndis.c
>> +++ b/drivers/usb/gadget/function/f_rndis.c
>> @@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
>> usb_function *f)
>>  if (rndis->manufacturer && rndis->vendorID &&
>>  rndis_set_param_vendor(rndis->config, rndis->vendorID,
>> rndis->manufacturer))
>> -goto fail;
>> +goto err;
>>
>>  /* NOTE:  all that is done without know

Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

2014-10-13 Thread Peter Chen
On Thu, Sep 25, 2014 at 07:37:50PM -0500, Felipe Balbi wrote:
> HI,
> 
> On Fri, Sep 26, 2014 at 07:39:34AM +0800, Peter Chen wrote:
> > On Thu, Sep 25, 2014 at 09:15:53AM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> > > > > 
> > > > > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > > > > (dwc3, musb, chipidea) you are talking about, right? Except for
> > > > > creating another platform driver as well as related DT node 
> > > > > (optional),
> > > > > are there any advantages compared to current IP core driver structure?
> > > > 
> > > > Having a library module usually requires less code, and is more
> > > > consistent with other drivers, which helps new developers understand
> > > > what the driver is doing.
> > > 
> > > I beg to differ. You end-up having to pass function pointers through
> > > platform_data to handle differences which could be handled by the core
> > > IP.
> > > 
> > > > The most important aspect though is the DT binding: once the structure
> > > > with separate kernel drivers leaks out into the DT format, you can't
> > > > easily change the driver any more, e.g. to make a property that was
> > > > introduced for one glue driver more general so it can get handled by
> > > > the common part. Having a single node lets us convert to the library
> > > > model later, so that would be a strong reason to keep the DT binding
> > > > simple, without child nodes.
> > > > 
> > > > Following that logic, it turns into another reason for using the library
> > > > model to start with, because otherwise the child device does not have
> > > > any DT properties itself and has to rely on the parent device 
> > > > properties,
> > > > which somewhat complicates the driver structure.
> > > 
> > > this is bullcrap. Take a look at
> > > Documentation/devicetree/bindings/usb/dwc3.txt:
> > > 
> > > synopsys DWC3 CORE
> > > 
> > > DWC3- USB3 CONTROLLER
> > > 
> > > Required properties:
> > >  - compatible: must be "snps,dwc3"
> > >  - reg : Address and length of the register set for the device
> > >  - interrupts: Interrupts used by the dwc3 controller.
> > > 
> > > Optional properties:
> > >  - usb-phy : array of phandle for the PHY device.  The first element
> > >in the array is expected to be a handle to the USB2/HS PHY and
> > >the second element is expected to be a handle to the USB3/SS PHY
> > >  - phys: from the *Generic PHY* bindings
> > >  - phy-names: from the *Generic PHY* bindings
> > >  - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
> > > 
> > > This is usually a subnode to DWC3 glue to which it is connected.
> > > 
> > > dwc3@4a03 {
> > >   compatible = "snps,dwc3";
> > >   reg = <0x4a03 0xcfff>;
> > >   interrupts = <0 92 4>
> > >   usb-phy = <&usb2_phy>, <&usb3,phy>;
> > >   tx-fifo-resize;
> > > };
> > > 
> 
> > Is it possible that the glue layer needs to access core register
> > (eg, portsc.phcd during suspend)? The wrapper IP may wait some signals
> > at core IP to change.
> 
> why would a glue layer need to access registers from the core ? That
> sounds very odd. I haven't seen that and will, definitely, NACK such a
> patch :-)
> 

Just have a case for glue layer needs to know core layer's status.
The controller's wakeup logic is usually at glue layer, but we
may need to enable different wakeup for roles. Eg, if the current
role is host (controller is otg capable), we can't enable id wakeup,
otherwise, the user will complain plug out Micro-AB cable wakes up
system which is not expected.

-- 
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: gadget: function: Remove redundant usb_free_all_descriptors

2014-10-13 Thread Robert Baldyga
Hi,

On 10/10/2014 03:09 PM, pavi1729 ivap wrote:
> From: Pavitra 
> Date: Fri, 10 Oct 2014 16:05:30 +0530
> Subject: [PATCH] usb: gadget: function: Remove redundant
>  usb_free_all_descriptors
> 
> Removed the usb_free_all_descriptors call in *_bind functions
> as this call is already present in usb_assign_descriptors.
> usb_assign_descriptors is the only call where usb descriptor
> allocation happens, also in case of error freeing of the
> allocated memory takes place in the same call. Hence the
> call in the *_bind functions are redundant.
> Also the presence of usb_free_all_descriptors in the *_bind
> functions might result in double-free corruption of the
> allocated mmeory.
> 
> Signed-off-by: Pavitra 

You should use your full real name in sign-off (see
Documentation/SubmittingPatches).

I see there are no maintainers in your CC list. You should use
scripts/get_maintainer.pl on your patch.

> ---
>  drivers/usb/gadget/function/f_eem.c|  1 -
>  drivers/usb/gadget/function/f_hid.c|  7 +++
>  drivers/usb/gadget/function/f_ncm.c|  1 -
>  drivers/usb/gadget/function/f_phonet.c |  1 -
>  drivers/usb/gadget/function/f_rndis.c  |  5 +++--
>  drivers/usb/gadget/function/f_subset.c |  1 -
>  drivers/usb/gadget/function/f_uac2.c   | 10 ++
>  7 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_eem.c
> b/drivers/usb/gadget/function/f_eem.c
> index 4d8b236..c9e90de 100644
> --- a/drivers/usb/gadget/function/f_eem.c
> +++ b/drivers/usb/gadget/function/f_eem.c
> @@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
> struct usb_function *f)
>  return 0;
> 
>  fail:
> -usb_free_all_descriptors(f);
>  if (eem->port.out_ep)
>  eem->port.out_ep->driver_data = NULL;
>  if (eem->port.in_ep)
> diff --git a/drivers/usb/gadget/function/f_hid.c
> b/drivers/usb/gadget/function/f_hid.c
> index a95290a..df4f390 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -621,12 +621,14 @@ static int __init hidg_bind(struct
> usb_configuration *c, struct usb_function *f)
>  dev = MKDEV(major, hidg->minor);
>  status = cdev_add(&hidg->cdev, dev, 1);
>  if (status)
> -goto fail;
> +goto err;
> 
>  device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);
> 
>  return 0;
> 
> +err:

Maybe it would be better to use label name more consistent with the
naming convention in modified file (for example when existing label name
is "fail", you can use name "fail_free_descs" for your label).

> +usb_free_all_descriptors(f);
>  fail:
>  ERROR(f->config->cdev, "hidg_bind FAILED\n");
>  if (hidg->req != NULL) {
> @@ -635,7 +637,6 @@ fail:
>  usb_ep_free_request(hidg->in_ep, hidg->req);
>  }
> 
> -usb_free_all_descriptors(f);
>  return status;
>  }
> 
> @@ -652,8 +653,6 @@ static void hidg_unbind(struct usb_configuration
> *c, struct usb_function *f)
>  kfree(hidg->req->buf);
>  usb_ep_free_request(hidg->in_ep, hidg->req);
> 
> -usb_free_all_descriptors(f);
> -
>  kfree(hidg->report_desc);
>  kfree(hidg);
>  }
> diff --git a/drivers/usb/gadget/function/f_ncm.c
> b/drivers/usb/gadget/function/f_ncm.c
> index bcdc882..38a9279 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
> struct usb_function *f)
>  return 0;
> 
>  fail:
> -usb_free_all_descriptors(f);
>  if (ncm->notify_req) {
>  kfree(ncm->notify_req->buf);
>  usb_ep_free_request(ncm->notify, ncm->notify_req);
> diff --git a/drivers/usb/gadget/function/f_phonet.c
> b/drivers/usb/gadget/function/f_phonet.c
> index b9cfc15..74ddcac 100644
> --- a/drivers/usb/gadget/function/f_phonet.c
> +++ b/drivers/usb/gadget/function/f_phonet.c
> @@ -571,7 +571,6 @@ err_req:
>  for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
>  usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
>  err:
> -usb_free_all_descriptors(f);
>  if (fp->out_ep)
>  fp->out_ep->driver_data = NULL;
>  if (fp->in_ep)
> diff --git a/drivers/usb/gadget/function/f_rndis.c
> b/drivers/usb/gadget/function/f_rndis.c
> index ddb09dc..dae8c4b 100644
> --- a/drivers/usb/gadget/function/f_rndis.c
> +++ b/drivers/usb/gadget/function/f_rndis.c
> @@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
> usb_function *f)
>  if (rndis->manufacturer && rndis->vendorID &&
>  rndis_set_param_vendor(rndis->config, rndis->vendorID,
> rndis->manufacturer))
> -goto fail;
> +goto err;
> 
>  /* NOTE:  all that is done without knowing or caring about
>   * the network link ... which is unavailable to this code
> @@ -817,10 +817,11 @@ rndis_bind(struct usb_configuration *c, struct
> usb_function *f)
>  rndis->notify->name);
>  return