Re: [Letux-kernel] [PATCH v2] musb: omap2430: do not assume balanced enable()/disable()

[Andreas Kemnade]

On Fri, 5 Aug 2016 23:21:35 -0700
Tony Lindgren  wrote:

> * Andreas Kemnade  [160805 08:35]:
> > I repeat the subject line of the patch:
> > [PATCH v2] musb: omap2430: do not assume balanced enable()/disable()
> > It is *not* fix charging.
> > 
> > So you mean that the phy should magically know at which refcounter
> > it should power on and off if power on/off is not called in the
> > balanced way?
> 
> No, I mean we need to figure out why things can be called in
> unbalanced way. With your patch I end up with unbalanced calls
> leaving the phy on after all the modules have been removed :)
> 
Well, causing trouble when modules are removed was not the intention.
I was just happy to have things working a bit again.
The phy is powered on/off via
musb_platform_enable()/musb_platform_disable().

Calls to musb_platform_enable() occur at only 1 place.
musb_platform_disable() is called at 4 places.

about balancing:
There is musb_start() and musb_stop(). They are called from
musb_gadget_start/stop()
These call musb_platform_enable() and musb_platform_disable().
Looks ok.

There is musb_suspend() and musb_resume():

musb_suspend() calls musb_platform_disable()
musb_resume() calls musb_plaform_enable() via musb_start()
looks balanced but why don't we use musb_stop() in musb_suspend()?

Now the odd things:
musb_platform_disable() in musb_remove() called upon module removal
musb_platform_disable() in musb_init_controller() called from
musb_probe()

This looks clearly unbalanced.


> > Maybe the phy-twl4030 uses the phy layer wrong. 
> > Now the relevant part of power on/off in phy-twl4030 is done via
> > struct phy_ops.power_off/power_on (*not* via pm_runtime). Maybe
> > that is wrong and more parts should be moved to the pm_runtime
> > stuff (which is also present). 
> 
> We should use phy power_off/power_on for the USB related parts.
> The parts needed by other components, like VBUS detection, should
> be handled by PM runtime. We should get phy-twl4030-usb and the
> charger driver working also when no musb modules are loaded.
> 
> > Then the phy subsystem has its own power refcounter in struct
> > phy.power_count. It it handled via phy_power_off()/phy_power_on().
> > And that is called from musb/omap2430.c 
> > But that is another story. 
> 
> Yes that's what the USB driver is expected to do. But obviously
> there are issues remaining also in the phy-twl4030-usb.c driver.
> And it seems that we should have some OTG parts always enabled
> when VBUS is detected when twl4030-charger is configured?
> 
Seems so. I am writing a patch for it.

> > > If there are MUSB specific PM runtime issues then let's fix
> > > those separately.
> > > 
> > And that exactly tries my patch to do. For that task it does not
> > even use the PM runtime system. Again please read the subject line
> > of the patch. Maybe it unveils some other pm issues in musb
> > which should first be fixed in a complete patch series.
> 
> Certainly that needs to be fixed, but let's do it in a way where
> things work for other test cases also. Care to describe how to
> to reproduce the issue you're seeing? It seems that you are
> seeing devices not being enmerated leading to the charger not
> working? Is this with built in MUSB and phy modules?
> 
Both as modules. I added some debug output to the driver/phy/phy-core.c
and have seen the phy->power_count sticking at -1 or 0. 
g_ether is also loaded.
Gadget stops for me (device not showing up at the other side) at 4.7rc4.
But I remember Nikolaus having the situation on the same type of device
that it was important on which side you replug the usb cable
(probably causing some timing differences) with 4.7rc1.

Regards,
Andreas


pgpxcl3A3FXFO.pgp
Description: OpenPGP digital signature

[PATCH 1/1] usb: misc: usbtest: add fix for driver hang

[Lu Baolu]

In sg_timeout(), req->status is set to "-ETIMEDOUT" before calling
into usb_sg_cancel(). usb_sg_cancel() will do nothing and return
directly if req->status has been set to a non-zero value. This will
cause driver hang as soon as transfer time out is triggered.

In my test case, below system log shows when port reset happens after
urb being submitted.

[55099.746213] usb 2-3: USB disconnect, device number 2
[55321.955451] INFO: task kworker/6:0:59 blocked for more than 120 seconds.
[55321.955463]   Not tainted 4.7.0-rc6+ #19
[55321.955466] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[55321.955472] kworker/6:0 D 88045ad3bbf8 059  2 0x
[55321.955489] Workqueue: usb_hub_wq hub_event
[55321.955493]  88045ad3bbf8 88040030 88045ad30f40 
88045ad3bbd0
[55321.955499]  88045ad3c000 8804590920fc 88045ad30f40 

[55321.955505]  880459092100 88045ad3bc10 817b5925 
8804590920f8
[55321.955511] Call Trace:
[55321.955521]  [] schedule+0x35/0x80
[55321.955527]  [] schedule_preempt_disabled+0xe/0x10
[55321.955533]  [] __mutex_lock_slowpath+0x95/0x110
[55321.955538]  [] mutex_lock+0x1f/0x2f
[55321.955544]  [] usb_disconnect+0x53/0x270
[55321.90]  [] hub_port_connect+0x71/0x900
[55321.96]  [] ? hub_port_reset+0x3de/0x630
[55321.955562]  [] hub_event+0x63e/0xb20
[55321.955569]  [] ? put_prev_entity+0x35/0x730
[55321.955577]  [] process_one_work+0x153/0x3f0
[55321.955583]  [] worker_thread+0x12b/0x4b0
[55321.955590]  [] ? rescuer_thread+0x340/0x340
[55321.955595]  [] kthread+0xc9/0xe0
[55321.955601]  [] ret_from_fork+0x1f/0x40
[55321.955606]  [] ? kthread_park+0x60/0x60
[55321.955638] INFO: task testusb:3011 blocked for more than 120 seconds.
[55321.955643]   Not tainted 4.7.0-rc6+ #19
[55321.955647] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[55321.955651] testusb D 880449a079f8 0  3011   2367 0x
[55321.955657]  880449a079f8 0002 88045754cc40 
815b91b9
[55321.955663]  880449a08000 7fff 880449a07cb0 
88045754cc40
[55321.955668]   880449a07a10 817b5925 
880449a07cb8
[55321.955673] Call Trace:
[55321.955680]  [] ? usb_hcd_submit_urb+0xa9/0xba0
[55321.955685]  [] schedule+0x35/0x80
[55321.955691]  [] schedule_timeout+0x231/0x2d0
[55321.955698]  [] ? __alloc_pages_nodemask+0x112/0x2b0
[55321.955703]  [] wait_for_completion+0xa4/0x110
[55321.955709]  [] ? wake_up_q+0x80/0x80
[55321.955713]  [] usb_sg_wait+0x138/0x190
[55321.955722]  [] perform_sglist+0xef/0x180 [usbtest]
[55321.955728]  [] ? usbtest_resume+0x10/0x10 [usbtest]
[55321.955734]  [] usbtest_do_ioctl+0x898/0x1520 [usbtest]
[55321.955740]  [] ? usb_hcd_reset_endpoint+0x25/0x60
[55321.955745]  [] ? usb_enable_endpoint+0x85/0x90
[55321.955751]  [] usbtest_ioctl+0x123/0x26e [usbtest]
[55321.955756]  [] ? filemap_map_pages+0x295/0x310
[55321.955763]  [] ? proc_ioctl+0x48/0x210
[55321.955769]  [] proc_ioctl+0x130/0x210
[55321.955776]  [] usbdev_do_ioctl+0x50a/0x1170
[55321.955782]  [] usbdev_ioctl+0xe/0x20
[55321.955789]  [] do_vfs_ioctl+0x96/0x590
[55321.955794]  [] ? putname+0x53/0x60
[55321.955800]  [] SyS_ioctl+0x79/0x90
[55321.955806]  [] entry_SYSCALL_64_fastpath+0x1e/0xa

This patch fixes this driver hang. It should be back-ported to stable
kernel with version after v3.15.

Cc: sta...@vger.kernel.org
Cc: Alan Stern 
Signed-off-by: Lu Baolu 
---
 drivers/usb/misc/usbtest.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 6b978f0..6c6586d 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -585,7 +585,6 @@ static void sg_timeout(unsigned long _req)
 {
struct usb_sg_request   *req = (struct usb_sg_request *) _req;
 
-   req->status = -ETIMEDOUT;
usb_sg_cancel(req);
 }
 
-- 
2.1.4

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

[RESEND PATCH] usb: hub: change CLEAR_FEATURE to SET_FEATURE

[Yonglong Wu]

From: Yonglong Wu 

According to USB30 specification, the Function Remote Wakeup field can be
modified by the SetFeature() requests. SetFeature() is recommended to use.

Signed-off-by: Yonglong Wu 
---
 drivers/usb/core/hub.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a6f5095 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3111,7 +3111,7 @@ static int usb_disable_remote_wakeup(struct usb_device 
*udev)
USB_CTRL_SET_TIMEOUT);
else
return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
+   USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
USB_CTRL_SET_TIMEOUT);
 }
-- 
1.7.9.5

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

[PATCH] usb: hub: change CLEAR_FEATURE to SET_FEATURE

[Yonglong Wu]

From: Yonglong Wu 

According to USB30 specification, the Function Remote Wakeup field can be
modified by the SetFeature() requests. SetFeature() is recommended to use.

Change-Id: Ie744b95f12d7d21d9519e77ed706c8cc33fa
Signed-off-by: Yonglong Wu 
---
 drivers/usb/core/hub.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a6f5095 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3111,7 +3111,7 @@ static int usb_disable_remote_wakeup(struct usb_device 
*udev)
USB_CTRL_SET_TIMEOUT);
else
return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
+   USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
USB_CTRL_SET_TIMEOUT);
 }
-- 
1.7.9.5

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

Re: [PATCH] scsi: introduce a quirk for false cache reporting

[Martin K. Petersen]

> "Oliver" == Oliver Neukum  writes:

Oliver> Some SATA to USB bridges fail to cooperate with some drives
Oliver> resulting in no cache being present being reported to the
Oliver> host. That causes the host to skip sending a command to
Oliver> synchronize caches. That causes data loss when the drive is
Oliver> powered down.

Alan?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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

[no subject]

[Jeffrey Chu]

unsubscsribe
--
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 08/22] usb: chipidea: Remove locking in ci_udc_start()

[Peter Chen]

>Quoting Peter Chen (2016-08-06 00:54:35)
>> On Fri, Aug 05, 2016 at 02:53:56PM -0700, Stephen Boyd wrote:
>> > Quoting Peter Chen (2016-07-08 02:45:28)
>> > > On Thu, Jul 07, 2016 at 03:20:59PM -0700, Stephen Boyd wrote:
>> > > > We don't call hw_device_reset() with the ci->lock held, so it
>> > > > doesn't seem like this lock here is protecting anything. Let's
>> > > > just remove it. This allows us to call sleeping functions like
>> > > > phy_init() from within the CI_HDRC_CONTROLLER_RESET_EVENT hook.
>> > > >
>> > > > Cc: Peter Chen 
>> > > > Cc: Greg Kroah-Hartman 
>> > > > Signed-off-by: Stephen Boyd 
>> > > > ---
>> > > >  drivers/usb/chipidea/udc.c | 3 ---
>> > > >  1 file changed, 3 deletions(-)
>> > > >
>> > > > diff --git a/drivers/usb/chipidea/udc.c
>> > > > b/drivers/usb/chipidea/udc.c index 065f5d97aa67..f16be4710cdb
>> > > > 100644
>> > > > --- a/drivers/usb/chipidea/udc.c
>> > > > +++ b/drivers/usb/chipidea/udc.c
>> > > > @@ -1719,7 +1719,6 @@ static int ci_udc_start(struct usb_gadget 
>> > > > *gadget,
>> > > >struct usb_gadget_driver *driver)  {
>> > > >   struct ci_hdrc *ci = container_of(gadget, struct ci_hdrc, 
>> > > > gadget);
>> > > > - unsigned long flags;
>> > > >   int retval = -ENOMEM;
>> > > >
>> > > >   if (driver->disconnect == NULL) @@ -1746,7 +1745,6 @@
>> > > > static int ci_udc_start(struct usb_gadget *gadget,
>> > > >
>> > > >   pm_runtime_get_sync(>gadget.dev);
>> > > >   if (ci->vbus_active) {
>> > > > - spin_lock_irqsave(>lock, flags);
>> > > >   hw_device_reset(ci);
>> > > >   } else {
>> > > >   usb_udc_vbus_handler(>gadget, false); @@
>> > > > -1755,7 +1753,6 @@ static int ci_udc_start(struct usb_gadget *gadget,
>> > > >   }
>> > > >
>> > > >   retval = hw_device_state(ci, ci->ep0out->qh.dma);
>> > > > - spin_unlock_irqrestore(>lock, flags);
>> > > >   if (retval)
>> > > >   pm_runtime_put_sync(>gadget.dev);
>> > > >
>> > >
>> > > The main purpose for this is disabling interrupt when reset
>> > > controller, in case the unexpected interrupts occur.
>> > >
>> > > You can move this between hw_device_state.
>> > >
>> >
>> > Ok, but we don't hold the ci->lock in ci_udc_vbus_session(). Is that
>> > a bug as well?
>>
>> I agree with your patch. In fact, during the reset controller, the
>> interrupt has not been enabled, it should no unexpected interrupt.
>>
>
>So then we can leave this patch as is? It still isn't clear to me what 
>sequence of
>events that would cause a problem if we don't hold the
>ci->lock around hw_device_state().

Yes. I am ok with this patch.  After thinking more, it is safe to be without 
lock.
And we have this unlock code in nxp internal tree, and without any problems
during 1-2 years.

Acked-by: Peter Chen 

Peter

Re: [PATCH v5 2/6] power: add power sequence library

[Matthias Kaehlcke]

El Mon, Aug 08, 2016 at 04:52:07PM +0800 Peter Chen ha dit:

> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
> 
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices.
> 
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover regulator and pinctrl
> in future. The host driver calls pwrseq_alloc_generic to create
> an generic pwrseq instance.
> 
> Signed-off-by: Peter Chen 
> Tested-by Joshua Clayton 

Reviewed-by: Matthias Kaehlcke 
Tested-by: Matthias Kaehlcke 
--
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: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[ican realizeum]

On Mon, Aug 8, 2016 at 11:01 PM, Alan Stern  wrote:
> On Mon, 8 Aug 2016, ican realizeum wrote:
>
>> >> 1. if I have line in debug.sh which attempts to poweroff manually via
>> >> sysrq, like this:
>> >> echo o > /proc/sysrq-trigger ; sleep 5
>> >> then, there are no 'controller won't resume' and no 'HC died'
>> >> messages, but there are only those with PME# and bus mastering. This
>> >> seems quite fine! (as a side, there's still the failure to flush SSD
>> >> cache and failure to turn off SSD gracefully, but that's another bug)
>> In case 1. it shows:
>> ohci_rh_resume: control 240
>>
>> (that's 100100)
>> https://i.imgur.com/5Wwwxwf.jpg
>> >>
>> >> 2. if I don't manually poweroff from debug.sh and therefore allow the
>> >> normal shutdown process to continue, then I get those errors with
>> >> 'controller won't resume' and 'HC died'
>> >> It looks like something is at least trying to prevent those from
>> >> waking up but I'm not sure if it's something in device_shutdown() or
>> >> something before do_poweroff(), because apparently both
>> >> sysrq+o(poweroff) and normal poweroff have the same stacktrace(?i
>> >> should probably check this)
>> In case 2. it shows:
>> ohci_rh_resume: control 2c0
>>
>> (that's 101100)
>> https://i.imgur.com/8sxP70x.jpg
>
> Well, 240 means the controller is in the RESUME state (which is what it
> should be at that point), and 2c0 means the controller is still in the
> SUSPEND state.  So even after the driver told the controller to change
> from SUSPEND to RESUME, it's still in SUSPEND.
>
> The spec doesn't list any reasons why this might happen.  All I can
> guess is that the controller needs some input that the shutdown
> procedure has turned off.
>
> Alan Stern
>
Thanks Alan.
I've no further ideas.
Peace out :)
--
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

[PACTH v3 1/2] usb: xhci: plat: Enable runtime PM

[robert . foss]

From: Andrew Bresticker 

Enable runtime PM for the xhci-plat device so that the parent device
may implement runtime PM.

Signed-off-by: Andrew Bresticker 
Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
---
 drivers/usb/host/xhci-plat.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..4f39d4e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;
 
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 
@@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
 
+   pm_runtime_disable(>dev);
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
 
@@ -292,7 +297,11 @@ static int xhci_plat_suspend(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;
 
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0)
+   return ret;
/*
 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
 * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -301,15 +310,26 @@ static int xhci_plat_suspend(struct device *dev)
 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
 * also applies to runtime suspend.
 */
-   return xhci_suspend(xhci, device_may_wakeup(dev));
+   ret = xhci_suspend(xhci, device_may_wakeup(dev));
+   pm_runtime_put(dev);
+
+   return ret;
 }
 
 static int xhci_plat_resume(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;
+
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0)
+   return ret;
 
-   return xhci_resume(xhci, 0);
+   ret = xhci_resume(xhci, 0);
+   pm_runtime_put(dev);
+
+   return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
-- 
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

[PACTH v3 2/2] usb: xhci: plat: Enable async suspend/resume

[robert . foss]

From: Andrew Bresticker 

USB host controllers can take a significant amount of time to suspend
and resume, adding several hundred miliseconds to the kernel resume
time. Since the XHCI controller has no outside dependencies (other than
clocks, which are suspended late/resumed early), allow it to suspend and
resume asynchronously.

Signed-off-by: Andrew Bresticker 
Tested-by: Andrew Bresticker 
Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
---
 drivers/usb/host/xhci-plat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 4f39d4e..89aef0e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -248,6 +248,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
pm_runtime_set_active(>dev);
pm_runtime_enable(>dev);
+   device_enable_async_suspend(>dev);
 
return 0;
 
-- 
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

[PACTH v3 0/2] usb: xhci: plat: Enable PM, async resume/suspend

[robert . foss]

From: Robert Foss 

This series enables runtime PM and asynchronous resume/suspend support for
xhci-plat devices.

This is a resumbmission of v3.

Changes since v1:
- Added Signed-off-by: Robert Foss 
- Added proper metadata tags to series.

Changes since v2:
- Added missing changelog to cover-letter.
- Added error checking to pm_runtime_get_sync().

Andrew Bresticker (2):
  usb: xhci: plat: Enable runtime PM
  usb: xhci: plat: Enable async suspend/resume

 drivers/usb/host/xhci-plat.c | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

-- 
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 v4 02/10] usb: gadget: change len to size_t on alloc_ep_req()

[Felipe F. Tonello]

Length of buffers should be of type size_t whenever possible. Altough
recommended, this change has no real practical change, unless a driver has a
uses a huge or negative buffer size - it might help find these bugs.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/u_f.c | 2 +-
 drivers/usb/gadget/u_f.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c
index 4bc7eea8bfc8..6811761fec64 100644
--- a/drivers/usb/gadget/u_f.c
+++ b/drivers/usb/gadget/u_f.c
@@ -13,7 +13,7 @@
 
 #include "u_f.h"
 
-struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len)
+struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len)
 {
struct usb_request  *req;
 
diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h
index 4247cc098a89..21b75710c4a4 100644
--- a/drivers/usb/gadget/u_f.h
+++ b/drivers/usb/gadget/u_f.h
@@ -48,7 +48,7 @@ struct usb_ep;
 struct usb_request;
 
 /* Requests allocated via alloc_ep_req() must be freed by free_ep_req(). */
-struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len);
+struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len);
 static inline void free_ep_req(struct usb_ep *ep, struct usb_request *req)
 {
kfree(req->buf);
-- 
2.9.2

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

[PATCH v4 04/10] usb: gadget: f_midi: remove alignment code for OUT endpoint

[Felipe F. Tonello]

The new version of alloc_ep_req() already aligns the buffer size to
wMaxPacketSize on OUT endpoints.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 58fc199a18ec..39018dea7035 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -360,9 +360,8 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->out_ep,
-   max_t(unsigned, midi->buflen,
-   bulk_out_desc.wMaxPacketSize));
+   midi_alloc_ep_req(midi->out_ep, midi->buflen);
+
if (req == NULL)
return -ENOMEM;
 
-- 
2.9.2

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

[PATCH v4 05/10] usb: gadget: f_midi: defaults buflen sizes to 512

[Felipe F. Tonello]

512 is the value used by wMaxPacketSize, as specified by the USB Spec. This
makes sure this driver uses, by default, the most optimal value for IN and OUT
endpoint requests.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 2 +-
 drivers/usb/gadget/legacy/gmidi.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 39018dea7035..a7b50ac947f8 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1122,7 +1122,7 @@ static struct usb_function_instance 
*f_midi_alloc_inst(void)
opts->func_inst.free_func_inst = f_midi_free_inst;
opts->index = SNDRV_DEFAULT_IDX1;
opts->id = SNDRV_DEFAULT_STR1;
-   opts->buflen = 256;
+   opts->buflen = 512;
opts->qlen = 32;
opts->in_ports = 1;
opts->out_ports = 1;
diff --git a/drivers/usb/gadget/legacy/gmidi.c 
b/drivers/usb/gadget/legacy/gmidi.c
index fc2ac150f5ff..0bf39c3ccdb1 100644
--- a/drivers/usb/gadget/legacy/gmidi.c
+++ b/drivers/usb/gadget/legacy/gmidi.c
@@ -47,7 +47,7 @@ static char *id = SNDRV_DEFAULT_STR1;
 module_param(id, charp, S_IRUGO);
 MODULE_PARM_DESC(id, "ID string for the USB MIDI Gadget adapter.");
 
-static unsigned int buflen = 256;
+static unsigned int buflen = 512;
 module_param(buflen, uint, S_IRUGO);
 MODULE_PARM_DESC(buflen, "MIDI buffer length");
 
-- 
2.9.2

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

[PATCH v4 01/10] usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align

[Felipe F. Tonello]

USB spec specifies wMaxPacketSize to be little endian (as other properties),
so when using this variable in the driver we should convert to the current
CPU endianness if necessary.

This patch also introduces usb_ep_align() which does always returns the
aligned buffer size for an endpoint. This is useful to be used by USB requests
allocator functions.

Signed-off-by: Felipe F. Tonello 
---
 include/linux/usb/gadget.h | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdfa388e..3cc93237ff98 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -418,8 +418,20 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
device *dev)
list_for_each_entry(tmp, &(gadget)->ep_list, ep_list)
 
 /**
+ * usb_ep_align - returns @len aligned to ep's maxpacketsize.
+ * @ep: the endpoint whose maxpacketsize is used to align @len
+ * @len: buffer size's length to align to @ep's maxpacketsize
+ *
+ * This helper is used to align buffer's size to an ep's maxpacketsize.
+ */
+static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
+{
+   return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
+}
+
+/**
  * usb_ep_align_maybe - returns @len aligned to ep's maxpacketsize if gadget
- * requires quirk_ep_out_aligned_size, otherwise reguens len.
+ * requires quirk_ep_out_aligned_size, otherwise returns len.
  * @g: controller to check for quirk
  * @ep: the endpoint whose maxpacketsize is used to align @len
  * @len: buffer size's length to align to @ep's maxpacketsize
@@ -430,8 +442,7 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
device *dev)
 static inline size_t
 usb_ep_align_maybe(struct usb_gadget *g, struct usb_ep *ep, size_t len)
 {
-   return !g->quirk_ep_out_aligned_size ? len :
-   round_up(len, (size_t)ep->desc->wMaxPacketSize);
+   return g->quirk_ep_out_aligned_size ? usb_ep_align(ep, len) : len;
 }
 
 /**
-- 
2.9.2

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

[PATCH v4 03/10] usb: gadget: align buffer size when allocating for OUT endpoint

[Felipe F. Tonello]

Using usb_ep_align() makes sure that the buffer size for OUT endpoints is
always aligned with wMaxPacketSize (512 usually). This makes sure
that no buffer has the wrong size, which can cause nasty bugs.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/u_f.c |  3 +++
 drivers/usb/gadget/u_f.h | 16 +++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c
index 6811761fec64..907f8144813c 100644
--- a/drivers/usb/gadget/u_f.c
+++ b/drivers/usb/gadget/u_f.c
@@ -12,6 +12,7 @@
  */
 
 #include "u_f.h"
+#include 
 
 struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len)
 {
@@ -20,6 +21,8 @@ struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t 
len, int default_len)
req = usb_ep_alloc_request(ep, GFP_ATOMIC);
if (req) {
req->length = len ?: default_len;
+   if (usb_endpoint_dir_out(ep->desc))
+   req->length = usb_ep_align(ep, req->length);
req->buf = kmalloc(req->length, GFP_ATOMIC);
if (!req->buf) {
usb_ep_free_request(ep, req);
diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h
index 21b75710c4a4..69a1d10df04f 100644
--- a/drivers/usb/gadget/u_f.h
+++ b/drivers/usb/gadget/u_f.h
@@ -47,8 +47,22 @@
 struct usb_ep;
 struct usb_request;
 
-/* Requests allocated via alloc_ep_req() must be freed by free_ep_req(). */
+/**
+ * alloc_ep_req - returns a usb_request allocated by the gadget driver and
+ * allocates the request's buffer.
+ *
+ * @ep: the endpoint to allocate a usb_request
+ * @len: usb_requests's buffer suggested size
+ * @default_len: used if @len is not provided, ie, is 0
+ *
+ * In case @ep direction is OUT, the @len will be aligned to ep's
+ * wMaxPacketSize. In order to avoid memory leaks or drops, *always* use
+ * usb_requests's length (req->length) to refer to the allocated buffer size.
+ * Requests allocated via alloc_ep_req() *must* be freed by free_ep_req().
+ */
 struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len);
+
+/* Frees a usb_request previously allocated by alloc_ep_req() */
 static inline void free_ep_req(struct usb_ep *ep, struct usb_request *req)
 {
kfree(req->buf);
-- 
2.9.2

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

[PATCH v4 00/10] Gadget endpoint request allocation and MIDI

[Felipe F. Tonello]

As discussed with Baolin Wang, Michal Nazarewicz and Felipe Balbi. I propose
the forced buffer alignment of OUT endpoints USB requests. This is implemented
by patches #1 and #3.

That not just simplifies the driver code, but it also prevents nasty bugs when
buflen is not aligned or even less than wMaxPacketSize.

Patch #10 removes direct calls to usb_ep_alloc_request() and use alloc_ep_req()
instead. If accepted, then we should apply to all other gadgets that uses
usb_ep_alloc_request() when possible and encorage drivers to use it instead.

Changes from v3:
 * Added patch #2 which uses size_t on alloc_ep_req() instead of int.

Changes from v2:
 * Simplified logic in patch #1;
 * Added documentation to alloc_ep_req and free_ep_req;
 * Improved commit message on patch #8.

Changes from v1:
 * Added patches #1, #3, #8, #9 ,#10;
 * Patch #4 removes max_t() for buffer alignment with wMaxPacketSize.

Felipe F. Tonello (10):
  usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align
  usb: gadget: change len to size_t on alloc_ep_req()
  usb: gadget: align buffer size when allocating for OUT endpoint
  usb: gadget: f_midi: remove alignment code for OUT endpoint
  usb: gadget: f_midi: defaults buflen sizes to 512
  usb: gadget: f_midi: refactor state machine
  usb: gadget: f_midi: drop substreams when disabling endpoint
  usb: gadget: remove useless parameter in alloc_ep_req()
  usb: gadget: f_hid: use free_ep_req()
  usb: gadget: f_hid: use alloc_ep_req()

 drivers/usb/gadget/function/f_hid.c|  26 +---
 drivers/usb/gadget/function/f_loopback.c   |   9 +-
 drivers/usb/gadget/function/f_midi.c   | 240 +
 drivers/usb/gadget/function/f_sourcesink.c |  11 +-
 drivers/usb/gadget/legacy/gmidi.c  |   2 +-
 drivers/usb/gadget/u_f.c   |   6 +-
 drivers/usb/gadget/u_f.h   |  17 +-
 include/linux/usb/gadget.h |  17 +-
 8 files changed, 188 insertions(+), 140 deletions(-)

-- 
2.9.2

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

[PATCH v4 07/10] usb: gadget: f_midi: drop substreams when disabling endpoint

[Felipe F. Tonello]

This change makes sure that the ALSA buffers are cleaned if an endpoint
becomes disabled.

Before this change, if the internal ALSA buffer did overflow, the MIDI
function would stop sending MIDI to the host.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 09d769e18b50..3a47596afcab 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -305,6 +305,19 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
}
 }
 
+static void f_midi_drop_out_substreams(struct f_midi *midi)
+{
+   unsigned int i;
+
+   for (i = 0; i < midi->in_ports; i++) {
+   struct gmidi_in_port *port = midi->in_ports_array + i;
+   struct snd_rawmidi_substream *substream = port->substream;
+
+   if (port->active && substream)
+   snd_rawmidi_drop_output(substream);
+   }
+}
+
 static int f_midi_start_ep(struct f_midi *midi,
   struct usb_function *f,
   struct usb_ep *ep)
@@ -402,6 +415,8 @@ static void f_midi_disable(struct usb_function *f)
/* release IN requests */
while (kfifo_get(>in_req_fifo, ))
free_ep_req(midi->in_ep, req);
+
+   f_midi_drop_out_substreams(midi);
 }
 
 static int f_midi_snd_free(struct snd_device *device)
@@ -571,18 +586,6 @@ static void f_midi_transmit_byte(struct usb_request *req,
port->state = next_state;
 }
 
-static void f_midi_drop_out_substreams(struct f_midi *midi)
-{
-   unsigned int i;
-
-   for (i = 0; i < midi->in_ports; i++) {
-   struct gmidi_in_port *port = midi->in_ports_array + i;
-   struct snd_rawmidi_substream *substream = port->substream;
-   if (port->active && substream)
-   snd_rawmidi_drop_output(substream);
-   }
-}
-
 static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
 {
struct usb_request *req = NULL;
-- 
2.9.2

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

[PATCH v4 09/10] usb: gadget: f_hid: use free_ep_req()

[Felipe F. Tonello]

We should always use free_ep_req() when allocating requests with
alloc_ep_req().

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_hid.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index e82a7468252e..a010496e4e05 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -671,11 +671,8 @@ fail_free_descs:
usb_free_all_descriptors(f);
 fail:
ERROR(f->config->cdev, "hidg_bind FAILED\n");
-   if (hidg->req != NULL) {
-   kfree(hidg->req->buf);
-   if (hidg->in_ep != NULL)
-   usb_ep_free_request(hidg->in_ep, hidg->req);
-   }
+   if (hidg->req != NULL)
+   free_ep_req(hidg->in_ep, hidg->req);
 
return status;
 }
@@ -904,8 +901,7 @@ static void hidg_unbind(struct usb_configuration *c, struct 
usb_function *f)
 
/* disable/free request and end point */
usb_ep_disable(hidg->in_ep);
-   kfree(hidg->req->buf);
-   usb_ep_free_request(hidg->in_ep, hidg->req);
+   free_ep_req(hidg->in_ep, hidg->req);
 
usb_free_all_descriptors(f);
 }
-- 
2.9.2

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

[PATCH v4 08/10] usb: gadget: remove useless parameter in alloc_ep_req()

[Felipe F. Tonello]

The default_length parameter of alloc_ep_req was not really necessary
and gadget drivers would almost always create an inline function to pass
the same value to len and default_len.

So this patch also removes duplicate code from few drivers.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_hid.c| 10 ++
 drivers/usb/gadget/function/f_loopback.c   |  9 +
 drivers/usb/gadget/function/f_midi.c   | 10 ++
 drivers/usb/gadget/function/f_sourcesink.c | 11 ++-
 drivers/usb/gadget/u_f.c   |  7 +++
 drivers/usb/gadget/u_f.h   |  3 +--
 6 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 51980c50546d..e82a7468252e 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, struct file 
*fd)
 /*-*/
 /*usb_function */
 
-static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
-   unsigned length)
-{
-   return alloc_ep_req(ep, length, length);
-}
-
 static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request 
*req)
 {
struct f_hidg *hidg = (struct f_hidg *) req->context;
@@ -549,8 +543,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
 */
for (i = 0; i < hidg->qlen && status == 0; i++) {
struct usb_request *req =
-   hidg_alloc_ep_req(hidg->out_ep,
- hidg->report_length);
+   alloc_ep_req(hidg->out_ep,
+   hidg->report_length);
if (req) {
req->complete = hidg_set_report_complete;
req->context  = hidg;
diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 3a9f8f9c77bd..701ee0f11c33 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -306,13 +306,6 @@ static void disable_loopback(struct f_loopback *loop)
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
-static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
-{
-   struct f_loopback   *loop = ep->driver_data;
-
-   return alloc_ep_req(ep, len, loop->buflen);
-}
-
 static int alloc_requests(struct usb_composite_dev *cdev,
  struct f_loopback *loop)
 {
@@ -333,7 +326,7 @@ static int alloc_requests(struct usb_composite_dev *cdev,
if (!in_req)
goto fail;
 
-   out_req = lb_alloc_ep_req(loop->out_ep, 0);
+   out_req = alloc_ep_req(loop->out_ep, loop->buflen);
if (!out_req)
goto fail_in;
 
diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 3a47596afcab..abf26364b46f 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -208,12 +208,6 @@ static struct usb_gadget_strings *midi_strings[] = {
NULL,
 };
 
-static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep,
-   unsigned length)
-{
-   return alloc_ep_req(ep, length, length);
-}
-
 static const uint8_t f_midi_cin_length[] = {
0, 0, 2, 3, 3, 1, 2, 3, 3, 3, 3, 3, 2, 2, 3, 1
 };
@@ -365,7 +359,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* pre-allocate write usb requests to use on f_midi_transmit. */
while (kfifo_avail(>in_req_fifo)) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->in_ep, midi->buflen);
+   alloc_ep_req(midi->in_ep, midi->buflen);
 
if (req == NULL)
return -ENOMEM;
@@ -379,7 +373,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->out_ep, midi->buflen);
+   alloc_ep_req(midi->out_ep, midi->buflen);
 
if (req == NULL)
return -ENOMEM;
diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index df0189ddfdd5..517d78d6da78 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ 

[PATCH v4 10/10] usb: gadget: f_hid: use alloc_ep_req()

[Felipe F. Tonello]

Use gadget's framework allocation function instead of directly calling
usb_ep_alloc_request().

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_hid.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index a010496e4e05..89d2e9a5a04f 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -611,14 +611,10 @@ static int hidg_bind(struct usb_configuration *c, struct 
usb_function *f)
 
/* preallocate request and buffer */
status = -ENOMEM;
-   hidg->req = usb_ep_alloc_request(hidg->in_ep, GFP_KERNEL);
+   hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length);
if (!hidg->req)
goto fail;
 
-   hidg->req->buf = kmalloc(hidg->report_length, GFP_KERNEL);
-   if (!hidg->req->buf)
-   goto fail;
-
/* set descriptor dynamic values */
hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
-- 
2.9.2

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

[PATCH v4 06/10] usb: gadget: f_midi: refactor state machine

[Felipe F. Tonello]

This refactor results in a cleaner state machine code and promotes
consistency, readability, and maintanability of this driver.

This refactor state machine was well tested and it is currently running in
production code and devices.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 204 ++-
 1 file changed, 129 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index a7b50ac947f8..09d769e18b50 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -51,6 +51,19 @@ static const char f_midi_longname[] = "MIDI Gadget";
  */
 #define MAX_PORTS 16
 
+/* MIDI message states */
+enum {
+   STATE_INITIAL = 0,  /* pseudo state */
+   STATE_1PARAM,
+   STATE_2PARAM_1,
+   STATE_2PARAM_2,
+   STATE_SYSEX_0,
+   STATE_SYSEX_1,
+   STATE_SYSEX_2,
+   STATE_REAL_TIME,
+   STATE_FINISHED, /* pseudo state */
+};
+
 /*
  * This is a gadget, and the IN/OUT naming is from the host's perspective.
  * USB -> OUT endpoint -> rawmidi
@@ -61,13 +74,6 @@ struct gmidi_in_port {
int active;
uint8_t cable;
uint8_t state;
-#define STATE_UNKNOWN  0
-#define STATE_1PARAM   1
-#define STATE_2PARAM_1 2
-#define STATE_2PARAM_2 3
-#define STATE_SYSEX_0  4
-#define STATE_SYSEX_1  5
-#define STATE_SYSEX_2  6
uint8_t data[2];
 };
 
@@ -403,118 +409,166 @@ static int f_midi_snd_free(struct snd_device *device)
return 0;
 }
 
-static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
-   uint8_t p1, uint8_t p2, uint8_t p3)
-{
-   unsigned length = req->length;
-   u8 *buf = (u8 *)req->buf + length;
-
-   buf[0] = p0;
-   buf[1] = p1;
-   buf[2] = p2;
-   buf[3] = p3;
-   req->length = length + 4;
-}
-
 /*
  * Converts MIDI commands to USB MIDI packets.
  */
 static void f_midi_transmit_byte(struct usb_request *req,
 struct gmidi_in_port *port, uint8_t b)
 {
-   uint8_t p0 = port->cable << 4;
+   uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
+   uint8_t next_state = STATE_INITIAL;
+
+   switch (b) {
+   case 0xf8 ... 0xff:
+   /* System Real-Time Messages */
+   p[0] |= 0x0f;
+   p[1] = b;
+   next_state = port->state;
+   port->state = STATE_REAL_TIME;
+   break;
+
+   case 0xf7:
+   /* End of SysEx */
+   switch (port->state) {
+   case STATE_SYSEX_0:
+   p[0] |= 0x05;
+   p[1] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   case STATE_SYSEX_1:
+   p[0] |= 0x06;
+   p[1] = port->data[0];
+   p[2] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   case STATE_SYSEX_2:
+   p[0] |= 0x07;
+   p[1] = port->data[0];
+   p[2] = port->data[1];
+   p[3] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   default:
+   /* Ignore byte */
+   next_state = port->state;
+   port->state = STATE_INITIAL;
+   }
+   break;
 
-   if (b >= 0xf8) {
-   f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0);
-   } else if (b >= 0xf0) {
+   case 0xf0 ... 0xf6:
+   /* System Common Messages */
+   port->data[0] = port->data[1] = 0;
+   port->state = STATE_INITIAL;
switch (b) {
case 0xf0:
port->data[0] = b;
-   port->state = STATE_SYSEX_1;
+   port->data[1] = 0;
+   next_state = STATE_SYSEX_1;
break;
case 0xf1:
case 0xf3:
port->data[0] = b;
-   port->state = STATE_1PARAM;
+   next_state = STATE_1PARAM;
break;
case 0xf2:
port->data[0] = b;
-   port->state = STATE_2PARAM_1;
+   next_state = STATE_2PARAM_1;
break;
case 0xf4:
case 0xf5:
-   port->state = STATE_UNKNOWN;
+   next_state = STATE_INITIAL;
break;
case 0xf6:
-   f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0);
-   port->state = STATE_UNKNOWN;
-   break;
-   case 0xf7:
-   

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[Alan Stern]

On Mon, 8 Aug 2016, ican realizeum wrote:

> >> 1. if I have line in debug.sh which attempts to poweroff manually via
> >> sysrq, like this:
> >> echo o > /proc/sysrq-trigger ; sleep 5
> >> then, there are no 'controller won't resume' and no 'HC died'
> >> messages, but there are only those with PME# and bus mastering. This
> >> seems quite fine! (as a side, there's still the failure to flush SSD
> >> cache and failure to turn off SSD gracefully, but that's another bug)
> In case 1. it shows:
> ohci_rh_resume: control 240
> 
> (that's 100100)
> https://i.imgur.com/5Wwwxwf.jpg
> >>
> >> 2. if I don't manually poweroff from debug.sh and therefore allow the
> >> normal shutdown process to continue, then I get those errors with
> >> 'controller won't resume' and 'HC died'
> >> It looks like something is at least trying to prevent those from
> >> waking up but I'm not sure if it's something in device_shutdown() or
> >> something before do_poweroff(), because apparently both
> >> sysrq+o(poweroff) and normal poweroff have the same stacktrace(?i
> >> should probably check this)
> In case 2. it shows:
> ohci_rh_resume: control 2c0
> 
> (that's 101100)
> https://i.imgur.com/8sxP70x.jpg

Well, 240 means the controller is in the RESUME state (which is what it 
should be at that point), and 2c0 means the controller is still in the 
SUSPEND state.  So even after the driver told the controller to change 
from SUSPEND to RESUME, it's still in SUSPEND.

The spec doesn't list any reasons why this might happen.  All I can 
guess is that the controller needs some input that the shutdown 
procedure has turned off.

Alan Stern

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

[PATCH resend 0/1] phy-sun4i-usb: Add support for peripheral-only mode

[Hans de Goede]

Hi Kishon,

As discussed before, here is a resend now that rc1 is out (and includes
the new of_usb_get_dr_mode_by_phy helper this patch needs).

Please merge this for 4.9-rcX some of the musb-sunxi glue changes
already merged have a runtime dependency on this patch, without
this patch boards using the musb controller in host-only mode
will break.

Thanks & Regards,

Hans


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

[PATCH resend] phy-sun4i-usb: Add support for peripheral-only mode

[Hans de Goede]

Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
from the musb controller node instead of assuming that having an id_det
gpio means otg mode, and not having one means host mode.

Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
helper which looks at the dr_mode, always registering our extcon and
always monitoring vbus.

If dr_mode is not specified in the dts, do not register phy0 as we then
do not know how to treat it. This is actually a good thing as this means
we will not be registering phy0 on devices where the otg controller is
not enabled in the devicetree.

Signed-off-by: Hans de Goede 
Acked-by: Kishon Vijay Abraham I 
---
Changes in v2:
-Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
Changes in v3:
-Only toggle the phy vbus-det bit on id-change on systems without vbus-det
 when in otg mode
Changes in v4:
-No changes
Changes in v5:
-Added Kishon's Acked-by
Changes in v6:
-Rebase on top of latest linux-phy/next
---
 drivers/phy/phy-sun4i-usb.c | 68 ++---
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 0a45bc6..8c7eb33 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define REG_ISCR   0x00
@@ -110,6 +111,7 @@ struct sun4i_usb_phy_cfg {
 struct sun4i_usb_phy_data {
void __iomem *base;
const struct sun4i_usb_phy_cfg *cfg;
+   enum usb_dr_mode dr_mode;
struct mutex mutex;
struct sun4i_usb_phy {
struct phy *phy;
@@ -120,6 +122,7 @@ struct sun4i_usb_phy_data {
bool regulator_on;
int index;
} phys[MAX_PHYS];
+   int first_phy;
/* phy0 / otg related variables */
struct extcon_dev *extcon;
bool phy0_init;
@@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
 
-   if (data->id_det_gpio) {
-   /* OTG mode, force ISCR and cable state updates */
-   data->id_det = -1;
-   data->vbus_det = -1;
-   queue_delayed_work(system_wq, >detect, 0);
-   } else {
-   /* Host only mode */
-   sun4i_usb_phy0_set_id_detect(_phy, 0);
-   sun4i_usb_phy0_set_vbus_detect(_phy, 1);
-   }
+   /* Force ISCR and cable state updates */
+   data->id_det = -1;
+   data->vbus_det = -1;
+   queue_delayed_work(system_wq, >detect, 0);
}
 
return 0;
@@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
return 0;
 }
 
+static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
+{
+   switch (data->dr_mode) {
+   case USB_DR_MODE_OTG:
+   return gpiod_get_value_cansleep(data->id_det_gpio);
+   case USB_DR_MODE_HOST:
+   return 0;
+   case USB_DR_MODE_PERIPHERAL:
+   default:
+   return 1;
+   }
+}
+
 static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
 {
if (data->vbus_det_gpio)
@@ -432,7 +442,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
struct phy *phy0 = data->phys[0].phy;
int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
 
-   id_det = gpiod_get_value_cansleep(data->id_det_gpio);
+   if (phy0 == NULL)
+   return;
+
+   id_det = sun4i_usb_phy0_get_id_det(data);
vbus_det = sun4i_usb_phy0_get_vbus_det(data);
 
mutex_lock(>mutex);
@@ -448,7 +461,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
 * without vbus detection report vbus low for long enough for
 * the musb-ip to end the current device session.
 */
-   if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
+   if (data->dr_mode == USB_DR_MODE_OTG &&
+   !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
sun4i_usb_phy0_set_vbus_detect(phy0, 0);
msleep(200);
sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -474,7 +488,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
 * without vbus detection report vbus low for long enough to
 * the musb-ip to end the current host session.
 */
-   if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
+   if (data->dr_mode == USB_DR_MODE_OTG &&
+   !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) 

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[ican realizeum]

On Mon, Aug 8, 2016 at 8:41 PM, Alan Stern  wrote:
> On Mon, 8 Aug 2016, ican realizeum wrote:
>
>> Ok, here's what I found out:
>> tlp being enabled in /etc/default/tlp (as TLP_ENABLE=1) would always
>> set those to 'auto' aka they show up as 'suspended', regardless of
>> what I did (eg. set them 'on') before executing shutdown(from X, in
>> menu, Shutdown)
>> Even if TLP_ENABLE=0 during startup, if I set it to =1 before
>> shutdown, it will put them in suspend.
>>
>> With TLP_ENABLE=1,
>> and in /usr/lib/systemd/system-shutdown/debug.sh I put some lines to
>> show me the runtime_status
>> it's always 'suspended' on all four (12.0 12.2 13.0 13.2) and:
>> 1. if I have line in debug.sh which attempts to poweroff manually via
>> sysrq, like this:
>> echo o > /proc/sysrq-trigger ; sleep 5
>> then, there are no 'controller won't resume' and no 'HC died'
>> messages, but there are only those with PME# and bus mastering. This
>> seems quite fine! (as a side, there's still the failure to flush SSD
>> cache and failure to turn off SSD gracefully, but that's another bug)
In case 1. it shows:
ohci_rh_resume: control 240

(that's 100100)
https://i.imgur.com/5Wwwxwf.jpg
>>
>> 2. if I don't manually poweroff from debug.sh and therefore allow the
>> normal shutdown process to continue, then I get those errors with
>> 'controller won't resume' and 'HC died'
>> It looks like something is at least trying to prevent those from
>> waking up but I'm not sure if it's something in device_shutdown() or
>> something before do_poweroff(), because apparently both
>> sysrq+o(poweroff) and normal poweroff have the same stacktrace(?i
>> should probably check this)
In case 2. it shows:
ohci_rh_resume: control 2c0

(that's 101100)
https://i.imgur.com/8sxP70x.jpg
>>
>> If I turn off tlp (=0) then all four(ohci-s) are 'active' and there
>> are no ohci messages whatsoever! (regardless of doing a manual sysrq+o
>> poweroff or allowing the normal poweroff to happen)
>>
>> //Now, this makes me wonder what have I tried with tlp off in the
>> first post that it still gave the errors - I'll assume I did something
>> wrong and they were still suspended - but they shouldn't have been  if
>> tlp was off (unless maybe I changed TLP_ENABLE  to =1 before shutdown
>> thinking it will not have effect on the current shutdown! but it
>> always has! as I've just found out)
>>
>> The only question remains then, why in the case 2. above, it can't
>> power them on(and thus cause the errors in the title), but in case 1.
>> it can(without the errors) ?
>
> Good question.  Unfortunately, I can't think of a good way to find the
> answer.
>
> What shows up in the log with the patch below?
>
> Alan Stern
>
>
>
> Index: usb-4.x/drivers/usb/host/ohci-hub.c
> ===
> --- usb-4.x.orig/drivers/usb/host/ohci-hub.c
> +++ usb-4.x/drivers/usb/host/ohci-hub.c
> @@ -212,6 +212,7 @@ __acquires(ohci->lock)
> msleep (20 /* usb 11.5.1.10 */ + 12 /* 32 msec counter */ + 1);
>
> temp = ohci_readl (ohci, >regs->control);
> +   ohci_info(ohci, "ohci_rh_resume: control %x\n", temp);
> temp &= OHCI_CTRL_HCFS;
> if (temp != OHCI_USB_RESUME) {
> ohci_err (ohci, "controller won't resume\n");
>
Thanks for this patch, Alan. Much appreciated as I had no idea how to
use ochi_info().
Cheers!
--
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 net-next] cdc_ether: Improve ZTE MF823/831/910 handling

[Bjørn Mork]

Oliver Neukum  writes:

> But why fix similar issues at two different places? And what about
> PCI or other cards that show the same problem?

I guess some sort of common helper would be nice to avoid open coding
this fix everywhere.  But you would still have to modify every driver
where it is applicable, as there is no existing common API.

Note that this doesn't include *every* ethernet driver, although there
certainly are some examples.  There are also a number of serious
vendors, providing vendor supported drivers for cards with no known
issues of this kind.

Where exactly would you like to see this implemented if it isn't going
into those specific usbnet drivers?


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

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[Alan Stern]

On Mon, 8 Aug 2016, ican realizeum wrote:

> Ok, here's what I found out:
> tlp being enabled in /etc/default/tlp (as TLP_ENABLE=1) would always
> set those to 'auto' aka they show up as 'suspended', regardless of
> what I did (eg. set them 'on') before executing shutdown(from X, in
> menu, Shutdown)
> Even if TLP_ENABLE=0 during startup, if I set it to =1 before
> shutdown, it will put them in suspend.
> 
> With TLP_ENABLE=1,
> and in /usr/lib/systemd/system-shutdown/debug.sh I put some lines to
> show me the runtime_status
> it's always 'suspended' on all four (12.0 12.2 13.0 13.2) and:
> 1. if I have line in debug.sh which attempts to poweroff manually via
> sysrq, like this:
> echo o > /proc/sysrq-trigger ; sleep 5
> then, there are no 'controller won't resume' and no 'HC died'
> messages, but there are only those with PME# and bus mastering. This
> seems quite fine! (as a side, there's still the failure to flush SSD
> cache and failure to turn off SSD gracefully, but that's another bug)
> 
> 2. if I don't manually poweroff from debug.sh and therefore allow the
> normal shutdown process to continue, then I get those errors with
> 'controller won't resume' and 'HC died'
> It looks like something is at least trying to prevent those from
> waking up but I'm not sure if it's something in device_shutdown() or
> something before do_poweroff(), because apparently both
> sysrq+o(poweroff) and normal poweroff have the same stacktrace(?i
> should probably check this)
> 
> If I turn off tlp (=0) then all four(ohci-s) are 'active' and there
> are no ohci messages whatsoever! (regardless of doing a manual sysrq+o
> poweroff or allowing the normal poweroff to happen)
> 
> //Now, this makes me wonder what have I tried with tlp off in the
> first post that it still gave the errors - I'll assume I did something
> wrong and they were still suspended - but they shouldn't have been  if
> tlp was off (unless maybe I changed TLP_ENABLE  to =1 before shutdown
> thinking it will not have effect on the current shutdown! but it
> always has! as I've just found out)
> 
> The only question remains then, why in the case 2. above, it can't
> power them on(and thus cause the errors in the title), but in case 1.
> it can(without the errors) ?

Good question.  Unfortunately, I can't think of a good way to find the 
answer.

What shows up in the log with the patch below?

Alan Stern



Index: usb-4.x/drivers/usb/host/ohci-hub.c
===
--- usb-4.x.orig/drivers/usb/host/ohci-hub.c
+++ usb-4.x/drivers/usb/host/ohci-hub.c
@@ -212,6 +212,7 @@ __acquires(ohci->lock)
msleep (20 /* usb 11.5.1.10 */ + 12 /* 32 msec counter */ + 1);
 
temp = ohci_readl (ohci, >regs->control);
+   ohci_info(ohci, "ohci_rh_resume: control %x\n", temp);
temp &= OHCI_CTRL_HCFS;
if (temp != OHCI_USB_RESUME) {
ohci_err (ohci, "controller won't resume\n");

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

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[ican realizeum]

On Mon, Aug 8, 2016 at 7:43 PM, ican realizeum  wrote:
> On Mon, Aug 8, 2016 at 5:38 PM, Alan Stern  wrote:
>> On Mon, 8 Aug 2016, ican realizeum wrote:
>>
>>> >> The caller was hcd_resume_work(), which is part of a workqueue.  This
>>> >> routine runs when the OHCI driver calls usb_hcd_resume_root_hub().
>>> >> There are only 3 places in the driver where this happens (two in
>>> >> ohci-hcd.c and one in ohci-hub.c), but as far as I can tell, none of
>>> >> them should have occurred if power/control was set to "on".  If you
>>> >> want, you can add an ohci_info() before each of those calls; knowing
>>> >> which one it was will probably tell us what happened.
>>> I didn't know how to use ohci_info() but I used dump_stack()
>>> https://i.imgur.com/VMak4ED.jpg
>>> it's from ohci_resume()
>>
>> Are you sure you wrote "on" to the power/control file for the usb3 and
>> usb4 devices?  The stack dump says that the :00:12.0 and
>> :00:13.0 controllers were being runtime-resumed, but they should
>> not have been suspended to begin with, because their children (the
>> usb3 and usb4 devices) should have been prevented from going into
>> runtime suspend.
>>
> Ok, here's what I found out:
> tlp being enabled in /etc/default/tlp (as TLP_ENABLE=1) would always
> set those to 'auto' aka they show up as 'suspended', regardless of
> what I did (eg. set them 'on') before executing shutdown(from X, in
> menu, Shutdown)
> Even if TLP_ENABLE=0 during startup, if I set it to =1 before
> shutdown, it will put them in suspend.
>
> With TLP_ENABLE=1,
> and in /usr/lib/systemd/system-shutdown/debug.sh I put some lines to
> show me the runtime_status
> it's always 'suspended' on all four (12.0 12.2 13.0 13.2) and:
> 1. if I have line in debug.sh which attempts to poweroff manually via
> sysrq, like this:
> echo o > /proc/sysrq-trigger ; sleep 5
> then, there are no 'controller won't resume' and no 'HC died'
> messages, but there are only those with PME# and bus mastering. This
> seems quite fine! (as a side, there's still the failure to flush SSD
> cache and failure to turn off SSD gracefully, but that's another bug)
forgot to include stacktrace for this case:
https://i.imgur.com/T32GYom.jpg

>
> 2. if I don't manually poweroff from debug.sh and therefore allow the
> normal shutdown process to continue, then I get those errors with
> 'controller won't resume' and 'HC died'
> It looks like something is at least trying to prevent those from
> waking up but I'm not sure if it's something in device_shutdown() or
> something before do_poweroff(), because apparently both
> sysrq+o(poweroff) and normal poweroff have the same stacktrace(?i
> should probably check this)
for reference the stacktrace for this case was this:
https://i.imgur.com/VMak4ED.jpg
>
> If I turn off tlp (=0) then all four(ohci-s) are 'active' and there
> are no ohci messages whatsoever! (regardless of doing a manual sysrq+o
> poweroff or allowing the normal poweroff to happen)
>
> //Now, this makes me wonder what have I tried with tlp off in the
> first post that it still gave the errors - I'll assume I did something
> wrong and they were still suspended - but they shouldn't have been  if
> tlp was off (unless maybe I changed TLP_ENABLE  to =1 before shutdown
> thinking it will not have effect on the current shutdown! but it
> always has! as I've just found out)
>
> The only question remains then, why in the case 2. above, it can't
> power them on(and thus cause the errors in the title), but in case 1.
> it can(without the errors) ?
>
>> You might want to check the power/runtime_status contents in the
>> relevant device directories before doing the shutdown.
>>
>> Alan Stern
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[Felipe Ferreri Tonello]

Hi,

On 05-08-2016 20:15, kbuild test robot wrote:
> Hi Felipe,
> 
> [auto build test ERROR on balbi-usb/next]
> [also build test ERROR on v4.7 next-20160805]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Felipe-F-Tonello/Gadget-endpoint-request-allocation-and-MIDI/20160806-024520
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> config: x86_64-randconfig-x013-201631 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> Note: the 
> linux-review/Felipe-F-Tonello/Gadget-endpoint-request-allocation-and-MIDI/20160806-024520
>  HEAD 5064a41cd5f89103e3b75c1a2ab9f6e98851503b builds fine.
>   It only hurts bisectibility.
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/usb/gadget/u_f.c:17:21: error: conflicting types for 'alloc_ep_req'
> struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int 
> default_len)
> ^~~~
>In file included from drivers/usb/gadget/u_f.c:14:0:
>drivers/usb/gadget/u_f.h:63:21: note: previous declaration of 
> 'alloc_ep_req' was here
> struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
> default_len);

Ok, I made the mistake to change to size_t the len type just on the
function declaration. I'll fix this on a v4.

> ^~~~
> 
> vim +/alloc_ep_req +17 drivers/usb/gadget/u_f.c
> 
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  11   * published by the Free 
> Software Foundation.
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  12   */
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  13  
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  14  #include "u_f.h"
> a7ffc68f Felipe F. Tonello 2016-08-05  15  #include 
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  16  
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07 @17  struct usb_request 
> *alloc_ep_req(struct usb_ep *ep, int len, int default_len)
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  18  {
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  19 struct usb_request  
> *req;
> 1efd54ea Andrzej Pietrasiewicz 2013-11-07  20  
> 
> :: The code at line 17 was first introduced by commit
> :: 1efd54eab2b60c68c2ce75ea635306cef847d751 usb: gadget: factor out 
> alloc_ep_req
> 
> :: TO: Andrzej Pietrasiewicz 
> :: CC: Felipe Balbi 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 

Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[ican realizeum]

On Mon, Aug 8, 2016 at 5:38 PM, Alan Stern  wrote:
> On Mon, 8 Aug 2016, ican realizeum wrote:
>
>> >> The caller was hcd_resume_work(), which is part of a workqueue.  This
>> >> routine runs when the OHCI driver calls usb_hcd_resume_root_hub().
>> >> There are only 3 places in the driver where this happens (two in
>> >> ohci-hcd.c and one in ohci-hub.c), but as far as I can tell, none of
>> >> them should have occurred if power/control was set to "on".  If you
>> >> want, you can add an ohci_info() before each of those calls; knowing
>> >> which one it was will probably tell us what happened.
>> I didn't know how to use ohci_info() but I used dump_stack()
>> https://i.imgur.com/VMak4ED.jpg
>> it's from ohci_resume()
>
> Are you sure you wrote "on" to the power/control file for the usb3 and
> usb4 devices?  The stack dump says that the :00:12.0 and
> :00:13.0 controllers were being runtime-resumed, but they should
> not have been suspended to begin with, because their children (the
> usb3 and usb4 devices) should have been prevented from going into
> runtime suspend.
>
Ok, here's what I found out:
tlp being enabled in /etc/default/tlp (as TLP_ENABLE=1) would always
set those to 'auto' aka they show up as 'suspended', regardless of
what I did (eg. set them 'on') before executing shutdown(from X, in
menu, Shutdown)
Even if TLP_ENABLE=0 during startup, if I set it to =1 before
shutdown, it will put them in suspend.

With TLP_ENABLE=1,
and in /usr/lib/systemd/system-shutdown/debug.sh I put some lines to
show me the runtime_status
it's always 'suspended' on all four (12.0 12.2 13.0 13.2) and:
1. if I have line in debug.sh which attempts to poweroff manually via
sysrq, like this:
echo o > /proc/sysrq-trigger ; sleep 5
then, there are no 'controller won't resume' and no 'HC died'
messages, but there are only those with PME# and bus mastering. This
seems quite fine! (as a side, there's still the failure to flush SSD
cache and failure to turn off SSD gracefully, but that's another bug)

2. if I don't manually poweroff from debug.sh and therefore allow the
normal shutdown process to continue, then I get those errors with
'controller won't resume' and 'HC died'
It looks like something is at least trying to prevent those from
waking up but I'm not sure if it's something in device_shutdown() or
something before do_poweroff(), because apparently both
sysrq+o(poweroff) and normal poweroff have the same stacktrace(?i
should probably check this)

If I turn off tlp (=0) then all four(ohci-s) are 'active' and there
are no ohci messages whatsoever! (regardless of doing a manual sysrq+o
poweroff or allowing the normal poweroff to happen)

//Now, this makes me wonder what have I tried with tlp off in the
first post that it still gave the errors - I'll assume I did something
wrong and they were still suspended - but they shouldn't have been  if
tlp was off (unless maybe I changed TLP_ENABLE  to =1 before shutdown
thinking it will not have effect on the current shutdown! but it
always has! as I've just found out)

The only question remains then, why in the case 2. above, it can't
power them on(and thus cause the errors in the title), but in case 1.
it can(without the errors) ?

> You might want to check the power/runtime_status contents in the
> relevant device directories before doing the shutdown.
>
> Alan Stern
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] xhci:Remove unused marco definitions from the file xhci-hub.c

[Greg KH]

On Mon, Aug 08, 2016 at 12:00:38PM -0400, Nicholas Krause wrote:
> This removes the unneeded marco definitions for the marcos
> of XHCI_PORT_RW1S, XHCI_PORT_RW1C, XHCI_PORT_RWand
> XHCI_PORT_RZ due to no uses of these marcos in the file
> xhci-hub.c or any other related kernel source code file
> related to supporting xhci host controllers.
> 
> Signed-off-by: Nicholas Krause 
> ---
>  drivers/usb/host/xhci-hub.c | 23 ---
>  1 file changed, 23 deletions(-)

Again, please stop sending patches, they will be ignored.
--
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

[PACTH v2] cdc-wdm: Clear read pipeline in case of error

[robert . foss]

From: Robert Foss 

Implemented queued response handling. This queue is processed every time the
WDM_READ flag is cleared.

In case of a read error, userspace may not actually read the data, since the
driver returns an error through wdm_poll. After this, the underlying device may
attempt to send us more data, but the queue is not processed. While userspace is
also blocked, because the read error is never cleared.

After this patch, we proactively process the queue on a read error. If there was
an outstanding response to handle, that will clear the error (or go through the
same logic again, if another read error occurs). If there was no outstanding
response, this will bring the queue size back to 0, unblocking a future response
from the underlying device.

Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
---

Changes since v1:
- Only set a new error code it there is no previous error code

 drivers/usb/class/cdc-wdm.c | 39 +--
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 337948c..f12a006 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -154,6 +154,9 @@ static void wdm_out_callback(struct urb *urb)
wake_up(>wait);
 }
 
+/* forward declaration */
+static int service_outstanding_interrupt(struct wdm_device *desc);
+
 static void wdm_in_callback(struct urb *urb)
 {
struct wdm_device *desc = urb->context;
@@ -188,7 +191,13 @@ static void wdm_in_callback(struct urb *urb)
}
}
 
-   desc->rerr = status;
+   /*
+* only set a new error if there is no previous error.
+* Errors are only cleared during read/open
+*/
+   if (desc->rerr  == 0)
+   desc->rerr = status;
+
if (length + desc->length > desc->wMaxCommand) {
/* The buffer would overflow */
set_bit(WDM_OVERFLOW, >flags);
@@ -201,9 +210,19 @@ static void wdm_in_callback(struct urb *urb)
}
}
 skip_error:
+   set_bit(WDM_READ, >flags);
wake_up(>wait);
 
-   set_bit(WDM_READ, >flags);
+   if (desc->rerr) {
+   /*
+* Since there was an error, userspace may decide to not read
+* any data after poll'ing.
+* We should respond to further attempts from the device to send
+* data, so that we can get unstuck.
+*/
+   service_outstanding_interrupt(desc);
+   }
+
spin_unlock(>iuspin);
 }
 
@@ -436,17 +455,14 @@ out_free_mem:
 }
 
 /*
- * clear WDM_READ flag and possibly submit the read urb if resp_count
- * is non-zero.
+ * Submit the read urb if resp_count is non-zero.
  *
  * Called with desc->iuspin locked
  */
-static int clear_wdm_read_flag(struct wdm_device *desc)
+static int service_outstanding_interrupt(struct wdm_device *desc)
 {
int rv = 0;
 
-   clear_bit(WDM_READ, >flags);
-
/* submit read urb only if the device is waiting for it */
if (!desc->resp_count || !--desc->resp_count)
goto out;
@@ -538,7 +554,8 @@ retry:
 
if (!desc->reslength) { /* zero length read */
dev_dbg(>intf->dev, "%s: zero length - clearing 
WDM_READ\n", __func__);
-   rv = clear_wdm_read_flag(desc);
+   clear_bit(WDM_READ, >flags);
+   rv = service_outstanding_interrupt(desc);
spin_unlock_irq(>iuspin);
if (rv < 0)
goto err;
@@ -563,8 +580,10 @@ retry:
 
desc->length -= cntr;
/* in case we had outstanding data */
-   if (!desc->length)
-   clear_wdm_read_flag(desc);
+   if (!desc->length) {
+   clear_bit(WDM_READ, >flags);
+   service_outstanding_interrupt(desc);
+   }
spin_unlock_irq(>iuspin);
rv = cntr;
 
-- 
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

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[Alan Stern]

On Mon, 8 Aug 2016, ican realizeum wrote:

> >> The caller was hcd_resume_work(), which is part of a workqueue.  This
> >> routine runs when the OHCI driver calls usb_hcd_resume_root_hub().
> >> There are only 3 places in the driver where this happens (two in
> >> ohci-hcd.c and one in ohci-hub.c), but as far as I can tell, none of
> >> them should have occurred if power/control was set to "on".  If you
> >> want, you can add an ohci_info() before each of those calls; knowing
> >> which one it was will probably tell us what happened.
> I didn't know how to use ohci_info() but I used dump_stack()
> https://i.imgur.com/VMak4ED.jpg
> it's from ohci_resume()

Are you sure you wrote "on" to the power/control file for the usb3 and
usb4 devices?  The stack dump says that the :00:12.0 and
:00:13.0 controllers were being runtime-resumed, but they should
not have been suspended to begin with, because their children (the
usb3 and usb4 devices) should have been prevented from going into 
runtime suspend.

You might want to check the power/runtime_status contents in the 
relevant device directories before doing the shutdown.

Alan Stern

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

Re: [PACTH v1] cdc-wdm: Clear read pipeline in case of error

[Robert Foss]

On 2016-08-07 04:59 AM, Oliver Neukum wrote:

On Thu, 2016-08-04 at 13:44 -0400, Robert Foss wrote:


On 2016-08-03 06:39 AM, Oliver Neukum wrote:

On Tue, 2016-08-02 at 10:37 -0400, Robert Foss wrote:



How can that depend on what we return to user space?
In the driver we can continue just ignoring errors.
Now, if user space stops reading because we reported an error,
that is the decision user space has made. We cannot ignore errors
in the kernel because we don't like what user space does when it
sees the error.


So perhaps the better solution is to be more intelligent about how
desc->rerr is written to during after an error to be able to maintain
the error condition?


Yes, good idea. I think an error condition should never be overwritten.
So we go to the current behaviour only if a second error before
user space has seen the3 first error arises. Would that fix your
issue?


Excellent!

As long as the device is able to keep pushing data during an -EPIPE 
error condition, I think my issues would be solved.

--
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 net-next] cdc_ether: Improve ZTE MF823/831/910 handling

[Oliver Neukum]

On Mon, 2016-08-08 at 14:44 +0200, Bjørn Mork wrote:
> Oliver Neukum  writes:

> > I don't see how it would be specific for a subsystem. If the patch
> > is correct, it belongs into the networking core.
> 
> The bug is in the firmware implementation of the "read unique vendor
> assigned mac address" functions, and should therefore be fixed there.

Well, if you define the semantics of the operation in that manner, it
certainly does. That however is not given. You could also define it
as returning the MAC the hardware listens to. The difference was just
unclear when the API was defined.

> It cannot be put into the networking core because "read unique vendor
> assigned mac address" is a hardware dependent function.  It's
> implemented in each ethernet driver based of whatever interface the
> firmware provides to that register.

Again, that depends on that particular semantics.

> IMHO, usbnet_get_ethernet_addr() would be the most appropriate place for
> cdc_ether and other CDC drivers. And generic_rndis_bind() is the correct
> place for rndis_host.
> 
> Putting this in usbnet_get_ethernet_addr() will also fix the XMM7160
> based devices having an FF:FF:FF:FF:FF:FF mac address (sic).  I'm pretty
> sure there are other examples too.  There is no end to the creative
> crazyness of firmware engineers...

It is clear that that would work. No question about that.

But why fix similar issues at two different places? And what about
PCI or other cards that show the same problem?

Regards
Oliver


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

Re: [PATCH v3,4/5] usb: Add MediaTek USB3 DRD Driver

[Greg Kroah-Hartman]

On Fri, Jun 10, 2016 at 03:32:41PM +0800, Chunfeng Yun wrote:
> --- /dev/null
> +++ b/drivers/usb/mtu3/Makefile
> @@ -0,0 +1,20 @@
> +
> +#ifeq ($(CONFIG_USB_DEBUG),y)
> + ccflags-y   += -DDEBUG
> +#endif

There is no CONFIG_USB_DEBUG in the tree anymore, are you sure you
tested this?  :)

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: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[ican realizeum]

On Mon, Aug 8, 2016 at 11:42 AM, ican realizeum  wrote:
> Sorry, I just realized that in gmail `Default reply behaviour` was set
> to `Reply` not `Reply All`, so I skipped the list when I replied
> last(for I forgot to select Reply All as I did before). Oops. So now
> hopefully everyone else can see the quoted previous email below.
>
> On Mon, Aug 8, 2016 at 4:35 AM, Alan Stern  wrote:
>> On Mon, 8 Aug 2016, ican realizeum wrote:
>>
>>> Roger that. Here's what I found out:
>>> if I try to sysrq+o (aka poweroff) manually from a file that systemd
>>> runs at shutdown such as manually created a+x file:
>>> /usr/lib/systemd/system-shutdown/debug.sh
>>>
>>> then I get no ohci errors,
>>> screenshot: https://i.imgur.com/VVeAyj7.jpg
>>> only the "PME# disabled" and "enabling bus mastering" are seen.
>>> But if I allow the kernel(and/or whatever else happens before that) to
>>> do it, then the errors are back just like in my original post.
>>
>> The difference isn't clear.
>>
>>> The stack is this:
>>> https://i.imgur.com/9wlbure.jpg
>>> but it looks like something else called that and I don't know
>>> what(looks forked?), unless it's something from the following stack
>>> from the point where "Stop disk" happens:
>>> https://i.imgur.com/z9uq4ez.jpg
>>> Maybe I'll find out something from there.
>>
>> The caller was hcd_resume_work(), which is part of a workqueue.  This
>> routine runs when the OHCI driver calls usb_hcd_resume_root_hub().
>> There are only 3 places in the driver where this happens (two in
>> ohci-hcd.c and one in ohci-hub.c), but as far as I can tell, none of
>> them should have occurred if power/control was set to "on".  If you
>> want, you can add an ohci_info() before each of those calls; knowing
>> which one it was will probably tell us what happened.
I didn't know how to use ohci_info() but I used dump_stack()
https://i.imgur.com/VMak4ED.jpg
it's from ohci_resume()

>>
>> However, this doesn't answer the question of why the resume failed.
>> My best guess is that it failed because something it needed had already
>> been shut down.
>>
>> Alan Stern
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] usb: renesas_usbhs: Fix receiving data corrupt on R-Car Gen3 with dmac

[Yoshihiro Shimoda]

Since R-Car Gen3 SoC has the USB-DMAC, this driver should set
dparam->has_usb_dmac to 1. Otherwise, behavior of this driver and
the usb-dmac driver will be mismatch, then sometimes receiving data will
be corrupt.

Fixes: de18757e272d ("usb: renesas_usbhs: add R-Car Gen3 power control")
Cc:  # v4.5+
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index 8fbbc2d..ac67bab 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -514,7 +514,8 @@ static struct renesas_usbhs_platform_info 
*usbhs_parse_dt(struct device *dev)
if (gpio > 0)
dparam->enable_gpio = gpio;
 
-   if (dparam->type == USBHS_TYPE_RCAR_GEN2)
+   if (dparam->type == USBHS_TYPE_RCAR_GEN2 ||
+   dparam->type == USBHS_TYPE_RCAR_GEN3)
dparam->has_usb_dmac = 1;
 
return info;
-- 
1.9.1

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

[PATCH 2/3] usb: renesas_usbhs: clear the BRDYSTS in usbhsg_ep_enable()

[Yoshihiro Shimoda]

This patch fixes an issue that unexpected BRDY interruption happens
when the usb_ep_{enable,disable}() are called with different direction.
In this case, the driver will cause the following message:

 renesas_usbhs e659.usb: irq_ready run_error 1 : -16

This issue causes the followings:
 1) A pipe is enabled as transmission
 2) The pipe sent a data
 3) The pipe is disabled and re-enabled as reception.
 4) The pipe got a queue

Since the driver doesn't clear the BRDYSTS flags after 2) above, the issue
happens. If we add such clearing the flags into the driver, the code will
become complicate. So, this patch clears the BRDYSTS flag of reception in
usbhsg_ep_enable() to avoid complicate.

Cc:  # v4.1+ (usbhs_xxxsts_clear() is needed)
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/mod_gadget.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c 
b/drivers/usb/renesas_usbhs/mod_gadget.c
index 50f3363..92bc83b 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -617,10 +617,13 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
 * use dmaengine if possible.
 * It will use pio handler if impossible.
 */
-   if (usb_endpoint_dir_in(desc))
+   if (usb_endpoint_dir_in(desc)) {
pipe->handler = _fifo_dma_push_handler;
-   else
+   } else {
pipe->handler = _fifo_dma_pop_handler;
+   usbhs_xxxsts_clear(priv, BRDYSTS,
+  usbhs_pipe_number(pipe));
+   }
 
ret = 0;
}
-- 
1.9.1

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

[PATCH 3/3] usb: renesas_usbhs: Use dmac only if the pipe type is bulk

[Yoshihiro Shimoda]

This patch fixes an issue that isochronous transfer's data is possible to
be lost as a workaround. Since this driver uses a workqueue to start
the dmac, the transfer is possible to be delayed when system load is high.

Fixes: 6e4b74e4690d ("usb: renesas: fix scheduling in atomic context bug")
Cc:  # v3.4+
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/fifo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 280ed5f..857e783 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -871,7 +871,7 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
 
/* use PIO if packet is less than pio_dma_border or pipe is DCP */
if ((len < usbhs_get_dparam(priv, pio_dma_border)) ||
-   usbhs_pipe_is_dcp(pipe))
+   usbhs_pipe_type_is(pipe, USB_ENDPOINT_XFER_ISOC))
goto usbhsf_pio_prepare_push;
 
/* check data length if this driver don't use USB-DMAC */
@@ -976,7 +976,7 @@ static int usbhsf_dma_prepare_pop_with_usb_dmac(struct 
usbhs_pkt *pkt,
 
/* use PIO if packet is less than pio_dma_border or pipe is DCP */
if ((pkt->length < usbhs_get_dparam(priv, pio_dma_border)) ||
-   usbhs_pipe_is_dcp(pipe))
+   usbhs_pipe_type_is(pipe, USB_ENDPOINT_XFER_ISOC))
goto usbhsf_pio_prepare_pop;
 
fifo = usbhsf_get_dma_fifo(priv, pkt);
-- 
1.9.1

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

[PATCH 0/3] usb: renesas_usbhs: fix the driver for v4.8

[Yoshihiro Shimoda]

This patch set is based on the latest Felipe's usb.git / testing/fixes branch.
(The commit id = 10493a3c0718 ("usb: dwc3: gadget: increment request->actual 
once")

Yoshihiro Shimoda (3):
  usb: renesas_usbhs: Fix receiving data corrupt on R-Car Gen3 with dmac
  usb: renesas_usbhs: clear the BRDYSTS in usbhsg_ep_enable()
  usb: renesas_usbhs: Use dmac only if the pipe type is bulk

 drivers/usb/renesas_usbhs/common.c | 3 ++-
 drivers/usb/renesas_usbhs/fifo.c   | 4 ++--
 drivers/usb/renesas_usbhs/mod_gadget.c | 7 +--
 3 files changed, 9 insertions(+), 5 deletions(-)

-- 
1.9.1

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

Re: [PATCH net-next] cdc_ether: Improve ZTE MF823/831/910 handling

[Bjørn Mork]

Oliver Neukum  writes:
> On Mon, 2016-07-18 at 16:10 +0200, Kristian Evensen wrote:
>> On Mon, Jul 18, 2016 at 3:50 PM, Oliver Neukum  wrote:
>> > No, you misunderstand me. I don't want quirks if we can avoid it.
>> > But if you need to do this for rndis_host and cdc_ether and maybe other
>> > drivers you should not be touching drivers. This belongs into the core
>> > ethernet code. Your code is good, but you are putting it in the wrong
>> > places.
>> 
>> Ok, sounds good. So far, I have only seen the random MAC issue with
>> the three previously mentioned devices, but who knows how many else is
>> out there with the same error ... I don't think it should be in the
>> core ethernet code, at least not yet, but I agree it would make sense
>> to move it to for example usbnet_core(). If you agree, I can prepare a
>> patch for it.
>
> I don't see how it would be specific for a subsystem. If the patch
> is correct, it belongs into the networking core.

The bug is in the firmware implementation of the "read unique vendor
assigned mac address" functions, and should therefore be fixed there.
It cannot be put into the networking core because "read unique vendor
assigned mac address" is a hardware dependent function.  It's
implemented in each ethernet driver based of whatever interface the
firmware provides to that register.

IMHO, usbnet_get_ethernet_addr() would be the most appropriate place for
cdc_ether and other CDC drivers. And generic_rndis_bind() is the correct
place for rndis_host.

Putting this in usbnet_get_ethernet_addr() will also fix the XMM7160
based devices having an FF:FF:FF:FF:FF:FF mac address (sic).  I'm pretty
sure there are other examples too.  There is no end to the creative
crazyness of firmware engineers...

An lsusb snippet example:

Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 2 Communications
  bInterfaceSubClass 13 
  bInterfaceProtocol  0 
  iInterface  5 Sierra Wireless EM7345 4G LTE (NCM)
  CDC Header:
bcdCDC   1.20
  CDC Union:
bMasterInterface0
bSlaveInterface 1 
  CDC NCM:
bcdNcmVersion1.00
bmNetworkCapabilities 0x00
  CDC Ethernet:
iMacAddress  6 
bmEthernetStatistics0x
wMaxSegmentSize   1514
wNumberMCFilters0x
bNumberPowerFilters  0


FWIW, putting the fix in usbnet_get_ethernet_addr() will not be a
problem for qmi_wwan.  It will further fix up the resulting random
address if required.


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

Re: [PATCH v3] cdc-wdm: fix "out-of-sync" due to missing notifications

[Greg Kroah-Hartman]

On Mon, Aug 08, 2016 at 02:12:06PM +0200, Bjørn Mork wrote:
> Oliver Neukum  writes:
> > On Sun, 2016-07-10 at 17:45 +0200, Bjørn Mork wrote:
> >> The workaround has been tested on a large number of different MBIM
> >> and QMI devices, as well as the Ericsson F5521gw and H5321gw modems
> >> with real Device Management functions.
> >> 
> >> Signed-off-by: Bjørn Mork 
> >
> > Acked-by: Oliver Neukum 
> 
> A bit late now I guess, but I'm just back from a laptop-less vacation
> and noticed that this patch is missing in v4.8-rc1.  Any particular
> reason for that?  Or was it just the too long discussion and general
> email overload?
> 
> I guess I should resend if it was the latter?  Will that be against
> usb-next then, or will you take it for -rc2?

I'll take it for -rc2, I'm just now starting to go through my patch
queue.  July was busy for me, vacation, conference, -rc1 merge window,
and moving across the ocean ate up my patch review time.  Just now
starting to dig out from that...

tanks,

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

Re: [PATCH v3] cdc-wdm: fix "out-of-sync" due to missing notifications

[Bjørn Mork]

Oliver Neukum  writes:
> On Sun, 2016-07-10 at 17:45 +0200, Bjørn Mork wrote:
>> The workaround has been tested on a large number of different MBIM
>> and QMI devices, as well as the Ericsson F5521gw and H5321gw modems
>> with real Device Management functions.
>> 
>> Signed-off-by: Bjørn Mork 
>
> Acked-by: Oliver Neukum 

A bit late now I guess, but I'm just back from a laptop-less vacation
and noticed that this patch is missing in v4.8-rc1.  Any particular
reason for that?  Or was it just the too long discussion and general
email overload?

I guess I should resend if it was the latter?  Will that be against
usb-next then, or will you take it for -rc2?


Bjørn

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

[PATCH] scsi: introduce a quirk for false cache reporting

[Oliver Neukum]

Some SATA to USB bridges fail to cooperate with some
drives resulting in no cache being present being reported
to the host. That causes the host to skip sending
a command to synchronize caches. That causes data loss
when the drive is powered down.

Signed-off-by: Oliver Neukum 
Reviewed-by: Johannes Thumshirn 
Tested-by: Egbert Eich 
---
 Documentation/kernel-parameters.txt | 2 ++
 drivers/scsi/sd.c   | 6 +++---
 drivers/usb/storage/scsiglue.c  | 4 
 drivers/usb/storage/unusual_devs.h  | 7 +++
 drivers/usb/storage/usb.c   | 6 +-
 include/linux/usb_usual.h   | 2 ++
 include/scsi/scsi_device.h  | 1 +
 7 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c9..c8c682e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -4182,6 +4182,8 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
u = IGNORE_UAS (don't bind to the uas driver);
w = NO_WP_DETECT (don't test whether the
medium is write-protected).
+   y = ALWAYS_SYNC (issue a SYNCHRONIZE_CACHE
+   even if the device claims no cache)
Example: quirks=0419:aaf5:rl,0421:0433:rc
 
user_debug= [KNL,ARM]
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 60bff78..3e8a6f1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -139,7 +139,7 @@ static void sd_set_flush_flag(struct scsi_disk *sdkp)
 {
bool wc = false, fua = false;
 
-   if (sdkp->WCE) {
+   if (sdkp->WCE || sdkp->device->always_sync) {
wc = true;
if (sdkp->DPOFUA)
fua = true;
@@ -3228,7 +3228,7 @@ static void sd_shutdown(struct device *dev)
if (pm_runtime_suspended(dev))
return;
 
-   if (sdkp->WCE && sdkp->media_present) {
+   if ((sdkp->WCE || sdkp->device->always_sync) && sdkp->media_present)  {
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
sd_sync_cache(sdkp);
}
@@ -3247,7 +3247,7 @@ static int sd_suspend_common(struct device *dev, bool 
ignore_stop_errors)
if (!sdkp)  /* E.g.: runtime suspend following sd_remove() */
return 0;
 
-   if (sdkp->WCE && sdkp->media_present) {
+   if ((sdkp->WCE || sdkp->device->always_sync) && sdkp->media_present) {
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
ret = sd_sync_cache(sdkp);
if (ret) {
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 33eb923..43e76ae 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -296,6 +296,10 @@ static int slave_configure(struct scsi_device *sdev)
if (us->fflags & US_FL_BROKEN_FUA)
sdev->broken_fua = 1;
 
+   /* Some even totally fail to indicate a cache */
+   if (us->fflags & US_FL_ALWAYS_SYNC)
+   sdev->always_sync = 1;
+
} else {
 
/*
diff --git a/drivers/usb/storage/unusual_devs.h 
b/drivers/usb/storage/unusual_devs.h
index aa35392..af3c7ee 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -338,6 +338,13 @@ UNUSUAL_DEV(  0x046b, 0xff40, 0x0100, 0x0100,
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_NO_WP_DETECT),
 
+/* Reported by Egbert Eich  */
+UNUSUAL_DEV(  0x0480, 0xd010, 0x0100, 0x,
+   "Toshiba",
+   "External USB 3.0",
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_ALWAYS_SYNC),
+
 /* Patch submitted by Philipp Friedrich  */
 UNUSUAL_DEV(  0x0482, 0x0100, 0x0100, 0x0100,
"Kyocera",
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index ef2d8cd..19255f1 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -498,7 +498,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
-   US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS);
+   US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS |
+   US_FL_ALWAYS_SYNC);
 
p = quirks;
while (*p) {
@@ -581,6 +582,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
case 'w':
f |= US_FL_NO_WP_DETECT;
  

Re: [PATCH] USB: serial: fix memleak on error path in usb-serial

[Johan Hovold]

On Mon, Aug 08, 2016 at 02:34:46AM +0100, Alexey Klimov wrote:
> udriver struct allocated by kzalloc() will not be freed
> if usb_register() and next calls fail. This patch fixes this
> by adding one more step with kfree(udriver) in error path.
> 
> Cc: Alan Stern 
> Signed-off-by: Alexey Klimov 

Now applied, thanks.

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 1/1] USB: serial: option: add support for Telit LE920A4

[Johan Hovold]

On Tue, Aug 02, 2016 at 11:29:25AM +0200, Daniele Palmas wrote:
> This patch adds a set of compositions for Telit LE920A4.
> 
> Compositions in short are:
> 
> 0x1207: tty + tty
> 0x1208: tty + adb + tty + tty
> 0x1211: tty + adb + ecm
> 0x1212: tty + adb
> 0x1213: ecm + tty
> 0x1214: tty + adb + ecm + tty
> 
> telit_le922_blacklist_usbcfg3 is reused for compositions 0x1211
> and 0x1214 due to the same interfaces positions.
> 
> Signed-off-by: Daniele Palmas 

Now applied.

Thanks,
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 v6] usb: serial: ftdi_sio Added 0a5c:6422 device ID for WICED USB UART dev board

[Johan Hovold]

On Thu, Jul 28, 2016 at 05:01:45PM -0400, Sheng-Hui J. Chu wrote:
> BCM20706V2_EVAL is a WICED dev board designed with FT2232H USB 2.0 UART/FIFO 
> IC.
> 
> To support BCM920706V2_EVAL dev board for WICED development on Linux.  Add 
> the VID(0a5c) and 
> PID(6422) to ftdi_sio driver to allow loading ftdi_sio for this board.
> 
> Signed-off-by: Sheng-Hui J. Chu 
> ---
>  drivers/usb/serial/ftdi_sio.c   | 1 +
>  drivers/usb/serial/ftdi_sio_ids.h | 6 
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 0082080..ef19af4 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1008,6 +1008,7 @@ static const struct usb_device_id id_table_combined[] = 
> {
>   { USB_DEVICE(ICPDAS_VID, ICPDAS_I7560U_PID) },
>   { USB_DEVICE(ICPDAS_VID, ICPDAS_I7561U_PID) },
>   { USB_DEVICE(ICPDAS_VID, ICPDAS_I7563U_PID) },
> + { USB_DEVICE(WICED_USB_VID, WICED_USB20706V2_PID) },
>   { } /* Terminating entry */
>  };
>  
> diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
> b/drivers/usb/serial/ftdi_sio_ids.h
> index c5d6c1e..b29f280 100644
> --- a/drivers/usb/serial/ftdi_sio_ids.h
> +++ b/drivers/usb/serial/ftdi_sio_ids.h
> @@ -1485,3 +1485,11 @@
>  #define CHETCO_SEASMART_DISPLAY_PID  0xA5AD /* SeaSmart NMEA2000 Display */
>  #define CHETCO_SEASMART_LITE_PID 0xA5AE /* SeaSmart Lite USB Adapter */
>  #define CHETCO_SEASMART_ANALOG_PID   0xA5AF /* SeaSmart Analog Adapter */
> +
> +/*
> + * WICED USB UART
> + */
> +#define WICED_USB_VID0x0A5C
> +#define WICED_USB20706V2_PID 0x6422

This patch still failed to apply:

Applying: usb: serial: ftdi_sio Added 0a5c:6422 device ID for WICED USB UART 
dev board
fatal: corrupt patch at line 11
Patch failed at 0001 usb: serial: ftdi_sio Added 0a5c:6422 device ID for WICED 
USB UART dev board

and checkpatch also complained about possible line wrapping.

I fixed it up here before applying, but next time please try sending the
patch to yourself and make sure it still applies before posting to the
list.

Thanks,
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] USB: ftdi_sio: Added custom PIDs for Ivium Technologies electrochemical interface products

[Johan Hovold]

On Thu, Jul 28, 2016 at 06:52:55PM +, Robert Deliën wrote:
> Ivium Technologies uses the FTDI VID with custom PIDs for their line of
> electrochemical interfaces and the PalmSens they developed for PalmSens BV.
> 
> PIDs are kept in numerical order, entries in id_table_combined[] are at
> the same position as far as possible.
> 
> Signed-off-by: Robert Delien 

Now applied, thanks.

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] USB: serial: option: add D-Link DWM-156/A3

[Johan Hovold]

On Sun, Jul 24, 2016 at 01:53:30PM +0200, Lubomir Rintel wrote:
> The device has three interfaces; the three serial ports ought to be

I assumed you meant "four interfaces" above and updated the commit
message before applying.

> handled by this driver:
> 
> 00 Diagnostic interface serial port
> 01 NMEA device serial port
> 02 Mass storage (sd card)
> 03 Modem serial port
> 
> The other product ids listed in the Windows driver are present already.
> 
> Signed-off-by: Lubomir Rintel 
> Cc: stable 

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

[PATCH v5 6/6] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

[Peter Chen]

The current dts describes USB HUB's property at USB controller's
entry, it is improper. The USB HUB should be the child node
under USB controller, and power sequence properties are under
it.

Signed-off-by: Peter Chen 
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi 
b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..f29a72c2f 100644
--- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
@@ -9,6 +9,8 @@
  *
  */
 
+#include 
+
 / {
aliases {
backlight = 
@@ -58,17 +60,6 @@
#address-cells = <1>;
#size-cells = <0>;
 
-   reg_usb_h1_vbus: regulator@0 {
-   compatible = "regulator-fixed";
-   reg = <0>;
-   regulator-name = "usb_h1_vbus";
-   regulator-min-microvolt = <500>;
-   regulator-max-microvolt = <500>;
-   enable-active-high;
-   startup-delay-us = <2>; /* USB2415 requires a POR of 1 
us minimum */
-   gpio = < 12 0>;
-   };
-
reg_panel: regulator@1 {
compatible = "regulator-fixed";
reg = <1>;
@@ -259,9 +250,18 @@
  {
pinctrl-names = "default";
pinctrl-0 = <_usbh>;
-   vbus-supply = <_usb_h1_vbus>;
-   clocks = < IMX6QDL_CLK_CKO>;
status = "okay";
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   usb2415: hub@1 {
+   compatible = "usb424,2514";
+   reg = <1>;
+
+   clocks = < IMX6QDL_CLK_CKO>;
+   reset-gpios = < 12 GPIO_ACTIVE_LOW>;
+   reset-duration-us = <3000>;
+   };
 };
 
  {
-- 
1.9.1

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

[PATCH v5 3/6] binding-doc: usb: usb-device: add optional properties for power sequence

[Peter Chen]

Add optional properties for power sequence.

Signed-off-by: Peter Chen 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/usb/usb-device.txt | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt 
b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..3661dd2 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,6 +13,10 @@ Required properties:
 - reg: the port number which this device is connecting to, the range
   is 1-31.
 
+Optional properties:
+power sequence properties, see
+Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt for detail
+
 Example:
 
  {
@@ -21,8 +25,12 @@ Example:
#address-cells = <1>;
#size-cells = <0>;
 
-   hub: genesys@1 {
+   genesys: hub@1 {
compatible = "usb5e3,608";
reg = <1>;
+
+   clocks = < IMX6SX_CLK_CKO>;
+   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+   reset-duration-us = <10>;
};
 }
-- 
1.9.1

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

[PATCH v5 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Peter Chen]

From: Peter Chen 

At device tree, we have no device node for chipidea core,
the glue layer's node is the parent node for host and udc
device. But in related driver, the parent device is chipidea
core. So, in order to let the common driver get parent's node,
we let the core's device node equals glue layer device node.

Signed-off-by: Peter Chen 
Tested-by: Maciej S. Szmigiero 
Tested-by Joshua Clayton 
---
 drivers/usb/chipidea/core.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..b189dc7 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -954,6 +954,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
dev_err(dev, "unable to init phy: %d\n", ret);
return ret;
}
+   /*
+* At device tree, we have no device node for chipidea core,
+* the glue layer's node is the parent node for host and udc
+* device. But in related driver, the parent device is chipidea
+* core. So, in order to let the common driver get parent's node,
+* we let the core's device node equals glue layer's node.
+*/
+   if (dev->parent && dev->parent->of_node)
+   dev->of_node = dev->parent->of_node;
 
ci->hw_bank.phys = res->start;
 
@@ -1057,6 +1066,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 stop:
ci_role_destroy(ci);
 deinit_phy:
+   dev->of_node = NULL;
ci_usb_phy_exit(ci);
 
return ret;
@@ -1076,6 +1086,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
ci_extcon_unregister(ci);
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
+   ci->dev->of_node = NULL;
ci_usb_phy_exit(ci);
 
return 0;
-- 
1.9.1

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

[PATCH v5 4/6] usb: core: add power sequence handling for USB devices

[Peter Chen]

Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.

In this patch, it calls power sequence library APIs to finish
the power sequence events. At first, it calls pwrseq_alloc_generic
to create a generic power sequence instance, then it will do power
on sequence at hub's probe for all devices under this hub
(includes root hub).

At hub_disconnect, it will do power off sequence which is at powered
on list.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
 drivers/usb/core/Makefile |   1 +
 drivers/usb/core/hub.c|  12 --
 drivers/usb/core/hub.h|  12 ++
 drivers/usb/core/pwrseq.c | 100 ++
 4 files changed, 122 insertions(+), 3 deletions(-)
 create mode 100644 drivers/usb/core/pwrseq.c

diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 9780877..39f2149 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
 
 usbcore-$(CONFIG_PCI)  += hcd-pci.o
 usbcore-$(CONFIG_ACPI) += usb-acpi.o
+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
 
 obj-$(CONFIG_USB)  += usbcore.o
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a346a8b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
 
+   hub_pwrseq_off(hub);
mutex_lock(_port_peer_mutex);
 
/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
struct usb_endpoint_descriptor *endpoint;
struct usb_device *hdev;
struct usb_hub *hub;
+   int ret = -ENODEV;
 
desc = intf->cur_altsetting;
hdev = interface_to_usbdev(intf);
@@ -1839,6 +1841,7 @@ descriptor_error:
INIT_DELAYED_WORK(>leds, led_work);
INIT_DELAYED_WORK(>init_work, NULL);
INIT_WORK(>events, hub_event);
+   INIT_LIST_HEAD(>pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
 
@@ -1852,11 +1855,14 @@ descriptor_error:
if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
hub->quirk_check_port_auto_suspend = 1;
 
-   if (hub_configure(hub, endpoint) >= 0)
-   return 0;
+   if (hub_configure(hub, endpoint) >= 0) {
+   ret = hub_pwrseq_on(hub);
+   if (!ret)
+   return 0;
+   }
 
hub_disconnect(intf);
-   return -ENODEV;
+   return ret;
 }
 
 static int
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..9473f6f 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -78,6 +78,7 @@ struct usb_hub {
struct delayed_work init_work;
struct work_struct  events;
struct usb_port **ports;
+   struct list_headpwrseq_on_list; /* powered pwrseq node list */
 };
 
 /**
@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
usb_hub *hub,
 {
return hub_port_debounce(hub, port1, false);
 }
+
+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
+int hub_pwrseq_on(struct usb_hub *hub);
+void hub_pwrseq_off(struct usb_hub *hub);
+#else
+static inline int hub_pwrseq_on(struct usb_hub *hub)
+{
+   return 0;
+}
+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
+#endif /* CONFIG_PWRSEQ_GENERIC */
diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
new file mode 100644
index 000..837fe66
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,100 @@
+/*
+ * pwrseq.cUSB device power sequence management
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hub.h"
+
+struct usb_pwrseq_node {
+   struct pwrseq *pwrseq;
+   struct list_head list;
+};
+
+static int hub_of_pwrseq_on(struct 

[PATCH v5 2/6] power: add power sequence library

[Peter Chen]

We have an well-known problem that the device needs to do some power
sequence before it can be recognized by related host, the typical
example like hard-wired mmc devices and usb devices.

This power sequence is hard to be described at device tree and handled by
related host driver, so we have created a common power sequence
library to cover this requirement. The core code has supplied
some common helpers for host driver, and individual power sequence
libraries handle kinds of power sequence for devices.

pwrseq_generic is intended for general purpose of power sequence, which
handles gpios and clocks currently, and can cover regulator and pinctrl
in future. The host driver calls pwrseq_alloc_generic to create
an generic pwrseq instance.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
 MAINTAINERS   |   9 ++
 drivers/power/Kconfig |   1 +
 drivers/power/Makefile|   1 +
 drivers/power/pwrseq/Kconfig  |  20 
 drivers/power/pwrseq/Makefile |   2 +
 drivers/power/pwrseq/core.c   |  62 +
 drivers/power/pwrseq/pwrseq_generic.c | 168 ++
 include/linux/power/pwrseq.h  |  47 ++
 8 files changed, 310 insertions(+)
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 include/linux/power/pwrseq.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1ae6c84..407254b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9283,6 +9283,15 @@ F:   include/linux/pm_*
 F: include/linux/powercap.h
 F: drivers/powercap/
 
+POWER SEQUENCE LIBRARY
+M: Peter Chen 
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
+L: linux...@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/power/pwrseq/
+F: drivers/power/pwrseq/
+F: include/linux/power/pwrseq.h/
+
 POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
 M: Sebastian Reichel 
 M: Dmitry Eremin-Solenikov 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..f6aa4fd 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -515,3 +515,4 @@ endif # POWER_SUPPLY
 
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
+source "drivers/power/pwrseq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d..4ed2e12 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65217)+= tps65217_charger.o
 obj-$(CONFIG_POWER_RESET)  += reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)   += axp288_charger.o
+obj-$(CONFIG_POWER_SEQUENCE)   += pwrseq/
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
new file mode 100644
index 000..188729e
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,20 @@
+#
+# Power Sequence library
+#
+
+config POWER_SEQUENCE
+   bool
+
+menu "Power Sequence Support"
+
+config PWRSEQ_GENERIC
+   bool "Generic power sequence control"
+   depends on OF
+   select POWER_SEQUENCE
+   help
+ It is used for drivers which needs to do power sequence
+ (eg, turn on clock, toggle reset gpio) before the related
+ devices can be found by hardware. This generic one can be
+ used for common power sequence control.
+
+endmenu
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 000..ad82389
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWER_SEQUENCE) += core.o
+obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 000..dcf96c4
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,62 @@
+/*
+ * core.c  power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+
+static DEFINE_MUTEX(pwrseq_list_mutex);
+static LIST_HEAD(pwrseq_list);
+
+int pwrseq_get(struct 

[PATCH v5 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

[Peter Chen]

Add binding doc for generic power sequence library.

Signed-off-by: Peter Chen 
Acked-by: Philipp Zabel 
Acked-by: Rob Herring 
---
 .../bindings/power/pwrseq/pwrseq-generic.txt   | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
new file mode 100644
index 000..ebf0d47
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,48 @@
+The generic power sequence library
+
+Some hard-wired devices (eg USB/MMC) need to do power sequence before
+the device can be enumerated on the bus, the typical power sequence
+like: enable USB PHY clock, toggle reset pin, etc. But current
+Linux device driver lacks of such code to do it, it may cause some
+hard-wired devices works abnormal or can't be recognized by
+controller at all. The power sequence will be done before this device
+can be found at the bus.
+
+The power sequence properties is under the device node.
+
+Optional properties:
+- clocks: the input clocks for device.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
+
+Below is the example of USB power sequence properties on USB device
+nodes which have two level USB hubs.
+
+ {
+   vbus-supply = <_usb_otg1_vbus>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_usb_otg1_id>;
+   status = "okay";
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   genesys: hub@1 {
+   compatible = "usb5e3,608";
+   reg = <1>;
+
+   clocks = < IMX6SX_CLK_CKO>;
+   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+   reset-duration-us = <10>;
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   asix: ethernet@1 {
+   compatible = "usbb95,1708";
+   reg = <1>;
+
+   clocks = < IMX6SX_CLK_IPG>;
+   reset-gpios = < 6 GPIO_ACTIVE_LOW>; /* 
ethernet_rst */
+   reset-duration-us = <15>;
+   };
+   };
+};
-- 
1.9.1

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

[PATCH v5 0/6] power: add power sequence library

[Peter Chen]

Hi all,

This is a follow-up for my last power sequence framework patch set [1].
According to Rob Herring and Ulf Hansson's comments[2], I use a generic
power sequence library for parsing the power sequence elements on DT,
and implement generic power sequence on library. The host driver
can allocate power sequence instance, and calls pwrseq APIs accordingly.

In future, if there are special power sequence requirements, the special
power sequence library can be created.

This patch set is tested on i.mx6 sabresx evk using a dts change, I use
two hot-plug devices to simulate this use case, the related binding
change is updated at patch [1/6], The udoo board changes were tested
using my last power sequence patch set.[3]

Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
need to power on itself before it can be found by ULPI bus.

[1] http://www.spinics.net/lists/linux-usb/msg142755.html
[2] http://www.spinics.net/lists/linux-usb/msg143106.html
[3] http://www.spinics.net/lists/linux-usb/msg142815.html

Changes for v5:
- Delete pwrseq_register/pwrseq_unregister, which is useless currently
- Fix the linker error when the pwrseq user is compiled as module

Changes for v4:
- Create the patch on next-20160722 
- Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
- Using more friendly wait method for reset gpio [Patch 2/6]
- Support multiple input clocks [Patch 2/6]
- Add Rob Herring's ack for DT changes
- Add Joshua Clayton's Tested-by

Changes for v3:
- Delete "power-sequence" property at binding-doc, and change related code
  at both library and user code.
- Change binding-doc example node name with Rob's comments
- of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
  add additional code request gpio with proper gpio flags
- Add Philipp Zabel's Ack and MAINTAINER's entry

Changes for v2:
- Delete "pwrseq" prefix and clock-names for properties at dt binding
- Should use structure not but its pointer for kzalloc
- Since chipidea core has no of_node, let core's of_node equals glue
  layer's at core's probe

Peter Chen (6):
  binding-doc: power: pwrseq-generic: add binding doc for generic power
sequence library
  power: add power sequence library
  binding-doc: usb: usb-device: add optional properties for power
sequence
  usb: core: add power sequence handling for USB devices
  usb: chipidea: let chipidea core device of_node equal's glue layer
device of_node
  ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

 .../bindings/power/pwrseq/pwrseq-generic.txt   |  48 ++
 .../devicetree/bindings/usb/usb-device.txt |  10 +-
 MAINTAINERS|   9 ++
 arch/arm/boot/dts/imx6qdl-udoo.dtsi|  26 ++--
 drivers/power/Kconfig  |   1 +
 drivers/power/Makefile |   1 +
 drivers/power/pwrseq/Kconfig   |  20 +++
 drivers/power/pwrseq/Makefile  |   2 +
 drivers/power/pwrseq/core.c|  62 
 drivers/power/pwrseq/pwrseq_generic.c  | 168 +
 drivers/usb/chipidea/core.c|  11 ++
 drivers/usb/core/Makefile  |   1 +
 drivers/usb/core/hub.c |  12 +-
 drivers/usb/core/hub.h |  12 ++
 drivers/usb/core/pwrseq.c  | 100 
 include/linux/power/pwrseq.h   |  47 ++
 16 files changed, 513 insertions(+), 17 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 drivers/usb/core/pwrseq.c
 create mode 100644 include/linux/power/pwrseq.h

-- 
1.9.1

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

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Oliver Neukum]

On Sun, 2016-08-07 at 23:37 +0200, Pavel Machek wrote:
> Hi!
> 
> > > With these boards, you will not see anything on the screen that is
> > > attached to a Type-C connector until the OS has booted to the point
> > > where it has negotiated the power contract and entered a mode.
> > > 
> > > If the system has BIOS/FW/EC capable of negotiating the power contract
> > > and enter a mode, but where we still are expected to take over the
> > > whole TCPM in OS, I think the connection will be reset.
> > 
> > Think about a DP over type C display with a USB PD power brick on a
> > daisy chain.
> > If the host needs more than 15W or more than 5V, a reset is suicide.
> > 
> > And losing earlyprintk hurts a lot.
> > This means we need USB PD statically in the kernel. And a kernel
> > based policy that brings up all displays.
> 
> Yes please.
> 
> Of course, even that will hurt, because I guess that means printk()
> after USB, and that means late in the boot process, but better late in
> kernel than in initrd.

i can confirm that in a laptop with a decent firmware a docking
station is switched into the mode that will make the display work.
In principle all is well if we can manage a handover.

Regards
Oliver


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

Re: ohci-pci controller won't resume and HC died; cleaning up - only on shutdown/reboot

[ican realizeum]

Sorry, I just realized that in gmail `Default reply behaviour` was set
to `Reply` not `Reply All`, so I skipped the list when I replied
last(for I forgot to select Reply All as I did before). Oops. So now
hopefully everyone else can see the quoted previous email below.

On Mon, Aug 8, 2016 at 4:35 AM, Alan Stern  wrote:
> On Mon, 8 Aug 2016, ican realizeum wrote:
>
>> Roger that. Here's what I found out:
>> if I try to sysrq+o (aka poweroff) manually from a file that systemd
>> runs at shutdown such as manually created a+x file:
>> /usr/lib/systemd/system-shutdown/debug.sh
>>
>> then I get no ohci errors,
>> screenshot: https://i.imgur.com/VVeAyj7.jpg
>> only the "PME# disabled" and "enabling bus mastering" are seen.
>> But if I allow the kernel(and/or whatever else happens before that) to
>> do it, then the errors are back just like in my original post.
>
> The difference isn't clear.
>
>> The stack is this:
>> https://i.imgur.com/9wlbure.jpg
>> but it looks like something else called that and I don't know
>> what(looks forked?), unless it's something from the following stack
>> from the point where "Stop disk" happens:
>> https://i.imgur.com/z9uq4ez.jpg
>> Maybe I'll find out something from there.
>
> The caller was hcd_resume_work(), which is part of a workqueue.  This
> routine runs when the OHCI driver calls usb_hcd_resume_root_hub().
> There are only 3 places in the driver where this happens (two in
> ohci-hcd.c and one in ohci-hub.c), but as far as I can tell, none of
> them should have occurred if power/control was set to "on".  If you
> want, you can add an ohci_info() before each of those calls; knowing
> which one it was will probably tell us what happened.
>
> However, this doesn't answer the question of why the resume failed.
> My best guess is that it failed because something it needed had already
> been shut down.
>
> Alan Stern
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: creating a virtual usb device from user-space

[Greg KH]

On Sun, Aug 07, 2016 at 06:55:36PM +0200, stéphane bryant wrote:
> Hi
> 
> We are working on a project where a user-space application (a device
> emulator) creates a USB device on the host machine (thus allowing the
> emulated device to emulate a USB slave and to act as a usb dongle on the
> host machine).
> We have done this using usbip so far, but we'd rather do it directly:
> for that envision to have a vhci_hcd kernel module driver, with hooks to
> the user-space to set-up the device & handle the data.

Why not just use the CONFIG_USB_DUMMY_HCD and use the existing userspace
USB gadget interfaces?  That's traditionally how that code is used.

Why do you want a userspace program to emulate a USB device in a
different way from this?

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