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

2016-10-21 Thread Tony Lindgren
* Johan Hovold  [161020 08:38]:
> Hi Tony,
> 
> I'm getting the splat below when booting 4.9-rc1 on a BBB and
> tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for
> MUSB DSPS glue layer") which added a synchronous RPM get in a timer
> callback:

OK, sorry to hear about that. Care to email me your .config and how
to reproduce and what do you have connected like a hub? Also do
you use built-in gadgets or configure them via configfs?

> [6.466226] BUG: sleeping function called from invalid context at 
> /home/johan/work/omicron/src/linux/drivers/base/power/runtime.c:955
> [6.478850] in_atomic(): 1, irqs_disabled(): 0, pid: 59, name: grep
> [6.485434] 1 lock held by grep/59:
> [6.489102]  #0: [6.490966]  (
> ((&glue->timer))[6.494206] ){+.-...}
> , at: [6.497194] [] call_timer_fn+0x0/0x41c
> [6.502048] Preemption disabled at:[6.505540] [] 
> __do_softirq+0x80/0x594
> [6.510393] 
> [6.511974] CPU: 0 PID: 59 Comm: grep Not tainted 4.9.0-rc1 #46
> [6.518190] Hardware name: Generic AM33XX (Flattened Device Tree)
> [6.524613] [] (unwind_backtrace) from [] 
> (show_stack+0x20/0x24)
> [6.532753] [] (show_stack) from [] 
> (dump_stack+0x24/0x28)
> [6.540351] [] (dump_stack) from [] 
> (___might_sleep+0x1d8/0x2c4)
> [6.548486] [] (___might_sleep) from [] 
> (__might_sleep+0x70/0xa8)
> [6.556720] [] (__might_sleep) from [] 
> (__pm_runtime_resume+0x9c/0xa0)
> [6.565404] [] (__pm_runtime_resume) from [] 
> (otg_timer+0x3c/0x254)
> [6.573813] [] (otg_timer) from [] 
> (call_timer_fn+0xfc/0x41c)
> [6.581675] [] (call_timer_fn) from [] 
> (expire_timers+0x120/0x210)
> [6.589990] [] (expire_timers) from [] 
> (run_timer_softirq+0xa4/0xdc)
> [6.598487] [] (run_timer_softirq) from [] 
> (__do_softirq+0x12c/0x594)
> [6.607079] [] (__do_softirq) from [] 
> (irq_exit+0xf4/0x130)
> [6.614766] [] (irq_exit) from [] 
> (__handle_domain_irq+0x84/0xec)
> [6.622991] [] (__handle_domain_irq) from [] 
> (omap_intc_handle_irq+0x44/0xa0)
> [6.632306] [] (omap_intc_handle_irq) from [] 
> (__irq_usr+0x58/0x80)
> [6.640716] Exception stack(0xcf669fb0 to 0xcf669ff8)
> [6.646028] 9fa0: b6fd5050 0012ebb0 
> b6e71058 b6e1c088
> [6.654614] 9fc0: b6e07000 0001 b6e1cb10 b6fd5000 0001  
> b6fd38e8 bec32a54
> [6.663198] 9fe0: b6e1c870 bec32990 b6fb0a20 b6fb9758 20030010 
> 
> Setting the irq_safe flag seems to do the trick, but not sure that's
> what you intended to do.

That's what we want to avoid as it keep the parent device permanently
enabled. To avoid that we want to just queue things and deal with them
from pm_runtime_resume.

> I saw you posted some regression fixes lately, but they did not look
> related to this at first glance at least.

Yeah this seems different. Can you still try v4.9-rc + patches from
thread "[PATCH 0/2] Fixes for two more musb regressions"?

Regards,

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


Re: [PATCH 0/2] musb-fixes for v4.9-rc2

2016-10-21 Thread Tony Lindgren
* Ladislav Michl  [161020 12:37]:
> On Thu, Oct 20, 2016 at 05:35:24AM -0700, Tony Lindgren wrote:
> > I don't think I've seen that error..
> 
> Comment in code reads: 'FIXME this is another "SHOULD NEVER HAPPEN"'

Seems like it does though..

> > There are few patches that we seem to need for v4.7 and v4.8 stable.
> > At least these two fixes that should be merged for v4.9 should be
> > in:
> > 
> > [PATCH 0/2] Fixes for two more musb regressions
> > 
> > Then two patches for phy-twl4030-usb.c:
> > 
> > b78ea84a7d45 ("phy-twl4030-usb: initialize charging-related stuff via
> > pm_runtime")
> > 78489c7c48d4 ("phy-twl4030-usb: better handle musb_mailbox() failure")
> > 
> > Are you using the twl4030 phy or something else? Also, care to try
> 
> twl4030.
> 
> > with v4.9-rc + [PATCH 0/2] Fixes for two more musb regressions?
> 
> Compiled recent Linus' git tree with those two patches on top of it.
> It doesn't work either, but I found that when I unplug hub from musb
> during bootup and connect is again after musb gets initialized, it works
> normally. Well, almost... It does survive only few reconnects, then
> ends with:

OK interesting, care to check if things work normally with the fixes
mentioned and without the hub? Maybe that's the difference here
compared to my setup.
...

> [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, 
>  
> And that's the end, since now it does not react on hub plug/unplug.
> 
> Also all that VBUS_ERROR conditions are strange as hub is powered separately
> and power lines from phy are not used.

Hmm yeah. I'd like to be able to reproduce this. Can you email me
your .config (again)? You have things in host mode with a powered
hub plus few devices with no USB gadgets configured?

Regards,

Tony
--
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: Interactive whiteboards

2016-10-21 Thread Greg KH
On Thu, Oct 20, 2016 at 10:46:11AM +0200, María Cano wrote:
> >> HITACHI STARBOARD (DOESN’T WORK)
> >> DIFF FOR cat /proc/bus/input/devices
> >> > T:  Bus=04 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 12 Spd=12   MxCh= 0
> >> > D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> >> > P:  Vendor=10c4 ProdID=0661 Rev= 0.00
> >> > S:  Manufacturer=SZiB0rdelectronics
> >> > S:  Product=IBOADDEVICE
> >> > C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=300mA
> >> > I:* If#= 0 Alt= 0 #EPs= 2 Cls=00(>ifc ) Sub=00 Prot=00 Driver=(none)
> >> > E:  Ad=81(I) Atr=03(Int.) MxPS=  32 Ivl=1ms
> >> > E:  Ad=02(O) Atr=03(Int.) MxPS=   8 Ivl=1ms
> >>
> >> DIFF FOR cat /proc/bus/input/devices
> >
> > That looks like /sys/kernel/debug/usb/devices, not input/devices.  Which
> > is fine :)
> >
> > That looks like it is going to take a "vendor-specific" driver to
> > communicate with it.  If you can get the specs from the vendor, we will
> > be glad to help you out with writing a driver for it, but without that,
> > or a bunch of reverse engineering effort, it's probably not going to be
> > possible :(
> 
> 
> Ok, I'll try to contact with the seller and the manufacturer, let's
> see if we are lucky.

Great!

> >> HITACHI STARBOARD (DOESN’T WORK)
> >> DIFF FOR cat /proc/bus/input/devices
> >> > T:  Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  4 Spd=12   MxCh= 0
> >> > D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> >> > P:  Vendor=1477 ProdID=0008 Rev=13.13
> >> > S:  Manufacturer=eIT-Xiroku
> >> > S:  Product=Optical Touch Sensor HTM131
> >> > S:  SerialNumber=002302314034
> >> > C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=500mA
> >> > I:* If#= 0 Alt= 0 #EPs= 0 Cls=ff(vend.) Sub=00 Prot=ff Driver=(none)
> >> > I:  If#= 0 Alt= 1 #EPs= 1 Cls=ff(vend.) Sub=00 Prot=ff Driver=(none)
> >> > E:  Ad=82(I) Atr=01(Isoc) MxPS= 900 Ivl=1ms
> >> >
> >> > T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  2 Spd=1.5  MxCh= 0
> >>
> >>
> >> DIFF FOR cat /proc/bus/input/devices
> >
> > Again, that's the usb devices file.
> >
> > And why the odd blank line?  Are you sure you didn't miss some lines
> > there?
> >
> > Anyway, a device with only one isoc endpoint seems really odd, is this a
> > camera?
> 
> 
> I don't know what tecnology it uses, but it is possible it is a
> camera, look at the top of the whiteboard:
> http://www.hitachistarboard.us/hitachi-fx-trio-77-inch-whiteboard.html

Hm, or, worst case, it might move the logic of trying to determine where
the user is touching to the host computer, running software to turn the
camera into an input device.  That's going to be messy :(

> >> MULTICLASS (DOESN’T WORK)
> >> DIFF FOR cat /proc/bus/input/devices
> >> > T:  Bus=04 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  7 Spd=12   MxCh= 0
> >> > D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> >> > P:  Vendor=10c4 ProdID=ea60 Rev= 1.00
> >> > S:  Manufacturer=Silicon Labs
> >> > S:  Product=CP2102 USB to UART Bridge Controller
> >> > S:  SerialNumber=0001
> >> > C:* #Ifs= 1 Cfg#= 1 Atr=80 MxPwr=100mA
> >> > I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=cp210x
> >> > E:  Ad=81(I) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> >> > E:  Ad=01(O) Atr=02(Bulk) MxPS=  64 Ivl=0ms
> >>
> >> DIFF FOR cat /proc/bus/input/devices
> >
> > Look, the serial driver bound to it properly!  So there is hope, now you
> > just need to have some userspace program to talk to the device, as it is
> > now being exported by the kernel as a "normal" serial port.
> >
> > That's again going to be hard without some specs from the manufacturer
> > on how to talk to the device.
> >
> 
> Ok, I'll try to get more information about specs, but what exactly I
> have to ask?

We need to know:
- the protocol the device uses in order to communicate with the
  host computer in order to properly detect and control the
  device.

good luck!

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: musb RPM sleep-while-atomic in 4.9-rc1

2016-10-21 Thread Johan Hovold
On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote:
> * Johan Hovold  [161020 08:38]:
> > Hi Tony,
> > 
> > I'm getting the splat below when booting 4.9-rc1 on a BBB and
> > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for
> > MUSB DSPS glue layer") which added a synchronous RPM get in a timer
> > callback:
> 
> OK, sorry to hear about that. Care to email me your .config and how
> to reproduce and what do you have connected like a hub? Also do
> you use built-in gadgets or configure them via configfs?

It happens even with no devices connected to the host port, and with no
gadgets configured (or the peripheral port simply disabled). Whenever
the glue timer fires and calls pm_runtime_get_sync() I get the splat.
For some reason the might_sleep() in get_sync() does not trigger the
first time, which means that I see this four seconds after probe, and
then every other second when the timer fires.

Attaching my defconfig.

> > [6.466226] BUG: sleeping function called from invalid context at 
> > /home/johan/work/omicron/src/linux/drivers/base/power/runtime.c:955
> > [6.478850] in_atomic(): 1, irqs_disabled(): 0, pid: 59, name: grep
> > [6.485434] 1 lock held by grep/59:
> > [6.489102]  #0: [6.490966]  (
> > ((&glue->timer))[6.494206] ){+.-...}
> > , at: [6.497194] [] call_timer_fn+0x0/0x41c
> > [6.502048] Preemption disabled at:[6.505540] [] 
> > __do_softirq+0x80/0x594
> > [6.510393] 
> > [6.511974] CPU: 0 PID: 59 Comm: grep Not tainted 4.9.0-rc1 #46
> > [6.518190] Hardware name: Generic AM33XX (Flattened Device Tree)
> > [6.524613] [] (unwind_backtrace) from [] 
> > (show_stack+0x20/0x24)
> > [6.532753] [] (show_stack) from [] 
> > (dump_stack+0x24/0x28)
> > [6.540351] [] (dump_stack) from [] 
> > (___might_sleep+0x1d8/0x2c4)
> > [6.548486] [] (___might_sleep) from [] 
> > (__might_sleep+0x70/0xa8)
> > [6.556720] [] (__might_sleep) from [] 
> > (__pm_runtime_resume+0x9c/0xa0)
> > [6.565404] [] (__pm_runtime_resume) from [] 
> > (otg_timer+0x3c/0x254)
> > [6.573813] [] (otg_timer) from [] 
> > (call_timer_fn+0xfc/0x41c)
> > [6.581675] [] (call_timer_fn) from [] 
> > (expire_timers+0x120/0x210)
> > [6.589990] [] (expire_timers) from [] 
> > (run_timer_softirq+0xa4/0xdc)
> > [6.598487] [] (run_timer_softirq) from [] 
> > (__do_softirq+0x12c/0x594)
> > [6.607079] [] (__do_softirq) from [] 
> > (irq_exit+0xf4/0x130)
> > [6.614766] [] (irq_exit) from [] 
> > (__handle_domain_irq+0x84/0xec)
> > [6.622991] [] (__handle_domain_irq) from [] 
> > (omap_intc_handle_irq+0x44/0xa0)
> > [6.632306] [] (omap_intc_handle_irq) from [] 
> > (__irq_usr+0x58/0x80)
> > [6.640716] Exception stack(0xcf669fb0 to 0xcf669ff8)
> > [6.646028] 9fa0: b6fd5050 0012ebb0 
> > b6e71058 b6e1c088
> > [6.654614] 9fc0: b6e07000 0001 b6e1cb10 b6fd5000 0001  
> > b6fd38e8 bec32a54
> > [6.663198] 9fe0: b6e1c870 bec32990 b6fb0a20 b6fb9758 20030010 
> > 
> > Setting the irq_safe flag seems to do the trick, but not sure that's
> > what you intended to do.
> 
> That's what we want to avoid as it keep the parent device permanently
> enabled. To avoid that we want to just queue things and deal with them
> from pm_runtime_resume.

I figured, so then that pm_runtime_get_sync() in the dsps timer callback
needs to go.

> > I saw you posted some regression fixes lately, but they did not look
> > related to this at first glance at least.
> 
> Yeah this seems different. Can you still try v4.9-rc + patches from
> thread "[PATCH 0/2] Fixes for two more musb regressions"?

As expected, those two fixes makes no difference.

Thanks,
Johan
CONFIG_COMPILE_TEST=y
CONFIG_DEFAULT_HOSTNAME="omicron"
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_POSIX_MQUEUE=y
# CONFIG_CROSS_MEMORY_ATTACH is not set
# CONFIG_FHANDLE is not set
CONFIG_USELIB=y
CONFIG_NO_HZ_IDLE=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=16
CONFIG_EXPERT=y
# CONFIG_ADVISE_SYSCALLS is not set
CONFIG_SLAB=y
CONFIG_PROFILING=y
CONFIG_MODULES=y
CONFIG_MODULE_FORCE_LOAD=y
CONFIG_MODULE_UNLOAD=y
# CONFIG_LBDAF is not set
# CONFIG_BLK_DEV_BSG is not set
CONFIG_PARTITION_ADVANCED=y
# CONFIG_IOSCHED_DEADLINE is not set
CONFIG_OMAP_RESET_CLOCKS=y
CONFIG_OMAP_MUX_DEBUG=y
CONFIG_ARCH_OMAP3=y
CONFIG_SOC_AM33XX=y
# CONFIG_SOC_OMAP3430 is not set
# CONFIG_SOC_TI81XX is not set
# CONFIG_MACH_OMAP3517EVM is not set
# CONFIG_MACH_OMAP3_PANDORA is not set
CONFIG_ARM_THUMBEE=y
# CONFIG_CACHE_L2X0 is not set
CONFIG_ARM_ERRATA_720789=y
CONFIG_ARM_ERRATA_754322=y
CONFIG_ARM_ERRATA_775420=y
CONFIG_PCI=y
CONFIG_HAVE_ARM_ARCH_TIMER=y
CONFIG_PREEMPT=y
CONFIG_HZ_1000=y
# CONFIG_HIGHPTE is not set
# CONFIG_COMPACTION is not set
CONFIG_ZBOOT_ROM_TEXT=0x0
CONFIG_ZBOOT_ROM_BSS=0x0
CONFIG_ARM_APPENDED_DTB=y
CONFIG_CMDLINE="root=/dev/nfs ip=dhcp debug"
CONFIG_CM

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

2016-10-21 Thread Tony Lindgren
* Johan Hovold  [161021 02:26]:
> On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote:
> > * Johan Hovold  [161020 08:38]:
> > > Hi Tony,
> > > 
> > > I'm getting the splat below when booting 4.9-rc1 on a BBB and
> > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for
> > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer
> > > callback:
> > 
> > OK, sorry to hear about that. Care to email me your .config and how
> > to reproduce and what do you have connected like a hub? Also do
> > you use built-in gadgets or configure them via configfs?
> 
> It happens even with no devices connected to the host port, and with no
> gadgets configured (or the peripheral port simply disabled). Whenever
> the glue timer fires and calls pm_runtime_get_sync() I get the splat.
> For some reason the might_sleep() in get_sync() does not trigger the
> first time, which means that I see this four seconds after probe, and
> then every other second when the timer fires.

OK. I'm totally baffled how come I did not hit this earlier with
my tests. I did have some extra patches for using the pmic vbus irq
for cable detection, but that's only for the peripheral instance and
the host instance is still using timer.

> Attaching my defconfig.

Not seeing anything special there, musb built in, using dma. And no
reason why this should not always happen when polling the cable status.

> > > Setting the irq_safe flag seems to do the trick, but not sure that's
> > > what you intended to do.
> > 
> > That's what we want to avoid as it keep the parent device permanently
> > enabled. To avoid that we want to just queue things and deal with them
> > from pm_runtime_resume.
> 
> I figured, so then that pm_runtime_get_sync() in the dsps timer callback
> needs to go.

Agreed, that is clearly wrong to call from softirq context. I need to
figure out what is right fix here but don't have access to my bbb
until next week.

> > > I saw you posted some regression fixes lately, but they did not look
> > > related to this at first glance at least.
> > 
> > Yeah this seems different. Can you still try v4.9-rc + patches from
> > thread "[PATCH 0/2] Fixes for two more musb regressions"?
> 
> As expected, those two fixes makes no difference.

Yeah thanks for checking.

Regards,

Tony
--
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: dwc3: Fix error handling for core init

2016-10-21 Thread Felipe Balbi

Hi,

Vivek Gautam  writes:
> Fixing the sequence of events in dwc3_core_init() error exit path.
> dwc3_core_exit() call is removed from the error path since,
> whatever it's doing is already done.
>
> Signed-off-by: Vivek Gautam 
> Cc: Felipe Balbi 

Care to blame the original commit and Cc stable if applicable?

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] usb: dwc3: Fix error handling for core init

2016-10-21 Thread Vivek Gautam
Hi,


On Fri, Oct 21, 2016 at 3:45 PM, Felipe Balbi  wrote:
>
> Hi,
>
> Vivek Gautam  writes:
>> Fixing the sequence of events in dwc3_core_init() error exit path.
>> dwc3_core_exit() call is removed from the error path since,
>> whatever it's doing is already done.
>>
>> Signed-off-by: Vivek Gautam 
>> Cc: Felipe Balbi 
>
> Care to blame the original commit and Cc stable if applicable?

Ok, will do that.


Thanks
Vivek


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/1] usb: xhci: clean up error_bitmask usage

2016-10-21 Thread Mathias Nyman

On 21.10.2016 06:14, Lu Baolu wrote:

In xhci_handle_event(), when errors are detected, driver always sets
a bit in error_bitmask (one member of the xhci private driver data).
That means users have to retrieve and decode the value of error_bitmask
in xhci private driver data if they want to know whether those erros
ever happened in xhci_handle_event(). Otherwise, those errors are just
ignored silently.

This patch cleans up this by replacing the setting of error_bitmask
with the kernel print functions, so that users can easily check and
report the errors happened in xhci_handle_event().

Signed-off-by: Lu Baolu 
---
Changelog:
V2:
  - Remove "return" for failed port status event;
  - Refine checking return value of handle_tx_event();
  - Print the right TRB type value in debug message.



Thanks, added

-Mathias

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


[PATCH v2] usb: dwc3: Fix error handling for core init

2016-10-21 Thread Vivek Gautam
Fixing the sequence of events in dwc3_core_init() error exit path.
dwc3_core_exit() call is also removed from the error path since,
whatever it's doing is already done.

Fixes: c499ff7 usb: dwc3: core: re-factor init and exit paths

Cc: Felipe Balbi 
Cc: Greg KH 
Cc: Stable  # 4.8+
Signed-off-by: Vivek Gautam 
---

Based on usb-next.
Build tested.

Changes since v1:
 - Added reference to original commit that this patch fixes.
 - CC'ed kernel stable.

 drivers/usb/dwc3/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 7287a76..fea4469 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -769,15 +769,14 @@ static int dwc3_core_init(struct dwc3 *dwc)
return 0;
 
 err4:
-   phy_power_off(dwc->usb2_generic_phy);
+   phy_power_off(dwc->usb3_generic_phy);
 
 err3:
-   phy_power_off(dwc->usb3_generic_phy);
+   phy_power_off(dwc->usb2_generic_phy);
 
 err2:
usb_phy_set_suspend(dwc->usb2_phy, 1);
usb_phy_set_suspend(dwc->usb3_phy, 1);
-   dwc3_core_exit(dwc);
 
 err1:
usb_phy_shutdown(dwc->usb2_phy);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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


Re: [PATCH v2 0/2] usb: host: xhci: plat: add support for Renesas r8a7796 SoC

2016-10-21 Thread Mathias Nyman

On 19.10.2016 11:50, Yoshihiro Shimoda wrote:

This patch set is based on the latest Greg's usb.git / usb-next branch.
(commit id = 1001354ca34179f3db924eb66672442a173147dc)

Changes from v1:
  - Revise the comment in patch 1.
  - Don't add a new macro because the macro will be not used in the future.
(Especially, the xhci-rcar driver will need to check SoC revision and
 change the firmware name.)

Yoshihiro Shimoda (2):
   usb: host: xhci: rcar: add a new firmware version for r8a7796
   usb: host: xhci: plat: add support for Renesas r8a7796 SoC

  Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
  drivers/usb/host/xhci-plat.c   | 9 +
  drivers/usb/host/xhci-rcar.c   | 4 
  drivers/usb/host/xhci-rcar.h   | 1 +
  4 files changed, 15 insertions(+)



Thanks, added it to my tree

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


[PATCH] USB: serial: fix potential NULL-dereference at probe

2016-10-21 Thread Johan Hovold
Make sure we have at least one port before attempting to register a
console.

Currently, at least one driver binds to a "dummy" interface and requests
zero ports for it. Should such an interface also lack endpoints, we get
a NULL-deref during probe.

Fixes: e5b1e2062e05 ("USB: serial: make minor allocation dynamic")
Cc: stable  # 3.11
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/usb-serial.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index d213cf44a7e4..4a037b4a79cf 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1078,7 +1078,8 @@ static int usb_serial_probe(struct usb_interface 
*interface,
 
serial->disconnected = 0;
 
-   usb_serial_console_init(serial->port[0]->minor);
+   if (num_ports > 0)
+   usb_serial_console_init(serial->port[0]->minor);
 exit:
module_put(type->driver.owner);
return 0;
-- 
2.7.3

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


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

2016-10-21 Thread Johan Hovold
On Fri, Oct 21, 2016 at 02:49:05AM -0700, Tony Lindgren wrote:
> * Johan Hovold  [161021 02:26]:
> > On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote:
> > > * Johan Hovold  [161020 08:38]:
> > > > Hi Tony,
> > > > 
> > > > I'm getting the splat below when booting 4.9-rc1 on a BBB and
> > > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for
> > > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer
> > > > callback:
> > > 
> > > OK, sorry to hear about that. Care to email me your .config and how
> > > to reproduce and what do you have connected like a hub? Also do
> > > you use built-in gadgets or configure them via configfs?
> > 
> > It happens even with no devices connected to the host port, and with no
> > gadgets configured (or the peripheral port simply disabled). Whenever
> > the glue timer fires and calls pm_runtime_get_sync() I get the splat.
> > For some reason the might_sleep() in get_sync() does not trigger the
> > first time, which means that I see this four seconds after probe, and
> > then every other second when the timer fires.
> 
> OK. I'm totally baffled how come I did not hit this earlier with
> my tests. I did have some extra patches for using the pmic vbus irq
> for cable detection, but that's only for the peripheral instance and
> the host instance is still using timer.

Yeah, I was a bit surprised as well given that other people have also
apparently run this code. Guess CONFIG_DEBUG_ATOMIC_SLEEP must have been
disabled?

> > > > Setting the irq_safe flag seems to do the trick, but not sure that's
> > > > what you intended to do.
> > > 
> > > That's what we want to avoid as it keep the parent device permanently
> > > enabled. To avoid that we want to just queue things and deal with them
> > > from pm_runtime_resume.
> > 
> > I figured, so then that pm_runtime_get_sync() in the dsps timer callback
> > needs to go.
> 
> Agreed, that is clearly wrong to call from softirq context. I need to
> figure out what is right fix here but don't have access to my bbb
> until next week.

Ok, I'll make do with irq_safe meanwhile.

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: musb RPM sleep-while-atomic in 4.9-rc1

2016-10-21 Thread Tony Lindgren
* Johan Hovold  [161021 04:08]:
> On Fri, Oct 21, 2016 at 02:49:05AM -0700, Tony Lindgren wrote:
> > * Johan Hovold  [161021 02:26]:
> > > On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote:
> > > > * Johan Hovold  [161020 08:38]:
> > > > > Hi Tony,
> > > > > 
> > > > > I'm getting the splat below when booting 4.9-rc1 on a BBB and
> > > > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support 
> > > > > for
> > > > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer
> > > > > callback:
> > > > 
> > > > OK, sorry to hear about that. Care to email me your .config and how
> > > > to reproduce and what do you have connected like a hub? Also do
> > > > you use built-in gadgets or configure them via configfs?
> > > 
> > > It happens even with no devices connected to the host port, and with no
> > > gadgets configured (or the peripheral port simply disabled). Whenever
> > > the glue timer fires and calls pm_runtime_get_sync() I get the splat.
> > > For some reason the might_sleep() in get_sync() does not trigger the
> > > first time, which means that I see this four seconds after probe, and
> > > then every other second when the timer fires.
> > 
> > OK. I'm totally baffled how come I did not hit this earlier with
> > my tests. I did have some extra patches for using the pmic vbus irq
> > for cable detection, but that's only for the peripheral instance and
> > the host instance is still using timer.
> 
> Yeah, I was a bit surprised as well given that other people have also
> apparently run this code. Guess CONFIG_DEBUG_ATOMIC_SLEEP must have been
> disabled?

Yeh that's it, I've been just using omap2plus_defconfig where it seems
to be disabled. Probably a good idea to enable it there.

> > > > > Setting the irq_safe flag seems to do the trick, but not sure that's
> > > > > what you intended to do.
> > > > 
> > > > That's what we want to avoid as it keep the parent device permanently
> > > > enabled. To avoid that we want to just queue things and deal with them
> > > > from pm_runtime_resume.
> > > 
> > > I figured, so then that pm_runtime_get_sync() in the dsps timer callback
> > > needs to go.
> > 
> > Agreed, that is clearly wrong to call from softirq context. I need to
> > figure out what is right fix here but don't have access to my bbb
> > until next week.
> 
> Ok, I'll make do with irq_safe meanwhile.

OK

Regards,

Tony
--
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: [balbi-usb:testing/next 57/84] drivers/usb/dwc3/dwc3-st.c:328:2: error: implicit declaration of function 'pinctrl_pm_select_sleep_state'

2016-10-21 Thread Linus Walleij
On Mon, Oct 17, 2016 at 1:50 PM, Felipe Balbi
 wrote:

> kbuild test robot  writes:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
>> testing/next
>> head:   4281ef86fae986e0a9bb553fb311fe6d8e039118
>> commit: 040f47e7330045feaa8c06bf2900db2eb2038e80 [57/84] usb: dwc3: Kconfig: 
>> allow all glues to build if COMPILE_TEST
>> config: blackfin-allmodconfig (attached as .config)
>> compiler: bfin-uclinux-gcc (GCC) 6.2.0
>> reproduce:
>> wget 
>> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>>  -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> git checkout 040f47e7330045feaa8c06bf2900db2eb2038e80
>> # save the attached .config to linux build tree
>> make.cross ARCH=blackfin
>>
>> All errors (new ones prefixed by >>):
>>
>>drivers/usb/dwc3/dwc3-st.c: In function 'st_dwc3_suspend':
 drivers/usb/dwc3/dwc3-st.c:328:2: error: implicit declaration of function 
 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
>>  pinctrl_pm_select_sleep_state(dev);
>>  ^
>>drivers/usb/dwc3/dwc3-st.c: In function 'st_dwc3_resume':
 drivers/usb/dwc3/dwc3-st.c:338:2: error: implicit declaration of function 
 'pinctrl_pm_select_default_state' [-Werror=implicit-function-declaration]
>>  pinctrl_pm_select_default_state(dev);
>>  ^~~
>>cc1: some warnings being treated as errors
>
> looks like a bug in . Something along the
> lines of below should be enough:

I don't think so, that include file looks pretty solid.

I think the actual problem is that dwc3-st.c doesn't
#include  even if it is using the
functions from it.

Yours,
Linus Walleij
--
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/2] usb: dwc3: st: add missing include

2016-10-21 Thread Felipe Balbi
dwc3-st uses pinctrl_pm_select_*_state() however it
doesn't include the necessary header. Fix the build
break caused by that, by simply including the
missing header.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/dwc3-st.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 89a2f712fdfe..aaaf256f71dd 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "core.h"
-- 
2.10.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/2] usb: dwc3: st: allow to build on COMPILE_TEST

2016-10-21 Thread Felipe Balbi
Every glue layer should be buildable on COMPILE_TEST

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index b97cde76914d..66943106a69a 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -98,7 +98,7 @@ config USB_DWC3_OF_SIMPLE
 
 config USB_DWC3_ST
tristate "STMicroelectronics Platforms"
-   depends on ARCH_STI && OF
+   depends on ARCH_STI && OF || COMPILE_TEST
default USB_DWC3
help
  STMicroelectronics SoCs with one DesignWare Core USB3 IP
-- 
2.10.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: [balbi-usb:testing/next 57/84] drivers/usb/dwc3/dwc3-st.c:328:2: error: implicit declaration of function 'pinctrl_pm_select_sleep_state'

2016-10-21 Thread Felipe Balbi

Hi,

Linus Walleij  writes:
> On Mon, Oct 17, 2016 at 1:50 PM, Felipe Balbi
>  wrote:
>
>> kbuild test robot  writes:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
>>> testing/next
>>> head:   4281ef86fae986e0a9bb553fb311fe6d8e039118
>>> commit: 040f47e7330045feaa8c06bf2900db2eb2038e80 [57/84] usb: dwc3: 
>>> Kconfig: allow all glues to build if COMPILE_TEST
>>> config: blackfin-allmodconfig (attached as .config)
>>> compiler: bfin-uclinux-gcc (GCC) 6.2.0
>>> reproduce:
>>> wget 
>>> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>>>  -O ~/bin/make.cross
>>> chmod +x ~/bin/make.cross
>>> git checkout 040f47e7330045feaa8c06bf2900db2eb2038e80
>>> # save the attached .config to linux build tree
>>> make.cross ARCH=blackfin
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>drivers/usb/dwc3/dwc3-st.c: In function 'st_dwc3_suspend':
> drivers/usb/dwc3/dwc3-st.c:328:2: error: implicit declaration of function 
> 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
>>>  pinctrl_pm_select_sleep_state(dev);
>>>  ^
>>>drivers/usb/dwc3/dwc3-st.c: In function 'st_dwc3_resume':
> drivers/usb/dwc3/dwc3-st.c:338:2: error: implicit declaration of function 
> 'pinctrl_pm_select_default_state' [-Werror=implicit-function-declaration]
>>>  pinctrl_pm_select_default_state(dev);
>>>  ^~~
>>>cc1: some warnings being treated as errors
>>
>> looks like a bug in . Something along the
>> lines of below should be enough:
>
> I don't think so, that include file looks pretty solid.
>
> I think the actual problem is that dwc3-st.c doesn't
> #include  even if it is using the
> functions from it.

Oh, I missed that there was already a stub further down the
file. Indeed, we need to add a patch to dwc3-st.c.

Sorry for the noise.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 1/2] usb: dwc3: st: add missing include

2016-10-21 Thread Sergei Shtylyov

Hello.

On 10/21/2016 03:29 PM, Felipe Balbi wrote:


dwc3-st uses pinctrl_pm_select_*_state() however it
doesn't include the necessary header. Fix the build
break caused by that, by simply including the
missing header.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/dwc3-st.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 89a2f712fdfe..aaaf256f71dd 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 


   s/consume.h/consumer.h/ in the subject?


 #include 

 #include "core.h"


MBR, Sergei

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


Re: [PATCH 1/2] usb: dwc3: st: add missing include

2016-10-21 Thread Felipe Balbi

Hi,

Sergei Shtylyov  writes:
> Hello.
>
> On 10/21/2016 03:29 PM, Felipe Balbi wrote:
>
>> dwc3-st uses pinctrl_pm_select_*_state() however it
>> doesn't include the necessary header. Fix the build
>> break caused by that, by simply including the
>> missing header.
>>
>> Signed-off-by: Felipe Balbi 
>> ---
>>  drivers/usb/dwc3/dwc3-st.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
>> index 89a2f712fdfe..aaaf256f71dd 100644
>> --- a/drivers/usb/dwc3/dwc3-st.c
>> +++ b/drivers/usb/dwc3/dwc3-st.c
>> @@ -31,6 +31,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>
> s/consume.h/consumer.h/ in the subject?

fixed, though it was misleading to place the comment here ;-)

From 4bdfa9825c4e4f48d889d92e11d171ec325d3bc3 Mon Sep 17 00:00:00 2001
From: Felipe Balbi 
Date: Fri, 21 Oct 2016 15:23:59 +0300
Subject: [PATCH 1/2] usb: dwc3: st: add missing 
 include

dwc3-st uses pinctrl_pm_select_*_state() however it
doesn't include the necessary header. Fix the build
break caused by that, by simply including the
missing header.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/dwc3-st.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 89a2f712fdfe..aaaf256f71dd 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "core.h"
-- 
2.10.1



-- 
balbi


signature.asc
Description: PGP signature


[GIT PULL] USB-serial fixes for v4.9-rc2

2016-10-21 Thread Johan Hovold
Hi Greg,

Here are some fixes for 4.9-rc2. Details below.

Thanks,
Johan


The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
tags/usb-serial-4.9-rc2

for you to fetch changes up to 126d26f66d9890a69158812a6caa248c05359daa:

  USB: serial: fix potential NULL-dereference at probe (2016-10-21 16:47:17 
+0200)


USB-serial fixes for v4.9-rc2

Here's a fix for a NULL-deref during probe which could be triggered by a
malicious device, and a fix for some missing error handling in cp210x
that also leaked some bits from the stack. Included is also a new device
id for ftdi_sio.

Signed-off-by: Johan Hovold 


Johan Hovold (2):
  USB: serial: cp210x: fix tiocmget error handling
  USB: serial: fix potential NULL-dereference at probe

Stefan Tauner (1):
  USB: serial: ftdi_sio: add support for Infineon TriBoard TC2X7

 drivers/usb/serial/cp210x.c   | 4 +++-
 drivers/usb/serial/ftdi_sio.c | 3 ++-
 drivers/usb/serial/ftdi_sio_ids.h | 5 +++--
 drivers/usb/serial/usb-serial.c   | 3 ++-
 4 files changed, 10 insertions(+), 5 deletions(-)
--
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: xhci: Workaround for erratum-A010129

2016-10-21 Thread Sriram Dash
For the USB3.0 controller, USB 2.0 reset not driven while
port is in Resume state. So, do not program the USB 2.0 reset
(PORTSC[PR]=1) while in Resume state.

Signed-off-by: Rajat Srivastava 
Signed-off-by: Sriram Dash 
Signed-off-by: Rajesh Bhagat 
---
 drivers/usb/host/xhci-hub.c | 28 +++-
 drivers/usb/host/xhci.h |  1 +
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 730b9fd..3def0dd 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -878,7 +878,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
int max_ports;
unsigned long flags;
-   u32 temp, status;
+   u32 temp, status, tmp_status = 0;
int retval = 0;
__le32 __iomem **port_array;
int slot_id;
@@ -1098,6 +1098,32 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
spin_lock_irqsave(&xhci->lock, flags);
break;
case USB_PORT_FEAT_RESET:
+   /*
+* Erratum : A010129
+* Synopsys STAR 9000962562.
+* USB 2.0 Reset Not Driven While Port in Resume
+* While in USB 2.0 resume state (the PORTSC.PLS
+* register bit is set to 4'd15), if the xHCI driver
+* programs the PORTSC.PR register bit to 1, the
+* controller does not drive a USB 2.0 reset
+* and it does not generate a PORTSC.PRC=1 interrupt.
+* So, The xHCI driver should not program a USB
+* 2.0 reset (PORTSC.PR=1) while in resume
+* (PORTSC.PLS=4'd15).
+*/
+   if (xhci->quirks & XHCI_PORT_RST_ON_RESUME) {
+   tmp_status = readl(port_array[wIndex]);
+   spin_unlock_irqrestore(&xhci->lock, flags);
+   if (!DEV_SUPERSPEED(tmp_status)
+   && (tmp_status & PORT_PLS_MASK)
+   == XDEV_RESUME) {
+   xhci_err(xhci, "skip port reset\n");
+   spin_lock_irqsave(&xhci->lock, flags);
+   break;
+   }
+   spin_lock_irqsave(&xhci->lock, flags);
+   }
+
temp = (temp | PORT_RESET);
writel(temp, port_array[wIndex]);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index b2c1dc5..c07e267 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1653,6 +1653,7 @@ struct xhci_hcd {
 #define XHCI_MTK_HOST  (1 << 21)
 #define XHCI_SSIC_PORT_UNUSED  (1 << 22)
 #define XHCI_NO_64BIT_SUPPORT  (1 << 23)
+#define XHCI_PORT_RST_ON_RESUME(1 << 24)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
-- 
2.1.0

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


Re: [PATCH] usb: xhci: Workaround for erratum-A010129

2016-10-21 Thread Alan Stern
On Fri, 21 Oct 2016, Sriram Dash wrote:

> For the USB3.0 controller, USB 2.0 reset not driven while
> port is in Resume state. So, do not program the USB 2.0 reset
> (PORTSC[PR]=1) while in Resume state.
> 
> Signed-off-by: Rajat Srivastava 
> Signed-off-by: Sriram Dash 
> Signed-off-by: Rajesh Bhagat 
> ---
>  drivers/usb/host/xhci-hub.c | 28 +++-
>  drivers/usb/host/xhci.h |  1 +
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 730b9fd..3def0dd 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -878,7 +878,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
> u16 wValue,
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>   int max_ports;
>   unsigned long flags;
> - u32 temp, status;
> + u32 temp, status, tmp_status = 0;
>   int retval = 0;
>   __le32 __iomem **port_array;
>   int slot_id;
> @@ -1098,6 +1098,32 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
> u16 wValue,
>   spin_lock_irqsave(&xhci->lock, flags);
>   break;
>   case USB_PORT_FEAT_RESET:
> + /*
> +  * Erratum : A010129
> +  * Synopsys STAR 9000962562.
> +  * USB 2.0 Reset Not Driven While Port in Resume
> +  * While in USB 2.0 resume state (the PORTSC.PLS
> +  * register bit is set to 4'd15), if the xHCI driver
> +  * programs the PORTSC.PR register bit to 1, the
> +  * controller does not drive a USB 2.0 reset
> +  * and it does not generate a PORTSC.PRC=1 interrupt.
> +  * So, The xHCI driver should not program a USB
> +  * 2.0 reset (PORTSC.PR=1) while in resume
> +  * (PORTSC.PLS=4'd15).
> +  */
> + if (xhci->quirks & XHCI_PORT_RST_ON_RESUME) {

It's always a bad idea to drive a reset signal while a port is 
resuming.  We don't need a quirk flag for this.  The HCD should never 
do it, and the USB core should never tell the HCD to do it.

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


[RESEND PATCH] usb: chipidea: Configure DMA properties and ops from DT

2016-10-21 Thread Bjorn Andersson
hcd_alloc_coherent() and usb_alloc_coherent() ends up allocating coherent
memory on behalf of ci_hdrc driver. But as the ci_hdrc is instantiated manually
it will not have any dma_mem or dma_ops assigned, which makes the
dma_alloc_coherent() fail on some platforms (e.g. arm64). This patch solves
this by assigning the dma_mem and dma_ops based on the parent's DeviceTree
node.

Cc: Stephen Boyd 
Signed-off-by: Bjorn Andersson 
---

Hi Peter,

After (once more) debugging why USB doesn't work up on the 64-bit Qualcomm
systems I realized that we never concluded on this patch. Unfortunately I can't
find it in my mailbox either, so resending it to restart the discussion.

Regards,
Bjorn

 drivers/usb/chipidea/core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e644d17..6218d83cca25 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -62,6 +62,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -837,6 +838,9 @@ struct platform_device *ci_hdrc_add_device(struct device 
*dev,
pdev->dev.dma_parms = dev->dma_parms;
dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
 
+   if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+   of_dma_configure(&pdev->dev, dev->of_node);
+
ret = platform_device_add_resources(pdev, res, nres);
if (ret)
goto err;
-- 
2.5.0

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


Re: [RESEND PATCH] usb: chipidea: Configure DMA properties and ops from DT

2016-10-21 Thread Stephen Boyd
On 10/21, Bjorn Andersson wrote:
> hcd_alloc_coherent() and usb_alloc_coherent() ends up allocating coherent
> memory on behalf of ci_hdrc driver. But as the ci_hdrc is instantiated 
> manually
> it will not have any dma_mem or dma_ops assigned, which makes the
> dma_alloc_coherent() fail on some platforms (e.g. arm64). This patch solves
> this by assigning the dma_mem and dma_ops based on the parent's DeviceTree
> node.
> 
> Cc: Stephen Boyd 
> Signed-off-by: Bjorn Andersson 
> ---
> 
> Hi Peter,
> 
> After (once more) debugging why USB doesn't work up on the 64-bit Qualcomm
> systems I realized that we never concluded on this patch. Unfortunately I 
> can't
> find it in my mailbox either, so resending it to restart the discussion.
> 

I thought we were going to go down the route that Arnd has been
pushing[1]? That should work, but I haven't tried it yet and
there are some more fixes on top from Sriram. I think Sriram is
taking over the patch now?

[1] https://patchwork.kernel.org/patch/9319527/

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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: [RESEND PATCH] usb: chipidea: Configure DMA properties and ops from DT

2016-10-21 Thread Bjorn Andersson
On Fri 21 Oct 10:38 PDT 2016, Stephen Boyd wrote:

> On 10/21, Bjorn Andersson wrote:
> > hcd_alloc_coherent() and usb_alloc_coherent() ends up allocating coherent
> > memory on behalf of ci_hdrc driver. But as the ci_hdrc is instantiated 
> > manually
> > it will not have any dma_mem or dma_ops assigned, which makes the
> > dma_alloc_coherent() fail on some platforms (e.g. arm64). This patch solves
> > this by assigning the dma_mem and dma_ops based on the parent's DeviceTree
> > node.
> > 
> > Cc: Stephen Boyd 
> > Signed-off-by: Bjorn Andersson 
> > ---
> > 
> > Hi Peter,
> > 
> > After (once more) debugging why USB doesn't work up on the 64-bit Qualcomm
> > systems I realized that we never concluded on this patch. Unfortunately I 
> > can't
> > find it in my mailbox either, so resending it to restart the discussion.
> > 
> 
> I thought we were going to go down the route that Arnd has been
> pushing[1]? That should work, but I haven't tried it yet and
> there are some more fixes on top from Sriram. I think Sriram is
> taking over the patch now?
> 
> [1] https://patchwork.kernel.org/patch/9319527/

Thanks for the pointer, I've heard about it but couldn't find it.

It does make me further wonder about the multi-device model of these
drivers, but I agree with you that it looks like the patch would
solve our issue.

Regards,
Bjorn
--
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 v9 1/2] usb: dwc2: assert phy reset when waking up in rk3288 platform

2016-10-21 Thread John Youn
On 10/20/2016 11:38 AM, Randy Li wrote:
> On the rk3288 USB host-only port (the one that's not the OTG-enabled
> port) the PHY can get into a bad state when a wakeup is asserted (not
> just a wakeup from full system suspend but also a wakeup from
> autosuspend).
> 
> We can get the PHY out of its bad state by asserting its "port reset",
> but unfortunately that seems to assert a reset onto the USB bus so it
> could confuse things if we don't actually deenumerate / reenumerate the
> device.
> 
> We can also get the PHY out of its bad state by fully resetting it using
> the reset from the CRU (clock reset unit) in chip, which does a more full
> reset.  The CRU-based reset appears to actually cause devices on the bus
> to be removed and reinserted, which fixes the problem (albeit in a hacky
> way).
> 
> It's unfortunate that we need to do a full re-enumeration of devices at
> wakeup time, but this is better than alternative of letting the bus get
> wedged.
> 
> Signed-off-by: Randy Li 
> ---
>  drivers/usb/dwc2/core.h  |  1 +
>  drivers/usb/dwc2/core_intr.c | 11 +++
>  drivers/usb/dwc2/platform.c  |  9 +
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 2a21a04..e91ddbc 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -859,6 +859,7 @@ struct dwc2_hsotg {
>   unsigned int ll_hw_enabled:1;
>  
>   struct phy *phy;
> + struct work_struct phy_rst_work;
>   struct usb_phy *uphy;
>   struct dwc2_hsotg_plat *plat;
>   struct regulator_bulk_data 
> supplies[ARRAY_SIZE(dwc2_hsotg_supply_names)];
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index d85c5c9..c3d2168 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -345,6 +345,7 @@ static void dwc2_handle_session_req_intr(struct 
> dwc2_hsotg *hsotg)
>  static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
>  {
>   int ret;
> + struct device_node *np = hsotg->dev->of_node;
>  
>   /* Clear interrupt */
>   dwc2_writel(GINTSTS_WKUPINT, hsotg->regs + GINTSTS);
> @@ -379,6 +380,16 @@ static void dwc2_handle_wakeup_detected_intr(struct 
> dwc2_hsotg *hsotg)
>   /* Restart the Phy Clock */
>   pcgcctl &= ~PCGCTL_STOPPCLK;
>   dwc2_writel(pcgcctl, hsotg->regs + PCGCTL);
> +
> + /*
> +  * It is a quirk in Rockchip RK3288, causing by
> +  * a hardware bug. This will propagate out and
> +  * eventually we'll re-enumerate the device. 
> +  * Not great but the best we can do 
> +  */
> + if (of_device_is_compatible(np, "rockchip,rk3288-usb"))
> + schedule_work(&hsotg->phy_rst_work);
> +
>   mod_timer(&hsotg->wkp_timer,
> jiffies + msecs_to_jiffies(71));
>   } else {
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 8e1728b..65953cf 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -366,6 +366,14 @@ int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
>   return ret;
>  }
>  
> +/* Only used to reset usb phy at interrupter runtime */
> +static void dwc2_reset_phy_work(struct work_struct *data)
> +{
> + struct dwc2_hsotg *hsotg = container_of(data, struct dwc2_hsotg,
> + phy_rst_work);
> + phy_reset(hsotg->phy);
> +}
> +
>  static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
>  {
>   int i, ret;
> @@ -410,6 +418,7 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
>   return ret;
>   }
>   }
> + INIT_WORK(&hsotg->phy_rst_work, dwc2_reset_phy_work);
>  
>   if (!hsotg->phy) {
>   hsotg->uphy = devm_usb_get_phy(hsotg->dev, USB_PHY_TYPE_USB2);
> 

Hi Randy,

This fails compile if CONFIG_GENERIC_PHY is disabled. I think you need
to make a fix to your phy_reset patch first.

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


Re: [RFC][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

2016-10-21 Thread John Youn
On 10/20/2016 5:43 PM, Chen Yu wrote:
> On 2016/10/19 6:21, John Youn wrote:
>> On 10/16/2016 7:42 PM, Chen Yu wrote:
>>>
>>>
>>> On 2016/10/15 3:37, John Youn wrote:
 On 10/13/2016 4:36 PM, John Stultz wrote:
> From: Chen Yu 
>
> The Hi6220's usb controller is limited in that it does not
> automatically autonegotiate the usb speed. Thus it requires a
> quirk so that we can manually negotiate the best usb speed for
> the attached device.

 Hi,

 Could you expand more on this by explaining what exactly is the
 limitation and the workaround?

>>>
>>> The USB host limitation of Hisilicon Hi6220 is full-speed and low-speed
>>> devices can not be enumerated when gets plugged behind a hub.
>>>
 [snip]

> +/*
> + * HPRT0_SPD_HIGH_SPEED: high speed
> + * HPRT0_SPD_FULL_SPEED: full speed
> + */
> +static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (hsotg->core_params->speed == speed)
> + return;
> +
> + hsotg->core_params->speed = speed;
> + queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> +}
> +
> +static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (!hsotg->change_speed_quirk)
> + return 1;
> +
> + hsotg->device_count++;

 Why do you need to track the device count?

> + dev_info(hsotg->dev, "Device count is %u after alloc dev\n",
> + hsotg->device_count);
> +
> + return 1;
> +}
> +
> +static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (!hsotg->change_speed_quirk)
> + return;
> +
> + if (hsotg->device_count)
> + hsotg->device_count--;
> +
> + dev_info(hsotg->dev, "Device count is %u after free dev\n",
> + hsotg->device_count);
> +
> + if (hsotg->device_count == 1 && udev->parent &&
> + udev->parent->speed > USB_SPEED_UNKNOWN &&
> + udev->parent->speed < USB_SPEED_HIGH) {
> + dev_info(hsotg->dev, "Set speed to default high-speed\n");
> + dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
> + }
> +}
> +
> +static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device 
> *udev)
> +{
> + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> +
> + if (!hsotg->change_speed_quirk)
> + return 0;
> +
> + if (udev->speed == USB_SPEED_HIGH) {
> + dev_info(hsotg->dev, "Set speed to high-speed\n");
> + dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
> + } else if (udev->speed == USB_SPEED_FULL
> + || udev->speed == USB_SPEED_LOW) {
> + dev_info(hsotg->dev, "Set speed to full-speed\n");
> + dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
> + }

 It seems you are reinitializing the core every time a device is reset
 and the udev->speed does not match the core_param speed. But how is
 the udev->speed being set correctly if the hw cannot negotiate the
 speed in the first place?

>>>
>>> The hardware can negotiate the speed, but communication with a full-speed or
>>> low-speed device behind a hub is the problem.
>>>
 Also should it be for every device? What about if a device gets
 plugged in behind a hub? I don't think you want to execute this code
 in that case.

 This should only affect devices plugged into the root hub, correct?
 And the hsotg controller only has one root hub port. It seems things
 could be simplified a bit.

>>>
>>> The patch is initially written for Hikey Hi6220 board, and there is a
>>> hub always connected to root hub, so the patch sets the configuration to
>>> HPRT0_SPD_HIGH_SPEED when there is only one device(the hub).
>>
>> Ok, I see.
>>
>>>
>>> Thanks for your suggestions, the patch needs modified in these aspect:
>>> 1. Change the speed setting only when the device is behind a hub in 
>>> dwc2_reset_device.
>>
>> I still think you will have issues with multiple devices. Since you
>> have a built-in hub after root hub, it will always be behind the
>> hub. So whenver you need to change speeds, it will always reset every
>> device in the tree. Have you tested with multiple devices and also
>> multiple levels of hubs?
>>
> 
> Yes, a full-speed device or low-speed device being plugged in will reset 
> other devices
> if current speed setting is not full-speed.
> I have tested with multiple devices and also two levels of high-speed hubs,
> the patch was working well for enumerating full-speed device and low-speed 
> device.
> Also it changed the speed to high when there was only one hub conne

[PATCH 1/2] PCI: check for PME in targeted sleep state

2016-10-21 Thread Alan Stern
One some systems, the firmware does not allow certain PCI devices to
be put in deep D-states.  This can cause problems for wakeup
signalling, if the device does not support PME# in the deepest allowed
suspend state.  For example, Pierre reports that on his system, ACPI
does not permit his xHCI host controller to go into D3 during runtime
suspend -- but D3 is the only state in which the controller can generate
PME# signals.  As a result, the controller goes into runtime suspend
but never wakes up, so it doesn't work properly.  USB devices plugged
into the controller are never detected.

If the device relies on PME# for wakeup signals but is not capable of
generating PME# in the target state, the PCI core should accurately
report that it cannot do wakeup from runtime suspend.  This patch
modifies the pci_dev_run_wake() routine to add this check.

Signed-off-by: Alan Stern 
CC: Lukas Wunner 
Reported-by: Pierre de Villemereuil 
Tested-by: Pierre de Villemereuil 
CC: 

---

[as1814]


 drivers/pci/pci.c |4 
 1 file changed, 4 insertions(+)

Index: usb-4.x/drivers/pci/pci.c
===
--- usb-4.x.orig/drivers/pci/pci.c
+++ usb-4.x/drivers/pci/pci.c
@@ -2064,6 +2064,10 @@ bool pci_dev_run_wake(struct pci_dev *de
if (!dev->pme_support)
return false;
 
+   /* PME-capable in principle, but not from the intended sleep state */
+   if (!pci_pme_capable(dev, pci_target_state(dev)))
+   return false;
+
while (bus->parent) {
struct pci_dev *bridge = bus->self;
 

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


Re: [PATCH 1/2] PCI: check for PME in targeted sleep state

2016-10-21 Thread Rafael J. Wysocki
On Friday, October 21, 2016 04:45:38 PM Alan Stern wrote:
> One some systems, the firmware does not allow certain PCI devices to
> be put in deep D-states.  This can cause problems for wakeup
> signalling, if the device does not support PME# in the deepest allowed
> suspend state.  For example, Pierre reports that on his system, ACPI
> does not permit his xHCI host controller to go into D3 during runtime
> suspend -- but D3 is the only state in which the controller can generate
> PME# signals.  As a result, the controller goes into runtime suspend
> but never wakes up, so it doesn't work properly.  USB devices plugged
> into the controller are never detected.
> 
> If the device relies on PME# for wakeup signals but is not capable of
> generating PME# in the target state, the PCI core should accurately
> report that it cannot do wakeup from runtime suspend.  This patch
> modifies the pci_dev_run_wake() routine to add this check.
> 
> Signed-off-by: Alan Stern 
> CC: Lukas Wunner 
> Reported-by: Pierre de Villemereuil 
> Tested-by: Pierre de Villemereuil 
> CC: 

Acked-by: Rafael J. Wysocki 

> ---
> 
> [as1814]
> 
> 
>  drivers/pci/pci.c |4 
>  1 file changed, 4 insertions(+)
> 
> Index: usb-4.x/drivers/pci/pci.c
> ===
> --- usb-4.x.orig/drivers/pci/pci.c
> +++ usb-4.x/drivers/pci/pci.c
> @@ -2064,6 +2064,10 @@ bool pci_dev_run_wake(struct pci_dev *de
>   if (!dev->pme_support)
>   return false;
>  
> + /* PME-capable in principle, but not from the intended sleep state */
> + if (!pci_pme_capable(dev, pci_target_state(dev)))
> + return false;
> +
>   while (bus->parent) {
>   struct pci_dev *bridge = bus->self;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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/2] USB: UHCI: report non-PME wakeup signalling for Intel hardware

2016-10-21 Thread Alan Stern
The UHCI controllers in Intel chipsets rely on a platform-specific
non-PME mechanism for wakeup signalling.  They can generate wakeup
signals even though they don't support PME.

We need to let the USB core know this so that it will enable runtime
suspend for UHCI controllers.

Signed-off-by: Alan Stern 
CC: 

---

Greg:

This patch is somewhat independent of the 1/2 patch I sent to Bjorn.  
Still, it will help to keep them together.  Is it okay to have him
merge both of them through his tree?

Alan Stern


[as1815]


 drivers/usb/host/uhci-pci.c |4 
 1 file changed, 4 insertions(+)

Index: usb-4.x/drivers/usb/host/uhci-pci.c
===
--- usb-4.x.orig/drivers/usb/host/uhci-pci.c
+++ usb-4.x/drivers/usb/host/uhci-pci.c
@@ -129,6 +129,10 @@ static int uhci_pci_init(struct usb_hcd
if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_HP)
uhci->wait_for_hp = 1;
 
+   /* Intel controllers use non-PME wakeup signalling */
+   if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_INTEL)
+   device_set_run_wake(uhci_dev(uhci), 1);
+
/* Set up pointers to PCI-specific functions */
uhci->reset_hc = uhci_pci_reset_hc;
uhci->check_and_reset_hc = uhci_pci_check_and_reset_hc;

--
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: PROBLEM: OHCI watchdog timeouts inside VirtualBox, probably due to timer wheel rework

2016-10-21 Thread Alan Stern
On Fri, 21 Oct 2016, Michal Necasek wrote:

>Alan,
> 
>  I'll get back to you on whether increasing the timeout helps, it'll
> take a bit of testing. Does it actually sound plausible that some
> controllers could not get things done in 250ms but could in 275ms?

Not very, I admit.

>  I will note that according to the table in <1>, a 250ms timeout with
> a HZ=250 kernel (what Ubuntu uses) falls into the 4ms granularity
> bucket, but a 275ms timeout goes into the 32ms granularity bucket.
> That could change things.

Thomas has noted that there is definitely a bug in the new timer code.

>  And as a bit of background... the 250ms timeout should not be a
> problem in a virtualized environment under normal conditions. A ~10ms
> timeout is. What makes things harder is that the watchdog routine
> reads the frame counter from the HCCA, which has to be updated
> asynchronously. Reading from a HC register would actually be more
> expensive but much more reliable in this case. But again, 250ms
> should be plenty...
> 
>  Another factor is that the OHCI watchdog just kills the driver the
> first time there's a problem, there's no recovery attempt. So it's
> very noticeable when this happens.

I should think so!  The whole point of the watchdog is to catch bad
hardware when it starts misbehaving.  If that happens, we need to
cancel everything and kill the driver so that it doesn't take large
parts of the system down with it -- which is what used to happen.

Alan Stern

> Regards,
>   Michal
> 
> 
> <1> http://lxr.free-electrons.com/source/kernel/time/timer.c


> - Original Message -
> From: st...@rowland.harvard.edu
> To: michael.tha...@oracle.com
> Cc: linux-ker...@vger.kernel.org, t...@linutronix.de, 
> michal.neca...@oracle.com, knut.osmund...@oracle.com
> Sent: Friday, October 21, 2016 6:54:13 PM GMT +01:00 Amsterdam / Berlin / 
> Bern / Rome / Stockholm / Vienna
> Subject: Re: PROBLEM: OHCI watchdog timeouts inside VirtualBox, probably due 
> to timer wheel rework
> 
> On Fri, 21 Oct 2016, Michael Thayer wrote:
> 
> > Hello Alan (LKML on CC),
> > 
> > Contacting you about this on Thomas Gleixner's (also on CC) suggestion. 
> > The short summary is that when Linux 4.8.0 (also tested with a few later 
> > kernels) is run on a VirtualBox virtual machine with USB enabled, OHCI 
> > fails with the log messages "frame counter not updated; disabled" and 
> > "HC died; cleaning up".  This seems to be due to the 250 ms interval 
> > watchdog running with far too short intervals, which we think is a 
> > consequence of the timer wheel code rework.  I will refer you to a bug 
> > filed in Launchpad<1> for a longer description.
> > 
> > Hope this is of interest to you.
> > 
> > Regards,
> > 
> > Michael
> > 
> > <1> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1634737
> 
> That bug description says the watchdog timer routine can be called
> twice in a 4-ms period, even though it requests a 250-ms delay.  Is
> this really true?  If it is, it sounds like a real bug in the timer 
> core.
> 
> Bryan Paluch reported a similar problem and said that increasing the
> timeout value to 275 ms fixed it:
> 
>   http://marc.info/?l=linux-usb&m=147670889009451&w=2
> 
> Does that patch also fix the "frame counter not updating" problem?
> 
> 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] USB: update intro of documentation

2016-10-21 Thread Jonathan Corbet
On Thu, 20 Oct 2016 15:15:00 +0200
Oliver Neukum  wrote:

> It does no good to mention The 2.4 kernel series and neglect
> USB 3.x and XHCI. Also with type C and micro/mini USB we better
> not talk about the shape of connectors.

...except that USB 2 connectors will be with us for some time yet.  I'm all
for updating the documentation, but stuff like this:

>  That master/slave asymmetry was designed-in for a number of
>  reasons, one being ease of use.  It is not physically possible to
> -assemble (legal) USB cables incorrectly:  all upstream "to the host"
> -connectors are the rectangular type (matching the sockets on
> -root hubs), and all downstream connectors are the squarish type
> +mistake upstream and downstream or it does not matter with a type C
> +plug

seems to me like it loses information.  Greg, do you have an opinion on the
changes here?  I'll apply this if it's OK with you.

Thanks,

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


link state problem with dwc3 in supper-speed device mode

2016-10-21 Thread Bin Liu
Hi,

I run into a link state problem when dwc3 is in supper-speed device
mode.

Modprobe g_zero, link state is U3 (checked in DSTS).

After dwc3 is enumerated by the host, the trace on the bus is as:

[both Device and Host are in U0 now]
D->H: LGO_U1
H->D: LAU
D->H: LPMA

but actually dwc3 goes to U2. Is this expected?

Now remove the cable from the host, but dwc3 does not generate any
change, link state is still in U2, no any ftrace log either from the
point when removing the cable.

Now connect the cable again, dwc3 goes to SS.Inactive, shown in ftrace.
Of cause the host does not detect the attach at this point.

Then remove the cable, dwc3 changes as follows in ftrace:

RX.Detect
U0
U0
RX.Detect
U3

Basically the host does not enumerate dwc3 in every another attach.

Any comments about this problem? BTY, I checked with kernel v4.9-rc1,
v4.8, v4.4, v3.14 if it matters...

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


[PATCH v2 RESEND] drivers/usb: Skip auto handoff for TI and RENESAS usb controllers

2016-10-21 Thread Babu Moger
Never seen XHCI auto handoff working on TI and RENESAS cards.
Eventually, we force handoff. This code forces the handoff
unconditionally. It saves 5 seconds boot time for each card.

Signed-off-by: Babu Moger 
---
v2: 
 Made changes per comments from Greg KH.
 Extra space removal in assignment
 Added both vendor and device id checks.
 Resending the patch. Original discussion here.
 https://marc.info/?t=14522116207&r=1&w=4

 drivers/usb/host/pci-quirks.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 35af362..31c9502 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -996,6 +996,14 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
}
val = readl(base + ext_cap_offset);
 
+   /* Auto handoff never worked for these devices. Force it and continue */
+   if ((pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) ||
+   (pdev->vendor == PCI_VENDOR_ID_RENESAS
+&& pdev->device == 0x0014)) {
+   val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
+   writel(val, base + ext_cap_offset);
+   }
+
/* If the BIOS owns the HC, signal that the OS wants it, and wait */
if (val & XHCI_HC_BIOS_OWNED) {
writel(val | XHCI_HC_OS_OWNED, base + ext_cap_offset);
-- 
1.7.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][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

2016-10-21 Thread Chen Yu
On 2016/10/22 4:00, John Youn wrote:
> On 10/20/2016 5:43 PM, Chen Yu wrote:
>> On 2016/10/19 6:21, John Youn wrote:
>>> On 10/16/2016 7:42 PM, Chen Yu wrote:


 On 2016/10/15 3:37, John Youn wrote:
> On 10/13/2016 4:36 PM, John Stultz wrote:
>> From: Chen Yu 
>>
>> The Hi6220's usb controller is limited in that it does not
>> automatically autonegotiate the usb speed. Thus it requires a
>> quirk so that we can manually negotiate the best usb speed for
>> the attached device.
>
> Hi,
>
> Could you expand more on this by explaining what exactly is the
> limitation and the workaround?
>

 The USB host limitation of Hisilicon Hi6220 is full-speed and low-speed
 devices can not be enumerated when gets plugged behind a hub.

> [snip]
>
>> +/*
>> + * HPRT0_SPD_HIGH_SPEED: high speed
>> + * HPRT0_SPD_FULL_SPEED: full speed
>> + */
>> +static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
>> +{
>> +struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>> +
>> +if (hsotg->core_params->speed == speed)
>> +return;
>> +
>> +hsotg->core_params->speed = speed;
>> +queue_work(hsotg->wq_otg, &hsotg->wf_otg);
>> +}
>> +
>> +static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>> +{
>> +struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>> +
>> +if (!hsotg->change_speed_quirk)
>> +return 1;
>> +
>> +hsotg->device_count++;
>
> Why do you need to track the device count?
>
>> +dev_info(hsotg->dev, "Device count is %u after alloc dev\n",
>> +hsotg->device_count);
>> +
>> +return 1;
>> +}
>> +
>> +static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
>> +{
>> +struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>> +
>> +if (!hsotg->change_speed_quirk)
>> +return;
>> +
>> +if (hsotg->device_count)
>> +hsotg->device_count--;
>> +
>> +dev_info(hsotg->dev, "Device count is %u after free dev\n",
>> +hsotg->device_count);
>> +
>> +if (hsotg->device_count == 1 && udev->parent &&
>> +udev->parent->speed > USB_SPEED_UNKNOWN &&
>> +udev->parent->speed < USB_SPEED_HIGH) {
>> +dev_info(hsotg->dev, "Set speed to default 
>> high-speed\n");
>> +dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
>> +}
>> +}
>> +
>> +static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device 
>> *udev)
>> +{
>> +struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>> +
>> +if (!hsotg->change_speed_quirk)
>> +return 0;
>> +
>> +if (udev->speed == USB_SPEED_HIGH) {
>> +dev_info(hsotg->dev, "Set speed to high-speed\n");
>> +dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
>> +} else if (udev->speed == USB_SPEED_FULL
>> +|| udev->speed == USB_SPEED_LOW) {
>> +dev_info(hsotg->dev, "Set speed to full-speed\n");
>> +dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
>> +}
>
> It seems you are reinitializing the core every time a device is reset
> and the udev->speed does not match the core_param speed. But how is
> the udev->speed being set correctly if the hw cannot negotiate the
> speed in the first place?
>

 The hardware can negotiate the speed, but communication with a full-speed 
 or
 low-speed device behind a hub is the problem.

> Also should it be for every device? What about if a device gets
> plugged in behind a hub? I don't think you want to execute this code
> in that case.
>
> This should only affect devices plugged into the root hub, correct?
> And the hsotg controller only has one root hub port. It seems things
> could be simplified a bit.
>

 The patch is initially written for Hikey Hi6220 board, and there is a
 hub always connected to root hub, so the patch sets the configuration to
 HPRT0_SPD_HIGH_SPEED when there is only one device(the hub).
>>>
>>> Ok, I see.
>>>

 Thanks for your suggestions, the patch needs modified in these aspect:
 1. Change the speed setting only when the device is behind a hub in 
 dwc2_reset_device.
>>>
>>> I still think you will have issues with multiple devices. Since you
>>> have a built-in hub after root hub, it will always be behind the
>>> hub. So whenver you need to change speeds, it will always reset every
>>> device in the tree. Have you tested with m

RE: [PATCH V2]usb: dwc2: Clear GUSBCFG.UsbTrdTim before setting

2016-10-21 Thread Lipengcheng


> -Original Message-
> From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> Sent: Monday, October 17, 2016 5:37 PM
> To: Lipengcheng; johny...@synopsys.com
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; 
> linux-ker...@vger.kernel.org; Xuejiancheng; Lidongpo; Caizhiyong; Lipengcheng
> Subject: Re: [PATCH V2]usb: dwc2: Clear GUSBCFG.UsbTrdTim before setting
> 
> 
> Hi,
> 
> Pengcheng Li  writes:
> > The USBTRDTIM field needs to be cleared before setting a new value.
> > Otherwise it will result in an incorrect value if phyif == GUSBCFG_PHYIF8.
> >
> > Change-Id: Ib3e33cf4fd15ada41dc070ff7b93858daafbd10f
> > Signed-off-by: Pengcheng Li 
> > Acked-by: John Youn 
> 
> which commit are you fixing? Seems like you're missing a:
> 
commit c45574ff02d1d1f35a6bf4b8ad051fc06c001fc7.
> Fixes: foobar 
> 
> line here. Also, you need to remove Gerritisms from commit log ;-)
> 
Ok, I will remove at the next patch.
> --
> Balbi

Best Regards
Pengcheng Li
N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH] USB: update intro of documentation

2016-10-21 Thread Oliver Neukum
On Fri, 2016-10-21 at 15:17 -0600, Jonathan Corbet wrote:
> On Thu, 20 Oct 2016 15:15:00 +0200
> Oliver Neukum  wrote:
> 
> > It does no good to mention The 2.4 kernel series and neglect
> > USB 3.x and XHCI. Also with type C and micro/mini USB we better
> > not talk about the shape of connectors.
> 
> ...except that USB 2 connectors will be with us for some time yet.  I'm all
> for updating the documentation, but stuff like this:
> 
> >  That master/slave asymmetry was designed-in for a number of
> >  reasons, one being ease of use.  It is not physically possible to
> > -assemble (legal) USB cables incorrectly:  all upstream "to the host"
> > -connectors are the rectangular type (matching the sockets on
> > -root hubs), and all downstream connectors are the squarish type
> > +mistake upstream and downstream or it does not matter with a type C
> > +plug
> 
> seems to me like it loses information.  Greg, do you have an opinion on the
> changes here?  I'll apply this if it's OK with you.

It absolutely loses information, because that information is no longer
true. We have micro and mini USB downstream and the upstream is usually
square but not universally true since On-the-Go.

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: [RESEND PATCH] usb: chipidea: Configure DMA properties and ops from DT

2016-10-21 Thread Sriram Dash
>From: Stephen Boyd [mailto:sb...@codeaurora.org]
>On 10/21, Bjorn Andersson wrote:
>> hcd_alloc_coherent() and usb_alloc_coherent() ends up allocating
>> coherent memory on behalf of ci_hdrc driver. But as the ci_hdrc is
>> instantiated manually it will not have any dma_mem or dma_ops
>> assigned, which makes the
>> dma_alloc_coherent() fail on some platforms (e.g. arm64). This patch
>> solves this by assigning the dma_mem and dma_ops based on the parent's
>> DeviceTree node.
>>
>> Cc: Stephen Boyd 
>> Signed-off-by: Bjorn Andersson 
>> ---
>>
>> Hi Peter,
>>
>> After (once more) debugging why USB doesn't work up on the 64-bit
>> Qualcomm systems I realized that we never concluded on this patch.
>> Unfortunately I can't find it in my mailbox either, so resending it to 
>> restart the
>discussion.
>>
>
>I thought we were going to go down the route that Arnd has been pushing[1]? 
>That
>should work, but I haven't tried it yet and there are some more fixes on top 
>from
>Sriram. I think Sriram is taking over the patch now?
>

Yes Stephen. I am incorporating the idea from Arnd and working on those patches.

Regards,
Sriram

>[1] https://patchwork.kernel.org/patch/9319527/
>
>--
>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
>Foundation Collaborative Project
--
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