Re: question about drivers/usb/musb/musb_core.c

[Julia Lawall]

On Fri, 28 Oct 2016, Bin Liu wrote:

> On Fri, Oct 28, 2016 at 04:53:03PM -0400, Greg Kroah-Hartman wrote:
> > On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Fri, 28 Oct 2016, Julia Lawall wrote:
> > >
> > > > The file drivers/usb/musb/musb_core.c contains the code:
> > > >
> > > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > > >
> > > > Is it correct to have NULL in the third argument for an attribute that 
> > > > can
> > > > be read?  Should the permission be 0444 instead?
> > >
> > > Sorry, I got that backwards.  Should the permission be 0200?
> >
> > Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> > make it impossible to get wrong.
> >
> > Unless this file is set up this way for userspace to later change the
> > permission so that others can write to it?  I don't know the history
> > here...
>
> The musb driver history has so many unknowns... I am wondering if there
> is still anyone uses srp on musb...
>
> But I think we can change the driver to whatever we need nowadays, just
> like what we did on parameter use_dma about a year ago (51676c8d).

Thanks for the feedback.

julia
--
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: question about drivers/usb/musb/musb_core.c

[Bin Liu]

On Fri, Oct 28, 2016 at 04:53:03PM -0400, Greg Kroah-Hartman wrote:
> On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> > 
> > 
> > On Fri, 28 Oct 2016, Julia Lawall wrote:
> > 
> > > The file drivers/usb/musb/musb_core.c contains the code:
> > >
> > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > >
> > > Is it correct to have NULL in the third argument for an attribute that can
> > > be read?  Should the permission be 0444 instead?
> > 
> > Sorry, I got that backwards.  Should the permission be 0200?
> 
> Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> make it impossible to get wrong.
> 
> Unless this file is set up this way for userspace to later change the
> permission so that others can write to it?  I don't know the history
> here...

The musb driver history has so many unknowns... I am wondering if there
is still anyone uses srp on musb...

But I think we can change the driver to whatever we need nowadays, just
like what we did on parameter use_dma about a year ago (51676c8d).

> 
> thanks,
> 
> greg k-h

Regards,
-Bin.
--
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 3/3] usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode

[Bin Liu]

On Fri, Oct 28, 2016 at 12:11:21PM -0500, David Lechner wrote:
> On 10/28/2016 07:39 AM, Alexandre Bailon wrote:
> >On 10/28/2016 04:56 AM, David Lechner wrote:
> >>On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
> >>>When the phy is forced in host mode, only the first hot plug and
> >>>hot remove works. That is actually because the driver execute the
> >>>OTG workaround, whereas it is not applicable in host or device mode.
> >>>Indeed, to work correctly, the VBUS sense and session end comparator
> >>>must be enabled, what is only possible when the phy is in OTG mode.
> >>>Only execute the workaround if the phy is in OTG mode.
> >>>
> >>>Signed-off-by: Alexandre Bailon 
> >>>---
> >>> drivers/usb/musb/da8xx.c | 11 +++
> >>> 1 file changed, 11 insertions(+)
> >>>
> >>>diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> >>>index 6749aa1..b8a6b65 100644
> >>>--- a/drivers/usb/musb/da8xx.c
> >>>+++ b/drivers/usb/musb/da8xx.c
> >>>@@ -145,6 +145,17 @@ static void otg_timer(unsigned long _musb)
> >>> unsigned longflags;
> >>>
> >>> /*
> >>>+ * We should only execute the OTG workaround when the phy is in OTG
> >>>+ * mode. The workaround require the VBUS sense and the session end
> >>>+ * comparator to be enabled, what is only possible if the phy is in
> >>>+ * OTG mode. As the workaround is only required to detect if the
> >>>+ * controller must act as host or device, we can safely exit OTG is
> >>>+ * not in use.
> >>>+ */
> >>>+if (musb->port_mode != MUSB_PORT_MODE_DUAL_ROLE)
> >>
> >>musb->port_mode is not valid if we have changed the mode via sysfs. It
> >>only reflects the mode set during driver probe.
> >>
> >>Furthermore, this breaks the host mode completely for me. The first hot
> >>plug is not even detected.
> >>
> >>>+return;
> >>>+
> >>>+/*
> >>>  * We poll because DaVinci's won't expose several OTG-critical
> >>>  * status change events (from the transceiver) otherwise.
> >>>  */
> >>>
> >>
> >>
> >>The way this is working for me (on AM1808) is this:
> >>
> >>The problem is not that the OTG workaround is being used. The problem is
> >>that after disconnect, the VBUSDRV is turned off. If you look at the
> >>handler for DA8XX_INTR_DRVVBUS in da8xx_musb_interrupt(), you will see
> >>that if VBUSDRV is off, then drvvbus == 0, which puts the musb state
> >>back to device mode.
> >>
> >>I also ran into a similar problem a while back[1] that if you use a
> >>self-powered device in host mode, it immediately becomes disconnected.
> >>This is for the exact same reason. When a port detects a self-powered
> >>device, it turns of VBUSDRV, which triggers the DA8XX_INTR_DRVVBUS
> >>interrupt. As we have seen above, this takes the port out of host mode.
> >>
> >>The workaround that I have found that seems to fix both cases is to add
> >>and else if statement that toggles the PHY host override when we are
> >>forcing host mode and the VBUSDRV is turned off.
> >I like this workaround.
> >>
> >>Here is a partial diff of drivers/usb/musb/da8xx.c to show what I mean:
> >>
> >>@@ -304,10 +309,14 @@ static irqreturn_t da8xx_musb_interrupt(int irq,
> >>void *hci)
> >> * Also, DRVVBUS pulses for SRP (but not at 5 V)...
> >> */
> >>if (status & (DA8XX_INTR_DRVVBUS << DA8XX_INTR_USB_SHIFT)) {
> >>+   struct da8xx_glue *glue =
> >>+   dev_get_drvdata(musb->controller->parent);
> >>int drvvbus = musb_readl(reg_base, DA8XX_USB_STAT_REG);
> >>void __iomem *mregs = musb->mregs;
> >>u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
> >>-   int err;
> >>+   int cfgchip2, err;
> >>+
> >>+   regmap_read(glue->cfgchip, CFGCHIP(2), );
> >>
> >>err = musb->int_usb & MUSB_INTR_VBUSERROR;
> >>if (err) {
> >>@@ -332,10 +341,25 @@ static irqreturn_t da8xx_musb_interrupt(int irq,
> >>void *hci)
> >>musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> >>portstate(musb->port1_status |=
> >>USB_PORT_STAT_POWER);
> >>del_timer(_workaround);
> >>+   } else if ((cfgchip2 & CFGCHIP2_OTGMODE_MASK)
> >>+  == CFGCHIP2_OTGMODE_FORCE_HOST) {
> >>+   /*
> >>+* If we are forcing host mode, VBUSDRV is
> >>turned off
> >>+* after a device is disconnected. We need to
> >>toggle the
> >>+* VBUS/ID override to trigger turn it back on,
> >>which
> >>+* has the effect of triggering
> >>DA8XX_INTR_DRVVBUS again.
> >>+*/
> >>+   regmap_write_bits(glue->cfgchip, CFGCHIP(2),
> >>+   CFGCHIP2_OTGMODE_MASK,
> >>+   CFGCHIP2_OTGMODE_NO_OVERRIDE);
> >>+ 

[PATCH] usb: gadget: mv_u3d: add check for dma mapping error

[Alexey Khoroshilov]

mv_u3d_req_to_trb() does not check for dma mapping errors.

By the way, the patch improves readability of mv_u3d_start_queue()
by rearranging its code with two semantic modifications:
- assignment zero to ep->processing if usb_gadget_map_request() fails;
- propagation of error code from mv_u3d_req_to_trb() instead of 
  hardcoded -ENOMEM.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/mv_u3d_core.c | 34 +-
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c 
b/drivers/usb/gadget/udc/mv_u3d_core.c
index b9e19a591322..8d726bd767fd 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
req->trb_head->trb_hw,
trb_num * sizeof(*trb_hw),
DMA_BIDIRECTIONAL);
+   if (dma_mapping_error(u3d->gadget.dev.parent,
+   req->trb_head->trb_dma)) {
+   kfree(req->trb_head->trb_hw);
+   kfree(req->trb_head);
+   return -EFAULT;
+   }
 
req->chain = 1;
}
@@ -487,30 +493,32 @@ mv_u3d_start_queue(struct mv_u3d_ep *ep)
ret = usb_gadget_map_request(>gadget, >req,
mv_u3d_ep_dir(ep));
if (ret)
-   return ret;
+   goto break_processing;
 
req->req.status = -EINPROGRESS;
req->req.actual = 0;
req->trb_count = 0;
 
-   /* build trbs and push them to device queue */
-   if (!mv_u3d_req_to_trb(req)) {
-   ret = mv_u3d_queue_trb(ep, req);
-   if (ret) {
-   ep->processing = 0;
-   return ret;
-   }
-   } else {
-   ep->processing = 0;
+   /* build trbs */
+   ret = mv_u3d_req_to_trb(req);
+   if (ret) {
dev_err(u3d->dev, "%s, mv_u3d_req_to_trb fail\n", __func__);
-   return -ENOMEM;
+   goto break_processing;
}
 
+   /* and push them to device queue */
+   ret = mv_u3d_queue_trb(ep, req);
+   if (ret)
+   goto break_processing;
+
/* irq handler advances the queue */
-   if (req)
-   list_add_tail(>queue, >queue);
+   list_add_tail(>queue, >queue);
 
return 0;
+
+break_processing:
+   ep->processing = 0;
+   return ret;
 }
 
 static int mv_u3d_ep_enable(struct usb_ep *_ep,
-- 
2.7.4

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

[PATCH] USB: core: add missing license information to some files

[Greg Kroah-Hartman]

Some of the USB core files were missing explicit license information.
As all files in the kernel tree are implicitly licensed under the
GPLv2-only, be explicit in case someone get confused looking at
individual files by using the SPDX nomenclature.

Signed-off-by: Greg Kroah-Hartman 

---
 drivers/usb/core/buffer.c   |3 +++
 drivers/usb/core/config.c   |5 +
 drivers/usb/core/driver.c   |3 +++
 drivers/usb/core/endpoint.c |4 +++-
 drivers/usb/core/file.c |2 ++
 drivers/usb/core/generic.c  |2 ++
 drivers/usb/core/hub.c  |2 ++
 drivers/usb/core/message.c  |3 +++
 drivers/usb/core/notify.c   |2 ++
 drivers/usb/core/sysfs.c|2 ++
 drivers/usb/core/urb.c  |5 +
 drivers/usb/core/usb.c  |3 +++
 drivers/usb/core/usb.h  |5 +
 13 files changed, 40 insertions(+), 1 deletion(-)

--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -3,6 +3,9 @@
  *
  * This implementation plugs in through generic "usb_bus" level methods,
  * and should work with all USB controllers, regardless of bus type.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 #include 
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -1,3 +1,8 @@
+/*
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
 #include 
 #include 
 #include 
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -15,6 +15,9 @@
  * (usb_device_id matching changes by Adam J. Richter)
  * (C) Copyright Greg Kroah-Hartman 2002-2003
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ *
  * NOTE! This is not actually a driver at all, rather this is
  * just a collection of helper routines that implement the
  * matching, probing, releasing, suspending and resuming for
--- a/drivers/usb/core/endpoint.c
+++ b/drivers/usb/core/endpoint.c
@@ -5,8 +5,10 @@
  * (C) Copyright 2002,2004 IBM Corp.
  * (C) Copyright 2006 Novell Inc.
  *
- * Endpoint sysfs stuff
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  *
+ * Endpoint sysfs stuff
  */
 
 #include 
--- a/drivers/usb/core/file.c
+++ b/drivers/usb/core/file.c
@@ -13,6 +13,8 @@
  * (usb_device_id matching changes by Adam J. Richter)
  * (C) Copyright Greg Kroah-Hartman 2002-2003
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 #include 
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -15,6 +15,8 @@
  * (usb_device_id matching changes by Adam J. Richter)
  * (C) Copyright Greg Kroah-Hartman 2002-2003
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 #include 
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -6,6 +6,8 @@
  * (C) Copyright 1999 Gregory P. Smith
  * (C) Copyright 2001 Brad Hards (bha...@bigpond.net.au)
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 #include 
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1,5 +1,8 @@
 /*
  * message.c - synchronous message handling
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 #include  /* for scatterlist macros */
--- a/drivers/usb/core/notify.c
+++ b/drivers/usb/core/notify.c
@@ -6,6 +6,8 @@
  * notifier functions originally based on those in kernel/sys.c
  * but fixed up to not be so broken.
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -7,6 +7,8 @@
  *
  * All of the sysfs file attributes for usb devices and interfaces.
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
  */
 
 
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -1,3 +1,8 @@
+/*
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
 #include 
 #include 
 #include 
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -12,6 +12,9 @@
  * (usb_device_id matching changes by Adam J. Richter)
  * (C) Copyright Greg Kroah-Hartman 2002-2003
  *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ *
  * NOTE! This is not actually a driver at all, rather this is
  * just a collection of helper routines that implement the
  * generic USB things that the real drivers can use..
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -1,3 +1,8 @@
+/*
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
 #include 
 #include 
 
--
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: question about drivers/usb/musb/musb_core.c

[Julia Lawall]

On Fri, 28 Oct 2016, Greg Kroah-Hartman wrote:

> On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> >
> >
> > On Fri, 28 Oct 2016, Julia Lawall wrote:
> >
> > > The file drivers/usb/musb/musb_core.c contains the code:
> > >
> > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > >
> > > Is it correct to have NULL in the third argument for an attribute that can
> > > be read?  Should the permission be 0444 instead?
> >
> > Sorry, I got that backwards.  Should the permission be 0200?
>
> Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> make it impossible to get wrong.
>
> Unless this file is set up this way for userspace to later change the
> permission so that others can write to it?  I don't know the history
> here...

Felipe,

This line has been there since your commit 550a7375fe720 introduced the
file in 2008.  Do you recall any reason why there should be a special case
here?

julia

>
> 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: question about drivers/usb/musb/musb_core.c

[Julia Lawall]

[Corrected email for Felipe]

On Fri, 28 Oct 2016, Greg Kroah-Hartman wrote:

> On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> >
> >
> > On Fri, 28 Oct 2016, Julia Lawall wrote:
> >
> > > The file drivers/usb/musb/musb_core.c contains the code:
> > >
> > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > >
> > > Is it correct to have NULL in the third argument for an attribute that can
> > > be read?  Should the permission be 0444 instead?
> >
> > Sorry, I got that backwards.  Should the permission be 0200?
>
> Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> make it impossible to get wrong.
>
> Unless this file is set up this way for userspace to later change the
> permission so that others can write to it?  I don't know the history
> here...

Felipe,

This line has been there since your commit 550a7375fe720 introduced the
file in 2008.  Do you recall any reason why there should be a special case
here?

julia

>
> 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: question about drivers/usb/musb/musb_core.c

[Greg Kroah-Hartman]

On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> 
> 
> On Fri, 28 Oct 2016, Julia Lawall wrote:
> 
> > The file drivers/usb/musb/musb_core.c contains the code:
> >
> > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> >
> > Is it correct to have NULL in the third argument for an attribute that can
> > be read?  Should the permission be 0444 instead?
> 
> Sorry, I got that backwards.  Should the permission be 0200?

Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
make it impossible to get wrong.

Unless this file is set up this way for userspace to later change the
permission so that others can write to it?  I don't know the history
here...

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: question about drivers/usb/musb/musb_core.c

[Julia Lawall]

On Fri, 28 Oct 2016, Julia Lawall wrote:

> The file drivers/usb/musb/musb_core.c contains the code:
>
> static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
>
> Is it correct to have NULL in the third argument for an attribute that can
> be read?  Should the permission be 0444 instead?

Sorry, I got that backwards.  Should the permission be 0200?

julia

>
> thanks,
> julia
>
--
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

question about drivers/usb/musb/musb_core.c

[Julia Lawall]

The file drivers/usb/musb/musb_core.c contains the code:

static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);

Is it correct to have NULL in the third argument for an attribute that can
be read?  Should the permission be 0444 instead?

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

Re: musb RPM sleep-while-atomic in 4.9-rc1

[Tony Lindgren]

* Johan Hovold  [161028 02:45]:
> On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote:
> > * Johan Hovold  [161027 11:46]:
> > > But then this looks like it could trigger an ABBA deadlock as musb->lock
> > > is held while queue_on_resume() takes musb->list_lock, and
> > > musb_run_pending() would take the same locks in the reverse order.
> > 
> > It seems we can avoid that by locking only list_add_tail() and list_del():
> > 
> > list_for_each_entry_safe(w, _w, >resume_work, node) {
> > spin_lock_irqsave(>list_lock, flags);
> > list_del(>node);
> > spin_unlock_irqrestore(>list_lock, flags);
> > if (w->callback)
> > w->callback(musb, w->data);
> > devm_kfree(musb->controller, w);
> > }
> 
> I think you still need to hold the lock while traversing the list (even
> if you temporarily release it during the callback).

Hmm yeah we need iterate through the list again to avoid missing new
elements being added. I've updated the patch to use a the common
while (!list_empty(>resume_work)) loop. Does that solve the
concern you had or did you also had some other concern there?

Regards,

Tony

8< ---
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren 
Date: Tue, 25 Oct 2016 08:42:00 -0700
Subject: [PATCH] usb: musb: Fix sleeping function called from invalid
 context for hdrc glue

Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer
that runs in softirq context. That causes a "BUG: sleeping function called
from invalid context" every time when polling the cable status:

[] (__might_sleep) from [] (__pm_runtime_resume+0x9c/0xa0)
[] (__pm_runtime_resume) from [] (otg_timer+0x3c/0x254)
[] (otg_timer) from [] (call_timer_fn+0xfc/0x41c)
[] (call_timer_fn) from [] (expire_timers+0x120/0x210)
[] (expire_timers) from [] (run_timer_softirq+0xa4/0xdc)
[] (run_timer_softirq) from [] (__do_softirq+0x12c/0x594)

I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled.

Let's fix the issue by adding dsps_check_status() and then register a
callback with musb_runtime_resume() so it gets called only when musb core
and it's parent devices are awake. Note that we don't want to do this from
PM runtime resume in musb_dsps.c as musb core is not awake yet at that
point as noted by Johan Hovold .

Note that musb_gadget_queue() also suffers from a similar issue when
connecting the cable and cannot use pm_runtime_get_sync().

Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer")
Reported-by: Johan Hovold 
Signed-off-by: Tony Lindgren 
---
 drivers/usb/musb/musb_core.c   | 52 +-
 drivers/usb/musb/musb_core.h   |  7 ++
 drivers/usb/musb/musb_dsps.c   | 29 +--
 drivers/usb/musb/musb_gadget.c | 21 ++---
 4 files changed, 98 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1896,6 +1896,51 @@ static void musb_pm_runtime_check_session(struct musb 
*musb)
musb->session = s;
 }
 
+struct musb_resume_work {
+   void (*callback)(struct musb *musb, void *data);
+   void *data;
+   struct list_head node;
+};
+
+void musb_queue_on_resume(struct musb *musb,
+ void (*callback)(struct musb *musb, void *data),
+ void *data)
+{
+   struct musb_resume_work *w;
+   unsigned long flags;
+
+   w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC);
+   if (!w)
+   return;
+
+   w->callback = callback;
+   w->data = data;
+   spin_lock_irqsave(>list_lock, flags);
+   list_add_tail(>node, >resume_work);
+   spin_unlock_irqrestore(>list_lock, flags);
+}
+EXPORT_SYMBOL_GPL(musb_queue_on_resume);
+
+static void musb_run_pending(struct musb *musb)
+{
+   struct musb_resume_work *w;
+   unsigned long flags;
+
+   spin_lock_irqsave(>list_lock, flags);
+   while (!list_empty(>resume_work)) {
+   w = list_first_entry(>resume_work,
+struct musb_resume_work,
+node);
+   list_del(>node);
+   spin_unlock_irqrestore(>list_lock, flags);
+   if (w->callback)
+   w->callback(musb, w->data);
+   devm_kfree(musb->controller, w);
+   spin_lock_irqsave(>list_lock, flags);
+   }
+   spin_unlock_irqrestore(>list_lock, flags);
+}
+
 /* Only used to provide driver mode change events */
 static void musb_irq_work(struct work_struct *data)
 {
@@ -1969,6 +2014,7 @@ static struct musb *allocate_instance(struct device *dev,
INIT_LIST_HEAD(>control);

Re: [PATCH v2 3/3] usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode

[David Lechner]

On 10/28/2016 07:39 AM, Alexandre Bailon wrote:

On 10/28/2016 04:56 AM, David Lechner wrote:

On 10/26/2016 05:58 AM, Alexandre Bailon wrote:

When the phy is forced in host mode, only the first hot plug and
hot remove works. That is actually because the driver execute the
OTG workaround, whereas it is not applicable in host or device mode.
Indeed, to work correctly, the VBUS sense and session end comparator
must be enabled, what is only possible when the phy is in OTG mode.
Only execute the workaround if the phy is in OTG mode.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/da8xx.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 6749aa1..b8a6b65 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -145,6 +145,17 @@ static void otg_timer(unsigned long _musb)
 unsigned longflags;

 /*
+ * We should only execute the OTG workaround when the phy is in OTG
+ * mode. The workaround require the VBUS sense and the session end
+ * comparator to be enabled, what is only possible if the phy is in
+ * OTG mode. As the workaround is only required to detect if the
+ * controller must act as host or device, we can safely exit OTG is
+ * not in use.
+ */
+if (musb->port_mode != MUSB_PORT_MODE_DUAL_ROLE)


musb->port_mode is not valid if we have changed the mode via sysfs. It
only reflects the mode set during driver probe.

Furthermore, this breaks the host mode completely for me. The first hot
plug is not even detected.


+return;
+
+/*
  * We poll because DaVinci's won't expose several OTG-critical
  * status change events (from the transceiver) otherwise.
  */




The way this is working for me (on AM1808) is this:

The problem is not that the OTG workaround is being used. The problem is
that after disconnect, the VBUSDRV is turned off. If you look at the
handler for DA8XX_INTR_DRVVBUS in da8xx_musb_interrupt(), you will see
that if VBUSDRV is off, then drvvbus == 0, which puts the musb state
back to device mode.

I also ran into a similar problem a while back[1] that if you use a
self-powered device in host mode, it immediately becomes disconnected.
This is for the exact same reason. When a port detects a self-powered
device, it turns of VBUSDRV, which triggers the DA8XX_INTR_DRVVBUS
interrupt. As we have seen above, this takes the port out of host mode.

The workaround that I have found that seems to fix both cases is to add
and else if statement that toggles the PHY host override when we are
forcing host mode and the VBUSDRV is turned off.

I like this workaround.


Here is a partial diff of drivers/usb/musb/da8xx.c to show what I mean:

@@ -304,10 +309,14 @@ static irqreturn_t da8xx_musb_interrupt(int irq,
void *hci)
 * Also, DRVVBUS pulses for SRP (but not at 5 V)...
 */
if (status & (DA8XX_INTR_DRVVBUS << DA8XX_INTR_USB_SHIFT)) {
+   struct da8xx_glue *glue =
+   dev_get_drvdata(musb->controller->parent);
int drvvbus = musb_readl(reg_base, DA8XX_USB_STAT_REG);
void __iomem *mregs = musb->mregs;
u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
-   int err;
+   int cfgchip2, err;
+
+   regmap_read(glue->cfgchip, CFGCHIP(2), );

err = musb->int_usb & MUSB_INTR_VBUSERROR;
if (err) {
@@ -332,10 +341,25 @@ static irqreturn_t da8xx_musb_interrupt(int irq,
void *hci)
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |=
USB_PORT_STAT_POWER);
del_timer(_workaround);
+   } else if ((cfgchip2 & CFGCHIP2_OTGMODE_MASK)
+  == CFGCHIP2_OTGMODE_FORCE_HOST) {
+   /*
+* If we are forcing host mode, VBUSDRV is
turned off
+* after a device is disconnected. We need to
toggle the
+* VBUS/ID override to trigger turn it back on,
which
+* has the effect of triggering
DA8XX_INTR_DRVVBUS again.
+*/
+   regmap_write_bits(glue->cfgchip, CFGCHIP(2),
+   CFGCHIP2_OTGMODE_MASK,
+   CFGCHIP2_OTGMODE_NO_OVERRIDE);
+   regmap_write_bits(glue->cfgchip, CFGCHIP(2),
+   CFGCHIP2_OTGMODE_MASK,
+   CFGCHIP2_OTGMODE_FORCE_HOST);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);


I haven't thought to this workaround.
Actually, my goal with this patch was to prevent VBUSDRV to be turned
off. When I hit the issues, I captured some traces and these traces
let think VBUSDRV is turned off when the OTG 

Re: [PATCH] Revert "usb: dwc2: gadget: fix TX FIFO size and address initialization"

[John Youn]

On 10/28/2016 8:52 AM, Leo Yan wrote:
> This reverts commit aa381a7259c3f53727bcaa8c5f9359e940a0e3fd.
> 
> Reverting this patch, as it incorrectly assumes TX FIFO size is fixed
> and cannot change FIFO size; it removes all related dt binding code
> and have no chance to set FIFO size at init phase.
> 
> As result, Hi6220 cannot set correct FIFO size for gadget driver and
> ethernet on USB function driver is easily to hang. And in the kernel
> there also have other platforms set FIFO size for usb controller,
> e.g. arch/arm64/boot/dts/rockchip/rk3368.dtsi.
> 
> This patch re-enables dt binding to set FIFO size.
> 
> Signed-off-by: Leo Yan 


This is already queued for 4.9-rc in Felipe's tree.

Hi Felipe,

It seems like your fixes branch for 4.9-rc2 didn't get merged
upstream. Can we get them pulled for the next -rc? There are platforms
that are broken on 4.9 without them.

Thanks,
John
--
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 0/3] usb: musb: da8xx: Fix few issues

[David Lechner]

On 10/28/2016 04:31 AM, Alexandre Bailon wrote:

On 10/27/2016 08:44 PM, David Lechner wrote:

On 10/27/2016 12:16 PM, David Lechner wrote:

On 10/26/2016 05:58 AM, Alexandre Bailon wrote:

Currently, the USB OTG of the da8xx doesn't work.
This series intend to fix them.

Change in v2:
* Fix the error path da8xx_musb_init()

Alexandre Bailon (3):
  usb: musb: da8xx: Call earlier clk_prepare_enable()
  phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
  usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode

 drivers/phy/phy-da8xx-usb.c | 17 -
 drivers/usb/musb/da8xx.c| 28 +++-
 2 files changed, 31 insertions(+), 14 deletions(-)



I have found another problem with peripheral mode. When we force
peripheral mode, the glue layer currently uses CFGCHIP2 to override the
VBUS and ID. This causes it to not be able to detect disconnection
because the VBUS is overridden.

How have you found it ? Does it cause any issues ?
I mean I had to enable traces to see that disconnect was not happening.


I am also using the device tree patch series. I specified dr_mode = 
"peripheral" in my device tree because the device is only wired for use 
as a peripheral port.


I have actually known about this issue for a long time. See
 and 






Here is a patch to fix the problem. I have tested this on LEGO
MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
pulled up on the SoC, so we don't need to override it.

Actually, I'm wonder if that if not related to VBUS sensing.
May be we should set CFGCHIP2_VBDTCTEN in device mode.


Yes, as we discussed in another thread, we should set CFGCHIP2_VBDTCTEN 
and CFGCHIP2_SESNDEN *always* regardless of mode. I still have the 
problem described above with these two enabled if and only if we are 
also setting CFGCHIP2_OTG_FORCE_PERIPHERAL.




---

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 2bc12a2..33daa3b 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
musb_mode)
case MUSB_HOST: /* Force VBUS valid, ID = 0 */
phy_mode = PHY_MODE_USB_HOST;
break;
-   case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
-   phy_mode = PHY_MODE_USB_DEVICE;
-   break;
+   case MUSB_PERIPHERAL:
case MUSB_OTG:  /* Don't override the VBUS/ID
comparators */
phy_mode = PHY_MODE_USB_OTG;
break;

---

If this works for other SoCs/boards, I think we should make this change.
If it doesn't work, we could work around the VBUS problem by polling
VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.


I have realized that due to the way my device is wired, I can actually
use OTG mode and it will behave exactly as peripheral mode because the
ID pin is not connected. So, maybe this patch is not needed after all.


Actually, I'm sure that is related to ID


The ID pin on my device is not connected, so I don't think this has to 
do with ID. It is always high (internally pulled up).



I have the same issue except the disconnect is called when I use the
OTG mode.



This is exactly the behavior I am seeing.

--
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 v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

On Fri, Oct 28, 2016 at 08:51:41PM +0800, Baolin Wang wrote:
> On 28 October 2016 at 06:00, NeilBrown  wrote:

> > 1/ I think we agreed that it doesn't make sense for there to be
> >  two chargers registered in a system.

> Yes, until now...

> >  However usb_charger_register() still allows that, and assigns
> >  and arbitrary name to each based on discovery order.
> >  This *cannot* make sense.

> Fine, I can change that to allow only one charger to register.

Yeah, it's a reasonable change.  I'm not sure the prior discussion was
100% conclusive on the issue (I remember there being some debate about
leaving things there to avoid any need for future refactoring to touch
the interface).

> > 2/ Why do you have usb_charger_set_current()??
> >   No code ever calls it.
> >   This updates the min and max current which are defined in a
> >   standard.  It never makes sense to change the min and max
> >   for a particular cable type.

> Mark, do we have some scenarios which want to change the current
> limitation? If not, okay, I agree with you to remove this function.

I'm not aware of any, we can always add it back if the need arises.

> >  Related:  I don't like charger_type_show().  I don't think
> >  the usb-charger should export that information to user-space because
> >  extcon already does that, and duplication is confusing and pointless.

> I think we should combine all charger related information into one
> place for user. Moreover if we don't get charger type from extcon, we
> should also need one place to export the charger type.

I had also thought there was some software negotation as well as the
physical charger in cases where the device is plugged into an active
host?  I could be wrong.

> > 5/ There is no convincing example usage of this framework.
> >   wm8931x_power.c just scratches the surface.
> >   If it is so good, it should be easy to convert a lot of other
> >   drivers over to it.  If you did that it would be much easier
> >   to see how it works and what the strengths/weaknesses were.

> Jun have send out one patchset[1] based on my patchset, and he tested
> mypatchset. Thanks for your comments.
> [1]http://www.spinics.net/lists/linux-usb/msg139809.html

I think it's a good idea to pick up Jun's patches into your patch set,
that way Jun doesn't need to rebase and it might help with review of
your patches too.


signature.asc
Description: PGP signature

Re: [PATCH] Revert "usb: dwc3: gadget: use allocated/queued reqs for LST bit"

[Ville Syrjälä]

On Fri, Oct 28, 2016 at 01:16:13PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Ville Syrjälä  writes:
> > On Thu, Oct 06, 2016 at 12:08:26PM +0300, Ville Syrjälä wrote:
> >> On Thu, Oct 06, 2016 at 10:36:09AM +0300, Felipe Balbi wrote:
> >> > 
> >> > Hi,
> >> > 
> >> > Felipe Balbi  writes:
> >> 
> >> > Okay, I have found a regression on dwc3 and another patch follows:
> >> > 
> >> > commit 5e1a2af3e46248c55098cdae643c4141851b703e
> >> > Author: Felipe Balbi 
> >> > Date:   Wed Oct 5 14:24:37 2016 +0300
> >> > 
> >> > usb: dwc3: gadget: properly account queued requests
> >> > 
> >> > Some requests could be accounted for multiple
> >> > times. Let's fix that so each and every requests is
> >> > accounted for only once.
> >> > 
> >> > Cc:  # v4.8
> >> > Fixes: 55a0237f8f47 ("usb: dwc3: gadget: use allocated/queued reqs 
> >> > for LST bit")
> >> > Signed-off-by: Felipe Balbi 
> >> > 
> >> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >> > index 07cc8929f271..3c3ced128c77 100644
> >> > --- a/drivers/usb/dwc3/gadget.c
> >> > +++ b/drivers/usb/dwc3/gadget.c
> >> > @@ -783,6 +783,7 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
> >> >  req->trb = trb;
> >> >  req->trb_dma = dwc3_trb_dma_offset(dep, trb);
> >> >  req->first_trb_index = dep->trb_enqueue;
> >> > +dep->queued_requests++;
> >> >  }
> >> >  
> >> >  dwc3_ep_inc_enq(dep);
> >> > @@ -833,8 +834,6 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
> >> >  
> >> >  trb->ctrl |= DWC3_TRB_CTRL_HWO;
> >> >  
> >> > -dep->queued_requests++;
> >> > -
> >> >  trace_dwc3_prepare_trb(dep, trb);
> >> >  }
> >> >  
> >> > @@ -1861,8 +1860,11 @@ static int __dwc3_cleanup_done_trbs(struct dwc3 
> >> > *dwc, struct dwc3_ep *dep,
> >> >  unsigned ints_pkt = 0;
> >> >  unsigned inttrb_status;
> >> >  
> >> > -dep->queued_requests--;
> >> >  dwc3_ep_inc_deq(dep);
> >> > +
> >> > +if (req->trb == trb)
> >> > +dep->queued_requests--;
> >> > +
> >> >  trace_dwc3_complete_trb(dep, trb);
> >> >  
> >> >  /*
> >> > 
> >> > I have also built a branch which you can use for testing. Here's a pull
> >> > request, once you tell me it works for you, then I can send proper
> >> > patches out:
> >> > 
> >> > The following changes since commit 
> >> > c8d2bc9bc39ebea8437fd974fdbc21847bb897a3:
> >> > 
> >> >   Linux 4.8 (2016-10-02 16:24:33 -0700)
> >> > 
> >> > are available in the git repository at:
> >> > 
> >> >   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> >> > tmp-test-v4.8
> >> > 
> >> > for you to fetch changes up to c968b8d1effe64a7980802d1eef29f4e1922faca:
> >> > 
> >> >   usb: dwc3: gadget: properly account queued requests (2016-10-06 
> >> > 10:16:37 +0300)
> >> > 
> >> > 
> >> > Felipe Balbi (2):
> >> >   usb: gadget: function: u_ether: don't starve tx request queue
> >> >   usb: dwc3: gadget: properly account queued requests
> >> > 
> >> >  drivers/usb/dwc3/gadget.c | 7 ---
> >> >  drivers/usb/gadget/function/u_ether.c | 5 +++--
> >> >  2 files changed, 7 insertions(+), 5 deletions(-)
> >> 
> >> Tried your branch, but unfortunately I'm still seeing the lags. New trace
> >> attached.
> >
> > Just a reminder that the regressions is still there in 4.9-rc2.
> > Unfortunateyly with all the stuff already piled on top, getting USB
> > working on my device is no longer a simple matter of reverting the
> > one commit. I had to revert 10 patches to get even a clean revert, but
> > unfortunately that approach just lead to the transfer hanging immediately.
> >
> > So what I ended up doing is reverting all of this:
> > git log --no-merges --format=oneline 
> > 55a0237f8f47957163125e20ee9260538c5c341c^..HEAD -- drivers/usb/dwc3/ 
> > include/linux/ulpi/interface.h drivers/usb/Kconfig drivers/usb/core/Kconfig
> >
> > which comes out at whopping 70 commits. Having to carry that around
> > is going to be quite a pain especially as more stuff might be piled on
> > top.
> >
> > /me thinks a stable backport of any fix (assuming one is found
> > eventually) is going to be quite the challenge...
> 
> Yeah, I'm guessing we're gonna need some help from networking folks. The
> only thing we did since v4.7 was actually respect req->no_interrupt flag
> coming from u_ether itself. No idea why that causes so much trouble for
> u_ether.
> 
> BTW, Instead of reverting so many patches, you can just remove
> throttling:
> 
> diff --git a/drivers/usb/gadget/function/u_ether.c 
> b/drivers/usb/gadget/function/u_ether.c
> index f4a640216913..119a2e5848e8 100644
> --- 

[PATCH] Revert "usb: dwc2: gadget: fix TX FIFO size and address initialization"

[Leo Yan]

This reverts commit aa381a7259c3f53727bcaa8c5f9359e940a0e3fd.

Reverting this patch, as it incorrectly assumes TX FIFO size is fixed
and cannot change FIFO size; it removes all related dt binding code
and have no chance to set FIFO size at init phase.

As result, Hi6220 cannot set correct FIFO size for gadget driver and
ethernet on USB function driver is easily to hang. And in the kernel
there also have other platforms set FIFO size for usb controller,
e.g. arch/arm64/boot/dts/rockchip/rk3368.dtsi.

This patch re-enables dt binding to set FIFO size.

Signed-off-by: Leo Yan 
---
 drivers/usb/dwc2/core.h   |  7 +++
 drivers/usb/dwc2/gadget.c | 53 +--
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index aad4107..2a21a04 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -259,6 +259,13 @@ enum dwc2_lx_state {
DWC2_L3,/* Off state */
 };
 
+/*
+ * Gadget periodic tx fifo sizes as used by legacy driver
+ * EP0 is not included
+ */
+#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
+  768, 0, 0, 0, 0, 0, 0, 0}
+
 /* Gadget ep0 states */
 enum dwc2_ep0_state {
DWC2_EP0_SETUP,
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4cd6403..24fbebc 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -186,10 +186,9 @@ static void dwc2_hsotg_ctrl_epint(struct dwc2_hsotg *hsotg,
  */
 static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 {
-   unsigned int fifo;
+   unsigned int ep;
unsigned int addr;
int timeout;
-   u32 dptxfsizn;
u32 val;
 
/* Reset fifo map if not correctly cleared during previous session */
@@ -217,16 +216,16 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 * them to endpoints dynamically according to maxpacket size value of
 * given endpoint.
 */
-   for (fifo = 1; fifo < MAX_EPS_CHANNELS; fifo++) {
-   dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(fifo));
-
-   val = (dptxfsizn & FIFOSIZE_DEPTH_MASK) | addr;
-   addr += dptxfsizn >> FIFOSIZE_DEPTH_SHIFT;
-
-   if (addr > hsotg->fifo_mem)
-   break;
+   for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
+   if (!hsotg->g_tx_fifo_sz[ep])
+   continue;
+   val = addr;
+   val |= hsotg->g_tx_fifo_sz[ep] << FIFOSIZE_DEPTH_SHIFT;
+   WARN_ONCE(addr + hsotg->g_tx_fifo_sz[ep] > hsotg->fifo_mem,
+ "insufficient fifo memory");
+   addr += hsotg->g_tx_fifo_sz[ep];
 
-   dwc2_writel(val, hsotg->regs + DPTXFSIZN(fifo));
+   dwc2_writel(val, hsotg->regs + DPTXFSIZN(ep));
}
 
/*
@@ -3807,10 +3806,36 @@ static void dwc2_hsotg_dump(struct dwc2_hsotg *hsotg)
 static void dwc2_hsotg_of_probe(struct dwc2_hsotg *hsotg)
 {
struct device_node *np = hsotg->dev->of_node;
+   u32 len = 0;
+   u32 i = 0;
 
/* Enable dma if requested in device tree */
hsotg->g_using_dma = of_property_read_bool(np, "g-use-dma");
 
+   /*
+   * Register TX periodic fifo size per endpoint.
+   * EP0 is excluded since it has no fifo configuration.
+   */
+   if (!of_find_property(np, "g-tx-fifo-size", ))
+   goto rx_fifo;
+
+   len /= sizeof(u32);
+
+   /* Read tx fifo sizes other than ep0 */
+   if (of_property_read_u32_array(np, "g-tx-fifo-size",
+   >g_tx_fifo_sz[1], len))
+   goto rx_fifo;
+
+   /* Add ep0 */
+   len++;
+
+   /* Make remaining TX fifos unavailable */
+   if (len < MAX_EPS_CHANNELS) {
+   for (i = len; i < MAX_EPS_CHANNELS; i++)
+   hsotg->g_tx_fifo_sz[i] = 0;
+   }
+
+rx_fifo:
/* Register RX fifo size */
of_property_read_u32(np, "g-rx-fifo-size", >g_rx_fifo_sz);
 
@@ -3832,10 +3857,13 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
struct device *dev = hsotg->dev;
int epnum;
int ret;
+   int i;
+   u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE;
 
/* Initialize to legacy fifo configuration values */
hsotg->g_rx_fifo_sz = 2048;
hsotg->g_np_g_tx_fifo_sz = 1024;
+   memcpy(>g_tx_fifo_sz[1], p_tx_fifo, sizeof(p_tx_fifo));
/* Device tree specific probe */
dwc2_hsotg_of_probe(hsotg);
 
@@ -3853,6 +3881,9 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
dev_dbg(dev, "NonPeriodic TXFIFO size: %d\n",
hsotg->g_np_g_tx_fifo_sz);
dev_dbg(dev, "RXFIFO size: %d\n", hsotg->g_rx_fifo_sz);
+   for (i = 0; i < MAX_EPS_CHANNELS; i++)
+   

Schnelle Darlehen

[Globale Finanzdienstleistungen]

Brauchen Sie einen Kredit? Wenn ja, mailen Sie uns für weitere Informationen.
--
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 0/3] usb: trivial cleanups with list_first_entry_or_null()

[Masahiro Yamada]

Hello Felipe,

If this series looks good, can you pick it up please?

Thanks,

2016-09-19 1:03 GMT+09:00 Masahiro Yamada :
> Replace the chain of list_empty() and list_first_entry()
> with list_first_entry_or_null().
>
> Changes in v2:
>  - Split into per-driver patches
>
>
> Masahiro Yamada (3):
>   usb: dwc2: cleanup with list_first_entry_or_null()
>   usb: dwc3: cleanup with list_first_entry_or_null()
>   usb: renesas_usbhs: cleanup with list_first_entry_or_null()
>
>  drivers/usb/dwc2/gadget.c| 6 ++
>  drivers/usb/dwc3/gadget.h| 5 +
>  drivers/usb/renesas_usbhs/fifo.c | 5 +
>  3 files changed, 4 insertions(+), 12 deletions(-)
>
> --
> 1.9.1
>



-- 
Best Regards
Masahiro Yamada
--
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: [alsa-devel] Jack sensing in snd_usb_audio ?

[Bastien Nocera]

On Tue, 2016-10-18 at 14:07 +0200, Takashi Iwai wrote:
> On Wed, 12 Oct 2016 18:15:04 +0200,
> Bastien Nocera wrote:
> > 
> > On Wed, 2016-10-12 at 18:06 +0200, Clemens Ladisch wrote:
> > > Bastien Nocera wrote:
> > > > On Wed, 2016-10-12 at 14:43 +0200, Clemens Ladisch wrote:
> > > > > Bastien Nocera wrote:
> > > > > > "
> > > > > > A change of state in the audio function is most often
> > > > > > caused by
> > > > > > a
> > > > > > certain event that takes place. An event can either be
> > > > > > user-
> > > > > > initiated
> > > > > > or device-initiated. User-initiated jack insertion or
> > > > > > removal
> > > > > > is a
> > > > > > typical example of a user-initiated event.
> > > > > > "
> > > > > 
> > > > > 
> > > > > There are not many USB Audio 2.0 devices, and I'm not aware
> > > > > of
> > > > > any
> > > > > that actually implements this.
> > > > 
> > > > 
> > > > I guess I would see whether there are events if I captured the
> > > > USB
> > > > traffic even without special handling/turning on a feature in
> > > > the
> > > > drivers, right?
> > > 
> > > 
> > > Most devices do not even have the status endpoint (see "lsusb
> > > -v").
> > > To check what events arrive, you can add logging to the
> > > snd_usb_mixer_interrupt() function.
> > 
> > I'm guessing it doesn't support it then (see attached log)
> 
> So this looks like a HID, not from the audio device class.
> It's an oft-seen implementation.
> 
> > I also checked the input device output when plugging in something,
> > with
> > evtest, and no feedback either.
> 
> Then at first you need to hack a HID driver to support this device.
> It'll create an input device, and then we'll need to find some way to
> couple the given input device and the audio device.  We can parse the
> sysfs device path to figure out, but I'm not sure what's the best way
> to tell it to applications.

You misunderstood. There's no input events on the input device, there's
also no hidraw events (hid-recorder didn't see any events) and using
usbmon also got me no USB events whatsoever when plugging or unplugging
a jack on either the headphones or the microphone jack.

So there's really nothing that we can do for this hardware. Shame, it
would have been pretty useful to me :)

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

Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

Hi,

On 28 October 2016 at 06:00, NeilBrown  wrote:
> On Thu, Oct 27 2016, Baolin Wang wrote:
>
>> Hi Felipe,
>>
>> On 19 October 2016 at 10:37, Baolin Wang  wrote:
>>> Currently the Linux kernel does not provide any standard integration of this
>>> feature that integrates the USB subsystem with the system power regulation
>>> provided by PMICs meaning that either vendors must add this in their kernels
>>> or USB gadget devices based on Linux (such as mobile phones) may not behave
>>> as they should. Thus provide a standard framework for doing this in kernel.
>>>
>>> Now introduce one user with wm831x_power to support and test the usb 
>>> charger,
>>> which is pending testing. Moreover there may be other potential users will 
>>> use
>>> it in future.
>>>
>>> Changes since v17:
>>>  - Remove goto section in usb_charger_register() function.
>>>  - Remove 'extern' in charger.h file.
>>>  - Move the kfree() to usb_charger_exit() function.
>>>
>>> Changes since v16:
>>>  - Modify the charger current range with introducing the maximum and minimum
>>>  current.
>>>  - Remove the getting charger type method from power supply.
>>>  - Add the getting charger type method from extcon system.
>>>  - Introduce new usb_charger_get_current() API for users to get the maximum 
>>> and
>>>  minimum current.
>>>  - Rename some APIs and other optimization.
>>>
>>> Changes since v15:
>>>  - Add charger state checking to avoid sending out duplicate notifies to 
>>> users.
>>>  - Add one work to notify power users the current has been changed.
>>>
>>> Changes since v14:
>>>  - Add kernel documentation for struct usb_cahrger.
>>>  - Remove some redundant WARN() functions.
>>>
>>> Changes since v13:
>>>  - Remove the charger checking in usb_gadget_vbus_draw() function.
>>>  - Rename some functions in charger.c file.
>>>  - Rebase on git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
>>> tags/usb-for-v4.8
>>>
>>> Changes since v12:
>>>  - Remove the class and device things.
>>>  - Link usb charger to udc-core.ko.
>>>  - Create one "charger" subdirectory which holds all charger-related 
>>> attributes.
>>>
>>> Changes since v11:
>>>  - Reviewed and tested by Li Jun.
>>>
>>> Changes since v10:
>>>  - Introduce usb_charger_get_state() function to check charger state.
>>>  - Remove the mutex lock in usb_charger_set_cur_limit_by_type() function
>>>  in case will be issued in atomic context.
>>
>> Could you apply this patchset into your branch if there are no other
>> comments? Thanks.
>
> Some of my previous comments are still outstanding.  You seem to have
> just brushed them off without apparently understanding.

I am very appreciate for your comments, and I've explained your
comments but you did not reply me..

> And no-one else seems to care enough to try to bridge the gap...
>
> Let me try again.
>
> 1/ I think we agreed that it doesn't make sense for there to be
>  two chargers registered in a system.

Yes, until now...

>  However usb_charger_register() still allows that, and assigns
>  and arbitrary name to each based on discovery order.
>  This *cannot* make sense.

Fine, I can change that to allow only one charger to register.

>
> 2/ Why do you have usb_charger_set_current()??
>   No code ever calls it.
>   This updates the min and max current which are defined in a
>   standard.  It never makes sense to change the min and max
>   for a particular cable type.

Mark, do we have some scenarios which want to change the current
limitation? If not, okay, I agree with you to remove this function.

>
> 3/ usb_charger_notify_state() does nothing if the state doesn't change.
>   When the extcon detects an SDP, it will be called to set the state
>   to USB_CHARGER_PRESENT.  The value of cur.sdp_max will be whatever
>   it happened to be before, which is probably wrong.

Sorry, I did not get your points here, could you please explain it explicitly?

>   When after USB negotiation completes,
>   usb_charger_set_cur_limit_by_gadget()
>   will call into usb_charger_notify_state() to set USB_CHARGER_PRESENT
>   again, but with a new current.  This will be ignored, as the state is
>   already USB_CHARGER_PRESENT.

No, we will notify the user the current has been changed by one work.

>
>  (as an aside
>  +enum usb_charger_state {
>  +  USB_CHARGER_DEFAULT,
>  +  USB_CHARGER_PRESENT,
>  +  USB_CHARGER_REMOVE,
>  +};
>
>  looks odd.  It should probably by
> USB_CHARGER_UNKNOWN
> USB_CHARGER_PRESENT
> USB_CHARGER_ABSENT
>
>  "REMOVE" isn't a state.  "REMOVED" might be.
>  )

Sure.

>
> 4/ I still strongly object to the ->get_charger_type() interface.
>  You previously said:
>
>  No. User can implement the get_charger_type() method to access the
>  PMIC registers to get the charger type, which is one very common method.
>
>  I suggest that if the PMIC registers report the charger type, then the
>  PMIC driver should register an EXTCON and report the charger 

Re: [PATCH v2 3/3] usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode

[Alexandre Bailon]

On 10/28/2016 04:56 AM, David Lechner wrote:
> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
>> When the phy is forced in host mode, only the first hot plug and
>> hot remove works. That is actually because the driver execute the
>> OTG workaround, whereas it is not applicable in host or device mode.
>> Indeed, to work correctly, the VBUS sense and session end comparator
>> must be enabled, what is only possible when the phy is in OTG mode.
>> Only execute the workaround if the phy is in OTG mode.
>>
>> Signed-off-by: Alexandre Bailon 
>> ---
>>  drivers/usb/musb/da8xx.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>> index 6749aa1..b8a6b65 100644
>> --- a/drivers/usb/musb/da8xx.c
>> +++ b/drivers/usb/musb/da8xx.c
>> @@ -145,6 +145,17 @@ static void otg_timer(unsigned long _musb)
>>  unsigned longflags;
>>
>>  /*
>> + * We should only execute the OTG workaround when the phy is in OTG
>> + * mode. The workaround require the VBUS sense and the session end
>> + * comparator to be enabled, what is only possible if the phy is in
>> + * OTG mode. As the workaround is only required to detect if the
>> + * controller must act as host or device, we can safely exit OTG is
>> + * not in use.
>> + */
>> +if (musb->port_mode != MUSB_PORT_MODE_DUAL_ROLE)
> 
> musb->port_mode is not valid if we have changed the mode via sysfs. It
> only reflects the mode set during driver probe.
> 
> Furthermore, this breaks the host mode completely for me. The first hot
> plug is not even detected.
> 
>> +return;
>> +
>> +/*
>>   * We poll because DaVinci's won't expose several OTG-critical
>>   * status change events (from the transceiver) otherwise.
>>   */
>>
> 
> 
> The way this is working for me (on AM1808) is this:
> 
> The problem is not that the OTG workaround is being used. The problem is
> that after disconnect, the VBUSDRV is turned off. If you look at the
> handler for DA8XX_INTR_DRVVBUS in da8xx_musb_interrupt(), you will see
> that if VBUSDRV is off, then drvvbus == 0, which puts the musb state
> back to device mode.
> 
> I also ran into a similar problem a while back[1] that if you use a
> self-powered device in host mode, it immediately becomes disconnected.
> This is for the exact same reason. When a port detects a self-powered
> device, it turns of VBUSDRV, which triggers the DA8XX_INTR_DRVVBUS
> interrupt. As we have seen above, this takes the port out of host mode.
> 
> The workaround that I have found that seems to fix both cases is to add
> and else if statement that toggles the PHY host override when we are
> forcing host mode and the VBUSDRV is turned off.
I like this workaround.
> 
> Here is a partial diff of drivers/usb/musb/da8xx.c to show what I mean:
> 
> @@ -304,10 +309,14 @@ static irqreturn_t da8xx_musb_interrupt(int irq,
> void *hci)
>  * Also, DRVVBUS pulses for SRP (but not at 5 V)...
>  */
> if (status & (DA8XX_INTR_DRVVBUS << DA8XX_INTR_USB_SHIFT)) {
> +   struct da8xx_glue *glue =
> +   dev_get_drvdata(musb->controller->parent);
> int drvvbus = musb_readl(reg_base, DA8XX_USB_STAT_REG);
> void __iomem *mregs = musb->mregs;
> u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
> -   int err;
> +   int cfgchip2, err;
> +
> +   regmap_read(glue->cfgchip, CFGCHIP(2), );
> 
> err = musb->int_usb & MUSB_INTR_VBUSERROR;
> if (err) {
> @@ -332,10 +341,25 @@ static irqreturn_t da8xx_musb_interrupt(int irq,
> void *hci)
> musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> portstate(musb->port1_status |=
> USB_PORT_STAT_POWER);
> del_timer(_workaround);
> +   } else if ((cfgchip2 & CFGCHIP2_OTGMODE_MASK)
> +  == CFGCHIP2_OTGMODE_FORCE_HOST) {
> +   /*
> +* If we are forcing host mode, VBUSDRV is
> turned off
> +* after a device is disconnected. We need to
> toggle the
> +* VBUS/ID override to trigger turn it back on,
> which
> +* has the effect of triggering
> DA8XX_INTR_DRVVBUS again.
> +*/
> +   regmap_write_bits(glue->cfgchip, CFGCHIP(2),
> +   CFGCHIP2_OTGMODE_MASK,
> +   CFGCHIP2_OTGMODE_NO_OVERRIDE);
> +   regmap_write_bits(glue->cfgchip, CFGCHIP(2),
> +   CFGCHIP2_OTGMODE_MASK,
> +   CFGCHIP2_OTGMODE_FORCE_HOST);
> } else {
> musb->is_active = 0;
> MUSB_DEV_MODE(musb);
> 
I haven't thought to this 

Re: [PATCH 3/6] Documentation: devicetree: dwc3: Add interrupt moderation

[Mark Rutland]

On Fri, Oct 28, 2016 at 01:30:07PM +0300, Felipe Balbi wrote:
> Mark Rutland  writes:
> > On Thu, Oct 27, 2016 at 02:08:25PM -0700, John Youn wrote:
> >> On 10/26/2016 3:57 AM, Mark Rutland wrote:
> >> > On Tue, Oct 25, 2016 at 12:42:46PM -0700, John Youn wrote:
> >> >> Add interrupt moderation interval binding for dwc3.
> >
> >> >> + - snps,imod_interval: the interrupt moderation interval.

> >> This series implements the feature and enables it as a workaround for
> >> a particular version of the controller.
> >
> > ... as a workaround for *what*? Is there a bug in that IP version, or an
> 
> you didn't receive the entire series, I guess. Here's last patch in the
> series:

No, I did not. Thanks for forwarding this.

>  This is a workaround for STAR 9000961433 which affects only version
>  3.00a of the DWC_usb3 core. This prevents the controller interrupt from
>  being masked while handling events. Enabling interrupt moderation allows
>  us to work around this issue because once the GEVNTCOUNT.count is
>  written the IRQ is immediately deasserted and won't be asserted again
>  until GEVNTCOUNT.EHB is cleared.
> 
>  Signed-off-by: John Youn 
>  ---
>   drivers/usb/dwc3/core.c | 12 
>   1 file changed, 12 insertions(+)
> 
>  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>  index 6733838..7fa0832 100644
>  --- a/drivers/usb/dwc3/core.c
>  +++ b/drivers/usb/dwc3/core.c
>  @@ -1050,6 +1050,18 @@ static void dwc3_check_params(struct dwc3 *dwc)
>dwc->imod_interval = 0;
>}
> 
>  +/*
>  + * Workaround for STAR 9000961433 which affects only version
>  + * 3.00a of the DWC_usb3 core. This prevents the controller
>  + * interrupt from being masked while handling events. IMOD
>  + * allows us to work around this issue. Enable it for the
>  + * affected version.
>  + */
>  +if (!dwc->imod_interval &&
>  +(dwc->revision == DWC3_REVISION_300A)) {
>  +dwc->imod_interval = 1;
>  +}
>  +
>/* Check the maximum_speed parameter */
>switch (dwc->maximum_speed) {
>case USB_SPEED_LOW:
> 
> > integration issue? Does the problem vary per-board?
> >
> > Generally, if there's a problem that needs to be worked around, we
> > describe the problem in the DT (perhaps implicitly in the compatible
> > string), and then the kernel chooses the workaround.
> 
> Regardless of the silicon erratum, interrupt moderation is a *feature*
> of the IP, common to all instances since revision v3.00a (IIRC). John is
> just using interrupt moderation in the context of implementing this
> workaround. But the actual feature is valid also without the erratum.

Sure, I understand this.

> Another thing to remember is that different applications (i.e. boards)
> might want to moderate the interrupt for different periods. That's,
> again, not related to the erratum at all.

... again, the question is *why*?

If this varies per use-case, then it would be better to handle this
dynamically -- people can run wildly different use-cases on the same
hardware.

I'm not sure that it makes sense for this to be in the DT, though I may
have misunderstood.

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

Re: [PATCH v7, 0/8] Add MediaTek USB3 DRD Driver

[Matthias Brugger]

Hi Chunfeng,

On 10/19/2016 04:28 AM, Chunfeng Yun wrote:

These patches introduce the MediaTek USB3 dual-role controller
driver.

The driver can be configured as Dual-Role Device (DRD),
Peripheral Only and Host Only (xHCI) modes. It works well
with Mass Storage, RNDIS and g_zero on FS/HS and SS. And it is
tested on MT8173 platform which only contains USB2.0 device IP,
and on MT6290 platform which contains USB3.0 device IP.

Change in v7:
1. split dual-role driver into four patchs
2. remove QMU done tasklet
3. add a bool in xhci_hcd_mtk to signal absence of IPPC

Change in v6:
1. handle endianness of GPD and SETUP data
2. remove dummy error log and return suitable error number
3. cancel delay work when deregiseter driver

Change in v5:
1. modify some comments
2. rename some unsuitable variables
3. add reg-names property for host node
4. add USB_MTU3_DEBUG to control debug messages

Change in v4:
1. fix build errors on non-mediatek platforms
2. provide manual dual-role switch via debugfs instead of sysfs

Change in v3:
1. fix some typo error
2. rename mtu3.txt to mt8173-mtu3.txt

Change in v2:
1. modify binding docs according to suggestions
2. modify some comments and remove some dummy blank lines
3. fix memory leakage


Chunfeng Yun (8):
  dt-bindings: mt8173-xhci: support host side of dual-role mode
  dt-bindings: mt8173-mtu3: add devicetree bindings
  usb: xhci-mtk: make IPPC register optional
  usb: Add MediaTek USB3 DRD driver
  usb: mtu3: Super-Speed Peripheral mode support
  usb: mtu3: host only mode support
  usb: mtu3: dual-role mode support
  arm64: dts: mediatek: add USB3 DRD driver



I tried the driver with my mt8173-evb, but wasn't able to get USB 
working (no usb stick detected when adding to the usb port).


# dmesg |grep mtu
[0.428420] mtu3 11271000.usb: failed to get vusb33
[0.510570] mtu3 11271000.usb: failed to get vbus
[0.592103] mtu3 11271000.usb: failed to get vbus


Relevant config options:
CONFIG_USB_MTU3=y
CONFIG_USB_MTU3_HOST=y
CONFIG_USB_MTU3_DEBUG=y
CONFIG_PHY_MT65XX_USB3=y


Looks like an error in the device tree. I can see that the mt6397 
regulater get's initialized *after* the mtu3 driver:

[0.505166] mt6397-regulator mt6397-regulator: Chip ID = 0x4097

Not sure if this is related.
Any idea whats going wrong here?

Cheers,
Matthias
--
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 3/6] Documentation: devicetree: dwc3: Add interrupt moderation

[Felipe Balbi]

Hi,

Mark Rutland  writes:
> On Thu, Oct 27, 2016 at 02:08:25PM -0700, John Youn wrote:
>> On 10/26/2016 3:57 AM, Mark Rutland wrote:
>> > On Tue, Oct 25, 2016 at 12:42:46PM -0700, John Youn wrote:
>> >> Add interrupt moderation interval binding for dwc3.
>
>> >> + - snps,imod_interval: the interrupt moderation interval.
>
>> > What is "interrupt moderation"? The cover mentions that this is to be
>> > used for some kind of workaround, but it's not clear to me what this is,
>> > and as such, whether it makes sense to describe it in this manner.
>> > 
>> 
>> Interrupt moderation throttles the interrupt rate to be no faster than
>> a specified interval. It's an optional feature of the controller.
>
> Ok.
>
>> This series implements the feature and enables it as a workaround for
>> a particular version of the controller.
>
> ... as a workaround for *what*? Is there a bug in that IP version, or an

you didn't receive the entire series, I guess. Here's last patch in the
series:

 This is a workaround for STAR 9000961433 which affects only version
 3.00a of the DWC_usb3 core. This prevents the controller interrupt from
 being masked while handling events. Enabling interrupt moderation allows
 us to work around this issue because once the GEVNTCOUNT.count is
 written the IRQ is immediately deasserted and won't be asserted again
 until GEVNTCOUNT.EHB is cleared.

 Signed-off-by: John Youn 
 ---
  drivers/usb/dwc3/core.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index 6733838..7fa0832 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -1050,6 +1050,18 @@ static void dwc3_check_params(struct dwc3 *dwc)
 dwc->imod_interval = 0;
 }

 +  /*
 +   * Workaround for STAR 9000961433 which affects only version
 +   * 3.00a of the DWC_usb3 core. This prevents the controller
 +   * interrupt from being masked while handling events. IMOD
 +   * allows us to work around this issue. Enable it for the
 +   * affected version.
 +   */
 +  if (!dwc->imod_interval &&
 +  (dwc->revision == DWC3_REVISION_300A)) {
 +  dwc->imod_interval = 1;
 +  }
 +
 /* Check the maximum_speed parameter */
 switch (dwc->maximum_speed) {
 case USB_SPEED_LOW:

> integration issue? Does the problem vary per-board?
>
> Generally, if there's a problem that needs to be worked around, we
> describe the problem in the DT (perhaps implicitly in the compatible
> string), and then the kernel chooses the workaround.

Regardless of the silicon erratum, interrupt moderation is a *feature*
of the IP, common to all instances since revision v3.00a (IIRC). John is
just using interrupt moderation in the context of implementing this
workaround. But the actual feature is valid also without the erratum.

Another thing to remember is that different applications (i.e. boards)
might want to moderate the interrupt for different periods. That's,
again, not related to the erratum at all.

We cannot use compatible to figure this one out. dwc3 does as much
runtime discovery as possible, but we cannot discover what is the
desired interrupt moderation interval for $this setup. It needs to be
passed in on a board-by-board basis.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 3/6] Documentation: devicetree: dwc3: Add interrupt moderation

[Mark Rutland]

On Thu, Oct 27, 2016 at 02:08:25PM -0700, John Youn wrote:
> On 10/26/2016 3:57 AM, Mark Rutland wrote:
> > On Tue, Oct 25, 2016 at 12:42:46PM -0700, John Youn wrote:
> >> Add interrupt moderation interval binding for dwc3.

> >> + - snps,imod_interval: the interrupt moderation interval.

> > What is "interrupt moderation"? The cover mentions that this is to be
> > used for some kind of workaround, but it's not clear to me what this is,
> > and as such, whether it makes sense to describe it in this manner.
> > 
> 
> Interrupt moderation throttles the interrupt rate to be no faster than
> a specified interval. It's an optional feature of the controller.

Ok.

> This series implements the feature and enables it as a workaround for
> a particular version of the controller.

... as a workaround for *what*? Is there a bug in that IP version, or an
integration issue? Does the problem vary per-board?

Generally, if there's a problem that needs to be worked around, we
describe the problem in the DT (perhaps implicitly in the compatible
string), and then the kernel chooses the workaround.

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

Re: [PATCH] Revert "usb: dwc3: gadget: use allocated/queued reqs for LST bit"

[Felipe Balbi]

Hi,

Ville Syrjälä  writes:
> On Thu, Oct 06, 2016 at 12:08:26PM +0300, Ville Syrjälä wrote:
>> On Thu, Oct 06, 2016 at 10:36:09AM +0300, Felipe Balbi wrote:
>> > 
>> > Hi,
>> > 
>> > Felipe Balbi  writes:
>> 
>> > Okay, I have found a regression on dwc3 and another patch follows:
>> > 
>> > commit 5e1a2af3e46248c55098cdae643c4141851b703e
>> > Author: Felipe Balbi 
>> > Date:   Wed Oct 5 14:24:37 2016 +0300
>> > 
>> > usb: dwc3: gadget: properly account queued requests
>> > 
>> > Some requests could be accounted for multiple
>> > times. Let's fix that so each and every requests is
>> > accounted for only once.
>> > 
>> > Cc:  # v4.8
>> > Fixes: 55a0237f8f47 ("usb: dwc3: gadget: use allocated/queued reqs for 
>> > LST bit")
>> > Signed-off-by: Felipe Balbi 
>> > 
>> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> > index 07cc8929f271..3c3ced128c77 100644
>> > --- a/drivers/usb/dwc3/gadget.c
>> > +++ b/drivers/usb/dwc3/gadget.c
>> > @@ -783,6 +783,7 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
>> >req->trb = trb;
>> >req->trb_dma = dwc3_trb_dma_offset(dep, trb);
>> >req->first_trb_index = dep->trb_enqueue;
>> > +  dep->queued_requests++;
>> >}
>> >  
>> >dwc3_ep_inc_enq(dep);
>> > @@ -833,8 +834,6 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
>> >  
>> >trb->ctrl |= DWC3_TRB_CTRL_HWO;
>> >  
>> > -  dep->queued_requests++;
>> > -
>> >trace_dwc3_prepare_trb(dep, trb);
>> >  }
>> >  
>> > @@ -1861,8 +1860,11 @@ static int __dwc3_cleanup_done_trbs(struct dwc3 
>> > *dwc, struct dwc3_ep *dep,
>> >unsigned ints_pkt = 0;
>> >unsigned inttrb_status;
>> >  
>> > -  dep->queued_requests--;
>> >dwc3_ep_inc_deq(dep);
>> > +
>> > +  if (req->trb == trb)
>> > +  dep->queued_requests--;
>> > +
>> >trace_dwc3_complete_trb(dep, trb);
>> >  
>> >/*
>> > 
>> > I have also built a branch which you can use for testing. Here's a pull
>> > request, once you tell me it works for you, then I can send proper
>> > patches out:
>> > 
>> > The following changes since commit 
>> > c8d2bc9bc39ebea8437fd974fdbc21847bb897a3:
>> > 
>> >   Linux 4.8 (2016-10-02 16:24:33 -0700)
>> > 
>> > are available in the git repository at:
>> > 
>> >   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tmp-test-v4.8
>> > 
>> > for you to fetch changes up to c968b8d1effe64a7980802d1eef29f4e1922faca:
>> > 
>> >   usb: dwc3: gadget: properly account queued requests (2016-10-06 10:16:37 
>> > +0300)
>> > 
>> > 
>> > Felipe Balbi (2):
>> >   usb: gadget: function: u_ether: don't starve tx request queue
>> >   usb: dwc3: gadget: properly account queued requests
>> > 
>> >  drivers/usb/dwc3/gadget.c | 7 ---
>> >  drivers/usb/gadget/function/u_ether.c | 5 +++--
>> >  2 files changed, 7 insertions(+), 5 deletions(-)
>> 
>> Tried your branch, but unfortunately I'm still seeing the lags. New trace
>> attached.
>
> Just a reminder that the regressions is still there in 4.9-rc2.
> Unfortunateyly with all the stuff already piled on top, getting USB
> working on my device is no longer a simple matter of reverting the
> one commit. I had to revert 10 patches to get even a clean revert, but
> unfortunately that approach just lead to the transfer hanging immediately.
>
> So what I ended up doing is reverting all of this:
> git log --no-merges --format=oneline 
> 55a0237f8f47957163125e20ee9260538c5c341c^..HEAD -- drivers/usb/dwc3/ 
> include/linux/ulpi/interface.h drivers/usb/Kconfig drivers/usb/core/Kconfig
>
> which comes out at whopping 70 commits. Having to carry that around
> is going to be quite a pain especially as more stuff might be piled on
> top.
>
> /me thinks a stable backport of any fix (assuming one is found
> eventually) is going to be quite the challenge...

Yeah, I'm guessing we're gonna need some help from networking folks. The
only thing we did since v4.7 was actually respect req->no_interrupt flag
coming from u_ether itself. No idea why that causes so much trouble for
u_ether.

BTW, Instead of reverting so many patches, you can just remove
throttling:

diff --git a/drivers/usb/gadget/function/u_ether.c 
b/drivers/usb/gadget/function/u_ether.c
index f4a640216913..119a2e5848e8 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 
req->length = length;
 
-   /* throttle high/super speed IRQ rate back slightly */
-   if (gadget_is_dualspeed(dev->gadget))
-   req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH ||
-  dev->gadget->speed == 

Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues

[Alexandre Bailon]

On 10/28/2016 11:31 AM, Alexandre Bailon wrote:
> On 10/27/2016 08:44 PM, David Lechner wrote:
>> On 10/27/2016 12:16 PM, David Lechner wrote:
>>> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
 Currently, the USB OTG of the da8xx doesn't work.
 This series intend to fix them.

 Change in v2:
 * Fix the error path da8xx_musb_init()

 Alexandre Bailon (3):
   usb: musb: da8xx: Call earlier clk_prepare_enable()
   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode

  drivers/phy/phy-da8xx-usb.c | 17 -
  drivers/usb/musb/da8xx.c| 28 +++-
  2 files changed, 31 insertions(+), 14 deletions(-)

>>>
>>> I have found another problem with peripheral mode. When we force
>>> peripheral mode, the glue layer currently uses CFGCHIP2 to override the
>>> VBUS and ID. This causes it to not be able to detect disconnection
>>> because the VBUS is overridden.
> How have you found it ? Does it cause any issues ?
> I mean I had to enable traces to see that disconnect was not happening.
>>>
>>> Here is a patch to fix the problem. I have tested this on LEGO
>>> MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
>>> pulled up on the SoC, so we don't need to override it.
> Actually, I'm wonder if that if not related to VBUS sensing.
> May be we should set CFGCHIP2_VBDTCTEN in device mode.
>>>
>>> ---
>>>
>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>>> index 2bc12a2..33daa3b 100644
>>> --- a/drivers/usb/musb/da8xx.c
>>> +++ b/drivers/usb/musb/da8xx.c
>>> @@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
>>> musb_mode)
>>> case MUSB_HOST: /* Force VBUS valid, ID = 0 */
>>> phy_mode = PHY_MODE_USB_HOST;
>>> break;
>>> -   case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
>>> -   phy_mode = PHY_MODE_USB_DEVICE;
>>> -   break;
>>> +   case MUSB_PERIPHERAL:
>>> case MUSB_OTG:  /* Don't override the VBUS/ID
>>> comparators */
>>> phy_mode = PHY_MODE_USB_OTG;
>>> break;
>>>
>>> ---
>>>
>>> If this works for other SoCs/boards, I think we should make this change.
I will try it but for me, it should be good.
It is actually similar to what is done in TI BSP.
>>> If it doesn't work, we could work around the VBUS problem by polling
>>> VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.
Actually, I don't think we need to do any polling.
We can just catch the suspend interrupt and use VBUSSENSE to
differentiate a real suspend from a disconnect.
>>
>> I have realized that due to the way my device is wired, I can actually
>> use OTG mode and it will behave exactly as peripheral mode because the
>> ID pin is not connected. So, maybe this patch is not needed after all.
>>
> Actually, I'm sure that is related to ID
> I have the same issue except the disconnect is called when I use the
> OTG mode.
> 

--
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 3/6] Documentation: devicetree: dwc3: Add interrupt moderation

[Felipe Balbi]

Hi,

John Youn  writes:
> On 10/27/2016 3:47 AM, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> John Youn  writes:
>>> Add interrupt moderation interval binding for dwc3.
>>>
>>> Signed-off-by: John Youn 
>>> ---
>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
>>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> index e3e6983..17de9fc 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> @@ -53,6 +53,7 @@ Optional properties:
>>>   - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of 
>>> GFLADJ
>>> register for post-silicon frame length adjustment when the
>>> fladj_30mhz_sdbnd signal is invalid or incorrect.
>>> + - snps,imod_interval: the interrupt moderation interval.
>> 
>> on top of all other comments, what's the unit here? nanoseconds? clock 
>> cycles?
>> 
>
> Number of 250 ns intervals. I'll update the description to clarify.

it's probably better to add it in nanoseconds itself, then let driver
compute register value with DIV_ROUND_UP()

-- 
balbi


signature.asc
Description: PGP signature

Re: musb RPM sleep-while-atomic in 4.9-rc1

[Johan Hovold]

On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote:
> * Johan Hovold  [161027 11:46]:
> > On Thu, Oct 27, 2016 at 10:40:17AM -0700, Tony Lindgren wrote:

> > > diff --git a/drivers/usb/musb/musb_gadget.c 
> > > b/drivers/usb/musb/musb_gadget.c
> > > --- a/drivers/usb/musb/musb_gadget.c
> > > +++ b/drivers/usb/musb/musb_gadget.c
> > > @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct 
> > > musb_request *req)
> > >   rxstate(musb, req);
> > >  }
> > >  
> > > +void musb_ep_restart_resume_work(struct musb *musb, void *data)
> > > +{
> > > + struct musb_request *req = data;
> > > +
> > > + musb_ep_restart(musb, req);
> > 
> > This one is supposed to be called with musb->lock held (according to the
> > function header anyway).
> 
> Good point, yeah that calls the monster functions txstate and rxstate.
> 
> > >  static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req,
> > >   gfp_t gfp_flags)
> > >  {
> > > @@ -1255,7 +1262,7 @@ static int musb_gadget_queue(struct usb_ep *ep, 
> > > struct usb_request *req,
> > >  
> > >   map_dma_buffer(request, musb, musb_ep);
> > >  
> > > - pm_runtime_get_sync(musb->controller);
> > > + pm_runtime_get(musb->controller);
> > >   spin_lock_irqsave(>lock, lockflags);
> > >  
> > >   /* don't queue if the ep is down */
> > > @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, 
> > > struct usb_request *req,
> > >   list_add_tail(>list, _ep->req_list);
> > >  
> > >   /* it this is the head of the queue, start i/o ... */
> > > - if (!musb_ep->busy && >list == musb_ep->req_list.next)
> > > - musb_ep_restart(musb, request);
> > > + if (!musb_ep->busy && >list == musb_ep->req_list.next) {
> > > + if (pm_runtime_active(musb->controller))
> > > + musb_ep_restart(musb, request);
> > > + else
> > > + musb_queue_on_resume(musb, musb_ep_restart_resume_work,
> > > +  request);
> > > + }
> > 
> > But then this looks like it could trigger an ABBA deadlock as musb->lock
> > is held while queue_on_resume() takes musb->list_lock, and
> > musb_run_pending() would take the same locks in the reverse order.
> 
> It seems we can avoid that by locking only list_add_tail() and list_del():
> 
> list_for_each_entry_safe(w, _w, >resume_work, node) {
>   spin_lock_irqsave(>list_lock, flags);
>   list_del(>node);
>   spin_unlock_irqrestore(>list_lock, flags);
>   if (w->callback)
>   w->callback(musb, w->data);
>   devm_kfree(musb->controller, w);
> }

I think you still need to hold the lock while traversing the list (even
if you temporarily release it during the callback).

But this is related to another issue; what if a new callback is
registered while musb_runtime_resume is running? It seems we could end
up queueing work which might never be executed (e.g. if queued just
after musb_run_pending() is done or while processing the last
callback).

> Or do you have some better ideas?

Not right now sorry, and I'll be away from keyboard the rest of the day,
I'm afraid.

Johan
--
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 0/3] usb: musb: da8xx: Fix few issues

[Alexandre Bailon]

On 10/27/2016 08:44 PM, David Lechner wrote:
> On 10/27/2016 12:16 PM, David Lechner wrote:
>> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
>>> Currently, the USB OTG of the da8xx doesn't work.
>>> This series intend to fix them.
>>>
>>> Change in v2:
>>> * Fix the error path da8xx_musb_init()
>>>
>>> Alexandre Bailon (3):
>>>   usb: musb: da8xx: Call earlier clk_prepare_enable()
>>>   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
>>>   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode
>>>
>>>  drivers/phy/phy-da8xx-usb.c | 17 -
>>>  drivers/usb/musb/da8xx.c| 28 +++-
>>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>>
>>
>> I have found another problem with peripheral mode. When we force
>> peripheral mode, the glue layer currently uses CFGCHIP2 to override the
>> VBUS and ID. This causes it to not be able to detect disconnection
>> because the VBUS is overridden.
How have you found it ? Does it cause any issues ?
I mean I had to enable traces to see that disconnect was not happening.
>>
>> Here is a patch to fix the problem. I have tested this on LEGO
>> MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
>> pulled up on the SoC, so we don't need to override it.
Actually, I'm wonder if that if not related to VBUS sensing.
May be we should set CFGCHIP2_VBDTCTEN in device mode.
>>
>> ---
>>
>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>> index 2bc12a2..33daa3b 100644
>> --- a/drivers/usb/musb/da8xx.c
>> +++ b/drivers/usb/musb/da8xx.c
>> @@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
>> musb_mode)
>> case MUSB_HOST: /* Force VBUS valid, ID = 0 */
>> phy_mode = PHY_MODE_USB_HOST;
>> break;
>> -   case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
>> -   phy_mode = PHY_MODE_USB_DEVICE;
>> -   break;
>> +   case MUSB_PERIPHERAL:
>> case MUSB_OTG:  /* Don't override the VBUS/ID
>> comparators */
>> phy_mode = PHY_MODE_USB_OTG;
>> break;
>>
>> ---
>>
>> If this works for other SoCs/boards, I think we should make this change.
>> If it doesn't work, we could work around the VBUS problem by polling
>> VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.
> 
> I have realized that due to the way my device is wired, I can actually
> use OTG mode and it will behave exactly as peripheral mode because the
> ID pin is not connected. So, maybe this patch is not needed after all.
> 
Actually, I'm sure that is related to ID
I have the same issue except the disconnect is called when I use the
OTG mode.

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

patch "tty: vgacon+sisusb, move scrolldelta to a common helper" added to tty-next

[gregkh]

This is a note to let you know that I've just added the patch titled

tty: vgacon+sisusb, move scrolldelta to a common helper

to my tty git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
in the tty-next branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will also be merged in the next major kernel release
during the merge window.

If you have any questions about this process, please let me know.


>From 35cc56f9a30480c8a0cca809cf341614a2144758 Mon Sep 17 00:00:00 2001
From: Jiri Slaby 
Date: Mon, 3 Oct 2016 11:18:35 +0200
Subject: tty: vgacon+sisusb, move scrolldelta to a common helper

The code is mirrorred in scrolldelta implementations of both vgacon
and sisusb. Let's move the code to a separate helper where we will
perform a common cleanup and further changes.

While we are moving the code, make it linear and save one indentation
level. This is done by returning from the "!lines" then-branch
immediatelly. This allows flushing the else-branch 1 level to the
left, obviously.

Few more new lines and comments were added too.

And do not forget to export the helper function given sisusb can be
built as module.

Signed-off-by: Jiri Slaby 
Cc: Thomas Winischhofer 
Cc: Tomi Valkeinen 
Cc: 
Cc: 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/tty/vt/vt.c | 38 +
 drivers/usb/misc/sisusbvga/sisusb_con.c | 37 ++--
 drivers/video/console/vgacon.c  | 27 ++-
 include/linux/vt_kern.h |  2 ++
 4 files changed, 44 insertions(+), 60 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index a0b7576747cd..2eab714aab67 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4279,6 +4279,44 @@ void vcs_scr_updated(struct vc_data *vc)
notify_update(vc);
 }
 
+void vc_scrolldelta_helper(struct vc_data *c, int lines,
+   unsigned int rolled_over, void *base, unsigned int size)
+{
+   unsigned long ubase = (unsigned long)base;
+   int margin = c->vc_size_row * 4;
+   int ul, we, p, st;
+
+   /* Turn scrollback off */
+   if (!lines) {
+   c->vc_visible_origin = c->vc_origin;
+   return;
+   }
+
+   /* Do we have already enough to allow jumping from 0 to the end? */
+   if (rolled_over > (c->vc_scr_end - ubase) + margin) {
+   ul = c->vc_scr_end - ubase;
+   we = rolled_over + c->vc_size_row;
+   } else {
+   ul = 0;
+   we = size;
+   }
+
+   p = (c->vc_visible_origin - ubase - ul + we) % we +
+   lines * c->vc_size_row;
+   st = (c->vc_origin - ubase - ul + we) % we;
+
+   /* Only a little piece would be left? Show all incl. the piece! */
+   if (st < 2 * margin)
+   margin = 0;
+   if (p < margin)
+   p = 0;
+   if (p > st - margin)
+   p = st;
+
+   c->vc_visible_origin = ubase + (p + ul) % we;
+}
+EXPORT_SYMBOL_GPL(vc_scrolldelta_helper);
+
 /*
  * Visible symbols for modules
  */
diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c 
b/drivers/usb/misc/sisusbvga/sisusb_con.c
index 6331965daa0b..4b5777ec1501 100644
--- a/drivers/usb/misc/sisusbvga/sisusb_con.c
+++ b/drivers/usb/misc/sisusbvga/sisusb_con.c
@@ -686,8 +686,6 @@ static void
 sisusbcon_scrolldelta(struct vc_data *c, int lines)
 {
struct sisusb_usb_data *sisusb;
-   int margin = c->vc_size_row * 4;
-   int ul, we, p, st;
 
sisusb = sisusb_get_sisusb_lock_and_check(c->vc_num);
if (!sisusb)
@@ -700,39 +698,8 @@ sisusbcon_scrolldelta(struct vc_data *c, int lines)
return;
}
 
-   if (!lines) /* Turn scrollback off */
-   c->vc_visible_origin = c->vc_origin;
-   else {
-
-   if (sisusb->con_rolled_over >
-   (c->vc_scr_end - sisusb->scrbuf) + margin) {
-
-   ul = c->vc_scr_end - sisusb->scrbuf;
-   we = sisusb->con_rolled_over + c->vc_size_row;
-
-   } else {
-
-   ul = 0;
-   we = sisusb->scrbuf_size;
-
-   }
-
-   p = (c->vc_visible_origin - sisusb->scrbuf - ul + we) % we +
-   lines * c->vc_size_row;
-
-   st = (c->vc_origin - sisusb->scrbuf - ul + we) % we;
-
-   if (st < 2 * margin)
-   margin = 0;
-
-   if (p < margin)
-   p = 0;
-
-   if (p > st - margin)
-   p = st;
-
-   c->vc_visible_origin = sisusb->scrbuf + (p + 

patch "tty: vt, cleanup and document con_scroll" added to tty-next

[gregkh]

This is a note to let you know that I've just added the patch titled

tty: vt, cleanup and document con_scroll

to my tty git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
in the tty-next branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will also be merged in the next major kernel release
during the merge window.

If you have any questions about this process, please let me know.


>From d705ff38189fcfbbfa6aa97363d30c23348ad166 Mon Sep 17 00:00:00 2001
From: Jiri Slaby 
Date: Mon, 3 Oct 2016 11:18:33 +0200
Subject: tty: vt, cleanup and document con_scroll

Scrolling helpers scrup and scrdown both accept 'top' and 'bottom' as
unsigned int. Number of lines 'nr' is accepted as int, but all callers
pass down unsigned too. So change the type of 'nr' to unsigned too.
Now, promote unsigned int from the helpers up to the con_scroll
hook which actually accepted all those as signed int.

Next, the 'dir' parameter can have only two values and we define
constants for that: SM_UP and SM_DOWN. Switch them to enum and do
proper type checking on 'dir' too.

Finally, document the behaviour of the hook.

Signed-off-by: Jiri Slaby 
Cc: Thomas Winischhofer 
Cc: Tomi Valkeinen 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: 
Cc: 
Cc: 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/tty/vt/vt.c |  6 --
 drivers/usb/misc/sisusbvga/sisusb_con.c | 18 ++
 drivers/video/console/fbcon.c   | 18 --
 drivers/video/console/mdacon.c  |  7 ---
 drivers/video/console/newport_con.c |  8 
 drivers/video/console/sticon.c  |  7 ---
 drivers/video/console/vgacon.c  | 12 +---
 include/linux/console.h | 16 +++-
 8 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 06fb39c1d6dd..c4bf96fee32e 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -315,7 +315,8 @@ void schedule_console_callback(void)
schedule_work(_work);
 }
 
-static void scrup(struct vc_data *vc, unsigned int t, unsigned int b, int nr)
+static void scrup(struct vc_data *vc, unsigned int t, unsigned int b,
+   unsigned int nr)
 {
unsigned short *d, *s;
 
@@ -332,7 +333,8 @@ static void scrup(struct vc_data *vc, unsigned int t, 
unsigned int b, int nr)
vc->vc_size_row * nr);
 }
 
-static void scrdown(struct vc_data *vc, unsigned int t, unsigned int b, int nr)
+static void scrdown(struct vc_data *vc, unsigned int t, unsigned int b,
+   unsigned int nr)
 {
unsigned short *s;
unsigned int step;
diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c 
b/drivers/usb/misc/sisusbvga/sisusb_con.c
index 460cebf322e3..6331965daa0b 100644
--- a/drivers/usb/misc/sisusbvga/sisusb_con.c
+++ b/drivers/usb/misc/sisusbvga/sisusb_con.c
@@ -808,9 +808,10 @@ sisusbcon_cursor(struct vc_data *c, int mode)
mutex_unlock(>lock);
 }
 
-static int
+static bool
 sisusbcon_scroll_area(struct vc_data *c, struct sisusb_usb_data *sisusb,
-   int t, int b, int dir, int lines)
+   unsigned int t, unsigned int b, enum con_scroll dir,
+   unsigned int lines)
 {
int cols = sisusb->sisusb_num_columns;
int length = ((b - t) * cols) * 2;
@@ -852,8 +853,9 @@ sisusbcon_scroll_area(struct vc_data *c, struct 
sisusb_usb_data *sisusb,
 }
 
 /* Interface routine */
-static int
-sisusbcon_scroll(struct vc_data *c, int t, int b, int dir, int lines)
+static bool
+sisusbcon_scroll(struct vc_data *c, unsigned int t, unsigned int b,
+   enum con_scroll dir, unsigned int lines)
 {
struct sisusb_usb_data *sisusb;
u16 eattr = c->vc_video_erase_char;
@@ -870,17 +872,17 @@ sisusbcon_scroll(struct vc_data *c, int t, int b, int 
dir, int lines)
 */
 
if (!lines)
-   return 1;
+   return true;
 
sisusb = sisusb_get_sisusb_lock_and_check(c->vc_num);
if (!sisusb)
-   return 0;
+   return false;
 
/* sisusb->lock is down */
 
if (sisusb_is_inactive(c, sisusb)) {
mutex_unlock(>lock);
-   return 0;
+   return false;
}
 
/* Special case */
@@ -971,7 +973,7 @@ sisusbcon_scroll(struct vc_data *c, int t, int b, int dir, 
int lines)
 
mutex_unlock(>lock);
 
-   return 1;
+   return true;
 }
 
 /* Interface routine */
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index b87f5cfdaea5..a44f5627b82a 

Re: Identifying Synopsys USB3 Controller on Baytrail Device

[Felipe Balbi]

Hi,

Joseph Kogut  writes:
> Hi Felipe,
>
> That's some great information, thanks!
>
> At first glance, the DSDT table has two interesting bits. (I would
> paste the decompiled source, but I think it may be copyrighted?
> Correct me if I'm wrong.) The first is the USB mux, an SMSC USB3750
> connected to I2C1. According to the datasheet, this IC supports
> switching the USB data lanes between functions. This looks promising.
>
> The second interesting bit is a device named "OTG1". The _STA method
> of OTG1 checks the value of OTGM--defined earlier as an 8-bit field in
> GNVS (NVRAM?)--and returns 0xF if it's not equal to zero, otherwise it
> returns zero. I'm guessing this is the host controller, based on the
> device name, "Baytrail XHCI controller (Synopsys core/OTG)".

this is the peripheral IP :-) Synopsys core/OTG tells me it's dwc3 :-)

> There's a field called OTGD in the power management capability
> registers opregion, but it's not referenced anywhere else.
>
> There doesn't appear to be anything else related to the device
> controller. The only OS interface string check appears to be in the
> configuration of the PCI bus, and it only checks against various
> versions of Windows.
>
> I'm thinking my next step is to boot into Android and dump the DSDT
> table, and see if it's the same. I'm betting it's not.

It might be the same, yes. But I'm guessing _STA will return a different
value for that OTG1 device :-)

And maybe this mux has something to do with it too. You can fiddle with
the mux by finding its doc, and using i2c-tools (specially i2cset and
i2cget) to write/read registers.

-- 
balbi


signature.asc
Description: PGP signature