Re: [PATCH 8/8] usb: dwc3: gadget: always enable IOC on bulk/interrupt transfers

2014-05-08 Thread Pratyush Anand
Hi Amit,

On Tue, May 06, 2014 at 02:22:12PM +0800, Amit VIRDI wrote:
> On 5/6/2014 12:56 AM, Felipe Balbi wrote:
> >> I understand that enabling XferInProgress event for bulk/interrupt
> >> >transfers will completely
> >> >change the design of the dwc3 driver and hence is not the viable solution
> >> >to the issue here.
> > just send a patch enabling XferInProgress.. I haven't gotten to it yet.
> > If you beat me to it, more power for you;-)
> 
> Enabling XferInProgress event for bulk and interrupt would incur 
> significant testing efforts. Till it is done can we revert this patch as 
> it isn't correct conceptually?

Its not just enabling the xferinprogress event for bulk and interrupt
and things will start working. DWC3 driver has been written in such a
way that it handle all isoc transfer using xferinprogress event and
other transfers using xfercomplete event. So you will have to make
changes at couple of more places to get that working and then a through
stress testing.

@Felip: As per my understanding too, this patch must be reverted.
Enabling IOC for bulk and interrupt without enabling xferinprogress
for them does not make sense.

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


CP210x Driver - Issue Enumerating Multiple Interfaces

2014-05-08 Thread Miner, Blake
To whom it may concern:

I am having trouble with the Silicon Labs CP2105 (usb serial driver
cp210x) enumerating both USB interfaces.

When the CP2105-EK evaluation board is connected to the PC, Linux only
creates a `/dev/ttyUSB0` device, but no `/dev/ttyUSB1` device is
created. Upon unplugging the device and plugging it back in, Linux
will correctly create both the `/dev/ttyUSB0` and `/dev/ttyUSB1`
devices. I am wondering if this might be a driver issue. Can you give
me any pointers?

Here is a snippet of `/var/log/syslog`:

May 6 15:59:31 dev kernel: [95779.749718] usb 1-2: new full-speed USB
device number 4 using ohci_hcd
May 6 15:59:32 dev kernel: [95780.222386] usb 1-2: New USB device
found, idVendor=10c4, idProduct=ea70
May 6 15:59:32 dev kernel: [95780.222404] usb 1-2: New USB device
strings: Mfr=1, Product=2, SerialNumber=5
May 6 15:59:32 dev kernel: [95780.222418] usb 1-2: Product: CP2105
Dual USB to UART Bridge Controller
May 6 15:59:32 dev kernel: [95780.222432] usb 1-2: Manufacturer: Silicon Labs
May 6 15:59:32 dev kernel: [95780.222588] usb 1-2: SerialNumber: 002D0751
May 6 15:59:32 dev kernel: [95780.267978] usbcore: registered new
interface driver usbserial
May 6 15:59:32 dev kernel: [95780.268002] USB Serial support
registered for generic
May 6 15:59:32 dev kernel: [95780.268008] usbcore: registered new
interface driver usbserial_generic
May 6 15:59:32 dev kernel: [95780.268552] usbserial: USB Serial Driver core
May 6 15:59:32 dev kernel: [95780.270777] USB Serial support
registered for cp210x
May 6 15:59:32 dev kernel: [95780.270806] cp210x 1-2:1.0: cp210x
converter detected
May 6 15:59:32 dev kernel: [95780.669352] usb 1-2: reset full-speed
USB device number 4 using ohci_hcd
May 6 15:59:33 dev kernel: [95781.268311] usb 1-2: cp210x converter
now attached to ttyUSB0
May 6 15:59:33 dev kernel: [95781.268354] cp210x 1-2:1.1: cp210x
converter detected
May 6 15:59:33 dev kernel: [95781.268536] cp210x ttyUSB0: cp210x
converter now disconnected from ttyUSB0
May 6 15:59:33 dev kernel: [95781.268573] cp210x 1-2:1.0: device disconnected
May 6 15:59:33 dev kernel: [95781.664834] usb 1-2: reset full-speed
USB device number 4 using ohci_hcd
May 6 15:59:34 dev kernel: [95782.192643] usb 1-2: cp210x converter
now attached to ttyUSB0
May 6 15:59:34 dev kernel: [95782.193768] usbcore: registered new
interface driver cp210x
May 6 15:59:34 dev kernel: [95782.193783] cp210x: v0.09:Silicon Labs
CP210x RS232 serial adaptor driver


Then, I unplug the device and plug it back in


May 6 16:00:05 dev kernel: [95813.481356] usb 1-2: USB disconnect,
device number 4
May 6 16:00:05 dev kernel: [95813.481875] cp210x ttyUSB0: cp210x
converter now disconnected from ttyUSB0
May 6 16:00:05 dev kernel: [95813.481914] cp210x 1-2:1.1: device disconnected
May 6 16:00:12 dev kernel: [95820.484792] usb 1-2: new full-speed USB
device number 5 using ohci_hcd
May 6 16:00:12 dev kernel: [95820.959247] usb 1-2: New USB device
found, idVendor=10c4, idProduct=ea70
May 6 16:00:12 dev kernel: [95820.959265] usb 1-2: New USB device
strings: Mfr=1, Product=2, SerialNumber=5
May 6 16:00:12 dev kernel: [95820.959279] usb 1-2: Product: CP2105
Dual USB to UART Bridge Controller
May 6 16:00:12 dev kernel: [95820.959292] usb 1-2: Manufacturer: Silicon Labs
May 6 16:00:12 dev kernel: [95820.959305] usb 1-2: SerialNumber: 002D0751
May 6 16:00:12 dev kernel: [95820.965198] cp210x 1-2:1.0: cp210x
converter detected
May 6 16:00:13 dev kernel: [95821.369406] usb 1-2: reset full-speed
USB device number 5 using ohci_hcd
May 6 16:00:13 dev kernel: [95821.925765] usb 1-2: cp210x converter
now attached to ttyUSB0
May 6 16:00:13 dev kernel: [95821.927820] cp210x 1-2:1.1: cp210x
converter detected
May 6 16:00:13 dev kernel: [95821.928959] cp210x ttyUSB0: cp210x
converter now disconnected from ttyUSB0
May 6 16:00:13 dev kernel: [95821.929002] cp210x 1-2:1.0: device disconnected
May 6 16:00:14 dev kernel: [95822.324582] usb 1-2: reset full-speed
USB device number 5 using ohci_hcd
May 6 16:00:14 dev kernel: [95822.903592] cp210x 1-2:1.0: cp210x
converter detected
May 6 16:00:15 dev kernel: [95823.301631] usb 1-2: reset full-speed
USB device number 5 using ohci_hcd
May 6 16:00:15 dev kernel: [95823.889378] usb 1-2: cp210x converter
now attached to ttyUSB0
May 6 16:00:15 dev kernel: [95823.892031] usb 1-2: cp210x converter
now attached to ttyUSB1

I am running Debian Linux 7 (with Linux kernel 3.2.0-4-amd64).  I have
tried the built-in Debian kernel driver and SiLabs's driver
(http://www.silabs.com/Support%20Documents/Software/Linux_3.x.x_VCP_Driver_Source.zip).
 Both behave the same way.

Also, I found out that running `modprobe cp210x` before plugging in
the device will properly register both interfaces (`ttyUSB0` and
`ttyUSB1`).

It would appear that the driver must be completely loaded before the
device can be properly detected.  Any thoughts on this?

Thanks,

Blake
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a

RE: [PATCH] usb: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Paul Zimmerman
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Thursday, May 08, 2014 2:18 PM
> 
> On Thu, 8 May 2014, Paul Zimmerman wrote:
> 
> > > When the host already timed out the control transfer and started a new
> > > one.  Here's what I'm talking about:
> > >
> > >   Host sends a Set-Configuration request.
> > >
> > >   The UDC driver calls the gadget driver's setup function.
> > >
> > >   The setup function returns DELAYED_STATUS.
> > >
> > >   After a few seconds, the host gets tired of waiting and
> > >   sends a Get-Descriptor request
> > >
> > >   The gadget driver finally submits the delayed request response
> > >   to the Set-Configuration request.  But it is now too late,
> > >   because the host expects a response to the Get-Descriptor
> > >   request.
> > >
> > > >  dwc3 can't move to SETUP phase until the status request arrives,
> > > > so any SETUP transaction from host will fail. If status request
> > > > eventually arrives, it already missed the first control transfer, and
> > > > I don't know how the controller will behave. If we still can get a
> > > > STATUS XferComplete event without actually transfer anything on the
> > > > bus, then we can move back to SETUP PHASE which will remove the stale
> > > > delayed status request and start the new SETUP transaction. But I think
> > > > in this situation, the host should already lose it patience and start
> > > > to reset the bus.
> > >
> > > My point is that the UDC driver can't handle this.  Therefore the
> > > gadget driver has to prevent this from happening.
> > >
> > > That means composite.c has to avoid sending delayed status responses if
> > > a new SETUP packet has been received already.
> > >
> > > > Per my understanding, it's impossible for dwc3 to send a stale STATUS
> > > > request for a new SETUP transaction.
> > >
> > > dwc3 won't know that the status response is stale.  It will think the
> > > response was meant for the new transfer, not the old one.
> >
> > The DWC3 controller will actually handle this case on its own. If
> > it sees another SETUP packet come in before the previous Control
> > transfer has completed, it will not send any DATA or STATUS phase
> > packets for the previous Control transfer to the host. But it will
> > "fake" the correct responses to the software, so the dwc3 driver will
> > think that the DATA/STATUS stages completed successfully, even though
> > nothing actually went out on the bus.
> 
> That doesn't handle the problem I described above.  When the dwc3
> driver gets the late delayed status response, it will think it is a
> response to the new SETUP packet, and so it will carry out a bogus
> transfer.  It won't know that the status request was meant to be a
> response to a defunct control transfer.

I think you misunderstood me. What I meant was, once the DWC3
controller sees the next SETUP packet, it will still accept the
commands from the dwc3 driver to start the DATA and STATUS phases
for the previous Control transfer, and will send back (fake) completion
events for those commands to the driver. But it won't actually send
anything on the wire.

So it should be impossible for the dwc3 driver to carry out a bogus
transfer. This is a feature of the DWC3 controller intended to
simplify what the software needs to handle, and to automatically
take care of the corner cases involved here.

> > For other controllers that can't do this, maybe it should be handled
> > in the UDC driver rather than in the composite gadget?
> 
> The only place this can be handled properly is in the gadget driver:
> composite.c for those gadgets using it, otherwise in the higher level
> driver (if there are any remaining gadgets that don't use the composite
> framework).

Why can't the UDC drivers handle this? AFAIK they all keep track of
which phase of a Control transfer they are in. If they see another
SETUP packet arrive, they could "fake" the DATA/STATUS stages of the
previous transfer, before passing on the next SETUP packet to the
gadget driver. Similar to what the DWC3 controller does in hardware.

Although, I guess it would be simpler to do it once in composite.c,
instead of in each individual UDC driver. But there would have to be
a quirk or something, to disable the code if the dwc3 driver is in
use. And that wouldn't fix the problem for gadgets that don't use
composite.c.

-- 
Paul

--
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: Bugs in xhci-hcd isochronous support

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Russel Hughes wrote:

> Hi,
> 
>Some more information from someone who has the same DAC as me and
> has got it working on USB3.0 under Linux. I dont know if this helps
> with a workround or just points to some fundamental problem with the
> Intel hardware.
> 
> "I was right in that MDAC works for me on USB3.0 (detected as NEC
> uPD720200, Asus P8Z68 Deluxe motherboard), using xhci_hcd on a
> more-or-less vanilla 3.12.3 kernel (yeah, I should upgrade soon):

Have you also tried 3.12.3, to verify that the very same kernel gives 
different results on different hardware, and that the changes between 
3.12 and 3.14 aren't responsible for the problem?

> Code:
> 
> # lsusb -t
> ...
> /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M
> |__ Port 1: Dev 3, If 0, Class=HID, Driver=usbhid, 12M
> |__ Port 1: Dev 3, If 1, Class=audio, Driver=snd-usb-audio, 12M
> |__ Port 1: Dev 3, If 2, Class=audio, Driver=snd-usb-audio, 12M
> ...
> 
> Manually watching /proc/interrupts confirms that it's not going
> through ehci_hcd.
> 
> I can even play 24/96k without any problems (unlike unpatched ehci_hcd).
> 
> Therfore your issue is either not xhci_hcd related, or is
> hardware-specific. Make sure to mention your USB3 xhci controller in
> that lkml thread."

This does seem to point to an incompatibility between the driver and 
the Intel xHCI hardware.  Figuring out what that incompability is may 
not be easy, though...

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: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Paul Zimmerman wrote:

> > When the host already timed out the control transfer and started a new
> > one.  Here's what I'm talking about:
> > 
> > Host sends a Set-Configuration request.
> > 
> > The UDC driver calls the gadget driver's setup function.
> > 
> > The setup function returns DELAYED_STATUS.
> > 
> > After a few seconds, the host gets tired of waiting and
> > sends a Get-Descriptor request
> > 
> > The gadget driver finally submits the delayed request response
> > to the Set-Configuration request.  But it is now too late,
> > because the host expects a response to the Get-Descriptor
> > request.
> > 
> > >  dwc3 can't move to SETUP phase until the status request arrives,
> > > so any SETUP transaction from host will fail. If status request
> > > eventually arrives, it already missed the first control transfer, and
> > > I don't know how the controller will behave. If we still can get a
> > > STATUS XferComplete event without actually transfer anything on the
> > > bus, then we can move back to SETUP PHASE which will remove the stale
> > > delayed status request and start the new SETUP transaction. But I think
> > > in this situation, the host should already lose it patience and start
> > > to reset the bus.
> > 
> > My point is that the UDC driver can't handle this.  Therefore the
> > gadget driver has to prevent this from happening.
> > 
> > That means composite.c has to avoid sending delayed status responses if
> > a new SETUP packet has been received already.
> > 
> > > Per my understanding, it's impossible for dwc3 to send a stale STATUS
> > > request for a new SETUP transaction.
> > 
> > dwc3 won't know that the status response is stale.  It will think the
> > response was meant for the new transfer, not the old one.
> 
> The DWC3 controller will actually handle this case on its own. If
> it sees another SETUP packet come in before the previous Control
> transfer has completed, it will not send any DATA or STATUS phase
> packets for the previous Control transfer to the host. But it will
> "fake" the correct responses to the software, so the dwc3 driver will
> think that the DATA/STATUS stages completed successfully, even though
> nothing actually went out on the bus.

That doesn't handle the problem I described above.  When the dwc3 
driver gets the late delayed status response, it will think it is a 
response to the new SETUP packet, and so it will carry out a bogus 
transfer.  It won't know that the status request was meant to be a 
response to a defunct control transfer.

> For other controllers that can't do this, maybe it should be handled
> in the UDC driver rather than in the composite gadget?

The only place this can be handled properly is in the gadget driver:
composite.c for those gadgets using it, otherwise in the higher level 
driver (if there are any remaining gadgets that don't use the composite 
framework).

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: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Felipe Balbi
On Thu, May 08, 2014 at 03:06:48PM -0400, Alan Stern wrote:
> On Thu, 8 May 2014, Paul Zimmerman wrote:
> 
> > Just FYI, the DWC3 core is designed to always respond to SETUP packets.
> > It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
> > properly according to the databook. If the buffer fills up, then
> > further SETUP packets will get lost, but they will still be ACKed.
> 
> Really?  That seems odd.  Isn't the most recent SETUP packet the one 
> you want to handle?

And we _will_ handle the most recent SETUP packet. It's just that we're
obligated to still start Data/Status phase before going back to that
SETUP packet. Note that neither Data/Status will shift anything into the
wire, they will complete as soon as they are started, but they _must_ be
started anyhow.

-- 
balbi


signature.asc
Description: Digital signature


Re: Bugs in xhci-hcd isochronous support

2014-05-08 Thread Russel Hughes
> Does your computer have any USB-2 ports?  Or is it possible to disable
> the USB-3 controllers in the BIOS?  It would be worthwhile to see if
> the audio works when the device is attached to a non-USB-3 controller.
>

Hi,

   Some more information from someone who has the same DAC as me and
has got it working on USB3.0 under Linux. I dont know if this helps
with a workround or just points to some fundamental problem with the
Intel hardware.

"I was right in that MDAC works for me on USB3.0 (detected as NEC
uPD720200, Asus P8Z68 Deluxe motherboard), using xhci_hcd on a
more-or-less vanilla 3.12.3 kernel (yeah, I should upgrade soon):
Code:

# lsusb -t
...
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M
|__ Port 1: Dev 3, If 0, Class=HID, Driver=usbhid, 12M
|__ Port 1: Dev 3, If 1, Class=audio, Driver=snd-usb-audio, 12M
|__ Port 1: Dev 3, If 2, Class=audio, Driver=snd-usb-audio, 12M
...

Manually watching /proc/interrupts confirms that it's not going
through ehci_hcd.

I can even play 24/96k without any problems (unlike unpatched ehci_hcd).

Therfore your issue is either not xhci_hcd related, or is
hardware-specific. Make sure to mention your USB3 xhci controller in
that lkml thread."


My output is:

lsusb -t
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/9p, 480M
|__ Port 3: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 12M
|__ Port 3: Dev 2, If 1, Class=Audio, Driver=snd-usb-audio, 12M
|__ Port 3: Dev 2, If 2, Class=Audio, Driver=snd-usb-audio, 12M
|__ Port 4: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M
|__ Port 4: Dev 3, If 1, Class=Human Interface Device, Driver=usbhid, 1.5M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
--
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: ep0: fix delayed status is queued too early

2014-05-08 Thread Felipe Balbi
On Thu, May 08, 2014 at 06:56:02PM +, Paul Zimmerman wrote:
> > From: linux-usb-ow...@vger.kernel.org 
> > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Paul Zimmerman
> > Sent: Thursday, May 08, 2014 11:42 AM
> > 
> > > From: linux-usb-ow...@vger.kernel.org 
> > > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern
> > > Sent: Thursday, May 08, 2014 10:40 AM
> > >
> > > On Thu, 8 May 2014, Felipe Balbi wrote:
> > >
> > > > > The dwc3 driver should always prepare a buffer for the next ep0 SETUP
> > > > > packet as soon as it retrieves the information for the current SETUP
> > > > > packet from the buffer.
> > > > >
> > > > > Otherwise, as you described it, if the gadget driver never sends its
> > > > > delayed status response then the UDC will never respond to any more
> > > > > control transfers.
> > > >
> > > > we _do_ prepare transfers for setup packet everytime a Status phase is
> > > > completed (we also restart ep0 in case of stalls):
> > >
> > > I said that dwc3 should prepare a buffer for a SETUP packet every time
> > > a _SETUP_ stage is completed -- not every time a _STATUS_ stage is
> > > completed.
> > >
> > > In principle the host can send a SETUP packet at any time, even in the
> > > middle of an ongoing transfer.  (Consider the case where a driver on
> > > the host unlinks a control transfer before it has completed and then
> > > submits a new one.)

Right, that's all taken care of and even properly commented around in
the code.

The way this IP works though is kinda funny. If we do get a new setup
packet before we go through all other phases, we still need to initiate
data/status phases. They will complete straight away without doing any
transfers, nothing will be shifted into the wire. We know about that
because of a SETUP_PENDING bit in the XferComplete event - which can
only be used for printing a debugging message.

Because of that, restarting ep0 on status completion alone is enough as
we are mandated to go through the ep0 phases anyway.

I assure you we had countless hours of stress testing ep0 handling with
all certification test cases. In fact, link layer test td7.9 (iirc) runs
with 8 (again, IIRC) back-to-back setup packets.

If you're interested in details, check git log on drivers/usb/dwc3/ep0.c

> > Just FYI, the DWC3 core is designed to always respond to SETUP packets.
> > It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
> > properly according to the databook. If the buffer fills up, then
> > further SETUP packets will get lost, but they will still be ACKed.
> 
> I forgot to say, this means that it is not necessary to prepare a
> buffer for the next SETUP immediately after the previous SETUP stage
> ends, since the next SETUP will be cached in the controller.
> 
> There is a flowchart in the databook that describes the programming
> flow for Control transfers. I think the DWC3 driver follows this
> pretty closely, although it has been a while since I last reviewed it.

yeah, we _do_ follow it pretty closely. We were very careful to take
care of all corner cases, including the one which caused the programming
model (and documentation) to be changed.

Quite frankly, I'd prefer to have XferNotReady(SETUP) so that we would
only start any TRBs (for ep0 at least) when we know what we're supposed
to start. EP0 would've been a lot easier to implement/read/maintain
should we have that working. But then again, XferNotReady(DATA) is
broken in back-to-back SETUP packets, so what the heck ? :-)

cheers

-- 
balbi


signature.asc
Description: Digital signature


RE: [PATCH] usb: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Paul Zimmerman
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Thursday, May 08, 2014 12:07 PM
> 
> On Thu, 8 May 2014, Paul Zimmerman wrote:
> 
> > Just FYI, the DWC3 core is designed to always respond to SETUP packets.
> > It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
> > properly according to the databook. If the buffer fills up, then
> > further SETUP packets will get lost, but they will still be ACKed.
> 
> Really?  That seems odd.  Isn't the most recent SETUP packet the one
> you want to handle?

I just spoke to one of the RTL designers about this, and I was wrong in
one of my previous statements.

The controller does have a 3-deep buffer for SETUP packets. But if the
buffer fills up and a 4th SETUP packet comes in, the controller will
not ACK it, it will time out the transfer instead.

So if that state is ever reached, the host should issue a USB reset and
start over.

-- 
Paul

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


Re: [PATCH] usb: gadget: fsl: check vbus presence on probe

2014-05-08 Thread Paul Fertser
On Thu, May 08, 2014 at 06:42:53PM +, suresh.gu...@freescale.com wrote:
> > > And Host might be attach after system bootup or after driver
> > > initialization. So putting this check in probe will not help much.
> > 
> > If the host is attached after the driver was initialised, the interrupt
> > will trigger and the driver will get notified that VBUS appeared and
> > everything will go smooth, at least that's how it should work (I do not
> > have any board handy to real-life check that, but AFAICT that's the
> > intent of the current code).
> 
> If is go through the code flow starting from usb_composite_probe the sequence 
> is
> usb_composite_probe->usb_gadget_probe_driver->udc_bind_to_driver->
> udc_bind_to_driver->usb_gadget_udc_start->fsl_udc_start->usbcmd=RUN
> udc_bind_to_driver->usb_gadget_connect->fsl_pullup
> 
> The function fsl_pullup make usbcmd=STOP if my fix is not there and if 
> controller 
> is stopped we do not get any interrupt. 

Are you really sure we can't get async VBUS state change notifications
until controller has USB_CMD_RUN_STOP bit set (and the same bit
actually controls internal 1.5k dataline pullup)? If yes, I guess it
means we need to check VBUS state _every_ time we set that bit to sync
the vbus_active variable with the actual hardware state (unless an
external OTG PHY is used and VBUS pad state is irrelevant)?

> > I actually do not have any iMX demoboards at all, I've only got some
> > custom-designed i.MX25 boards where I can't control VBUS, it's
> > permanently pulled up.
> > 
> > But I was fixing the problem that was clearly, 100% reproducibly
> > happening when VBUS was applied before the interrupt was configured. So
> 
> Wait a minute, are you using OTG. If your VBUS is permanently pulled up, that 
> means you are only Gadget(I might miss something) then why you use OTG mode.

I'm using FSL_USB2_DR_DEVICE mode (FSL_USB2_PHY_UTMI phy_mode), so the
OTG controller should always work in the device mode only.

> > what exactly do you mean here? Do you mean this check I've added doesn't
> > fix the bug? Or do you mean this bug should be fixed somehow else? Or do
> 
> What expertly my concern is, probe is not proper place to check VBUS valid.
> I think we should wait to hear what Balbi has to say. 

Yes, I hope he can understand both of us :)

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercer...@gmail.com
--
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: ep0: fix delayed status is queued too early

2014-05-08 Thread Paul Zimmerman
> From: linux-usb-ow...@vger.kernel.org 
> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern
> Sent: Wednesday, May 07, 2014 9:59 AM
> 
> On Thu, 8 May 2014, Zhuang Jin Can wrote:
> 
> > > A similar problem can occur in the opposite sense: The thread queuing
> > > the delayed status request might be delayed for so long that another
> > > SETUP packet arrives from the host first.  In that case, the delayed
> > > status request is a response for a stale transfer, so it must not be
> > > sent to the host.
> > >
> > > Do dwc3 and composite.c handle this case correctly?
> > >
> > So the situation you describe is that we get the STATUS XferNotReady
> > event, but gadget queues a status request when control transfer already
> > failed.
> 
> When the host already timed out the control transfer and started a new
> one.  Here's what I'm talking about:
> 
>   Host sends a Set-Configuration request.
> 
>   The UDC driver calls the gadget driver's setup function.
> 
>   The setup function returns DELAYED_STATUS.
> 
>   After a few seconds, the host gets tired of waiting and
>   sends a Get-Descriptor request
> 
>   The gadget driver finally submits the delayed request response
>   to the Set-Configuration request.  But it is now too late,
>   because the host expects a response to the Get-Descriptor
>   request.
> 
> >  dwc3 can't move to SETUP phase until the status request arrives,
> > so any SETUP transaction from host will fail. If status request
> > eventually arrives, it already missed the first control transfer, and
> > I don't know how the controller will behave. If we still can get a
> > STATUS XferComplete event without actually transfer anything on the
> > bus, then we can move back to SETUP PHASE which will remove the stale
> > delayed status request and start the new SETUP transaction. But I think
> > in this situation, the host should already lose it patience and start
> > to reset the bus.
> 
> My point is that the UDC driver can't handle this.  Therefore the
> gadget driver has to prevent this from happening.
> 
> That means composite.c has to avoid sending delayed status responses if
> a new SETUP packet has been received already.
> 
> > Per my understanding, it's impossible for dwc3 to send a stale STATUS
> > request for a new SETUP transaction.
> 
> dwc3 won't know that the status response is stale.  It will think the
> response was meant for the new transfer, not the old one.

The DWC3 controller will actually handle this case on its own. If
it sees another SETUP packet come in before the previous Control
transfer has completed, it will not send any DATA or STATUS phase
packets for the previous Control transfer to the host. But it will
"fake" the correct responses to the software, so the dwc3 driver will
think that the DATA/STATUS stages completed successfully, even though
nothing actually went out on the bus.

For other controllers that can't do this, maybe it should be handled
in the UDC driver rather than in the composite gadget?

-- 
Paul

--
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 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 08:56:24 AM Dan Williams wrote:
> On Thu, May 8, 2014 at 4:09 AM, Rafael J. Wysocki  wrote:
> > On Wednesday, May 07, 2014 10:37:24 PM Dan Williams wrote:
> >> Unconditionally wake up the child device when the power session is
> >> recovered.
> >
> > [cut]
> >
> >> + /*
> >> +  * Revalidate the device if it was requested 
> >> by
> >> +  * usb_port_runtime_resume.
> >> +  */
> >> + if (test_and_clear_bit(i, hub->wakeup_bits) 
> >> && udev) {
> >> + pm_runtime_get_noresume(&udev->dev);
> >> + pm_runtime_barrier(&udev->dev);
> >> + 
> >> pm_runtime_put_autosuspend(&udev->dev);
> >
> > It looks like you could just call pm_runtime_idle(&udev->dev) after the 
> > barrier
> > without the _get and _put around it.  Wouldn't that work?
> >
> 
> Sure, but the surprise was that an auto suspend enabled device stops
> auto suspending after a barrier.

I see.

Well, _barrier() works pretty much as documented, but its interaction with
autosuspend is weird due to the way the latter is implemented.

I guess we could add a second argument to _barrier() telling it to do
pm_runtime_put() instead of pm_runtime_put_noidle().

Thanks!


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


Re: [PATCH v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Dan Williams wrote:

> On Thu, May 8, 2014 at 11:09 AM, Alan Stern  wrote:
> > On Thu, 8 May 2014, Dan Williams wrote:
> >
> >> On Thu, May 8, 2014 at 9:09 AM, Alan Stern  
> >> wrote:
> >> > On Thu, 8 May 2014, Dan Williams wrote:
> >> >
> >> >> > I don't understand this last part.  Why do we need to guarantee the
> >> >> > child device has recovered from power loss?  Why not proceed the same
> >> >> > way we do now when the child is suspended?
> >> >>
> >> >> Two reasons I believe:
> >> >>
> >> >> 1/ The child may be gone, and usb_port_resume() will mark it for 
> >> >> disconnect
> >> >>
> >> >> 2/ Currently port_event() knows how to handle suspended devices
> >> >> (USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the
> >> >> status and change bits are different.  I figure why special case
> >> >> port_event()?  Just make it so it handles all the same cases that are
> >> >> presented when the port does not lose power.
> >> >
> >> > How much special casing would really be needed?
> >>
> >> Don't know.  Is the forced wakeup really that onerous, vs the risk of
> >> touch established code?  I've broken the port_event() path enough
> >> times through this exercise that I'd want a driving reason beyond "why
> >> not?".
> >
> > As far as I can see, it looks like no special casing is needed.  Now, I
> > haven't gone through all the changes introduced by the patch series to
> > make sure, but it seems that all the various port status tests in
> > port_event() will see everything appearing normal, because that's how
> > hub_port_debounce_be_connected() -- or whatever its current name is --
> > will leave the port.
> >
> > Thus, nothing will happen up until the place in
> > hub_port_connect_change() headed by the "Try to resuscitate an existing
> > device" comment.  At that point we will resume the child device.  That
> > is, assuming khubd thinks it has any reason for for calling
> > hub_port_connect_change(), which it probably won't.
> >
> > So at first sight, it appears that nothing would need to be changed.  I
> > suggest you try it and see.
> >
> 
> To be clear what I am trying to remove is the udev runtime_barrier
> prior to the port_event, right?  We still agree that
> pm_request_resume() needs to be there in the port_dev resume path?

Yes, pm_request_resume() is necessary.  But setting wakeup_bits and
calling usb_kick_khubd() aren't, which means the code you added to
hub_events() with the comment "Revalidate the device if it was
requested by usb_port_runtime_resume" can be left out.

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: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Paul Zimmerman wrote:

> > Just FYI, the DWC3 core is designed to always respond to SETUP packets.
> > It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
> > properly according to the databook. If the buffer fills up, then
> > further SETUP packets will get lost, but they will still be ACKed.
> 
> I forgot to say, this means that it is not necessary to prepare a
> buffer for the next SETUP immediately after the previous SETUP stage
> ends, since the next SETUP will be cached in the controller.
> 
> There is a flowchart in the databook that describes the programming
> flow for Control transfers. I think the DWC3 driver follows this
> pretty closely, although it has been a while since I last reviewed it.

Okay.  I'm not really interested in pursuing this in depth.  I just 
wanted to bring it to people's attention -- particularly since it seems 
like the most important synchronization work needs to be done in 
composite.c but isn't there now.

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: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Paul Zimmerman wrote:

> Just FYI, the DWC3 core is designed to always respond to SETUP packets.
> It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
> properly according to the databook. If the buffer fills up, then
> further SETUP packets will get lost, but they will still be ACKed.

Really?  That seems odd.  Isn't the most recent SETUP packet the one 
you want to handle?

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: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Paul Zimmerman
> From: linux-usb-ow...@vger.kernel.org 
> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Paul Zimmerman
> Sent: Thursday, May 08, 2014 11:42 AM
> 
> > From: linux-usb-ow...@vger.kernel.org 
> > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern
> > Sent: Thursday, May 08, 2014 10:40 AM
> >
> > On Thu, 8 May 2014, Felipe Balbi wrote:
> >
> > > > The dwc3 driver should always prepare a buffer for the next ep0 SETUP
> > > > packet as soon as it retrieves the information for the current SETUP
> > > > packet from the buffer.
> > > >
> > > > Otherwise, as you described it, if the gadget driver never sends its
> > > > delayed status response then the UDC will never respond to any more
> > > > control transfers.
> > >
> > > we _do_ prepare transfers for setup packet everytime a Status phase is
> > > completed (we also restart ep0 in case of stalls):
> >
> > I said that dwc3 should prepare a buffer for a SETUP packet every time
> > a _SETUP_ stage is completed -- not every time a _STATUS_ stage is
> > completed.
> >
> > In principle the host can send a SETUP packet at any time, even in the
> > middle of an ongoing transfer.  (Consider the case where a driver on
> > the host unlinks a control transfer before it has completed and then
> > submits a new one.)
> 
> Just FYI, the DWC3 core is designed to always respond to SETUP packets.
> It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
> properly according to the databook. If the buffer fills up, then
> further SETUP packets will get lost, but they will still be ACKed.

I forgot to say, this means that it is not necessary to prepare a
buffer for the next SETUP immediately after the previous SETUP stage
ends, since the next SETUP will be cached in the controller.

There is a flowchart in the databook that describes the programming
flow for Control transfers. I think the DWC3 driver follows this
pretty closely, although it has been a while since I last reviewed it.

-- 
Paul

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


RE: [PATCH] usb: gadget: fsl: check vbus presence on probe

2014-05-08 Thread suresh.gu...@freescale.com


> -Original Message-
> From: Paul Fertser [mailto:fercer...@gmail.com]
> Sent: Thursday, May 08, 2014 9:09 PM
> To: Gupta Suresh-B42813
> Cc: ba...@ti.com; Li Yang-Leo-R58472; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe
> 
> Hi,
> 
> On Thu, May 08, 2014 at 02:30:39PM +, suresh.gu...@freescale.com
> wrote:
> > As per my limited knowledge, the purpose of OTGSC_STS_B_SESSION_VALID
> > bit is to tell either VBUS is above the B session valid threshold and
> > which comes only Host is attached.
> 
> Yes, that matches the datasheet I've read.
> 
> > And Host might be attach after system bootup or after driver
> > initialization. So putting this check in probe will not help much.
> 
> If the host is attached after the driver was initialised, the interrupt
> will trigger and the driver will get notified that VBUS appeared and
> everything will go smooth, at least that's how it should work (I do not
> have any board handy to real-life check that, but AFAICT that's the
> intent of the current code).

If is go through the code flow starting from usb_composite_probe the sequence is
usb_composite_probe->usb_gadget_probe_driver->udc_bind_to_driver->
udc_bind_to_driver->usb_gadget_udc_start->fsl_udc_start->usbcmd=RUN
udc_bind_to_driver->usb_gadget_connect->fsl_pullup

The function fsl_pullup make usbcmd=STOP if my fix is not there and if 
controller 
is stopped we do not get any interrupt. 
 
> 
> I actually do not have any iMX demoboards at all, I've only got some
> custom-designed i.MX25 boards where I can't control VBUS, it's
> permanently pulled up.
> 
> But I was fixing the problem that was clearly, 100% reproducibly
> happening when VBUS was applied before the interrupt was configured. So

Wait a minute, are you using OTG. If your VBUS is permanently pulled up, that 
means you are only Gadget(I might miss something) then why you use OTG mode.

> what exactly do you mean here? Do you mean this check I've added doesn't
> fix the bug? Or do you mean this bug should be fixed somehow else? Or do

What expertly my concern is, probe is not proper place to check VBUS valid.
I think we should wait to hear what Balbi has to say. 

> you mean there's no bug in the first place and my board doesn't work
> because of something else?
> 
> --
> Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
> mailto:fercer...@gmail.com
--
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: ep0: fix delayed status is queued too early

2014-05-08 Thread Paul Zimmerman
> From: linux-usb-ow...@vger.kernel.org 
> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Alan Stern
> Sent: Thursday, May 08, 2014 10:40 AM
> 
> On Thu, 8 May 2014, Felipe Balbi wrote:
> 
> > > The dwc3 driver should always prepare a buffer for the next ep0 SETUP
> > > packet as soon as it retrieves the information for the current SETUP
> > > packet from the buffer.
> > >
> > > Otherwise, as you described it, if the gadget driver never sends its
> > > delayed status response then the UDC will never respond to any more
> > > control transfers.
> >
> > we _do_ prepare transfers for setup packet everytime a Status phase is
> > completed (we also restart ep0 in case of stalls):
> 
> I said that dwc3 should prepare a buffer for a SETUP packet every time
> a _SETUP_ stage is completed -- not every time a _STATUS_ stage is
> completed.
> 
> In principle the host can send a SETUP packet at any time, even in the
> middle of an ongoing transfer.  (Consider the case where a driver on
> the host unlinks a control transfer before it has completed and then
> submits a new one.)

Just FYI, the DWC3 core is designed to always respond to SETUP packets.
It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up
properly according to the databook. If the buffer fills up, then
further SETUP packets will get lost, but they will still be ACKed.

-- 
Paul

--
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 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 11:09 AM, Alan Stern  wrote:
> On Thu, 8 May 2014, Dan Williams wrote:
>
>> On Thu, May 8, 2014 at 9:09 AM, Alan Stern  wrote:
>> > On Thu, 8 May 2014, Dan Williams wrote:
>> >
>> >> > I don't understand this last part.  Why do we need to guarantee the
>> >> > child device has recovered from power loss?  Why not proceed the same
>> >> > way we do now when the child is suspended?
>> >>
>> >> Two reasons I believe:
>> >>
>> >> 1/ The child may be gone, and usb_port_resume() will mark it for 
>> >> disconnect
>> >>
>> >> 2/ Currently port_event() knows how to handle suspended devices
>> >> (USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the
>> >> status and change bits are different.  I figure why special case
>> >> port_event()?  Just make it so it handles all the same cases that are
>> >> presented when the port does not lose power.
>> >
>> > How much special casing would really be needed?
>>
>> Don't know.  Is the forced wakeup really that onerous, vs the risk of
>> touch established code?  I've broken the port_event() path enough
>> times through this exercise that I'd want a driving reason beyond "why
>> not?".
>
> As far as I can see, it looks like no special casing is needed.  Now, I
> haven't gone through all the changes introduced by the patch series to
> make sure, but it seems that all the various port status tests in
> port_event() will see everything appearing normal, because that's how
> hub_port_debounce_be_connected() -- or whatever its current name is --
> will leave the port.
>
> Thus, nothing will happen up until the place in
> hub_port_connect_change() headed by the "Try to resuscitate an existing
> device" comment.  At that point we will resume the child device.  That
> is, assuming khubd thinks it has any reason for for calling
> hub_port_connect_change(), which it probably won't.
>
> So at first sight, it appears that nothing would need to be changed.  I
> suggest you try it and see.
>

To be clear what I am trying to remove is the udev runtime_barrier
prior to the port_event, right?  We still agree that
pm_request_resume() needs to be there in the port_dev resume path?
--
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 16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Dan Williams wrote:

> > Also, instead of adding another #ifdef here, you could add a #else
> > section to the existing #ifdef in which you define an inline version of
> > hub_handle_remote_wakeup() (or a macro version) that always returns 0.
> 
> I originally started down that path, and then noticed:
> 
> #ifdef CONFIG_PM_RUNTIME
> } else if (udev->state == USB_STATE_SUSPENDED &&
> udev->persist_enabled) {
> /* For a suspended device, treat this as a
>  * remote wakeup event.
>  */
> usb_unlock_port(port_dev);
> status = usb_remote_wakeup(udev);
> usb_lock_port(port_dev);
> #endif
> 
> ...right above and made the wrong call to add more ugliness.  Will
> fix, but will leave the other ifdef excursions alone for now.

Yeah, I never tried very hard to encapsulate that bit away from the 
rest of the subroutine.

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 v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Dan Williams wrote:

> On Thu, May 8, 2014 at 9:09 AM, Alan Stern  wrote:
> > On Thu, 8 May 2014, Dan Williams wrote:
> >
> >> > I don't understand this last part.  Why do we need to guarantee the
> >> > child device has recovered from power loss?  Why not proceed the same
> >> > way we do now when the child is suspended?
> >>
> >> Two reasons I believe:
> >>
> >> 1/ The child may be gone, and usb_port_resume() will mark it for disconnect
> >>
> >> 2/ Currently port_event() knows how to handle suspended devices
> >> (USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the
> >> status and change bits are different.  I figure why special case
> >> port_event()?  Just make it so it handles all the same cases that are
> >> presented when the port does not lose power.
> >
> > How much special casing would really be needed?
> 
> Don't know.  Is the forced wakeup really that onerous, vs the risk of
> touch established code?  I've broken the port_event() path enough
> times through this exercise that I'd want a driving reason beyond "why
> not?".

As far as I can see, it looks like no special casing is needed.  Now, I
haven't gone through all the changes introduced by the patch series to
make sure, but it seems that all the various port status tests in
port_event() will see everything appearing normal, because that's how
hub_port_debounce_be_connected() -- or whatever its current name is --
will leave the port.

Thus, nothing will happen up until the place in
hub_port_connect_change() headed by the "Try to resuscitate an existing
device" comment.  At that point we will resume the child device.  That
is, assuming khubd thinks it has any reason for for calling
hub_port_connect_change(), which it probably won't.

So at first sight, it appears that nothing would need to be changed.  I 
suggest you try it and see.

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 v9 16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 9:01 AM, Alan Stern  wrote:
> On Wed, 7 May 2014, Dan Williams wrote:
>
>> Per Alan:
>> "You mean from within hub_handle_remote_wakeup()?  That routine will
>> never get called if CONFIG_PM_RUNTIME isn't enabled, because khubd
>> never sees wakeup requests if they arise during system suspend.
>>
>> In fact, that routine ought to go inside the "#ifdef CONFIG_PM_RUNTIME"
>> portion of hub.c, along with the other suspend/resume code."
>>
>> Suggested-by: Alan Stern 
>> Signed-off-by: Dan Williams 
>> ---
>>  drivers/usb/core/hub.c |4 
>>  drivers/usb/core/usb.h |5 -
>>  2 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index d91e6493abf7..45ded752c2d8 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -4700,6 +4700,7 @@ static void hub_port_connect_change(struct usb_hub 
>> *hub, int port1,
>>   usb_lock_port(port_dev);
>>  }
>>
>> +#ifdef CONFIG_PM_RUNTIME
>>  /* Returns 1 if there was a remote wakeup and a connect status change. */
>>  static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
>>   u16 portstatus, u16 portchange)
>> @@ -4740,6 +4741,7 @@ static int hub_handle_remote_wakeup(struct usb_hub 
>> *hub, unsigned int port,
>>   dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
>>   return connect_change;
>>  }
>> +#endif
>
> I meant that instead of adding a new #ifdef section here, you could
> move the subroutine inside the #ifdef that protects
> usb_remote_wakeup().
>
>>  static void port_event(struct usb_hub *hub, int port1)
>>   __must_hold(&port_dev->status_lock)
>> @@ -4818,8 +4820,10 @@ static void port_event(struct usb_hub *hub, int port1)
>>   if (!pm_runtime_active(&port_dev->dev))
>>   return;
>>
>> +#ifdef CONFIG_PM_RUNTIME
>>   if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
>>   connect_change = 1;
>> +#endif
>
> Also, instead of adding another #ifdef here, you could add a #else
> section to the existing #ifdef in which you define an inline version of
> hub_handle_remote_wakeup() (or a macro version) that always returns 0.

I originally started down that path, and then noticed:

#ifdef CONFIG_PM_RUNTIME
} else if (udev->state == USB_STATE_SUSPENDED &&
udev->persist_enabled) {
/* For a suspended device, treat this as a
 * remote wakeup event.
 */
usb_unlock_port(port_dev);
status = usb_remote_wakeup(udev);
usb_lock_port(port_dev);
#endif

...right above and made the wrong call to add more ugliness.  Will
fix, but will leave the other ifdef excursions alone for now.
--
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: ep0: fix delayed status is queued too early

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Felipe Balbi wrote:

> > The dwc3 driver should always prepare a buffer for the next ep0 SETUP
> > packet as soon as it retrieves the information for the current SETUP 
> > packet from the buffer.
> > 
> > Otherwise, as you described it, if the gadget driver never sends its 
> > delayed status response then the UDC will never respond to any more 
> > control transfers.
> 
> we _do_ prepare transfers for setup packet everytime a Status phase is
> completed (we also restart ep0 in case of stalls):

I said that dwc3 should prepare a buffer for a SETUP packet every time
a _SETUP_ stage is completed -- not every time a _STATUS_ stage is
completed.

In principle the host can send a SETUP packet at any time, even in the 
middle of an ongoing transfer.  (Consider the case where a driver on 
the host unlinks a control transfer before it has completed and then 
submits a new one.)

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


[RFC/PATCH] usb: musb: Prevent musb_am335x from being removed

2014-05-08 Thread Ezequiel Garcia
At probe time, the musb_am335x driver registers its childs by
calling of_platform_populate(), which registers all childs in
the devicetree hierarchy recursively.

On the other side, the driver's remove() function uses of_device_unregister()
to remove each child in the musb_am335x driver device struct.

However, when musb_dsps is loaded, its devices are attached to the musb_am335x
driver device struct as musb_am335x childs. Hence, musb_am335x remove()
will attempt to unregister the devices registered by musb_dsps, which produces
a kernel panic.

In other words, the childs in the "struct device" hierarchy are not the same
as the childs in the "devicetree" hierarchy.

Ideally, we should be enforcing the removal of the devices registered by
musb_am335x *only*, instead of all the child devices. However, because of the
recursive nature of of_platform_populate, this doesn't seem possible.

Therefore, as the only solution at hand, this commit disables musb_am335x driver
removal capability, preventing it from being ever removed.

Just for reference, here's the panic upon module removal:

musb-hdrc musb-hdrc.0.auto: remove, state 4
usb usb1: USB disconnect, device number 1
musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered
Unable to handle kernel NULL pointer dereference at virtual address 008c
pgd = de11c000
[008c] *pgd=9e174831, *pte=, *ppte=
Internal error: Oops: 17 [#1] ARM
Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common
CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-1-g24efd13 #69
task: de1b7500 ti: de122000 task.ti: de122000
PC is at am335x_shutdown+0x10/0x28
LR is at am335x_shutdown+0xc/0x28
pc : []lr : []psr: a013
sp : de123df8  ip : 0004  fp : 00028f00
r10:   r9 : de122000  r8 : c000e6c4
r7 : de0e3c10  r6 : de0e3800  r5 : de624010  r4 : de1ec750
r3 : de0e3810  r2 :   r1 : 0001  r0 : 
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c5387d  Table: 9e11c019  DAC: 0015
Process modprobe (pid: 623, stack limit = 0xde122240)
Stack: (0xde123df8 to 0xde124000)
3de0:   de0e3810 bf054488
3e00: bf05444c de624010 6013 bf043650 12fc de624010 de0e3810 bf043a20
3e20: de0e3810 bf04b240 c0635b88 c02ca37c c02ca364 c02c8db0 de1b7500 de0e3844
3e40: de0e3810 c02c8e28 c0635b88 de02824c de0e3810 c02c884c de0e3800 de0e3810
3e60: de0e3818 c02c5b20 bf05417c de0e3800 de0e3800 c0635b88 de0f2410 c02ca838
3e80: bf05417c de0e3800 bf055438 c02ca8cc de0e3c10 bf054194 de0e3c10 c02ca37c
3ea0: c02ca364 c02c8db0 de1b7500 de0e3c44 de0e3c10 c02c8e28 c0635b88 de02824c
3ec0: de0e3c10 c02c884c de0e3c10 de0e3c10 de0e3c18 c02c5b20 de0e3c10 de0e3c10
3ee0:  bf059000 a013 c02c5bc0  bf05900c de0e3c10 c02c5c48
3f00: de0dd0c0 de1ec970 de0f2410 bf05929c de0f2444 bf05902c de0f2410 c02ca37c
3f20: c02ca364 c02c8db0 bf05929c de0f2410 bf05929c c02c94c8 bf05929c 
3f40: 0800 c02c8ab4 bf0592e0 c007fc40 c00dd820 6273756d 336d615f 00783533
3f60: c064a0ac de1b7500 de122000 de1b7500 c000e590 0001 c000e6c4 c0060160
3f80: 00028e70 00028e70 00028ea4 0081 6010 00028e70 00028e70 00028ea4
3fa0: 0081 c000e500 00028e70 00028e70 00028ea4 0800 becb59f8 00027608
3fc0: 00028e70 00028e70 00028ea4 0081 0001 0001  00028f00
3fe0: b6e6b6f0 becb59d4 000160e8 b6e6b6fc 6010 00028ea4  
[] (am335x_shutdown) from [] (dsps_musb_exit+0x3c/0x4c 
[musb_dsps])
[] (dsps_musb_exit [musb_dsps]) from [] 
(musb_shutdown+0x80/0x90 [musb_hdrc])
[] (musb_shutdown [musb_hdrc]) from [] 
(musb_remove+0x24/0x68 [musb_hdrc])
[] (musb_remove [musb_hdrc]) from [] 
(platform_drv_remove+0x18/0x1c)
[] (platform_drv_remove) from [] 
(__device_release_driver+0x70/0xc8)
[] (__device_release_driver) from [] 
(device_release_driver+0x20/0x2c)
[] (device_release_driver) from [] 
(bus_remove_device+0xdc/0x10c)
[] (bus_remove_device) from [] (device_del+0x104/0x198)
[] (device_del) from [] (platform_device_del+0x14/0x9c)
[] (platform_device_del) from [] 
(platform_device_unregister+0xc/0x20)
[] (platform_device_unregister) from [] 
(dsps_remove+0x18/0x38 [musb_dsps])
[] (dsps_remove [musb_dsps]) from [] 
(platform_drv_remove+0x18/0x1c)
[] (platform_drv_remove) from [] 
(__device_release_driver+0x70/0xc8)
[] (__device_release_driver) from [] 
(device_release_driver+0x20/0x2c)
[] (device_release_driver) from [] 
(bus_remove_device+0xdc/0x10c)
[] (bus_remove_device) from [] (device_del+0x104/0x198)
[] (device_del) from [] (device_unregister+0xc/0x20)
[] (device_unregister) from [] 
(of_remove_populated_child+0xc/0x14 [musb_am335x])
[] (of_remove_populated_child [musb_am335x]) from [] 
(device_for_each_child+0x44/0x70)
[] (device_for_each_child) from [] 
(am335x_child_remove+0x18/0x30 [musb_am335x])
[] (am335x_child_remove [musb_am335x]) from [] 
(platform_drv_remove+0x18/0x1c)
[] (platform_drv_remove) from [] 
(__device_release_driver+0x70/0x

Re: [PATCH] usb: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Zhuang Jin Can
On Thu, May 08, 2014 at 11:22:36AM -0400, Alan Stern wrote:
> On Thu, 8 May 2014, Zhuang Jin Can wrote:
> 
> > > dwc3 _cannot_ return NYET to a SETUP packet.  The USB protocol does not 
> > > allow it.  A device must always respond to SETUP with ACK.
> > It true that device can not return NYET to a SETUP packet.
> > A device must always respond to SETUP with ACK _if_ the SETUP packet is
> > correctly received. Because there's no buffer prepared in ep0 for dwc3
> > to receive the SETUP packet, I guess there will be no handshake
> > returned to host. I can confirm this by doing an experiment tomorrow:)
> 
> The dwc3 driver should always prepare a buffer for the next ep0 SETUP
> packet as soon as it retrieves the information for the current SETUP 
> packet from the buffer.
> 
In current model dwc3 doesn't prepare a buffer for the next ep0 SETUP
packet, and normally host should not send another SETUP packet if the
current control transfer is not finished. So below model works well
if every control transfer succeeds one by one.

Here's the 2-stage control transfer model in dwc3.
***
* SETUP PHASE:*
*Setup a Control-Setup TRB, start Transfer*<-
*** |
|   |
   XferComplete irq |
|   |
V   |
*** |
*Interpret Setup bytes* |
*** |
|   |
  2 stage  XferComplete
|   Setup Pending=0 or 1

V   |
*   |
* Wait for Host *   |
*   |
|   |
Status XferNotReady irq |
|   |
V   |
*** |
*   STATUS PHASE: * |
*Setup Control-Status2 TRB, Start Transfer*-|
***
(note: in *STATUS PHASE* it can't start transfer if
the delayed status request is not queued by gadget driver).

If the gadget driver can't queue the delayed status request in time,
dwc3 will stay at *STATUS PHASE*.
Then, current control transfer fails. Host starts to send a new SETUP
packet.
At this point, it really depends on how dwc3 controller will behave when
it receives the new SETUP packet. If it can move on to *SETUP PHASE*
with similar way to [Tree-stage back to back SETUP handling] (see
below), then the stale delayed status request could cause a problem.

[ Tree-stage back to back SETUP handling]
For a tree-stage control transfer, dwc3 can handle back to back
SETUP packets. Below is quoted from dwc3 ep0.c
/*
 * Unfortunately we have uncovered a limitation wrt the Data
 * Phase.
 *
 * Section 9.4 says we can wait for the XferNotReady(DATA) event
 * to
 * come before issueing Start Transfer command, but if we do, we
 * will
 * miss situations where the host starts another SETUP phase
 * instead of
 * the DATA phase.  Such cases happen at least on TD.7.6 of the
 * Link
 * Layer Compliance Suite.
 *
 * The problem surfaces due to the fact that in case of
 * back-to-back
 * SETUP packets there will be no XferNotReady(DATA) generated
 * and we
 * will be stuck waiting for XferNotReady(DATA) forever.
 *
 * By looking at tables 9-13 and 9-14 of the Databook, we can
 * see that
 * it tells us to start Data Phase right away. It also mentions
 * that if
 * we receive a SETUP phase instead of the DATA phase, core will
 * issue
 * XferComplete for the DATA phase, before actually initiating
 * it in
 * the wire, with the TRB's status set to "SETUP_PENDING". Such
 * status
 * can only be used to print some debugging logs, as the core
 * expects
 * us to go through to the STATUS phase and start a
 * CONTROL_STATUS TRB,
 * just so it completes right away, without transferring
 * anything and,
 * only then, we can go back to the SETUP phase.
 *
 * Because of this scenario, SNPS decided to change the
 * programming
 * model of control transfers and support on-demand transfers
 * only for
 * the STATUS phase. To fix the issue we have now, we will
 * always wait
 * for gadget driver to queue the DATA phase's struct
 * usb_request, then
 * start it right away.
 *
 * If we're actually in a 2-stage transfer, we will wait for
 * XferNotReady(STATUS).
 */

> Oth

Re: [PATCH 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 9:47 AM, Joe Perches  wrote:
> On Thu, 2014-05-08 at 19:25 +0300, Mathias Nyman wrote:
>> Save someone else the debug cycles of figuring out why a driver's
>> transfer request is failing or causing undefined system behavior.
>> Buffers submitted for dma must come from GFP allocated / DMA-able
>> memory.
>>
>> Return -EAGAIN matching the return value for dma_mapping_error() cases.
> []
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> []
>> @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, 
>> struct urb *urb,
>>   ret = -EAGAIN;
>>   else
>>   urb->transfer_flags |= 
>> URB_DMA_MAP_PAGE;
>> + } else if (is_vmalloc_addr(urb->transfer_buffer)) {
>> + WARN_ONCE(1, "transfer buffer not dma 
>> capable\n");
>> + ret = -EAGAIN;
>>   } else {
>>   urb->transfer_dma = dma_map_single(
>>   hcd->self.controller,
>
> Perhaps this could be #ifdef'd here or moved to and
> tested in dma_map_single/dma_map_single_attr instead.
>

What problem are you trying to solve?  Adding it to dma_map() means
incurring the overhead of checking on every call and it would be akin
to adding dma_mapping_error() to dma_map().  Otherwise, if it is
ifdef'd that will miss the very same driver developers that didn't
know they couldn't do that.  USB core has already committed to
validating the input buffers in this routine, what's wrong with one
more check?
--
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 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

2014-05-08 Thread Joe Perches
On Thu, 2014-05-08 at 19:25 +0300, Mathias Nyman wrote:
> Save someone else the debug cycles of figuring out why a driver's
> transfer request is failing or causing undefined system behavior.
> Buffers submitted for dma must come from GFP allocated / DMA-able
> memory.
> 
> Return -EAGAIN matching the return value for dma_mapping_error() cases.
[]
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
[]
> @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct 
> urb *urb,
>   ret = -EAGAIN;
>   else
>   urb->transfer_flags |= URB_DMA_MAP_PAGE;
> + } else if (is_vmalloc_addr(urb->transfer_buffer)) {
> + WARN_ONCE(1, "transfer buffer not dma 
> capable\n");
> + ret = -EAGAIN;
>   } else {
>   urb->transfer_dma = dma_map_single(
>   hcd->self.controller,

Perhaps this could be #ifdef'd here or moved to and
tested in dma_map_single/dma_map_single_attr instead.

--
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 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 9:22 AM, David Laight  wrote:
> From: Mathias Nyman
>> From: Dan Williams 
>>
>> Save someone else the debug cycles of figuring out why a driver's
>> transfer request is failing or causing undefined system behavior.
>> Buffers submitted for dma must come from GFP allocated / DMA-able
>> memory.
>>
>> Return -EAGAIN matching the return value for dma_mapping_error() cases.
>
> Won't that just cause the request to be resubmitted a few clock
> cycles later?
> Surely you either need to error the request, or panic.

No, panic() is too drastic for this.

> In any case is this the right place for this sort of test?

Yes.

The system is already compromised because the driver is broken.  The
expectation is that this is a clue bat for a driver developer to fix
their code before it goes upstream.  I found this while debugging an
interaction with a new driver that was causing the xhci controller to
lock up.  Had this warning been there I likely never would have
received the xhci bug report.
--
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 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

2014-05-08 Thread David Laight
From: Mathias Nyman
> From: Dan Williams 
> 
> Save someone else the debug cycles of figuring out why a driver's
> transfer request is failing or causing undefined system behavior.
> Buffers submitted for dma must come from GFP allocated / DMA-able
> memory.
> 
> Return -EAGAIN matching the return value for dma_mapping_error() cases.

Won't that just cause the request to be resubmitted a few clock
cycles later?
Surely you either need to error the request, or panic.
In any case is this the right place for this sort of test?

(Yes I've spent some time before realising that Linux doesn't
have a proper vtop() function )

David

...
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 9c4e292..adddc66 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct 
> urb *urb,
>   ret = -EAGAIN;
>   else
>   urb->transfer_flags |= URB_DMA_MAP_PAGE;
> + } else if (is_vmalloc_addr(urb->transfer_buffer)) {
> + WARN_ONCE(1, "transfer buffer not dma 
> capable\n");
> + ret = -EAGAIN;
>   } else {
>   urb->transfer_dma = dma_map_single(
>   hcd->self.controller,


--
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 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 9:25 AM, Mathias Nyman
 wrote:
> From: Dan Williams 
>
> Save someone else the debug cycles of figuring out why a driver's
> transfer request is failing or causing undefined system behavior.
> Buffers submitted for dma must come from GFP allocated / DMA-able
> memory.
>
> Return -EAGAIN matching the return value for dma_mapping_error() cases.
>
> Cc: Alan Stern 
> Cc: Sarah Sharp 
> Cc: Mathias Nyman 
> Signed-off-by: Dan Williams 
> Signed-off-by: Mathias Nyman 

Thanks Mathias.

One note, this was acked-by Alan here:

http://marc.info/?l=linux-usb&m=139327920501989&w=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 05/10] xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()

2014-05-08 Thread Mathias Nyman
From: Alexander Gordeev 

As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range()  or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Signed-off-by: Alexander Gordeev 
Cc: Sarah Sharp 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 708cb29..88ec076 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -291,7 +291,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
xhci->msix_entries[i].vector = 0;
}
 
-   ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count);
+   ret = pci_enable_msix_exact(pdev, xhci->msix_entries, xhci->msix_count);
if (ret) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Failed to enable MSI-X");
-- 
1.8.3.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 06/10] xhci: Report max device limit when Enable Slot command fails.

2014-05-08 Thread Mathias Nyman
From: Sarah Sharp 

xHCI host controllers may only support a limited number of device slot
IDs, which is usually far less than the theoretical maximum number of
devices (255) that the USB specifications advertise.  This is
frustrating to consumers that expect to be able to plug in a large
number of devices.

Add a print statement when the Enable Slot command fails to show how
many devices the host supports.  We can't change hardware manufacturer's
design decisions, but hopefully we can save customers a little bit of
time trying to debug why their host mysteriously fails when too many
devices are plugged in.

Signed-off-by: Sarah Sharp 
Reported-by: Amund Hov 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 88ec076..92e1dda 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3696,6 +3696,9 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
 
if (!xhci->slot_id) {
xhci_err(xhci, "Error while assigning device slot ID\n");
+   xhci_err(xhci, "Max number of devices this xHCI host supports 
is %u.\n",
+   HCS_MAX_SLOTS(
+   readl(&xhci->cap_regs->hcs_params1)));
return 0;
}
 
-- 
1.8.3.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 00/10] xhci: features for usb-next

2014-05-08 Thread Mathias Nyman
Hi Greg

These following xhci patches are for usb-next and hopefully for 3.16

This patcheseries includes a bigger change in xhci command queue code,
(last four patches), a task that I've been working on for a longer time.
Sarah gave green light for v5 before she went on her sabbatical.

http://marc.info/?l=linux-usb&m=139889908701592&w=2

-Mathias

Alexander Gordeev (1):
  xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()

Dan Williams (2):
  xhci: 'noxhci_port_switch' kernel parameter
  usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

Fabio Estevam (1):
  usb: xhci: Use IS_ENABLED() macro

Lin Wang (1):
  xhci: fix wrong port number reported when setting USB2.0 hardware LPM.

Mathias Nyman (4):
  xhci: Use command structures when queuing commands on the command ring
  xhci: Add a global command queue
  xhci: Use completion and status in global command queue
  xhci: rework command timeout and cancellation,

Sarah Sharp (1):
  xhci: Report max device limit when Enable Slot command fails.

 Documentation/kernel-parameters.txt |   3 +
 drivers/usb/core/hcd.c  |   3 +
 drivers/usb/host/pci-quirks.c   |  15 +-
 drivers/usb/host/xhci-hub.c |  43 ++-
 drivers/usb/host/xhci-mem.c |  17 +-
 drivers/usb/host/xhci-ring.c| 587 ++--
 drivers/usb/host/xhci.c | 271 +
 drivers/usb/host/xhci.h |  47 +--
 8 files changed, 440 insertions(+), 546 deletions(-)

-- 
1.8.3.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 02/10] xhci: 'noxhci_port_switch' kernel parameter

2014-05-08 Thread Mathias Nyman
From: Dan Williams 

Add a command line switch for disabling ehci port switchover.  Useful
for working around / debugging xhci incompatibilities where ehci
operation is available.

Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2

Cc: Sarah Sharp 
Cc: Mathias Nyman 
Cc: Holger Hans Peter Freyther 
Suggested-by: Alan Stern 
Signed-off-by: Dan Williams 
Signed-off-by: Mathias Nyman 
---
 Documentation/kernel-parameters.txt |  3 +++
 drivers/usb/host/pci-quirks.c   | 15 +--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 4384217..fc3403114 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
 
nox2apic[X86-64,APIC] Do not enable x2APIC mode.
 
+   noxhci_port_switch
+   [USB] Use EHCI instead of XHCI where available
+
cpu0_hotplug[X86] Turn on CPU0 hotplug feature when
CONFIG_BOOTPARAM_HOTPLUG_CPU0 is off.
Some features depend on CPU0. Known dependencies are:
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 00661d3..38bfe3d 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -823,6 +823,15 @@ static int handshake(void __iomem *ptr, u32 mask, u32 done,
return -ETIMEDOUT;
 }
 
+static int noxhci_port_switch;
+
+static int __init noxhci_port_switch_setup(char *str)
+{
+   noxhci_port_switch = 1;
+   return 0;
+}
+early_param("noxhci_port_switch", noxhci_port_switch_setup);
+
 /*
  * Intel's Panther Point chipset has two host controllers (EHCI and xHCI) that
  * share some number of ports.  These ports can be switched between either
@@ -860,8 +869,7 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
return;
 
/* Don't switchover the ports if the user hasn't compiled the xHCI
-* driver.  Otherwise they will see "dead" USB ports that don't power
-* the devices.
+* driver or has explicitly disabled switchover
 */
if (!IS_ENABLED(CONFIG_USB_XHCI_HCD)) {
dev_warn(&xhci_pdev->dev,
@@ -871,6 +879,9 @@ void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
"USB 3.0 devices will work at USB 2.0 
speeds.\n");
usb_disable_xhci_ports(xhci_pdev);
return;
+   } else if (noxhci_port_switch) {
+   usb_disable_xhci_ports(xhci_pdev);
+   return;
}
 
/* Read USB3PRM, the USB 3.0 Port Routing Mask Register
-- 
1.8.3.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 04/10] usb: xhci: Use IS_ENABLED() macro

2014-05-08 Thread Mathias Nyman
From: Fabio Estevam 

Using the IS_ENABLED() macro can make the code shorter and easier to read.

Signed-off-by: Fabio Estevam 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 4746816..cc67c76 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1738,8 +1738,7 @@ static inline int xhci_register_pci(void) { return 0; }
 static inline void xhci_unregister_pci(void) {}
 #endif
 
-#if defined(CONFIG_USB_XHCI_PLATFORM) \
-   || defined(CONFIG_USB_XHCI_PLATFORM_MODULE)
+#if IS_ENABLED(CONFIG_USB_XHCI_PLATFORM)
 int xhci_register_plat(void);
 void xhci_unregister_plat(void);
 #else
-- 
1.8.3.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 07/10] xhci: Use command structures when queuing commands on the command ring

2014-05-08 Thread Mathias Nyman
To create a global command queue we require that each command put on the
command ring is submitted with a command structure.

Functions that queue commands and wait for completion need to allocate a command
before submitting it, and free it once completed. The following command queuing
functions need to be modified.

xhci_configure_endpoint()
xhci_address_device()
xhci_queue_slot_control()
xhci_queue_stop_endpoint()
xhci_queue_new_dequeue_state()
xhci_queue_reset_ep()
xhci_configure_endpoint()

xhci_configure_endpoint() could already be called with a command structure,
and only xhci_check_maxpacket and xhci_check_bandwidth did not do so. These
are changed and a command structure is now required. This change also simplifies
the configure endpoint command completion handling and the "goto 
bandwidth_change"
handling code can be removed.

In some cases the command queuing function is called in interrupt context.
These commands needs to be allocated atomically, and they can't wait for
completion. These commands will in this patch be freed directly after queuing,
but freeing will be moved to the command completion event handler in a later
patch once we get the global command queue up.(Just so that we won't leak
memory in the middle of the patch set)

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  |  21 +++--
 drivers/usb/host/xhci-ring.c | 107 +---
 drivers/usb/host/xhci.c  | 194 ---
 drivers/usb/host/xhci.h  |  31 +++
 4 files changed, 216 insertions(+), 137 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1ad6bc1..3ce9c0a 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -20,7 +20,8 @@
  * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#include 
+
+#include 
 #include 
 
 #include "xhci.h"
@@ -284,12 +285,22 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
 
spin_lock_irqsave(&xhci->lock, flags);
for (i = LAST_EP_INDEX; i > 0; i--) {
-   if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue)
-   xhci_queue_stop_endpoint(xhci, slot_id, i, suspend);
+   if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
+   struct xhci_command *command;
+   command = xhci_alloc_command(xhci, false, false,
+GFP_NOIO);
+   if (!command) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
+   xhci_free_command(xhci, cmd);
+   return -ENOMEM;
+
+   }
+   xhci_queue_stop_endpoint(xhci, command, slot_id, i,
+suspend);
+   }
}
-   cmd->command_trb = xhci_find_next_enqueue(xhci->cmd_ring);
list_add_tail(&cmd->cmd_list, &virt_dev->cmd_list);
-   xhci_queue_stop_endpoint(xhci, slot_id, 0, suspend);
+   xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7a0e3c7..b172a7d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -123,16 +123,6 @@ static int enqueue_is_link_trb(struct xhci_ring *ring)
return TRB_TYPE_LINK_LE32(link->control);
 }
 
-union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
-{
-   /* Enqueue pointer can be left pointing to the link TRB,
-* we must handle that
-*/
-   if (TRB_TYPE_LINK_LE32(ring->enqueue->link.control))
-   return ring->enq_seg->next->trbs;
-   return ring->enqueue;
-}
-
 /* Updates trb to point to the next TRB in the ring, and updates seg if the 
next
  * TRB is in a new segment.  This does not skip over link TRBs, and it does not
  * effect the ring dequeue or enqueue pointers.
@@ -684,12 +674,14 @@ static void td_to_noop(struct xhci_hcd *xhci, struct 
xhci_ring *ep_ring,
}
 }
 
-static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id,
+static int queue_set_tr_deq(struct xhci_hcd *xhci,
+   struct xhci_command *cmd, int slot_id,
unsigned int ep_index, unsigned int stream_id,
struct xhci_segment *deq_seg,
union xhci_trb *deq_ptr, u32 cycle_state);
 
 void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
+   struct xhci_command *cmd,
unsigned int slot_id, unsigned int ep_index,
unsigned int stream_id,
struct xhci_dequeue_state *deq_state)
@@ -704,7 +696,7 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
deq_state->new_deq_ptr,
(unsigned long 
long)xhci_trb_virt_to_dma(deq_

[PATCH 08/10] xhci: Add a global command queue

2014-05-08 Thread Mathias Nyman
Create a list to store command structures, add a structure to it every time
a command is submitted, and remove it from the list once we get a
command completion event matching the command.

Callers that wait for completion will free their command structures themselves.
The other command structures are freed in the command completion event handler.

Also add a check that prevents queuing commands if host is dying

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c  |  2 ++
 drivers/usb/host/xhci-ring.c | 34 ++
 drivers/usb/host/xhci.c  |  2 --
 drivers/usb/host/xhci.h  |  2 ++
 4 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index c089668..b070a17 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1821,6 +1821,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
list_del(&cur_cd->cancel_cmd_list);
kfree(cur_cd);
}
+   xhci_cleanup_command_queue(xhci);
 
for (i = 1; i < MAX_HC_SLOTS; ++i)
xhci_free_virt_device(xhci, i);
@@ -2324,6 +2325,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
int i;
 
INIT_LIST_HEAD(&xhci->cancel_cmd_list);
+   INIT_LIST_HEAD(&xhci->cmd_list);
 
page_size = readl(&xhci->op_regs->page_size);
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b172a7d..89b8745 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1520,6 +1520,20 @@ static void xhci_handle_cmd_nec_get_fw(struct xhci_hcd 
*xhci,
NEC_FW_MINOR(le32_to_cpu(event->status)));
 }
 
+static void xhci_del_and_free_cmd(struct xhci_command *cmd)
+{
+   list_del(&cmd->cmd_list);
+   if (!cmd->completion)
+   kfree(cmd);
+}
+
+void xhci_cleanup_command_queue(struct xhci_hcd *xhci)
+{
+   struct xhci_command *cur_cmd, *tmp_cmd;
+   list_for_each_entry_safe(cur_cmd, tmp_cmd, &xhci->cmd_list, cmd_list)
+   xhci_del_and_free_cmd(cur_cmd);
+}
+
 static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
 {
@@ -1528,6 +1542,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
union xhci_trb *cmd_trb;
+   struct xhci_command *cmd;
u32 cmd_type;
 
cmd_dma = le64_to_cpu(event->cmd_trb);
@@ -1545,6 +1560,13 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
 
+   cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);
+
+   if (cmd->command_trb != xhci->cmd_ring->dequeue) {
+   xhci_err(xhci,
+"Command completion event does not match command\n");
+   return;
+   }
trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
 
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
@@ -1614,6 +1636,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci->error_bitmask |= 1 << 6;
break;
}
+
+   xhci_del_and_free_cmd(cmd);
+
inc_deq(xhci, xhci->cmd_ring);
 }
 
@@ -3998,6 +4023,8 @@ static int queue_command(struct xhci_hcd *xhci, struct 
xhci_command *cmd,
 {
int reserved_trbs = xhci->cmd_ring_reserved_trbs;
int ret;
+   if (xhci->xhc_state & XHCI_STATE_DYING)
+   return -ESHUTDOWN;
 
if (!command_must_succeed)
reserved_trbs++;
@@ -4011,10 +4038,9 @@ static int queue_command(struct xhci_hcd *xhci, struct 
xhci_command *cmd,
"unfailable commands failed.\n");
return ret;
}
-   if (cmd->completion)
-   cmd->command_trb = xhci->cmd_ring->enqueue;
-   else
-   kfree(cmd);
+
+   cmd->command_trb = xhci->cmd_ring->enqueue;
+   list_add_tail(&cmd->cmd_list, &xhci->cmd_list);
 
queue_trb(xhci, xhci->cmd_ring, false, field1, field2, field3,
field4 | xhci->cmd_ring->cycle_state);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9a4c6df..8dbc410 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3732,7 +3732,6 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
timeleft == 0 ? "Timeout" : "Signal");
/* cancel the enable slot request */
ret = xhci_cancel_cmd(xhci, NULL, command->command_trb);
-   kfree(command);
return ret;
}
 
@@ -3891,7 +3890,6 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct 
usb_device *udev,
  timeleft == 0 ? "Timeout" : "Signal", act);
/* cancel the address device command */
 

[PATCH 09/10] xhci: Use completion and status in global command queue

2014-05-08 Thread Mathias Nyman
Remove the per-device command list and handle_cmd_in_cmd_wait_list()
and use the completion and status variables found in the
command structure in the global command list.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  | 11 --
 drivers/usb/host/xhci-mem.c  |  1 -
 drivers/usb/host/xhci-ring.c | 84 
 drivers/usb/host/xhci.c  | 16 ++---
 drivers/usb/host/xhci.h  |  3 --
 5 files changed, 17 insertions(+), 98 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 3ce9c0a..12871b5 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -299,7 +299,6 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
 suspend);
}
}
-   list_add_tail(&cmd->cmd_list, &virt_dev->cmd_list);
xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
@@ -311,18 +310,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
if (timeleft <= 0) {
xhci_warn(xhci, "%s while waiting for stop endpoint command\n",
timeleft == 0 ? "Timeout" : "Signal");
-   spin_lock_irqsave(&xhci->lock, flags);
-   /* The timeout might have raced with the event ring handler, so
-* only delete from the list if the item isn't poisoned.
-*/
-   if (cmd->cmd_list.next != LIST_POISON1)
-   list_del(&cmd->cmd_list);
-   spin_unlock_irqrestore(&xhci->lock, flags);
ret = -ETIME;
-   goto command_cleanup;
}
-
-command_cleanup:
xhci_free_command(xhci, cmd);
return ret;
 }
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b070a17..38dc721 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1020,7 +1020,6 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int 
slot_id,
dev->num_rings_cached = 0;
 
init_completion(&dev->cmd_completion);
-   INIT_LIST_HEAD(&dev->cmd_list);
dev->udev = udev;
 
/* Point to output device context in dcbaa. */
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 89b8745..3d60865 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -69,10 +69,6 @@
 #include "xhci.h"
 #include "xhci-trace.h"
 
-static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci,
-   struct xhci_virt_device *virt_dev,
-   struct xhci_event_cmd *event);
-
 /*
  * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA
  * address of the TRB.
@@ -765,7 +761,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
union xhci_trb *trb, struct xhci_event_cmd *event)
 {
unsigned int ep_index;
-   struct xhci_virt_device *virt_dev;
struct xhci_ring *ep_ring;
struct xhci_virt_ep *ep;
struct list_head *entry;
@@ -775,11 +770,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
struct xhci_dequeue_state deq_state;
 
if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb->generic.field[3] {
-   virt_dev = xhci->devs[slot_id];
-   if (virt_dev)
-   handle_cmd_in_cmd_wait_list(xhci, virt_dev,
-   event);
-   else
+   if (!xhci->devs[slot_id])
xhci_warn(xhci, "Stop endpoint command "
"completion for disabled slot %u\n",
slot_id);
@@ -1229,29 +1220,6 @@ static void xhci_complete_cmd_in_cmd_wait_list(struct 
xhci_hcd *xhci,
 }
 
 
-/* Check to see if a command in the device's command queue matches this one.
- * Signal the completion or free the command, and return 1.  Return 0 if the
- * completed command isn't at the head of the command list.
- */
-static int handle_cmd_in_cmd_wait_list(struct xhci_hcd *xhci,
-   struct xhci_virt_device *virt_dev,
-   struct xhci_event_cmd *event)
-{
-   struct xhci_command *command;
-
-   if (list_empty(&virt_dev->cmd_list))
-   return 0;
-
-   command = list_entry(virt_dev->cmd_list.next,
-   struct xhci_command, cmd_list);
-   if (xhci->cmd_ring->dequeue != command->command_trb)
-   return 0;
-
-   xhci_complete_cmd_in_cmd_wait_list(xhci, command,
-   GET_COMP_CODE(le32_to_cpu(event->status)));
-   return 1;
-}
-
 /*
  * Finding the command trb need to be cancelled and modifying it to
  * NO OP command. And if the command is in device's command wait
@@ -1403,7 +1371,6 @@ static void xhci_handle_cmd_enable_sl

[PATCH 03/10] usb: catch attempts to submit urbs with a vmalloc'd transfer buffer

2014-05-08 Thread Mathias Nyman
From: Dan Williams 

Save someone else the debug cycles of figuring out why a driver's
transfer request is failing or causing undefined system behavior.
Buffers submitted for dma must come from GFP allocated / DMA-able
memory.

Return -EAGAIN matching the return value for dma_mapping_error() cases.

Cc: Alan Stern 
Cc: Sarah Sharp 
Cc: Mathias Nyman 
Signed-off-by: Dan Williams 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/core/hcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 9c4e292..adddc66 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1502,6 +1502,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct 
urb *urb,
ret = -EAGAIN;
else
urb->transfer_flags |= URB_DMA_MAP_PAGE;
+   } else if (is_vmalloc_addr(urb->transfer_buffer)) {
+   WARN_ONCE(1, "transfer buffer not dma 
capable\n");
+   ret = -EAGAIN;
} else {
urb->transfer_dma = dma_map_single(
hcd->self.controller,
-- 
1.8.3.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 01/10] xhci: fix wrong port number reported when setting USB2.0 hardware LPM.

2014-05-08 Thread Mathias Nyman
From: Lin Wang 

This patch fix wrong port number reported when trying to enable/disable
USB2.0 hardware LPM.

Signed-off-by: Lin Wang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3008369..708cb29 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4092,7 +4092,7 @@ int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 
xhci_dbg(xhci, "%s port %d USB2 hardware LPM\n",
-   enable ? "enable" : "disable", port_num);
+   enable ? "enable" : "disable", port_num + 1);
 
if (enable) {
/* Host supports BESL timeout instead of HIRD */
-- 
1.8.3.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 10/10] xhci: rework command timeout and cancellation,

2014-05-08 Thread Mathias Nyman
Use one timer to control command timeout.

start/kick the timer every time a command is completed and a
new command is waiting, or a new command is added to a empty list.

If the timer runs out, then tag the current command as "aborted", and
start the xhci command abortion process.

Previously each function that submitted a command had its own timer.
If that command timed out, a new command structure for the
command was created and it was put on a cancel_cmd_list list,
then a pci write to abort the command ring was issued.

when the ring was aborted, it checked if the current command
was the one to be canceled, later when the ring was stopped the
driver got ownership of the TRBs in the command ring,
compared then to the TRBs in the cancel_cmd_list,
and turned them into No-ops.

Now, instead, at timeout we tag the status of the command in the
command queue to be aborted, and start the ring abortion.
Ring abortion stops the command ring and gives control of the
commands to us.
All the aborted commands are now turned into No-ops.

If the ring is already stopped when the command times outs its not possible
to start the ring abortion, in this case the command is turnd to No-op
right away.

All these changes allows us to remove the entire cancel_cmd_list code.

The functions waiting for a command to finish no longer have their own timeouts.
They will wait either until the command completes normally,
or until the whole command abortion is done.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  |  11 +-
 drivers/usb/host/xhci-mem.c  |  14 +-
 drivers/usb/host/xhci-ring.c | 378 +++
 drivers/usb/host/xhci.c  |  78 +++--
 drivers/usb/host/xhci.h  |   8 +-
 5 files changed, 169 insertions(+), 320 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 12871b5..6231ce6 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -271,7 +271,6 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
struct xhci_virt_device *virt_dev;
struct xhci_command *cmd;
unsigned long flags;
-   int timeleft;
int ret;
int i;
 
@@ -304,12 +303,10 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
spin_unlock_irqrestore(&xhci->lock, flags);
 
/* Wait for last stop endpoint command to finish */
-   timeleft = wait_for_completion_interruptible_timeout(
-   cmd->completion,
-   XHCI_CMD_DEFAULT_TIMEOUT);
-   if (timeleft <= 0) {
-   xhci_warn(xhci, "%s while waiting for stop endpoint command\n",
-   timeleft == 0 ? "Timeout" : "Signal");
+   wait_for_completion(cmd->completion);
+
+   if (cmd->status == COMP_CMD_ABORT || cmd->status == COMP_CMD_STOP) {
+   xhci_warn(xhci, "Timeout while waiting for stop endpoint 
command\n");
ret = -ETIME;
}
xhci_free_command(xhci, cmd);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 38dc721..6a57e81 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1793,10 +1793,11 @@ void xhci_free_command(struct xhci_hcd *xhci,
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
struct device   *dev = xhci_to_hcd(xhci)->self.controller;
-   struct xhci_cd  *cur_cd, *next_cd;
int size;
int i, j, num_ports;
 
+   del_timer_sync(&xhci->cmd_timer);
+
/* Free the Event Ring Segment Table and the actual Event Ring */
size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
if (xhci->erst.entries)
@@ -1815,11 +1816,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
xhci_ring_free(xhci, xhci->cmd_ring);
xhci->cmd_ring = NULL;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring");
-   list_for_each_entry_safe(cur_cd, next_cd,
-   &xhci->cancel_cmd_list, cancel_cmd_list) {
-   list_del(&cur_cd->cancel_cmd_list);
-   kfree(cur_cd);
-   }
xhci_cleanup_command_queue(xhci);
 
for (i = 1; i < MAX_HC_SLOTS; ++i)
@@ -2323,7 +2319,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
u32 page_size, temp;
int i;
 
-   INIT_LIST_HEAD(&xhci->cancel_cmd_list);
INIT_LIST_HEAD(&xhci->cmd_list);
 
page_size = readl(&xhci->op_regs->page_size);
@@ -2510,6 +2505,11 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
"Wrote ERST address to ir_set 0.");
xhci_print_ir_set(xhci, 0);
 
+   /* init command timeout timer */
+   init_timer(&xhci->cmd_timer);
+   xhci->cmd_timer.data = (unsigned long) xhci;
+   xhci->cmd_timer.function = xhci_handle_command_timeout;
+
/*
 * XXX: Might need to set the Interrupter Moderation Register to
 * something 

Re: [PATCH v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 9:09 AM, Alan Stern  wrote:
> On Thu, 8 May 2014, Dan Williams wrote:
>
>> > I don't understand this last part.  Why do we need to guarantee the
>> > child device has recovered from power loss?  Why not proceed the same
>> > way we do now when the child is suspended?
>>
>> Two reasons I believe:
>>
>> 1/ The child may be gone, and usb_port_resume() will mark it for disconnect
>>
>> 2/ Currently port_event() knows how to handle suspended devices
>> (USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the
>> status and change bits are different.  I figure why special case
>> port_event()?  Just make it so it handles all the same cases that are
>> presented when the port does not lose power.
>
> How much special casing would really be needed?

Don't know.  Is the forced wakeup really that onerous, vs the risk of
touch established code?  I've broken the port_event() path enough
times through this exercise that I'd want a driving reason beyond "why
not?".

>> > If you take that stuff out, it seems that there won't be any need to
>> > use wakeup_bits or usb_kick_khubd() for this purpose.
>>
>> See 1/ I think we want to handle disconnects right away, hence the khubd 
>> kick.
>
> If the child has been disconnected, pm_request_resume()'s callback
> will determine that fact quickly enough.

True.  We can now skip the kick, but the runtime_barrier is still
needed pending the resolution of the question above.
--
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] ARM: dts: Enable USB 3503 hub on exynos5250-snow

2014-05-08 Thread Tomasz Figa
On 08.05.2014 17:59, Andreas Färber wrote:
> Hello,
> 
> Am 05.05.2014 14:30, schrieb Vivek Gautam:
>> The exynos5250-snow has a SMSC USB3503 connected in
>> hardware only mode like a PHY. Enable support for it,
>> and add necessary 'reset-gpio' for it.
>>
>> This is in correspondance to similar patch by Mark Brown
>> 7c1b0ec ARM: dts: Enable USB hub on Arndale
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>
>> Based on 'for-next' branch of kgene's linux-samsung tree.
>>
>>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi |7 +++
>>  arch/arm/boot/dts/exynos5250-snow.dts |   14 ++
>>  2 files changed, 21 insertions(+)
> 
> The same snippet as for snow also fixed my USB issues on spring.
> Should it go into exynos5250-cros-common.dtsi instead?

The GPIO pins used are different, so I don't think the hub node added by
these patches could be shared.

Also, I believe all you need to add is a simple node as follows

usb_hub {
compatible = "smsc,usb3503a";
reset-gpios = <&gpe1 0 1>;
};

at root level (see my comments to this patch in another reply), which is
affordable to be duplicated for every platform that requires it,
especially if the reset-gpios property differs between boards.

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


Re: [PATCH v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Dan Williams wrote:

> > I don't understand this last part.  Why do we need to guarantee the
> > child device has recovered from power loss?  Why not proceed the same
> > way we do now when the child is suspended?
> 
> Two reasons I believe:
> 
> 1/ The child may be gone, and usb_port_resume() will mark it for disconnect
> 
> 2/ Currently port_event() knows how to handle suspended devices
> (USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the
> status and change bits are different.  I figure why special case
> port_event()?  Just make it so it handles all the same cases that are
> presented when the port does not lose power.

How much special casing would really be needed?

> > If you take that stuff out, it seems that there won't be any need to
> > use wakeup_bits or usb_kick_khubd() for this purpose.
> 
> See 1/ I think we want to handle disconnects right away, hence the khubd kick.

If the child has been disconnected, pm_request_resume()'s callback
will determine that fact quickly enough.

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 1/2] ARM: dts: Enable USB 3503 hub on exynos5250-snow

2014-05-08 Thread Tomasz Figa
Hi Vivek,

On 05.05.2014 14:30, Vivek Gautam wrote:
> The exynos5250-snow has a SMSC USB3503 connected in
> hardware only mode like a PHY. Enable support for it,
> and add necessary 'reset-gpio' for it.
> 
> This is in correspondance to similar patch by Mark Brown
> 7c1b0ec ARM: dts: Enable USB hub on Arndale
> 
> Signed-off-by: Vivek Gautam 
> ---
> 
> Based on 'for-next' branch of kgene's linux-samsung tree.
> 
>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi |7 +++
>  arch/arm/boot/dts/exynos5250-snow.dts |   14 ++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi 
> b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> index 9a49e68..bd8c8f1 100644
> --- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> @@ -706,6 +706,13 @@
>   samsung,pin-pud = <0>;
>   samsung,pin-drv = <0>;
>   };
> +
> + usb_hub_reset: usb-hub-reset {
> + samsung,pins = "gpe1-0";
> + samsung,pin-function = <1>;
> + samsung,pin-pud = <0>;
> + samsung,pin-drv = <0>;
> + };

This is not a generic pin config group and should not be added to this
file. Btw. it just sets the pin to output, which is what the consumer
driver can do as well by calling gpio_direction_output. Do you really
need this?

>   };
>  
>   pinctrl@10d1 {
> diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
> b/arch/arm/boot/dts/exynos5250-snow.dts
> index 1ce1088..2e48f27 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -200,6 +200,20 @@
>   samsung,vbus-gpio = <&gpx1 1 0>;
>   };
>  
> + usb_hub_bus {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <0>;

Why do you need this bus?

> +
> + // SMSC USB3503 connected in hardware only mode as a PHY

Wrong comment style. Also basically the comment says exactly the same as
could be inferred from the node below.

> + usb_hub {
> + compatible = "smsc,usb3503a";
> + reset-gpios = <&gpe1 0 1>;
> + pinctrl-0 = <&usb_hub_reset>;
> + pinctrl-names = "default";

As above, do you really need to use pinctrl here?

I believe the same comments apply for patch 2/2 as well.

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


Re: [PATCH v9 16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y

2014-05-08 Thread Alan Stern
On Wed, 7 May 2014, Dan Williams wrote:

> Per Alan:
> "You mean from within hub_handle_remote_wakeup()?  That routine will
> never get called if CONFIG_PM_RUNTIME isn't enabled, because khubd
> never sees wakeup requests if they arise during system suspend.
> 
> In fact, that routine ought to go inside the "#ifdef CONFIG_PM_RUNTIME"
> portion of hub.c, along with the other suspend/resume code."
> 
> Suggested-by: Alan Stern 
> Signed-off-by: Dan Williams 
> ---
>  drivers/usb/core/hub.c |4 
>  drivers/usb/core/usb.h |5 -
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index d91e6493abf7..45ded752c2d8 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4700,6 +4700,7 @@ static void hub_port_connect_change(struct usb_hub 
> *hub, int port1,
>   usb_lock_port(port_dev);
>  }
>  
> +#ifdef CONFIG_PM_RUNTIME
>  /* Returns 1 if there was a remote wakeup and a connect status change. */
>  static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
>   u16 portstatus, u16 portchange)
> @@ -4740,6 +4741,7 @@ static int hub_handle_remote_wakeup(struct usb_hub 
> *hub, unsigned int port,
>   dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
>   return connect_change;
>  }
> +#endif

I meant that instead of adding a new #ifdef section here, you could
move the subroutine inside the #ifdef that protects
usb_remote_wakeup().

>  static void port_event(struct usb_hub *hub, int port1)
>   __must_hold(&port_dev->status_lock)
> @@ -4818,8 +4820,10 @@ static void port_event(struct usb_hub *hub, int port1)
>   if (!pm_runtime_active(&port_dev->dev))
>   return;
>  
> +#ifdef CONFIG_PM_RUNTIME
>   if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
>   connect_change = 1;
> +#endif

Also, instead of adding another #ifdef here, you could add a #else
section to the existing #ifdef in which you define an inline version of
hub_handle_remote_wakeup() (or a macro version) that always returns 0.

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 v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 8:44 AM, Alan Stern  wrote:
> On Wed, 7 May 2014, Dan Williams wrote:
>
>> Unconditionally wake up the child device when the power session is
>> recovered.
>
> ...
>
>> 1, 2, and 4 are not a problem in the system dpm_resume() case because,
>> although the power-session is lost, khubd is frozen until after device
>> resume.  For the rpm_resume() case pm_request_resume() and
>> runtime-pm-synchronization is used to guarantee the same sequence of
>> events.  When pm_request_resume() is invoked on the child usb_device
>> port device is in the RPM_RESUMING state.  khubd in turn performs a
>> pm_runtime_barrier() on the port device to flush the port recovery, and
>> holds the port active while it resumes the child with another
>> pm_runtime_barrier().  This guarantees that the portstatus khubd
>> evaluates via port_event() is always on an active port and a usb_device
>> that has recovered from power loss.
>
> I don't understand this last part.  Why do we need to guarantee the
> child device has recovered from power loss?  Why not proceed the same
> way we do now when the child is suspended?

Two reasons I believe:

1/ The child may be gone, and usb_port_resume() will mark it for disconnect

2/ Currently port_event() knows how to handle suspended devices
(USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the
status and change bits are different.  I figure why special case
port_event()?  Just make it so it handles all the same cases that are
presented when the port does not lose power.

> If you take that stuff out, it seems that there won't be any need to
> use wakeup_bits or usb_kick_khubd() for this purpose.

See 1/ I think we want to handle disconnects right away, hence the khubd kick.

>> Given that this patch de-references port_dev->child we need to make sure
>> not to collide with usb_disconnect().
>
> I don't like the way this was done.  We shouldn't disable runtime PM
> unless there's no choice.  In this case, the disable would interfere
> with the immediately preceding pm_runtime_put().
>
> You could change the test_and_clear_bit()/pm_runtime_put() calls to
> !test_and_set_bit()/pm_runtime_get_sync.  Then replace the
> runtime-enable with clear_bit/pm_runtime_put.

Ok, I'll fix this up.
--
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] ARM: dts: Enable USB 3503 hub on exynos5250-snow

2014-05-08 Thread Andreas Färber
Hello,

Am 05.05.2014 14:30, schrieb Vivek Gautam:
> The exynos5250-snow has a SMSC USB3503 connected in
> hardware only mode like a PHY. Enable support for it,
> and add necessary 'reset-gpio' for it.
> 
> This is in correspondance to similar patch by Mark Brown
> 7c1b0ec ARM: dts: Enable USB hub on Arndale
> 
> Signed-off-by: Vivek Gautam 
> ---
> 
> Based on 'for-next' branch of kgene's linux-samsung tree.
> 
>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi |7 +++
>  arch/arm/boot/dts/exynos5250-snow.dts |   14 ++
>  2 files changed, 21 insertions(+)

The same snippet as for snow also fixed my USB issues on spring.
Should it go into exynos5250-cros-common.dtsi instead?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
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 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Dan Williams
On Thu, May 8, 2014 at 4:09 AM, Rafael J. Wysocki  wrote:
> On Wednesday, May 07, 2014 10:37:24 PM Dan Williams wrote:
>> Unconditionally wake up the child device when the power session is
>> recovered.
>
> [cut]
>
>> + /*
>> +  * Revalidate the device if it was requested by
>> +  * usb_port_runtime_resume.
>> +  */
>> + if (test_and_clear_bit(i, hub->wakeup_bits) && 
>> udev) {
>> + pm_runtime_get_noresume(&udev->dev);
>> + pm_runtime_barrier(&udev->dev);
>> + pm_runtime_put_autosuspend(&udev->dev);
>
> It looks like you could just call pm_runtime_idle(&udev->dev) after the 
> barrier
> without the _get and _put around it.  Wouldn't that work?
>

Sure, but the surprise was that an auto suspend enabled device stops
auto suspending after a barrier.
--
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: ep0: fix delayed status is queued too early

2014-05-08 Thread Felipe Balbi
Hi,

(using private email as I'm having issue with company's VPN, will get
that sorted out by tomorrow hopefully)

> On Thu, 8 May 2014, Zhuang Jin Can wrote:
> > > dwc3 _cannot_ return NYET to a SETUP packet.  The USB protocol does
> > > not 
> > > allow it.  A device must always respond to SETUP with ACK.
> > It true that device can not return NYET to a SETUP packet.
> > A device must always respond to SETUP with ACK _if_ the SETUP packet
> > is
> > correctly received. Because there's no buffer prepared in ep0 for dwc3
> > to receive the SETUP packet, I guess there will be no handshake
> > returned to host. I can confirm this by doing an experiment tomorrow:)
> 
> The dwc3 driver should always prepare a buffer for the next ep0 SETUP
> packet as soon as it retrieves the information for the current SETUP 
> packet from the buffer.
> 
> Otherwise, as you described it, if the gadget driver never sends its 
> delayed status response then the UDC will never respond to any more 
> control transfers.

we _do_ prepare transfers for setup packet everytime a Status phase is
completed (we also restart ep0 in case of stalls):

| static void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
| {
|   struct dwc3_ep  *dep;
| 
|   /* reinitialize physical ep1 */
|   dep = dwc->eps[1];
|   dep->flags = DWC3_EP_ENABLED;
| 
|   /* stall is always issued on EP0 */
|   dep = dwc->eps[0];
|   __dwc3_gadget_ep_set_halt(dep, 1);
|   dep->flags = DWC3_EP_ENABLED;
|   dwc->delayed_status = false;
| 
|   if (!list_empty(&dep->request_list)) {
|   struct dwc3_request *req;
| 
|   req = next_request(&dep->request_list);
|   dwc3_gadget_giveback(dep, req, -ECONNRESET);
|   }
| 
|   dwc->ep0state = EP0_SETUP_PHASE;
|   dwc3_ep0_out_start(dwc);
| }

[ ... ]

| void dwc3_ep0_out_start(struct dwc3 *dwc)
| {
|   int ret;
| 
|   ret = dwc3_ep0_start_trans(dwc, 0, dwc->ctrl_req_addr, 8,
|   DWC3_TRBCTL_CONTROL_SETUP);
|   WARN_ON(ret < 0);
| }

[ ... ]

| static void dwc3_ep0_complete_status(struct dwc3 *dwc,
|   const struct dwc3_event_depevt *event)
| {
|   struct dwc3_request *r;
|   struct dwc3_ep  *dep;
|   struct dwc3_trb *trb;
|   u32 status;
| 
|   dep = dwc->eps[0];
|   trb = dwc->ep0_trb;
| 
|   if (!list_empty(&dep->request_list)) {
|   r = next_request(&dep->request_list);
| 
|   dwc3_gadget_giveback(dep, r, 0);
|   }
| 
|   if (dwc->test_mode) {
|   int ret;
| 
|   ret = dwc3_gadget_set_test_mode(dwc, dwc->test_mode_nr);
|   if (ret < 0) {
|   dev_dbg(dwc->dev, "Invalid Test #%d\n",
|   dwc->test_mode_nr);
|   dwc3_ep0_stall_and_restart(dwc);
|   return;
|   }
|   }
| 
|   status = DWC3_TRB_SIZE_TRBSTS(trb->size);
|   if (status == DWC3_TRBSTS_SETUP_PENDING)
|   dev_dbg(dwc->dev, "Setup Pending received\n");
| 
|   dwc->ep0state = EP0_SETUP_PHASE;
|   dwc3_ep0_out_start(dwc);
| }

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 7/8] usb: ohci: sort out dependencies for lpc32xx and omap

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Arnd Bergmann wrote:

> The dependency on the isp1301 driver is not something that
> should be in the main OHCI driver but rather the SoC specific
> part of it.
> 
> This moves the dependency for LPC32xx into USB_OHCI_HCD_LPC32XX,
> and changes the 'select ISP1301_OMAP' to a similar 'depends on'.
> Since the same dependency exists for the client driver, do the
> same change there.
> 
> Signed-off-by: Arnd Bergmann 
> Cc: linux-o...@vger.kernel.org
> Cc: Alan Stern 

For the host side changes:

Acked-by: Alan Stern 

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


Re: [PATCH 8/8] usb: ohci-da8xx can only be built-in

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Arnd Bergmann wrote:

> The PHY setup code of the TI DaVinci DA8xx OHCI controller
> uses ad-hoc register access using a pointer that is meant to
> be used only by the DaVinci platform implementation and that
> is intentionally not exported to loadable modules. This results
> in a link error on configurations that use a modular OHCI
> code on this platform.
> 
> While the proper solution for this problem would be to
> implement a real PHY driver shared by ohci-da8xx and musb-da8xx,
> this patch for now just works around the build error by
> only allowing the ohci-da8xx code to be built-in.
> 
> Signed-off-by: Arnd Bergmann 
> Cc: Alan Stern 
> ---
>  drivers/usb/host/Kconfig| 10 ++
>  drivers/usb/host/ohci-hcd.c |  2 +-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index e229a47..3af10b7 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -417,6 +417,16 @@ config USB_OHCI_HCD_OMAP3
> Enables support for the on-chip OHCI controller on
> OMAP3 and later chips.
>  
> +config USB_OHCI_HCD_DAVINCI
> + bool "OHCI support for TI DaVinci DA8xx"
> + depends on ARCH_DAVINCI_DA8XX
> + depends on USB_OHCI_HCD=y
> + default y
> + help
> +   Enables support for the DaVinci DA8xx integrated OHCI
> +   controller. This driver cannot currently be a loadable
> +   module because it lacks a proper PHY abstraction.
> +
>  config USB_OHCI_ATH79
>   bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)"
>   depends on (SOC_AR71XX || SOC_AR724X)
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 3586460..f98d03f 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -1178,7 +1178,7 @@ MODULE_LICENSE ("GPL");
>  #define SA_DRIVERohci_hcd_sa_driver
>  #endif
>  
> -#ifdef CONFIG_ARCH_DAVINCI_DA8XX
> +#ifdef CONFIG_USB_OHCI_HCD_DAVINCI
>  #include "ohci-da8xx.c"
>  #define DAVINCI_PLATFORM_DRIVER  ohci_hcd_da8xx_driver
>  #endif

Acked-by: Alan Stern 

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


Re: [PATCH v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Alan Stern
On Wed, 7 May 2014, Dan Williams wrote:

> Unconditionally wake up the child device when the power session is
> recovered.

...

> 1, 2, and 4 are not a problem in the system dpm_resume() case because,
> although the power-session is lost, khubd is frozen until after device
> resume.  For the rpm_resume() case pm_request_resume() and
> runtime-pm-synchronization is used to guarantee the same sequence of
> events.  When pm_request_resume() is invoked on the child usb_device
> port device is in the RPM_RESUMING state.  khubd in turn performs a
> pm_runtime_barrier() on the port device to flush the port recovery, and
> holds the port active while it resumes the child with another
> pm_runtime_barrier().  This guarantees that the portstatus khubd
> evaluates via port_event() is always on an active port and a usb_device
> that has recovered from power loss.

I don't understand this last part.  Why do we need to guarantee the 
child device has recovered from power loss?  Why not proceed the same 
way we do now when the child is suspended?

If you take that stuff out, it seems that there won't be any need to
use wakeup_bits or usb_kick_khubd() for this purpose.

> Given that this patch de-references port_dev->child we need to make sure
> not to collide with usb_disconnect().

I don't like the way this was done.  We shouldn't disable runtime PM 
unless there's no choice.  In this case, the disable would interfere 
with the immediately preceding pm_runtime_put().

You could change the test_and_clear_bit()/pm_runtime_put() calls to 
!test_and_set_bit()/pm_runtime_get_sync.  Then replace the 
runtime-enable with clear_bit/pm_runtime_put.

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: gadget: fsl: check vbus presence on probe

2014-05-08 Thread Paul Fertser
Hi,

On Thu, May 08, 2014 at 02:30:39PM +, suresh.gu...@freescale.com wrote:
> As per my limited knowledge, the purpose of
> OTGSC_STS_B_SESSION_VALID bit is to tell either VBUS is above the B
> session valid threshold and which comes only Host is attached.

Yes, that matches the datasheet I've read.

> And Host might be attach after system bootup or after driver
> initialization. So putting this check in probe will not help much.

If the host is attached after the driver was initialised, the
interrupt will trigger and the driver will get notified that VBUS
appeared and everything will go smooth, at least that's how it should
work (I do not have any board handy to real-life check that, but
AFAICT that's the intent of the current code).

I actually do not have any iMX demoboards at all, I've only got some
custom-designed i.MX25 boards where I can't control VBUS, it's
permanently pulled up.

But I was fixing the problem that was clearly, 100% reproducibly
happening when VBUS was applied before the interrupt was
configured. So what exactly do you mean here? Do you mean this check
I've added doesn't fix the bug? Or do you mean this bug should be
fixed somehow else? Or do you mean there's no bug in the first place
and my board doesn't work because of something else?

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercer...@gmail.com
--
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: Clarification regarding USB Data Card (3G Dongle) enumeration in Linux udev

2014-05-08 Thread Dan Williams
On Wed, 2014-05-07 at 15:30 +0530, Suresh Kumar N. wrote:
> On Tue, May 6, 2014 at 9:42 PM, Dan Williams  wrote:
> > On Tue, 2014-05-06 at 14:27 +0530, Suresh Kumar N. wrote:
> >> On Mon, May 5, 2014 at 8:38 PM, Dan Williams  wrote:
> >> > On Mon, 2014-05-05 at 11:07 +0530, Suresh Kumar N. wrote:
> >> >> Hi,
> >> >>
> >> >> I am new to udev and device enumeration.
> >> >>
> >> >> I am interested to know how USB Data Card would get enumerated.
> >> >>
> >> >> Based on my understanding Data Card can get enumerated below 2 possible 
> >> >> ways -
> >> >>   1. As a modem
> >> >>   2. As a Network Interface Card (NIC)
> >> >
> >> > Or both at the same time.
> >> >
> >> >> Is there a Standard defining the way a Data Card should be enumerated?
> >> >
> >> > There is no single standard.  There is a few "standards" and multiple
> >> > proprietary mechanisms, and sometimes these are combined in the same
> >> > device.
> >> >
> >> > The device simply enumerates as a normal USB device, providing to the
> >> > host computer one or more USB interfaces.
> >> >
> >> > Each USB interface can be any one of:
> >> >
> >> > 1) serial interface (AT, QCDM, WMC, WDM, CDC-ACM, etc)
> >> > 2) pseudo-ethernet NIC (proprietary, CDC-ETHER, CDC-NCM, etc)
> >>
> >> In such case (pseudo-ethernet NIC) do we assume that the firmware on
> >> the USB Dongle is responsible to establish IP address?
> >> In other words how does IP address allocation occur in this case?
> >
> 
> Thank you very much for such a clear and exhaustive explanation.
> 
> > The firmware is always involved, because the IP address comes from the
> > cellular network, and the firmware is what receives it.  The firmware
> > must then provide that address to the host, so that the host can
> > communicate with the network.  This happens in a few ways:
> >
> > 1) firmware implements a DHCP server, IPv6 Router, etc; host uses DHCP
> > client or IPv6 Router Discovery to obtain IP address and DNS details
> >
> > 2) firmware provides static IP address and DNS details over AT commands
> >
> > 3) firmware provides static IP address and DNS details via proprietary
> > protocols (QMI, MBIM, etc)
> >
> > 4) firmware implements PPP over serial port and provides IP address and
> > DNS details via IPCP/IPV6CP
> 
> Based on 1) and 2), Firmware refers to be driver code implemented on
> USB device-side, but 3) and 4) refer to Firmware implemented on
> Host-side.
> Does Firmware here refer to Host-side or USB device-side?

"Firmware" here always refers to USB device side.  So all 4 of these
refer to things that the USB device can do, based on what the USB device
has implemented in its firmware.  It's up to the host to determine which
protocols the USB device firmware implements, and then to speak to the
USB device with those protocols to determine the IP address the host
must use to communicate with the USB device firmware.

> >
> > Most modems still support #4 over at least one AT-capable serial port.
> > Many modems implement multiple methods (eg, Qualcomm firmware often does
> > #1, #3, and #4).
> >
> > Note that in all cases, connection setup (with the APN and other
> > details) must occur via control channels (with AT, QMI, MBIM, etc)
> > before you can use any of these methods to obtain an IP address from the
> > firmware.
> >
> > There is actually another class of "plug and play" 3G dongle that
> > provides a NAT-ed IP network (192.168.0.x or 10.x) to the host over what
> > looks like a USB ethernet NIC.  These appear exactly like a normal USB
> > ethernet dongle or a home router to the host, and to configure the
> > device, you use a web browser on the host like you would with a home
> > router.  None of the 3G stuff is exposed to the host.  One example is
> > Huawei HiLink devices like the E3256, though not all HiLink-branded
> > devices operate this way.
> 
> In all of the above, how do we visualize the Connect/Disconnect option
> provided by the Host side application?

The steps you must take are *always* dependent on the specific modem
that you are communicating with.  Depending on which protocols the modem
implements your host application must do many different things.

> 1. Is it defined to just Enable/Disable a network interface (USB
> ethernet NIC) on Host side, but in that case would the Active context
> get Deactivated?

Your host application must perform setup and context activation using
the control channel protocols (AT commands, proprietary protocols, etc)
before it can start real IP networking.

Perhaps a concrete example will help.  This is the procedure for
connecting an Ericsson "MBM" (F3507, F3607, F5521, etc) modem.  This
modem provides 2 CDC-ACM ports which are used for AT command
communication, and a CDC-ETHER port which emulates an ethernet
interface.  Both of these ports use standard protocols and are handled
automatically by the Linux kernel drivers.

1) first, check and ensure that the modem is registered on the network
with AT+CGREG.  Once the modem has reg

Re: [PATCH] usb: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Zhuang Jin Can wrote:

> > dwc3 _cannot_ return NYET to a SETUP packet.  The USB protocol does not 
> > allow it.  A device must always respond to SETUP with ACK.
> It true that device can not return NYET to a SETUP packet.
> A device must always respond to SETUP with ACK _if_ the SETUP packet is
> correctly received. Because there's no buffer prepared in ep0 for dwc3
> to receive the SETUP packet, I guess there will be no handshake
> returned to host. I can confirm this by doing an experiment tomorrow:)

The dwc3 driver should always prepare a buffer for the next ep0 SETUP
packet as soon as it retrieves the information for the current SETUP 
packet from the buffer.

Otherwise, as you described it, if the gadget driver never sends its 
delayed status response then the UDC will never respond to any more 
control transfers.

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: dwc3: ep0: fix delayed status is queued too early

2014-05-08 Thread Zhuang Jin Can
On Thu, May 08, 2014 at 10:25:46AM -0400, Alan Stern wrote:
> On Thu, 8 May 2014, Zhuang Jin Can wrote:
> 
> > > When the host already timed out the control transfer and started a new 
> > > one.  Here's what I'm talking about:
> > > 
> > >   Host sends a Set-Configuration request.
> > > 
> > >   The UDC driver calls the gadget driver's setup function.
> > > 
> > >   The setup function returns DELAYED_STATUS.
> > > 
> > >   After a few seconds, the host gets tired of waiting and
> > >   sends a Get-Descriptor request
> > My understanding is dwc3 will return NYET to host for this
> > Get-Descriptor request transaction, as dwc3 is still in STATUS phase,
> > there's no buffer to receive anything in ep0-out.
> 
> dwc3 _cannot_ return NYET to a SETUP packet.  The USB protocol does not 
> allow it.  A device must always respond to SETUP with ACK.
It true that device can not return NYET to a SETUP packet.
A device must always respond to SETUP with ACK _if_ the SETUP packet is
correctly received. Because there's no buffer prepared in ep0 for dwc3
to receive the SETUP packet, I guess there will be no handshake
returned to host. I can confirm this by doing an experiment tomorrow:)

Jincan
--
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 5/8] usb: phy: msm: reset controller is mandatory now

2014-05-08 Thread Ivan T. Ivanov
On Thu, 2014-05-08 at 16:27 +0200, Arnd Bergmann wrote:
> On Thursday 08 May 2014 17:21:49 Ivan T. Ivanov wrote:
> > > @@ -168,7 +168,7 @@ config USB_EHCI_HCD_AT91
> > >  
> > >  config USB_EHCI_MSM
> > >   tristate "Support for Qualcomm QSD/MSM on-chip EHCI USB controller"
> > > - depends on ARCH_MSM
> > > + depends on ARCH_MSM && RESET_CONTROLLER
> > 
> > This driver did not use reset controller API.
> > 
> > >   select USB_EHCI_ROOT_HUB_TT
> > >   select USB_MSM_OTG
> 
> It does select USB_MSM_OTG though, which uses it:
> 
> Maybe that 'select' is wrong?

Driver have run time dependency to USB_PHY framework.
Probably just removing  select line will be fine?

Regards,
Ivan



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


RE: [PATCH] usb: gadget: fsl: check vbus presence on probe

2014-05-08 Thread suresh.gu...@freescale.com
Hi,

As per my limited knowledge, the purpose of OTGSC_STS_B_SESSION_VALID bit is to 
tell either VBUS is above the B session valid threshold and which comes only 
Host is attached.
And Host might be attach after system bootup or after driver initialization. So 
putting this check in probe will not help much.

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of suresh.gu...@freescale.com
> Sent: Thursday, May 01, 2014 7:58 PM
> To: ba...@ti.com; Paul Fertser
> Cc: Li Yang-Leo-R58472; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: RE: [PATCH] usb: gadget: fsl: check vbus presence on probe
> 
> Give me some time (actually some days), I will try this and update you.
> 
> > -Original Message-
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Thursday, May 01, 2014 1:42 AM
> > To: Paul Fertser
> > Cc: Felipe Balbi; Li Yang-Leo-R58472; linux-usb@vger.kernel.org;
> > linux- ker...@vger.kernel.org; Gupta Suresh-B42813
> > Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe
> >
> > +Suresh
> >
> > (top-posting, yeah yeah :-)
> >
> > On Wed, Apr 30, 2014 at 11:27:45PM +0400, Paul Fertser wrote:
> > > Hello,
> > >
> > > Thank you for the review.
> > >
> > > On Wed, Apr 30, 2014 at 11:06:25AM -0500, Felipe Balbi wrote:
> > > > On Thu, Apr 24, 2014 at 12:54:13PM +0400, Paul Fertser wrote:
> > > > > If VBUS is already present during the driver initialisation, the
> > > >
> > > > s/initialisation/initialization
> > >
> > > If I'm understanding [1] properly, both spelling variants are
> correct.
> > >
> > > > > + /* Now let it settle a bit and sense VBUS */
> > > > > + msleep_interruptible(1);
> > > > > + if (fsl_readl(&dr_regs->otgsc) & OTGSC_STS_B_SESSION_VALID)
> > > > > + udc_controller->vbus_active = 1;
> > > >
> > > > good fix, should this go to stable ?
> > >
> > > I was reluctant to Cc stable because I'm not sure which versions
> > > need to be fixed, as I've seen an earlier version (3.0 something or
> > > a bit before that probably) working without this (no idea why); also
> > > I'm not sure about that msleep as it's deduced solely from
> > > experimenting with a particular i.MX model, I hoped someone from
> > > Freescale would comment on it. I've seen your review [2] on a
> > > related patch and my fix should have been obvious to Suresh (and
> > > trivial to test on different boards), yet he didn't implement it,
> that worries me a bit.
> > >
> > > [1]
> > > https://en.wikipedia.org/wiki/American_and_British_English_spelling_
> > > di
> > > fferences#-ise.2C_-ize_.28-isation.2C_-ization.29
> > > [2] https://patchwork.kernel.org/patch/3822321/
> > > --
> > > Be free, use free (http://www.gnu.org/philosophy/free-sw.html)
> > software!
> > > mailto:fercer...@gmail.com
> >
> > --
> > balbi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
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 5/8] usb: phy: msm: reset controller is mandatory now

2014-05-08 Thread Arnd Bergmann
On Thursday 08 May 2014 17:21:49 Ivan T. Ivanov wrote:
> > @@ -168,7 +168,7 @@ config USB_EHCI_HCD_AT91
> >  
> >  config USB_EHCI_MSM
> >   tristate "Support for Qualcomm QSD/MSM on-chip EHCI USB controller"
> > - depends on ARCH_MSM
> > + depends on ARCH_MSM && RESET_CONTROLLER
> 
> This driver did not use reset controller API.
> 
> >   select USB_EHCI_ROOT_HUB_TT
> >   select USB_MSM_OTG

It does select USB_MSM_OTG though, which uses it:

Maybe that 'select' is wrong?

Arnd
--
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: ep0: fix delayed status is queued too early

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Zhuang Jin Can wrote:

> > When the host already timed out the control transfer and started a new 
> > one.  Here's what I'm talking about:
> > 
> > Host sends a Set-Configuration request.
> > 
> > The UDC driver calls the gadget driver's setup function.
> > 
> > The setup function returns DELAYED_STATUS.
> > 
> > After a few seconds, the host gets tired of waiting and
> > sends a Get-Descriptor request
> My understanding is dwc3 will return NYET to host for this
> Get-Descriptor request transaction, as dwc3 is still in STATUS phase,
> there's no buffer to receive anything in ep0-out.

dwc3 _cannot_ return NYET to a SETUP packet.  The USB protocol does not 
allow it.  A device must always respond to SETUP with ACK.

On the other hand, dwc3 _can_ return NYET to the token packet that
follows the SETUP transaction.  That's what it should do.  But at this
point it should be in the DATA stage, not the STATUS stage.  Receiving
a SETUP packet should abort a STATUS stage.

>  And your below
> comments is not applicapable to dwc3.

True, they apply to composite.c rather than dwc3.  However, they
address an issue very similar to your patch, so I raised this topic in
your email thread.

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 5/8] usb: phy: msm: reset controller is mandatory now

2014-05-08 Thread Ivan T. Ivanov

Hi Arnd, 

On Thu, 2014-05-08 at 15:52 +0200, Arnd Bergmann wrote:
> Commit a27345434134 "usb: phy: msm: Use reset framework for LINK
> and PHY resets" introduced a mandatory call to reset_control_get
> into the msm usb phy driver, which means we have to add a Kconfig
> dependency on the API to avoid this build error:
> 
> phy/phy-msm-usb.c: In function 'msm_otg_read_dt':
> phy/phy-msm-usb.c:1461:2: error: implicit declaration of function 
> 'devm_reset_control_get' [-Werror=implicit-function-declaration]
>   motg->link_rst = devm_reset_control_get(&pdev->dev, "link");
>   ^
> 
> Signed-off-by: Arnd Bergmann 
> Cc: "Ivan T. Ivanov" 
> ---
>  drivers/usb/host/Kconfig | 2 +-
>  drivers/usb/phy/Kconfig  | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 3d9e540..890fc8c 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -168,7 +168,7 @@ config USB_EHCI_HCD_AT91
>  
>  config USB_EHCI_MSM
>   tristate "Support for Qualcomm QSD/MSM on-chip EHCI USB controller"
> - depends on ARCH_MSM
> + depends on ARCH_MSM && RESET_CONTROLLER

This driver did not use reset controller API.

>   select USB_EHCI_ROOT_HUB_TT
>   select USB_MSM_OTG
>   ---help---
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 359a6c1..65bec8f 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -165,6 +165,7 @@ config USB_ISP1301
>  config USB_MSM_OTG
>   tristate "Qualcomm on-chip USB OTG controller support"
>   depends on (USB || USB_GADGET) && (ARCH_MSM || ARCH_QCOM)
> + depends on RESET_CONTROLLER

This is fine. 

Thank you.
Ivan

--
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 4/7] usb: ehci-platform: add optional reset controller retrieval

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Hans de Goede wrote:

> Hi,
> 
> On 05/08/2014 12:00 AM, Maxime Ripard wrote:
> > On Wed, May 07, 2014 at 10:25:55AM -0400, Alan Stern wrote:
> >> On Tue, 6 May 2014, Maxime Ripard wrote:
> >>
> >>> From: Boris BREZILLON 
> >>>
> >>> On the Allwinner's A31 SoC the reset line connected to the EHCI IP has to
> >>> be deasserted for the EHCI block to be usable.
> >>>
> >>> Add support for an optional reset controller that will be deasserted on
> >>> power off and asserted on power on.
> >>>
> >>> Signed-off-by: Boris BREZILLON 
> >>> Signed-off-by: Maxime Ripard 
> >>
> >> Is this really a _reset_ line?  That is, when you assert the reset 
> >> line, does it actually reset the EHCI controller, or does it merely 
> >> leave the controller in a partially powered-down state?
> > 
> > It actually resets the whole controller.
> > 
> >> The difference is important.  During suspend, the controller is
> >> supposed to remember the state of the port connections as well as other
> >> settings.  If it doesn't, the controller and all attached USB devices
> >> will have to be reinitialized every time the controller resumes, which
> >> will increase the latency.
> > 
> > So you're saying that we should move this to the probe then?
> 
> Yes.

That's right.  The controller should not be reset during suspend, if
you can possibly avoid it.

There isn't any real benefit to asserting the reset signal during 
suspend, is there?  I mean, it won't use any less power, right?

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 8/8] usb: ohci-da8xx can only be built-in

2014-05-08 Thread Arnd Bergmann
The PHY setup code of the TI DaVinci DA8xx OHCI controller
uses ad-hoc register access using a pointer that is meant to
be used only by the DaVinci platform implementation and that
is intentionally not exported to loadable modules. This results
in a link error on configurations that use a modular OHCI
code on this platform.

While the proper solution for this problem would be to
implement a real PHY driver shared by ohci-da8xx and musb-da8xx,
this patch for now just works around the build error by
only allowing the ohci-da8xx code to be built-in.

Signed-off-by: Arnd Bergmann 
Cc: Alan Stern 
---
 drivers/usb/host/Kconfig| 10 ++
 drivers/usb/host/ohci-hcd.c |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index e229a47..3af10b7 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -417,6 +417,16 @@ config USB_OHCI_HCD_OMAP3
  Enables support for the on-chip OHCI controller on
  OMAP3 and later chips.
 
+config USB_OHCI_HCD_DAVINCI
+   bool "OHCI support for TI DaVinci DA8xx"
+   depends on ARCH_DAVINCI_DA8XX
+   depends on USB_OHCI_HCD=y
+   default y
+   help
+ Enables support for the DaVinci DA8xx integrated OHCI
+ controller. This driver cannot currently be a loadable
+ module because it lacks a proper PHY abstraction.
+
 config USB_OHCI_ATH79
bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)"
depends on (SOC_AR71XX || SOC_AR724X)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 3586460..f98d03f 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1178,7 +1178,7 @@ MODULE_LICENSE ("GPL");
 #define SA_DRIVER  ohci_hcd_sa_driver
 #endif
 
-#ifdef CONFIG_ARCH_DAVINCI_DA8XX
+#ifdef CONFIG_USB_OHCI_HCD_DAVINCI
 #include "ohci-da8xx.c"
 #define DAVINCI_PLATFORM_DRIVERohci_hcd_da8xx_driver
 #endif
-- 
1.8.3.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 0/8] usb: various fixes for ARM randconfig failures

2014-05-08 Thread Arnd Bergmann
These is the usb part of my longer ARM randconfig build bug
series. None of these are actually needed for 3.15 as far as
I can tell, but it would be good to have them included in the
next merge window.

The xhci patch is only for a build warning, not a failure, but
we see this one all the time because it happens in one of the
defconfigs.

Arnd Bergmann (8):
  usb/gadget: s3c2410_udc: don't use pr_debug return value
  usb: musb: tusb-dma can't be built-in if tusb is not
  usb: musb: omap2plus bus glue needs USB host support
  usb: phy: fix isp1301-omap dependency on tps65010
  usb: phy: msm: reset controller is mandatory now
  usb: xhci: avoid warning for !PM_SLEEP
  usb: ohci: sort out dependencies for lpc32xx and omap
  usb: ohci-da8xx can only be built-in

 drivers/usb/gadget/Kconfig |  4 ++--
 drivers/usb/gadget/s3c2410_udc.c   |  3 ++-
 drivers/usb/host/Kconfig   | 16 +---
 drivers/usb/host/ohci-hcd.c|  2 +-
 drivers/usb/host/xhci-plat.c   |  2 +-
 drivers/usb/musb/Kconfig   |  4 ++--
 drivers/usb/phy/Kconfig|  1 +
 drivers/usb/phy/phy-isp1301-omap.c |  2 +-
 8 files changed, 23 insertions(+), 11 deletions(-)

-- 
1.8.3.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 7/8] usb: ohci: sort out dependencies for lpc32xx and omap

2014-05-08 Thread Arnd Bergmann
The dependency on the isp1301 driver is not something that
should be in the main OHCI driver but rather the SoC specific
part of it.

This moves the dependency for LPC32xx into USB_OHCI_HCD_LPC32XX,
and changes the 'select ISP1301_OMAP' to a similar 'depends on'.
Since the same dependency exists for the client driver, do the
same change there.

Signed-off-by: Arnd Bergmann 
Cc: linux-o...@vger.kernel.org
Cc: Alan Stern 
---
 drivers/usb/gadget/Kconfig | 4 ++--
 drivers/usb/host/Kconfig   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 7fca52b..ba18e9c 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -157,7 +157,7 @@ config USB_AT91
 
 config USB_LPC32XX
tristate "LPC32XX USB Peripheral Controller"
-   depends on ARCH_LPC32XX
+   depends on ARCH_LPC32XX && I2C
select USB_ISP1301
help
   This option selects the USB device controller in the LPC32xx SoC.
@@ -226,7 +226,7 @@ config USB_GR_UDC
 config USB_OMAP
tristate "OMAP USB Device Controller"
depends on ARCH_OMAP1
-   select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3
+   depends on ISP1301_OMAP || !(MACH_OMAP_H2 || MACH_OMAP_H3)
help
   Many Texas Instruments OMAP processors have flexible full
   speed USB device controllers, with support for up to 30
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 890fc8c..e229a47 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -345,8 +345,6 @@ config USB_FOTG210_HCD
 
 config USB_OHCI_HCD
tristate "OHCI HCD (USB 1.1) support"
-   select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3
-   depends on USB_ISP1301 || !ARCH_LPC32XX
---help---
  The Open Host Controller Interface (OHCI) is a standard for accessing
  USB 1.1 host controller hardware.  It does more in hardware than 
Intel's
@@ -365,6 +363,7 @@ if USB_OHCI_HCD
 config USB_OHCI_HCD_OMAP1
tristate "OHCI support for OMAP1/2 chips"
depends on ARCH_OMAP1
+   depends on ISP1301_OMAP || !(MACH_OMAP_H2 || MACH_OMAP_H3)
default y
---help---
  Enables support for the OHCI controller on OMAP1/2 chips.
@@ -388,6 +387,7 @@ config USB_OHCI_HCD_S3C2410
 config USB_OHCI_HCD_LPC32XX
tristate "Support for LPC on-chip OHCI USB controller"
depends on USB_OHCI_HCD && ARCH_LPC32XX
+   depends on USB_ISP1301
default y
---help---
   Enables support for the on-chip OHCI controller on
-- 
1.8.3.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 2/8] usb: musb: tusb-dma can't be built-in if tusb is not

2014-05-08 Thread Arnd Bergmann
A configuration with CONFIG_USB_MUSB_HDRC=y, CONFIG_USB_TUSB_OMAP_DMA=y
and CONFIG_USB_MUSB_TUSB6010=m causes a link failure because of the
dependency on the tusb_get_revision symbol:

(.text+0x154ce8): undefined reference to `tusb_get_revision'

This patch ensures that either MUSB_HDRC and MUSB_TUSB6010 are
both modules or both built-in, which are the valid configurations.

Signed-off-by: Arnd Bergmann 
Cc: linux-o...@vger.kernel.org
---
 drivers/usb/musb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 8b78979..618b152 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -144,7 +144,7 @@ config USB_TI_CPPI41_DMA
 
 config USB_TUSB_OMAP_DMA
bool 'TUSB 6010'
-   depends on USB_MUSB_TUSB6010
+   depends on USB_MUSB_TUSB6010 = USB_MUSB_HDRC # both built-in or both 
modules
depends on ARCH_OMAP
help
  Enable DMA transfers on TUSB 6010 when OMAP DMA is available.
-- 
1.8.3.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 3/8] usb: musb: omap2plus bus glue needs USB host support

2014-05-08 Thread Arnd Bergmann
The musb/omap2430.c bus glue driver calls usb_hcd_poll_rh_status,
which is only available if CONFIG_USB is also set, i.e. we
are building USB host mode and not just endpoint mode.

Signed-off-by: Arnd Bergmann 
Cc: linux-o...@vger.kernel.org
---
 drivers/usb/musb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 618b152..fce762c 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -76,7 +76,7 @@ config USB_MUSB_TUSB6010
 
 config USB_MUSB_OMAP2PLUS
tristate "OMAP2430 and onwards"
-   depends on ARCH_OMAP2PLUS
+   depends on ARCH_OMAP2PLUS && USB
select GENERIC_PHY
 
 config USB_MUSB_AM35X
-- 
1.8.3.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 1/8] usb/gadget: s3c2410_udc: don't use pr_debug return value

2014-05-08 Thread Arnd Bergmann
pr_debug() may be defined as "do { } while (0)" in some configurations,
which means one cannot rely on the return value to be available.

In the dprintk function in this driver, we can work around the
resulting build error trivially by returning the length that
this function already knows and ignoring the return value of
pr_debug.

Signed-off-by: Arnd Bergmann 
Cc: Ben Dooks 
Cc: Kukjin Kim 
Cc: linux-samsung-...@vger.kernel.org
---
 drivers/usb/gadget/s3c2410_udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c
index dd9678f..7987aa0 100644
--- a/drivers/usb/gadget/s3c2410_udc.c
+++ b/drivers/usb/gadget/s3c2410_udc.c
@@ -117,7 +117,8 @@ static int dprintk(int level, const char *fmt, ...)
sizeof(printk_buf)-len, fmt, args);
va_end(args);
 
-   return pr_debug("%s", printk_buf);
+   pr_debug("%s", printk_buf);
+   return len;
 }
 #else
 static int dprintk(int level, const char *fmt, ...)
-- 
1.8.3.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 5/8] usb: phy: msm: reset controller is mandatory now

2014-05-08 Thread Arnd Bergmann
Commit a27345434134 "usb: phy: msm: Use reset framework for LINK
and PHY resets" introduced a mandatory call to reset_control_get
into the msm usb phy driver, which means we have to add a Kconfig
dependency on the API to avoid this build error:

phy/phy-msm-usb.c: In function 'msm_otg_read_dt':
phy/phy-msm-usb.c:1461:2: error: implicit declaration of function 
'devm_reset_control_get' [-Werror=implicit-function-declaration]
  motg->link_rst = devm_reset_control_get(&pdev->dev, "link");
  ^

Signed-off-by: Arnd Bergmann 
Cc: "Ivan T. Ivanov" 
---
 drivers/usb/host/Kconfig | 2 +-
 drivers/usb/phy/Kconfig  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 3d9e540..890fc8c 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -168,7 +168,7 @@ config USB_EHCI_HCD_AT91
 
 config USB_EHCI_MSM
tristate "Support for Qualcomm QSD/MSM on-chip EHCI USB controller"
-   depends on ARCH_MSM
+   depends on ARCH_MSM && RESET_CONTROLLER
select USB_EHCI_ROOT_HUB_TT
select USB_MSM_OTG
---help---
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 359a6c1..65bec8f 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -165,6 +165,7 @@ config USB_ISP1301
 config USB_MSM_OTG
tristate "Qualcomm on-chip USB OTG controller support"
depends on (USB || USB_GADGET) && (ARCH_MSM || ARCH_QCOM)
+   depends on RESET_CONTROLLER
select USB_PHY
help
  Enable this to support the USB OTG transceiver on Qualcomm chips. It
-- 
1.8.3.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 4/8] usb: phy: fix isp1301-omap dependency on tps65010

2014-05-08 Thread Arnd Bergmann
The isp1301-omap driver cannot be built-in if the tps65010 driver
is a module, otherwise we get a link error from the reference to
the tps65010_set_vbus_draw function.

There is already a hack in the driver to work around the problem
of tps65010 being not available at all. This patch extends that
hack to ensure that the real tps65010_set_vbus_draw() function
is only called when it's avaiable.

Signed-off-by: Arnd Bergmann 
Cc: linux-o...@vger.kernel.org
---
 drivers/usb/phy/phy-isp1301-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-isp1301-omap.c 
b/drivers/usb/phy/phy-isp1301-omap.c
index 6e146d7..35a0dd2 100644
--- a/drivers/usb/phy/phy-isp1301-omap.c
+++ b/drivers/usb/phy/phy-isp1301-omap.c
@@ -94,7 +94,7 @@ struct isp1301 {
 
 #if defined(CONFIG_MACH_OMAP_H2) || defined(CONFIG_MACH_OMAP_H3)
 
-#ifdefined(CONFIG_TPS65010) || defined(CONFIG_TPS65010_MODULE)
+#ifdefined(CONFIG_TPS65010) || (defined(CONFIG_TPS65010_MODULE) && 
defined(MODULE))
 
 #include 
 
-- 
1.8.3.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 6/8] usb: xhci: avoid warning for !PM_SLEEP

2014-05-08 Thread Arnd Bergmann
If we build a kernel with PM_SUSPEND set and no PM_SLEEP,
we get a build warning in the xhci-plat driver about unused
functions.

To fix this, use "#ifdef CONFIG_PM_SLEEP", like we do in most
other drivers nowadays.

Signed-off-by: Arnd Bergmann 
Cc: Mathias Nyman 
---
 drivers/usb/host/xhci-plat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 151901c..3473296 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -202,7 +202,7 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int xhci_plat_suspend(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
-- 
1.8.3.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


Re: [PATCH 5/7] usb: ohci-platform: Enable optional use of reset controller

2014-05-08 Thread Sergei Shtylyov

Hello.

On 05/08/2014 02:03 AM, Maxime Ripard wrote:


The OHCI controllers used in the Allwinner A31 are asserted in reset using a



s/asserted/powered up/?



No. There's an external reset controller that maintains these devices
into reset. It's not only a power on thing, it can also be put back
into reset later in the life of the system.


   However, the expression that "the OHCI controllers are asserted in reset" 
still seems dubious to me...



Maxime


WBR, 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] usb: gadget: gr_udc: unconditionally use GFP_ATOMIC in gr_queue_ext()

2014-05-08 Thread Andreas Larsson

On 2014-05-07 22:26, Alexey Khoroshilov wrote:

As far as gr_queue() is called with spinlock held,
we have to pass GFP_ATOMIC regardless of gfp argument.

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

Signed-off-by: Alexey Khoroshilov 


Acked-by: Andreas Larsson 


---
  drivers/usb/gadget/gr_udc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index f984ee75324d..19a1b52c4210 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -1679,7 +1679,7 @@ static int gr_queue_ext(struct usb_ep *_ep, struct 
usb_request *_req,
if (ep->is_in)
gr_dbgprint_request("EXTERN", ep, req);

-   ret = gr_queue(ep, req, gfp_flags);
+   ret = gr_queue(ep, req, GFP_ATOMIC);

spin_unlock(&ep->dev->lock);



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


[PATCHv2 4/8] usb: gadget: f_rndis: OS descriptors support

2014-05-08 Thread Andrzej Pietrasiewicz
In order for usb functions to expose OS descriptors they
need to be made aware of OS descriptors. This involves
extending the "options" structure and setting up
appropriate associations.

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Michal Nazarewicz 
---
 drivers/usb/gadget/f_rndis.c | 24 +---
 drivers/usb/gadget/u_rndis.h |  3 +++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
index 004b3a1..a7633d6 100644
--- a/drivers/usb/gadget/f_rndis.c
+++ b/drivers/usb/gadget/f_rndis.c
@@ -682,6 +682,15 @@ rndis_bind(struct usb_configuration *c, struct 
usb_function *f)
 
rndis_opts = container_of(f->fi, struct f_rndis_opts, func_inst);
 
+   if (cdev->use_os_string) {
+   f->os_desc_table = kzalloc(sizeof(*f->os_desc_table),
+  GFP_KERNEL);
+   if (!f->os_desc_table)
+   return PTR_ERR(f->os_desc_table);
+   f->os_desc_n = 1;
+   f->os_desc_table[0].os_desc = &rndis_opts->rndis_os_desc;
+   }
+
/*
 * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
 * configurations are bound in sequence with list_for_each_entry,
@@ -693,14 +702,16 @@ rndis_bind(struct usb_configuration *c, struct 
usb_function *f)
gether_set_gadget(rndis_opts->net, cdev->gadget);
status = gether_register_netdev(rndis_opts->net);
if (status)
-   return status;
+   goto fail;
rndis_opts->bound = true;
}
 
us = usb_gstrings_attach(cdev, rndis_strings,
 ARRAY_SIZE(rndis_string_defs));
-   if (IS_ERR(us))
-   return PTR_ERR(us);
+   if (IS_ERR(us)) {
+   status = PTR_ERR(us);
+   goto fail;
+   }
rndis_control_intf.iInterface = us[0].id;
rndis_data_intf.iInterface = us[1].id;
rndis_iad_descriptor.iFunction = us[2].id;
@@ -802,6 +813,8 @@ rndis_bind(struct usb_configuration *c, struct usb_function 
*f)
return 0;
 
 fail:
+   kfree(f->os_desc_table);
+   f->os_desc_n = 0;
usb_free_all_descriptors(f);
 
if (rndis->notify_req) {
@@ -892,6 +905,8 @@ static struct usb_function_instance *rndis_alloc_inst(void)
opts = kzalloc(sizeof(*opts), GFP_KERNEL);
if (!opts)
return ERR_PTR(-ENOMEM);
+   opts->rndis_os_desc.ext_compat_id = opts->rndis_ext_compat_id;
+
mutex_init(&opts->lock);
opts->func_inst.free_func_inst = rndis_free_inst;
opts->net = gether_setup_default();
@@ -900,6 +915,7 @@ static struct usb_function_instance *rndis_alloc_inst(void)
kfree(opts);
return ERR_CAST(net);
}
+   INIT_LIST_HEAD(&opts->rndis_os_desc.ext_prop);
 
config_group_init_type_name(&opts->func_inst.group, "",
&rndis_func_type);
@@ -925,6 +941,8 @@ static void rndis_unbind(struct usb_configuration *c, 
struct usb_function *f)
 {
struct f_rndis  *rndis = func_to_rndis(f);
 
+   kfree(f->os_desc_table);
+   f->os_desc_n = 0;
usb_free_all_descriptors(f);
 
kfree(rndis->notify_req->buf);
diff --git a/drivers/usb/gadget/u_rndis.h b/drivers/usb/gadget/u_rndis.h
index 7291b15..e902aa4 100644
--- a/drivers/usb/gadget/u_rndis.h
+++ b/drivers/usb/gadget/u_rndis.h
@@ -26,6 +26,9 @@ struct f_rndis_opts {
boolbound;
boolborrowed_net;
 
+   struct usb_os_desc  rndis_os_desc;
+   charrndis_ext_compat_id[16];
+
/*
 * Read/write access to configfs attributes is handled by configfs.
 *
-- 
1.8.3.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


[PATCHv2 2/8] usb: gadget: OS String support

2014-05-08 Thread Andrzej Pietrasiewicz
There is a custom (non-USB IF) extension to the USB standard:

http://msdn.microsoft.com/library/windows/hardware/gg463182

They grant permission to use the specification - there is
"Microsoft OS Descriptor Specification License Agreement"
under the link mentioned above, and its Section 2 "Grant
of License", letter (b) reads:

"Patent license. Microsoft hereby grants to You a nonexclusive,
royalty-free, nontransferable, worldwide license under Microsoft’s
patents embodied solely within the Specification and that are owned
or licensable by Microsoft to make, use, import, offer to sell,
sell and distribute directly or indirectly to Your Licensees Your
Implementation. You may sublicense this patent license to Your
Licensees under the same terms and conditions."

The said extension is maintained by Microsoft for Microsoft.

Yet it is fairly common for various devices to use it, and a
popular proprietary operating system expects devices to provide
"OS descriptors", so Linux-based USB gadgets whishing to be able
to talk to a variety of operating systems should be able to provide
the "OS descriptors".

This patch adds optional support for gadgets whishing to expose
the so called "OS String" under index 0xEE of language 0.
The contents of the string is generated based on the qw_sign
array and b_vendor_code.

Interested gadgets need to set the cdev->use_os_string flag,
fill cdev->qw_sign with appropriate values and fill cdev->b_vendor_code
with a value of their choice.

This patch does not however implement responding to any vendor-specific
USB requests.

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Michal Nazarewicz 
---
 drivers/usb/gadget/composite.c | 29 +
 include/linux/usb/composite.h  | 11 +++
 2 files changed, 40 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index fab9064..f27aec6 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -21,6 +21,22 @@
 #include 
 #include 
 
+/**
+ * struct usb_os_string - represents OS String to be reported by a gadget
+ * @bLength: total length of the entire descritor, always 0x12
+ * @bDescriptorType: USB_DT_STRING
+ * @qwSignature: the OS String proper
+ * @bMS_VendorCode: code used by the host for subsequent requests
+ * @bPad: not used, must be zero
+ */
+struct usb_os_string {
+   __u8bLength;
+   __u8bDescriptorType;
+   __u8qwSignature[OS_STRING_QW_SIGN_LEN];
+   __u8bMS_VendorCode;
+   __u8bPad;
+} __packed;
+
 /*
  * The code in this file is utility code, used to build a gadget driver
  * from one or more "function" drivers, one or more "configuration"
@@ -960,6 +976,19 @@ static int get_string(struct usb_composite_dev *cdev,
return s->bLength;
}
 
+   if (cdev->use_os_string && language == 0 && id == OS_STRING_IDX) {
+   struct usb_os_string *b = buf;
+   b->bLength = sizeof(*b);
+   b->bDescriptorType = USB_DT_STRING;
+   compiletime_assert(
+   sizeof(b->qwSignature) == sizeof(cdev->qw_sign),
+   "qwSignature size must be equal to qw_sign");
+   memcpy(&b->qwSignature, cdev->qw_sign, sizeof(b->qwSignature));
+   b->bMS_VendorCode = cdev->b_vendor_code;
+   b->bPad = 0;
+   return sizeof(*b);
+   }
+
list_for_each_entry(uc, &cdev->gstrings, list) {
struct usb_gadget_strings **sp;
 
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index d3ca3b5..7d29ee9 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -335,11 +335,17 @@ static inline struct usb_composite_driver *to_cdriver(
return container_of(gdrv, struct usb_composite_driver, gadget_driver);
 }
 
+#define OS_STRING_QW_SIGN_LEN  14
+#define OS_STRING_IDX  0xEE
+
 /**
  * struct usb_composite_device - represents one composite usb gadget
  * @gadget: read-only, abstracts the gadget's usb peripheral controller
  * @req: used for control responses; buffer is pre-allocated
  * @config: the currently active configuration
+ * @qw_sign: qwSignature part of the OS string
+ * @b_vendor_code: bMS_VendorCode part of the OS string
+ * @use_os_string: false by default, interested gadgets set it
  *
  * One of these devices is allocated and initialized before the
  * associated device driver's bind() is called.
@@ -372,6 +378,11 @@ struct usb_composite_dev {
 
struct usb_configuration*config;
 
+   /* OS String is a custom (yet popular) extension to the USB standard. */
+   u8  qw_sign[OS_STRING_QW_SIGN_LEN];
+   u8  b_vendor_code;
+   unsigned intuse_os_string:1;
+
/* private: */
/* internals */
unsigned intsuspended:1;
-- 
1.8.3.2

--
To unsubscri

[PATCHv2 8/8] usb: gadget: configfs: OS Extended Properties descriptors support

2014-05-08 Thread Andrzej Pietrasiewicz
Add handling of OS Extended Properties descriptors from configfs interface.
One kind of "OS Descriptors" are "Extended Properties" descriptors, which
need to be specified per interface or per group of interfaces described
by an IAD. This patch adds support for creating subdirectories
in interface. directory located in the function's directory.
Names of subdirectories created become names of properties.
Each property contains two attributes: "type" and "data".
The type can be a numeric value 1..7 while data is a blob interpreted
depending on the type specified.
The types are:
1 - unicode string
2 - unicode string with environment variables
3 - binary
4 - little-endian 32-bit
5 - big-endian 32-bit
6 - unicode string with a symbolic link
7 - multiple unicode strings

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/ABI/testing/configfs-usb-gadget |  21 +++
 drivers/usb/gadget/configfs.c | 201 ++
 include/linux/usb/composite.h |   4 +
 3 files changed, 226 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget 
b/Documentation/ABI/testing/configfs-usb-gadget
index 5c0b3e6..95a3658 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget
+++ b/Documentation/ABI/testing/configfs-usb-gadget
@@ -75,6 +75,27 @@ Description:
compatible_id   - 8-byte string for "Compatible ID"
sub_compatible_id   - 8-byte string for "Sub Compatible ID"
 
+What:  
/config/usb-gadget/gadget/functions/./interface./
+Date:  May 2014
+KernelVersion: 3.16
+Description:
+   This group contains "Extended Property Descriptors" specific 
for one
+   gadget's USB interface or one interface group described
+   by an IAD.
+
+   The attributes:
+
+   type- value 1..7 for interpreting the data
+   1: unicode string
+   2: unicode string with environment variable
+   3: binary
+   4: little-endian 32-bit
+   5: big-endian 32-bit
+   6: unicode string with a symbolic link
+   7: multiple unicode strings
+   data- blob of data to be interpreted depending on
+   type
+
 What:  /config/usb-gadget/gadget/strings
 Date:  Jun 2013
 KernelVersion: 3.11
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 1fd1e14..87ba35c 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -7,6 +7,7 @@
 #include 
 #include "configfs.h"
 #include "u_f.h"
+#include "u_os_desc.h"
 
 int check_user_usb_string(const char *name,
struct usb_gadget_strings *stringtab_dev)
@@ -941,6 +942,204 @@ static struct config_item_type os_desc_type = {
 CONFIGFS_ATTR_STRUCT(usb_os_desc);
 CONFIGFS_ATTR_OPS(usb_os_desc);
 
+
+static inline struct usb_os_desc_ext_prop
+*to_usb_os_desc_ext_prop(struct config_item *item)
+{
+   return container_of(item, struct usb_os_desc_ext_prop, item);
+}
+
+CONFIGFS_ATTR_STRUCT(usb_os_desc_ext_prop);
+CONFIGFS_ATTR_OPS(usb_os_desc_ext_prop);
+
+static ssize_t ext_prop_type_show(struct usb_os_desc_ext_prop *ext_prop,
+ char *page)
+{
+   return sprintf(page, "%d", ext_prop->type);
+}
+
+static ssize_t ext_prop_type_store(struct usb_os_desc_ext_prop *ext_prop,
+  const char *page, size_t len)
+{
+   struct usb_os_desc *desc = to_usb_os_desc(ext_prop->item.ci_parent);
+   u8 type;
+   int ret;
+
+   if (desc->opts_mutex)
+   mutex_lock(desc->opts_mutex);
+   ret = kstrtou8(page, 0, &type);
+   if (ret)
+   goto end;
+   if (type < USB_EXT_PROP_UNICODE || type > USB_EXT_PROP_UNICODE_MULTI) {
+   ret = -EINVAL;
+   goto end;
+   }
+
+   if ((ext_prop->type == USB_EXT_PROP_BINARY ||
+   ext_prop->type == USB_EXT_PROP_LE32 ||
+   ext_prop->type == USB_EXT_PROP_BE32) &&
+   (type == USB_EXT_PROP_UNICODE ||
+   type == USB_EXT_PROP_UNICODE_ENV ||
+   type == USB_EXT_PROP_UNICODE_LINK))
+   ext_prop->data_len <<= 1;
+   else if ((ext_prop->type == USB_EXT_PROP_UNICODE ||
+  ext_prop->type == USB_EXT_PROP_UNICODE_ENV ||
+  ext_prop->type == USB_EXT_PROP_UNICODE_LINK) &&
+  (type == USB_EXT_PROP_BINARY ||
+  type == USB_EXT_PROP_LE32 ||
+  type == USB_EXT_PROP_BE32))
+   ext_prop->data_len >>= 1;
+   ext_prop->type = type;
+   ret = len;
+
+end:
+   if (desc->opts_mutex)
+   mutex_unlock(desc->opts_mutex);
+   return ret;
+}
+
+static ssize_t ext_prop_data_show(struct usb_os_desc_ext_prop *ext_prop,
+  

[PATCHv2 6/8] usb: gadget: configfs: OS Extended Compatibility descriptors support

2014-05-08 Thread Andrzej Pietrasiewicz
Add handling of OS Extended Compatibility descriptors from configfs interface.
Hosts which expect the "OS Descriptors" ask only for configurations @ index 0,
but linux-based USB devices can provide more than one configuration.
This patch adds marking one of gadget's configurations the configuration
to be reported at index 0, regardless of the actual sequence of usb_add_config
invocations used for adding the configurations. The configuration is selected
by creating a symbolic link pointing to it from the "os_desc" directory
located at the top of a gadget's directory hierarchy.

One kind of "OS Descriptors" are "Extended Compatibility Descriptors",
which need to be specified per interface. This patch adds interface.
directory in function's configfs directory to represent each interface
defined by the function. Each interface's directory contains two attributes:
"compatible_id" and "sub_compatible_id", which represent 8-byte
strings to be reported to the host as the "Compatible ID" and "Sub Compatible
ID".

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/ABI/testing/configfs-usb-gadget |  13 ++
 drivers/usb/gadget/configfs.c | 190 ++
 drivers/usb/gadget/configfs.h |  12 ++
 include/linux/usb/composite.h |   6 +
 4 files changed, 221 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget 
b/Documentation/ABI/testing/configfs-usb-gadget
index 0e7b786..5c0b3e6 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget
+++ b/Documentation/ABI/testing/configfs-usb-gadget
@@ -62,6 +62,19 @@ KernelVersion:   3.11
 Description:
This group contains functions available to this USB gadget.
 
+What:  /config/usb-gadget/gadget/functions/./interface.
+Date:  May 2014
+KernelVersion: 3.16
+Description:
+   This group contains "Feature Descriptors" specific for one
+   gadget's USB interface or one interface group described
+   by an IAD.
+
+   The attributes:
+
+   compatible_id   - 8-byte string for "Compatible ID"
+   sub_compatible_id   - 8-byte string for "Sub Compatible ID"
+
 What:  /config/usb-gadget/gadget/strings
 Date:  Jun 2013
 KernelVersion: 3.11
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 4c781fc..1fd1e14 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include "configfs.h"
+#include "u_f.h"
 
 int check_user_usb_string(const char *name,
struct usb_gadget_strings *stringtab_dev)
@@ -872,10 +873,63 @@ static void os_desc_attr_release(struct config_item *item)
kfree(os_desc);
 }
 
+static int os_desc_link(struct config_item *os_desc_ci,
+   struct config_item *usb_cfg_ci)
+{
+   struct gadget_info *gi = container_of(to_config_group(os_desc_ci),
+   struct gadget_info, os_desc_group);
+   struct usb_composite_dev *cdev = &gi->cdev;
+   struct config_usb_cfg *c_target =
+   container_of(to_config_group(usb_cfg_ci),
+struct config_usb_cfg, group);
+   struct usb_configuration *c;
+   int ret;
+
+   mutex_lock(&gi->lock);
+   list_for_each_entry(c, &cdev->configs, list) {
+   if (c == &c_target->c)
+   break;
+   }
+   if (c != &c_target->c) {
+   ret = -EINVAL;
+   goto out;
+   }
+
+   if (cdev->os_desc_config) {
+   ret = -EBUSY;
+   goto out;
+   }
+
+   cdev->os_desc_config = &c_target->c;
+   ret = 0;
+
+out:
+   mutex_unlock(&gi->lock);
+   return ret;
+}
+
+static int os_desc_unlink(struct config_item *os_desc_ci,
+ struct config_item *usb_cfg_ci)
+{
+   struct gadget_info *gi = container_of(to_config_group(os_desc_ci),
+   struct gadget_info, os_desc_group);
+   struct usb_composite_dev *cdev = &gi->cdev;
+
+   mutex_lock(&gi->lock);
+   if (gi->udc_name)
+   unregister_gadget(gi);
+   cdev->os_desc_config = NULL;
+   WARN_ON(gi->udc_name);
+   mutex_unlock(&gi->lock);
+   return 0;
+}
+
 static struct configfs_item_operations os_desc_ops = {
.release= os_desc_attr_release,
.show_attribute = os_desc_attr_show,
.store_attribute= os_desc_attr_store,
+   .allow_link = os_desc_link,
+   .drop_link  = os_desc_unlink,
 };
 
 static struct config_item_type os_desc_type = {
@@ -884,6 +938,133 @@ static struct config_item_type os_desc_type = {
.ct_owner   = THIS_MODULE,
 };
 
+CONFIGFS_ATTR_STRUCT(usb_os_desc);
+CONFIGFS_ATTR_OPS(usb_os_desc);
+
+static struct configfs_item_operations interf_item_ops = {
+   .show_at

[PATCHv2 7/8] usb: gadget: f_rndis: OS Descriptors configfs support

2014-05-08 Thread Andrzej Pietrasiewicz
Added handling of OS Descriptors support for f_rndis.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/f_rndis.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
index a7633d6..eed3ad8 100644
--- a/drivers/usb/gadget/f_rndis.c
+++ b/drivers/usb/gadget/f_rndis.c
@@ -27,6 +27,7 @@
 #include "u_ether_configfs.h"
 #include "u_rndis.h"
 #include "rndis.h"
+#include "configfs.h"
 
 /*
  * This function is an RNDIS Ethernet port -- a Microsoft protocol that's
@@ -895,12 +896,15 @@ static void rndis_free_inst(struct usb_function_instance 
*f)
else
free_netdev(opts->net);
}
+
+   kfree(opts->rndis_os_desc.group.default_groups); /* single VLA chunk */
kfree(opts);
 }
 
 static struct usb_function_instance *rndis_alloc_inst(void)
 {
struct f_rndis_opts *opts;
+   struct usb_os_desc *descs[1];
 
opts = kzalloc(sizeof(*opts), GFP_KERNEL);
if (!opts)
@@ -917,6 +921,9 @@ static struct usb_function_instance *rndis_alloc_inst(void)
}
INIT_LIST_HEAD(&opts->rndis_os_desc.ext_prop);
 
+   descs[0] = &opts->rndis_os_desc;
+   usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
+  THIS_MODULE);
config_group_init_type_name(&opts->func_inst.group, "",
&rndis_func_type);
 
-- 
1.8.3.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


[PATCHv2 3/8] usb: gadget: OS Feature Descriptors support

2014-05-08 Thread Andrzej Pietrasiewicz
There is a custom (non-USB IF) extension to the USB standard:

http://msdn.microsoft.com/library/windows/hardware/gg463182

They grant permission to use the specification - there is
"Microsoft OS Descriptor Specification License Agreement"
under the link mentioned above, and its Section 2 "Grant
of License", letter (b) reads:

"Patent license. Microsoft hereby grants to You a nonexclusive,
royalty-free, nontransferable, worldwide license under Microsoft’s
patents embodied solely within the Specification and that are owned
or licensable by Microsoft to make, use, import, offer to sell,
sell and distribute directly or indirectly to Your Licensees Your
Implementation. You may sublicense this patent license to Your
Licensees under the same terms and conditions."

The said extension is maintained by Microsoft for Microsoft.

Yet it is fairly common for various devices to use it, and a
popular proprietary operating system expects devices to provide
"OS descriptors", so Linux-based USB gadgets whishing to be able
to talk to a variety of operating systems should be able to provide
the "OS descriptors".

This patch adds optional support for gadgets whishing to expose
the so called "OS Feature Descriptors", that is "Extended Compatibility ID"
and "Extended Properties".

Hosts which do request "OS descriptors" from gadgets do so during
the enumeration phase and before the configuration is set with
SET_CONFIGURATION. What is more, those hosts never ask for configurations
at indices other than 0. Therefore, gadgets whishing to provide
"OS descriptors" must designate one configuration to be used with
this kind of hosts - this is what os_desc_config is added for in
struct usb_composite_dev. There is an additional advantage to it:
if a gadget provides "OS descriptors" and designates one configuration
to be used with such non-USB-compliant hosts it can invoke
"usb_add_config" in any order because the designated configuration
will be reported to be at index 0 anyway.

This patch also adds handling vendor-specific requests addressed
at device or interface and related to handling "OS descriptors".

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Michal Nazarewicz 
---
 drivers/usb/gadget/composite.c | 288 -
 drivers/usb/gadget/u_os_desc.h |  90 +
 include/linux/usb/composite.h  |  58 +
 3 files changed, 435 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/gadget/u_os_desc.h

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index f27aec6..75494c2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -21,6 +21,8 @@
 #include 
 #include 
 
+#include "u_os_desc.h"
+
 /**
  * struct usb_os_string - represents OS String to be reported by a gadget
  * @bLength: total length of the entire descritor, always 0x12
@@ -438,6 +440,7 @@ static int config_desc(struct usb_composite_dev *cdev, 
unsigned w_value)
 {
struct usb_gadget   *gadget = cdev->gadget;
struct usb_configuration*c;
+   struct list_head*pos;
u8  type = w_value >> 8;
enum usb_device_speed   speed = USB_SPEED_UNKNOWN;
 
@@ -456,7 +459,20 @@ static int config_desc(struct usb_composite_dev *cdev, 
unsigned w_value)
 
/* This is a lookup by config *INDEX* */
w_value &= 0xff;
-   list_for_each_entry(c, &cdev->configs, list) {
+
+   pos = &cdev->configs;
+   c = cdev->os_desc_config;
+   if (c)
+   goto check_config;
+
+   while ((pos = pos->next) !=  &cdev->configs) {
+   c = list_entry(pos, typeof(*c), list);
+
+   /* skip OS Descriptors config which is handled separately */
+   if (c == cdev->os_desc_config)
+   continue;
+
+check_config:
/* ignore configs that won't work at this speed */
switch (speed) {
case USB_SPEED_SUPER:
@@ -1235,6 +1251,158 @@ static void composite_setup_complete(struct usb_ep *ep, 
struct usb_request *req)
req->status, req->actual, req->length);
 }
 
+static int count_ext_compat(struct usb_configuration *c)
+{
+   int i, res;
+
+   res = 0;
+   for (i = 0; i < c->next_interface_id; ++i) {
+   struct usb_function *f;
+   int j;
+
+   f = c->interface[i];
+   for (j = 0; j < f->os_desc_n; ++j) {
+   struct usb_os_desc *d;
+
+   if (i != f->os_desc_table[j].if_id)
+   continue;
+   d = f->os_desc_table[j].os_desc;
+   if (d && d->ext_compat_id)
+   ++res;
+   }
+   }
+   BUG_ON(res > 255);
+   return res;
+}
+
+static void fill_ext_compat(struct usb_configuration *c, u8 *buf)
+{
+   int i, count;
+
+   count = 16;
+  

[PATCHv2 0/8] OS Descriptors support

2014-05-08 Thread Andrzej Pietrasiewicz
There is a custom (non-USB IF) extension to the USB standard:

http://msdn.microsoft.com/library/windows/hardware/gg463182

They grant permission to use the specification - there is
"Microsoft OS Descriptor Specification License Agreement"
under the link mentioned above, and its Section 2 "Grant
of License", letter (b) reads:

"Patent license. Microsoft hereby grants to You a nonexclusive,
royalty-free, nontransferable, worldwide license under Microsoft’s
patents embodied solely within the Specification and that are owned
or licensable by Microsoft to make, use, import, offer to sell,
sell and distribute directly or indirectly to Your Licensees Your
Implementation. You may sublicense this patent license to Your
Licensees under the same terms and conditions."

The said extension is maintained by Microsoft for Microsoft.

Yet it is fairly common for various devices to use it, and a
popular proprietary operating system expects devices to provide
"OS descriptors", so Linux-based USB gadgets whishing to be able
to talk to a variety of operating systems should be able to provide
the "OS descriptors".

This patch series adds (optional) support for gadgets whishing to expose
the so-called "OS Descriptors".

The sequence of operations when a gadget is enumerated by an OS which expects
the said descriptors is as following:

1) the host issues a standard "GET_DESCRIPTOR" request of type 0x03 (string)
for a string at index 0xEE in language 0, named "OS String"
2) if the device responds, the hosts interprets the response, and if it is
of expected form the host assumes the device supports "OS Descriptors"
3) the host issues requests to get "Feature Descriptors" it expects

There are two kinds of "Feature Descriptors": "Extended Compatibility"
descriptors and "Extended Properties" descriptors.
They are specified per USB interface or per USB interface group described
by an IAD.

Legacy gadgets (compiled as separate modules, like g_ether, g_serial etc)
can stay as they are and not use this extension.
The intended use of the extension is with configfs interface.

This series contains an extension to the f_rndis function; if a function
whishes to support "OS Descriptors" it needs to be patched with a fairly
simple change: PATCH 4/8 contains code for supporting the extension proper,
while PATCH 7/8 contains code for using it with configfs.

v1..v2:

- included fixes suggested by Michal: fixed the "not used" comment,
replaced BUG_ON() with compiletime_assert(), folded handling of OS Descriptors
config into the loop (@Michal: it seems to work)

Example gadget is created with the following commands
(I assume s3c-hsotg is used):

$ #USUAL STUFF
$ #===
$
$ modprobe libcomposite
$ mount none cfg -t configfs
$ mkdir cfg/usb_gadget/g1
$ cd cfg/usb_gadget/g1
$ mkdir configs/c.1
$ mkdir functions/rndis.usb0
$ mkdir strings/0x409
$ mkdir configs/c.1/strings/0x409
$ echo 0x > idProduct
$ echo 0x > idVendor
$ echo 123 > strings/0x409/serialnumber
$ echo manufacturer > strings/0x409/manufacturer
$ echo RNDIS Gadget > strings/0x409/product
$ echo "Conf 1" > configs/c.1/strings/0x409/configuration
$ echo 120 > configs/c.1/MaxPower
$
$ #OS DESCRIPTORS
$ #==
$
$ #OS STRING
$ #-
$
$echo 1 > os_desc/use
$ echo 0xcd > os_desc/b_vendor_code
$ echo MSFT100 > os_desc/qw_sign
$
$ #COMPATIBLE ID (SUB COMPATIBLE ID UNUSED)
$ #-
$
$ echo RNDIS > functions/rndis.usb0/os_desc/interface.0/compatible_id
$
$ #MAKE c.1 THE ONE ASSOCIATED WITH OS DESCRIPTORS
$ #---
$
$ ln -s configs/c.1 os_desc
$
$ #MAKE "Icons" EXTENDED PROPERTY
$ #--
$
$ mkdir functions/rndis.usb0/os_desc/interface.0/Icons
$ echo 2 > functions/rndis.usb0/os_desc/interface.0/Icons/type
$ echo "%SystemRoot%\\system32\\shell32.dll,-233" > 
functions/rndis.usb0/os_desc/interface.0/Icons/data
$
$ #MAKE "Label" EXTENDED PROPERTY
$ #--
$
$ mkdir functions/rndis.usb0/os_desc/interface.0/Label
$ echo 1 > functions/rndis.usb0/os_desc/interface.0/Label/type
$ echo "XYZ Device" > functions/rndis.usb0/os_desc/interface.0/Label/data
$
$ #USUAL STUFF, CONTINUED
$ #==
$
$ ln -s functions/rndis.usb0 configs/c.1
$ echo s3c-hsotg > UDC


Andrzej Pietrasiewicz (8):
  usb: gadget: FunctionFS: share VLA macros with all usb gadget files
  usb: gadget: OS String support
  usb: gadget: OS Feature Descriptors support
  usb: gadget: f_rndis: OS descriptors support
  usb: gadget: configfs: OS String support
  usb: gadget: configfs: OS Extended Compatibility descriptors support
  usb: gadget: f_rndis: OS Descriptors configfs support
  usb: gadget: configfs: OS Extended Properties descriptors support

 Documentation/ABI/testing/configfs-usb-gadget |  45 +++
 drivers/usb/gadget/composite.c| 317 ++-
 drivers/usb/gadget/configfs.c | 550 +-
 drivers/usb/gadget/configfs.h |  12 

[PATCHv2 5/8] usb: gadget: configfs: OS String support

2014-05-08 Thread Andrzej Pietrasiewicz
Add handling of OS String extension from the configfs interface.
A directory "os_desc" is added at the top level of a gadget's
directories hierarchy. In the "os_desc" directory there are
three attributes: "use", "b_vendor_code" and "qw_sign".
If "use" contains "0" the OS string is not reported to the host.
"b_vendor_code" contains a one-byte value which is used
for custom per-device and per-interface requests.
"qw_sign" contains an identifier to be reported as the "OS String"
proper.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/ABI/testing/configfs-usb-gadget |  11 ++
 drivers/usb/gadget/configfs.c | 159 +-
 2 files changed, 169 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget 
b/Documentation/ABI/testing/configfs-usb-gadget
index 37559a0..0e7b786 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget
+++ b/Documentation/ABI/testing/configfs-usb-gadget
@@ -79,3 +79,14 @@ Description:
product - gadget's product description
manufacturer- gadget's manufacturer description
 
+What:  /config/usb-gadget/gadget/os_desc
+Date:  May 2014
+KernelVersion: 3.16
+Description:
+   This group contains "OS String" extension handling attributes.
+
+   use - flag turning "OS Desctiptors" support on/off
+   b_vendor_code   - one-byte value used for custom per-device and
+   per-interface requests
+   qw_sign - an identifier to be reported as "OS String"
+   proper
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index dcead55..4c781fc 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -2,6 +2,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "configfs.h"
@@ -43,7 +44,8 @@ struct gadget_info {
struct config_group functions_group;
struct config_group configs_group;
struct config_group strings_group;
-   struct config_group *default_groups[4];
+   struct config_group os_desc_group;
+   struct config_group *default_groups[5];
 
struct mutex lock;
struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
@@ -56,6 +58,9 @@ struct gadget_info {
 #endif
struct usb_composite_driver composite;
struct usb_composite_dev cdev;
+   bool use_os_desc;
+   char b_vendor_code;
+   char qw_sign[OS_STRING_QW_SIGN_LEN];
 };
 
 struct config_usb_cfg {
@@ -79,6 +84,10 @@ struct gadget_strings {
struct list_head list;
 };
 
+struct os_desc {
+   struct config_group group;
+};
+
 struct gadget_config_name {
struct usb_gadget_strings stringtab_dev;
struct usb_string strings;
@@ -736,6 +745,145 @@ static void gadget_strings_attr_release(struct 
config_item *item)
 USB_CONFIG_STRING_RW_OPS(gadget_strings);
 USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
 
+static inline struct os_desc *to_os_desc(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct os_desc, group);
+}
+
+CONFIGFS_ATTR_STRUCT(os_desc);
+CONFIGFS_ATTR_OPS(os_desc);
+
+static ssize_t os_desc_use_show(struct os_desc *os_desc, char *page)
+{
+   struct gadget_info *gi;
+
+   gi = to_gadget_info(os_desc->group.cg_item.ci_parent);
+
+   return sprintf(page, "%d", gi->use_os_desc);
+}
+
+static ssize_t os_desc_use_store(struct os_desc *os_desc, const char *page,
+size_t len)
+{
+   struct gadget_info *gi;
+   int ret;
+   bool use;
+
+   gi = to_gadget_info(os_desc->group.cg_item.ci_parent);
+
+   mutex_lock(&gi->lock);
+   ret = strtobool(page, &use);
+   if (!ret) {
+   gi->use_os_desc = use;
+   ret = len;
+   }
+   mutex_unlock(&gi->lock);
+
+   return ret;
+}
+
+static struct os_desc_attribute os_desc_use =
+   __CONFIGFS_ATTR(use, S_IRUGO | S_IWUSR,
+   os_desc_use_show,
+   os_desc_use_store);
+
+static ssize_t os_desc_b_vendor_code_show(struct os_desc *os_desc, char *page)
+{
+   struct gadget_info *gi;
+
+   gi = to_gadget_info(os_desc->group.cg_item.ci_parent);
+
+   return sprintf(page, "%d", gi->b_vendor_code);
+}
+
+static ssize_t os_desc_b_vendor_code_store(struct os_desc *os_desc,
+  const char *page, size_t len)
+{
+   struct gadget_info *gi;
+   int ret;
+   u8 b_vendor_code;
+
+   gi = to_gadget_info(os_desc->group.cg_item.ci_parent);
+
+   mutex_lock(&gi->lock);
+   ret = kstrtou8(page, 0, &b_vendor_code);
+   if (!ret) {
+   gi->b_vendor_code = b_vendor_code;
+   ret = len;
+   }
+   mutex_unlock(&gi->lock);
+
+   return ret;
+}
+
+static struct os_desc_attribute os_desc_b_vendor_code =
+   __C

[PATCHv2 1/8] usb: gadget: FunctionFS: share VLA macros with all usb gadget files

2014-05-08 Thread Andrzej Pietrasiewicz
Variable Length Array macros allow portable (compilable with both gcc
and clang) way of allocating a number of structures using a single
memory chunk. They can be useful for files other than f_fs.c,
so move them to a header file.

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Michal Nazarewicz 
---
 drivers/usb/gadget/f_fs.c | 27 +--
 drivers/usb/gadget/u_f.h  | 26 ++
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index de92bd3..74202d6 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -33,36 +33,11 @@
 #include 
 
 #include "u_fs.h"
+#include "u_f.h"
 #include "configfs.h"
 
 #define FUNCTIONFS_MAGIC   0xa647361 /* Chosen by a honest dice roll ;) */
 
-/* Variable Length Array Macros **/
-#define vla_group(groupname) size_t groupname##__next = 0
-#define vla_group_size(groupname) groupname##__next
-
-#define vla_item(groupname, type, name, n) \
-   size_t groupname##_##name##__offset = ({   \
-   size_t align_mask = __alignof__(type) - 1; \
-   size_t offset = (groupname##__next + align_mask) & ~align_mask;\
-   size_t size = (n) * sizeof(type);  \
-   groupname##__next = offset + size; \
-   offset;\
-   })
-
-#define vla_item_with_sz(groupname, type, name, n) \
-   size_t groupname##_##name##__sz = (n) * sizeof(type);  \
-   size_t groupname##_##name##__offset = ({   \
-   size_t align_mask = __alignof__(type) - 1; \
-   size_t offset = (groupname##__next + align_mask) & ~align_mask;\
-   size_t size = groupname##_##name##__sz;\
-   groupname##__next = offset + size; \
-   offset;\
-   })
-
-#define vla_ptr(ptr, groupname, name) \
-   ((void *) ((char *)ptr + groupname##_##name##__offset))
-
 /* Reference counter handling */
 static void ffs_data_get(struct ffs_data *ffs);
 static void ffs_data_put(struct ffs_data *ffs);
diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h
index 71034c0..1d5f0eb 100644
--- a/drivers/usb/gadget/u_f.h
+++ b/drivers/usb/gadget/u_f.h
@@ -16,6 +16,32 @@
 #ifndef __U_F_H__
 #define __U_F_H__
 
+/* Variable Length Array Macros **/
+#define vla_group(groupname) size_t groupname##__next = 0
+#define vla_group_size(groupname) groupname##__next
+
+#define vla_item(groupname, type, name, n) \
+   size_t groupname##_##name##__offset = ({   \
+   size_t align_mask = __alignof__(type) - 1; \
+   size_t offset = (groupname##__next + align_mask) & ~align_mask;\
+   size_t size = (n) * sizeof(type);  \
+   groupname##__next = offset + size; \
+   offset;\
+   })
+
+#define vla_item_with_sz(groupname, type, name, n) \
+   size_t groupname##_##name##__sz = (n) * sizeof(type);  \
+   size_t groupname##_##name##__offset = ({   \
+   size_t align_mask = __alignof__(type) - 1; \
+   size_t offset = (groupname##__next + align_mask) & ~align_mask;\
+   size_t size = groupname##_##name##__sz;\
+   groupname##__next = offset + size; \
+   offset;\
+   })
+
+#define vla_ptr(ptr, groupname, name) \
+   ((void *) ((char *)ptr + groupname##_##name##__offset))
+
 struct usb_ep;
 struct usb_request;
 
-- 
1.8.3.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


Re: [PATCH v9 17/19] usb: resume (wakeup) child device when port is powered on

2014-05-08 Thread Rafael J. Wysocki
On Wednesday, May 07, 2014 10:37:24 PM Dan Williams wrote:
> Unconditionally wake up the child device when the power session is
> recovered.

[cut]

> + /*
> +  * Revalidate the device if it was requested by
> +  * usb_port_runtime_resume.
> +  */
> + if (test_and_clear_bit(i, hub->wakeup_bits) && 
> udev) {
> + pm_runtime_get_noresume(&udev->dev);
> + pm_runtime_barrier(&udev->dev);
> + pm_runtime_put_autosuspend(&udev->dev);

It looks like you could just call pm_runtime_idle(&udev->dev) after the barrier
without the _get and _put around it.  Wouldn't that work?


-- 
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] usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled

2014-05-08 Thread George Cherian
AM335x MUSB supports both PIO and DMA mode. When DMA mode is
selected users need to explicitly enable the DMA driver. To avoid the
extra configuration select the DMA driver if DMA mode is set for AM335x MUSB.

Signed-off-by: George Cherian 
---
 drivers/usb/musb/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 8b78979..6b2a4b8 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -141,6 +141,7 @@ config USB_TI_CPPI_DMA
 config USB_TI_CPPI41_DMA
bool 'TI CPPI 4.1 (AM335x)'
depends on ARCH_OMAP
+   select TI_CPPI41
 
 config USB_TUSB_OMAP_DMA
bool 'TUSB 6010'
-- 
1.8.3.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 v2 3/5] usb: musb: core: Convert the musb_platform_reset to have a return value.

2014-05-08 Thread George Cherian

On 5/8/2014 3:17 PM, Daniel Mack wrote:

Hi Geroge,

On 05/08/2014 11:35 AM, George Cherian wrote:

-static inline void musb_platform_reset(struct musb *musb)
+static inline int  musb_platform_reset(struct musb *musb)
  {
-   if (musb->ops->reset)
-   musb->ops->reset(musb);
+   if (!musb->ops->reset)
+   return -EINVAL;
+
+   return musb->ops->reset(musb);
  }
  
  static inline int musb_platform_get_vbus_status(struct musb *musb)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 74c4193..8438200 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -536,7 +536,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
return 0;
  }
  
-static void dsps_musb_reset(struct musb *musb)

+static int dsps_musb_reset(struct musb *musb)
  {
struct device *dev = musb->controller;
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
@@ -548,6 +548,7 @@ static void dsps_musb_reset(struct musb *musb)
usleep_range(100, 200);
usb_phy_init(musb->xceiv);
  
+	return 1;

Could we follow the general kernel rule here and return 0 for success,
and < 0 on failure?
I made this to return 1, just because the next patch in the series would 
make it
return session_restart. Depending on which the musb_platform_reset() 
would return

whether actual reset of IP (reset_done) is done or not .





Thanks,
Daniel




--
-George

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


Re: [PATCH v2 3/5] usb: musb: core: Convert the musb_platform_reset to have a return value.

2014-05-08 Thread Daniel Mack
Hi Geroge,

On 05/08/2014 11:35 AM, George Cherian wrote:
> -static inline void musb_platform_reset(struct musb *musb)
> +static inline int  musb_platform_reset(struct musb *musb)
>  {
> - if (musb->ops->reset)
> - musb->ops->reset(musb);
> + if (!musb->ops->reset)
> + return -EINVAL;
> +
> + return musb->ops->reset(musb);
>  }
>  
>  static inline int musb_platform_get_vbus_status(struct musb *musb)
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 74c4193..8438200 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -536,7 +536,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
>   return 0;
>  }
>  
> -static void dsps_musb_reset(struct musb *musb)
> +static int dsps_musb_reset(struct musb *musb)
>  {
>   struct device *dev = musb->controller;
>   struct dsps_glue *glue = dev_get_drvdata(dev->parent);
> @@ -548,6 +548,7 @@ static void dsps_musb_reset(struct musb *musb)
>   usleep_range(100, 200);
>   usb_phy_init(musb->xceiv);
>  
> + return 1;

Could we follow the general kernel rule here and return 0 for success,
and < 0 on failure?



Thanks,
Daniel

--
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 0/5] Add support for SW babble Control

2014-05-08 Thread George Cherian
Series add support for SW babble control logic found in 
new silicon versions of AM335x. Runtime differentiation of
silicon version is done by checking the BABBLE_CTL register.
For newer silicon the register default value read is 0x4 and
for older versions its 0x0.

Patch 1 -> Convert recover work to delayed work.
Patch 2 -> usb_phy_vbus_(off/_on) are NOPs for am335x PHY
   so use usb_phy(_shutdown/_init) in musb_platform_reset()
Patch 3 -> Add return value for musb_platform_reset() in prepration
   to support SW babble_ctrl
Patch 4 -> Add the sw_babble_control()
Patch 5 -> Enable sw babble control for newer silicon

v1 -> v2 : Fixed the issue with Patch 5. In v1 it was not calling 
   sw_babble_control().

George Cherian (5):
  usb: musb: core: Convert babble recover work to delayed work
  usb: musb: dsps: Call usb_phy(_shutdown/_init) during
musb_platform_reset()
  usb: musb: core: Convert the musb_platform_reset to have a return
value.
  usb: musb: dsps: Add the sw_babble_control()
  usb: musb: dsps: Enable sw babble control for newer silicon

 drivers/usb/musb/musb_core.c | 49 +
 drivers/usb/musb/musb_core.h | 12 ---
 drivers/usb/musb/musb_dsps.c | 85 ++--
 drivers/usb/musb/musb_regs.h |  7 
 4 files changed, 124 insertions(+), 29 deletions(-)

-- 
1.8.3.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 v2 2/5] usb: musb: dsps: Call usb_phy(_shutdown/_init) during musb_platform_reset()

2014-05-08 Thread George Cherian
For DSPS platform usb_phy_vbus(_off/_on) are NOPs.
So during musb_platform_reset() call usb_phy(_shutdown/_init)

Signed-off-by: George Cherian 
---
 drivers/usb/musb/musb_dsps.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 51beb13..74c4193 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -543,7 +543,11 @@ static void dsps_musb_reset(struct musb *musb)
const struct dsps_musb_wrapper *wrp = glue->wrp;
 
dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
-   udelay(100);
+   usleep_range(100, 200);
+   usb_phy_shutdown(musb->xceiv);
+   usleep_range(100, 200);
+   usb_phy_init(musb->xceiv);
+
 }
 
 static struct musb_platform_ops dsps_ops = {
-- 
1.8.3.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 v2 1/5] usb: musb: core: Convert babble recover work to delayed work

2014-05-08 Thread George Cherian
During babble condition first disconnect of devices are
initiated. Make sure MUSB controller is reset and re-initialized
after all disconnects.

To acheive this schedule a delayed work for babble rrecovery.

While at that convert udelay to usleep_range.
Refer Documentation/timers/timers-howto.txt

Signed-off-by: George Cherian 
---
 drivers/usb/musb/musb_core.c | 15 ---
 drivers/usb/musb/musb_core.h |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 61da471..dcadc62 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -850,7 +850,8 @@ b_host:
 
/* handle babble condition */
if (int_usb & MUSB_INTR_BABBLE)
-   schedule_work(&musb->recover_work);
+   schedule_delayed_work(&musb->recover_work,
+ msecs_to_jiffies(100));
 
 #if 0
 /* REVISIT ... this would be for multiplexing periodic endpoints, or
@@ -1753,16 +1754,16 @@ static void musb_irq_work(struct work_struct *data)
 /* Recover from babble interrupt conditions */
 static void musb_recover_work(struct work_struct *data)
 {
-   struct musb *musb = container_of(data, struct musb, recover_work);
+   struct musb *musb = container_of(data, struct musb, recover_work.work);
int status;
 
musb_platform_reset(musb);
 
usb_phy_vbus_off(musb->xceiv);
-   udelay(100);
+   usleep_range(100, 200);
 
usb_phy_vbus_on(musb->xceiv);
-   udelay(100);
+   usleep_range(100, 200);
 
/*
 * When a babble condition occurs, the musb controller removes the
@@ -1945,7 +1946,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
 
/* Init IRQ workqueue before request_irq */
INIT_WORK(&musb->irq_work, musb_irq_work);
-   INIT_WORK(&musb->recover_work, musb_recover_work);
+   INIT_DELAYED_WORK(&musb->recover_work, musb_recover_work);
INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset);
INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume);
 
@@ -2041,7 +2042,7 @@ fail4:
 
 fail3:
cancel_work_sync(&musb->irq_work);
-   cancel_work_sync(&musb->recover_work);
+   cancel_delayed_work_sync(&musb->recover_work);
cancel_delayed_work_sync(&musb->finish_resume_work);
cancel_delayed_work_sync(&musb->deassert_reset_work);
if (musb->dma_controller)
@@ -2107,7 +2108,7 @@ static int musb_remove(struct platform_device *pdev)
dma_controller_destroy(musb->dma_controller);
 
cancel_work_sync(&musb->irq_work);
-   cancel_work_sync(&musb->recover_work);
+   cancel_delayed_work_sync(&musb->recover_work);
cancel_delayed_work_sync(&musb->finish_resume_work);
cancel_delayed_work_sync(&musb->deassert_reset_work);
musb_free(musb);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 47e8874..423cd00 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -297,7 +297,7 @@ struct musb {
 
irqreturn_t (*isr)(int, void *);
struct work_struct  irq_work;
-   struct work_struct  recover_work;
+   struct delayed_work recover_work;
struct delayed_work deassert_reset_work;
struct delayed_work finish_resume_work;
u16 hwvers;
-- 
1.8.3.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 v2 5/5] usb: musb: dsps: Enable sw babble control for newer silicon

2014-05-08 Thread George Cherian
Find whether we are running on newer silicon. The babble control
register reads 0x4 by default in newer silicon as opposed to 0
in old versions of AM335x. Based on this enable the sw babble
control logic.

Signed-off-by: George Cherian 
---
 drivers/usb/musb/musb_dsps.c | 34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 6647e7e..665d1dd 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -136,6 +136,7 @@ struct dsps_glue {
const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
struct timer_list timer;/* otg_workaround timer */
unsigned long last_timer;/* last timer data for each instance */
+   bool sw_babble_enabled;
 
struct dsps_context context;
struct debugfs_regset32 regset;
@@ -469,6 +470,16 @@ static int dsps_musb_init(struct musb *musb)
val &= ~(1 << wrp->otg_disable);
dsps_writel(musb->ctrl_base, wrp->phy_utmi, val);
 
+   /*
+*  Check whether the dsps version has babble control enabled.
+* In latest silicon revision the babble control logic is enabled.
+* If MUSB_BABBLE_CTL returns 0x4 then we have the babble control
+* logic enabled.
+*/
+   val = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+   if (val == MUSB_BABBLE_RCV_DISABLE)
+   glue->sw_babble_enabled = true;
+
ret = dsps_musb_dbg_init(musb, glue);
if (ret)
return ret;
@@ -591,14 +602,25 @@ static int dsps_musb_reset(struct musb *musb)
struct device *dev = musb->controller;
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
const struct dsps_musb_wrapper *wrp = glue->wrp;
+   int session_restart = 0;
 
-   dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
-   usleep_range(100, 200);
-   usb_phy_shutdown(musb->xceiv);
-   usleep_range(100, 200);
-   usb_phy_init(musb->xceiv);
+   if (glue->sw_babble_enabled)
+   session_restart = sw_babble_control(musb);
+   /*
+* In case of new silicon version babble condition can be recovered
+* without resetting the MUSB. But for older silicon versions, MUSB
+* reset is needed
+*/
+   if (session_restart || !glue->sw_babble_enabled) {
+   dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
+   usleep_range(100, 200);
+   usb_phy_shutdown(musb->xceiv);
+   usleep_range(100, 200);
+   usb_phy_init(musb->xceiv);
+   session_restart = 1;
+   }
 
-   return 1;
+   return session_restart;
 }
 
 static struct musb_platform_ops dsps_ops = {
-- 
1.8.3.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 v2 4/5] usb: musb: dsps: Add the sw_babble_control()

2014-05-08 Thread George Cherian
Add sw_babble_control() logic to differentiate between transient
babble and real babble condition. Also add the SW babble control
register definitions.

Babble control register logic is implemented in the latest
revision of AM335x.

Signed-off-by: George Cherian 
---
 drivers/usb/musb/musb_dsps.c | 50 
 drivers/usb/musb/musb_regs.h |  7 +++
 2 files changed, 57 insertions(+)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 8438200..6647e7e 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -536,6 +536,56 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
return 0;
 }
 
+static int sw_babble_control(struct musb *musb)
+{
+   int timeout = 10;
+   u8 babble_ctl, session_restart = 0;
+
+   babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+   dev_dbg(musb->controller, "babble: MUSB_BABBLE_CTL value %x\n",
+   babble_ctl);
+   /*
+* check line monitor flag to check whether babble is
+* due to noise
+*/
+   dev_dbg(musb->controller, "STUCK_J is %s\n",
+   babble_ctl & MUSB_BABBLE_STUCK_J ? "set" : "reset");
+
+   if (babble_ctl & MUSB_BABBLE_STUCK_J) {
+   /*
+* babble is due to noise, then set transmit idle (d7 bit)
+* to resume normal operation
+*/
+   babble_ctl = musb_readb(musb->mregs, MUSB_BABBLE_CTL);
+   babble_ctl |= MUSB_BABBLE_FORCE_TXIDLE;
+   dsps_writeb(musb->mregs, MUSB_BABBLE_CTL, babble_ctl);
+
+   /* wait till line monitor flag cleared */
+   dev_dbg(musb->controller, "Set TXIDLE, wait J to clear\n");
+   do {
+   babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+   udelay(1);
+   } while ((babble_ctl & MUSB_BABBLE_STUCK_J) && timeout--);
+
+   /* check whether stuck_at_j bit cleared */
+   if (babble_ctl & MUSB_BABBLE_STUCK_J) {
+   /*
+* real babble condition is occured
+* restart the controller to start the
+* session again
+*/
+   dev_dbg(musb->controller, "J not cleared, misc (%x)\n",
+   babble_ctl);
+   session_restart = 1;
+   }
+
+   } else {
+   session_restart = 1;
+   }
+
+   return session_restart;
+}
+
 static int dsps_musb_reset(struct musb *musb)
 {
struct device *dev = musb->controller;
diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h
index 03f2655..b9bcda5 100644
--- a/drivers/usb/musb/musb_regs.h
+++ b/drivers/usb/musb/musb_regs.h
@@ -72,6 +72,12 @@
 #define MUSB_DEVCTL_HR 0x02
 #define MUSB_DEVCTL_SESSION0x01
 
+/* BABBLE_CTL */
+#define MUSB_BABBLE_FORCE_TXIDLE   0x80
+#define MUSB_BABBLE_SW_SESSION_CTRL0x40
+#define MUSB_BABBLE_STUCK_J0x20
+#define MUSB_BABBLE_RCV_DISABLE0x04
+
 /* MUSB ULPI VBUSCONTROL */
 #define MUSB_ULPI_USE_EXTVBUS  0x01
 #define MUSB_ULPI_USE_EXTVBUSIND 0x02
@@ -246,6 +252,7 @@
  */
 
 #define MUSB_DEVCTL0x60/* 8 bit */
+#define MUSB_BABBLE_CTL0x61/* 8 bit */
 
 /* These are always controlled through the INDEX register */
 #define MUSB_TXFIFOSZ  0x62/* 8-bit (see masks) */
-- 
1.8.3.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 v2 3/5] usb: musb: core: Convert the musb_platform_reset to have a return value.

2014-05-08 Thread George Cherian
Currently musb_platform_reset() is only used by dsps.
In case of BABBLE interrupt for other platforms the  musb_platform_reset()
is a NOP. In such situations no need to re-initialize the endpoints.
Also in the latest silicon revision of AM335x, we do have a babble recovery
mechanism without resetting the IP block. In preperation to add that support
its better to have a rest_done return for  musb_platform_reset().

Signed-off-by: George Cherian 
---
 drivers/usb/musb/musb_core.c | 38 +-
 drivers/usb/musb/musb_core.h | 10 ++
 drivers/usb/musb/musb_dsps.c |  3 ++-
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index dcadc62..983a591 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1755,28 +1755,32 @@ static void musb_irq_work(struct work_struct *data)
 static void musb_recover_work(struct work_struct *data)
 {
struct musb *musb = container_of(data, struct musb, recover_work.work);
-   int status;
+   int status, reset_done;
 
-   musb_platform_reset(musb);
+   reset_done  = musb_platform_reset(musb);
+   if (reset_done  < 0)
+   return;
 
-   usb_phy_vbus_off(musb->xceiv);
-   usleep_range(100, 200);
+   if (reset_done) {
+   usb_phy_vbus_off(musb->xceiv);
+   usleep_range(100, 200);
 
-   usb_phy_vbus_on(musb->xceiv);
-   usleep_range(100, 200);
+   usb_phy_vbus_on(musb->xceiv);
+   usleep_range(100, 200);
 
-   /*
-* When a babble condition occurs, the musb controller removes the
-* session bit and the endpoint config is lost.
-*/
-   if (musb->dyn_fifo)
-   status = ep_config_from_table(musb);
-   else
-   status = ep_config_from_hw(musb);
+   /*
+* When a babble condition occurs, the musb controller removes 
the
+* session bit and the endpoint config is lost.
+*/
+   if (musb->dyn_fifo)
+   status = ep_config_from_table(musb);
+   else
+   status = ep_config_from_hw(musb);
 
-   /* start the session again */
-   if (status == 0)
-   musb_start(musb);
+   /* start the session again */
+   if (status == 0)
+   musb_start(musb);
+   }
 }
 
 /* --
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 423cd00..3ccb428 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -192,7 +192,7 @@ struct musb_platform_ops {
 
int (*set_mode)(struct musb *musb, u8 mode);
void(*try_idle)(struct musb *musb, unsigned long timeout);
-   void(*reset)(struct musb *musb);
+   int (*reset)(struct musb *musb);
 
int (*vbus_status)(struct musb *musb);
void(*set_vbus)(struct musb *musb, int on);
@@ -554,10 +554,12 @@ static inline void musb_platform_try_idle(struct musb 
*musb,
musb->ops->try_idle(musb, timeout);
 }
 
-static inline void musb_platform_reset(struct musb *musb)
+static inline int  musb_platform_reset(struct musb *musb)
 {
-   if (musb->ops->reset)
-   musb->ops->reset(musb);
+   if (!musb->ops->reset)
+   return -EINVAL;
+
+   return musb->ops->reset(musb);
 }
 
 static inline int musb_platform_get_vbus_status(struct musb *musb)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 74c4193..8438200 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -536,7 +536,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
return 0;
 }
 
-static void dsps_musb_reset(struct musb *musb)
+static int dsps_musb_reset(struct musb *musb)
 {
struct device *dev = musb->controller;
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
@@ -548,6 +548,7 @@ static void dsps_musb_reset(struct musb *musb)
usleep_range(100, 200);
usb_phy_init(musb->xceiv);
 
+   return 1;
 }
 
 static struct musb_platform_ops dsps_ops = {
-- 
1.8.3.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/5] Cleanup and fixes for dwc3-omap

2014-05-08 Thread George Cherian
The series does some refactoring on dwc3_probe()

Patch 1-3 - reduce the size of dwc3_probe()
Patch 4 - Fix the crash on dwc3_omap removal
Patch 5 - Addresses the issue of  xhci hang while resuming from system sleep.


George Cherian (5):
  usb: dwc3: dwc3-omap: Add dwc3_omap_map_offset function
  usb: dwc3: dwc3-omap: Add dwc3_omap_set_utmi_mode() function
  usb: dwc3: dwc3-omap: Add dwc3_omap_extcon_register function
  usb: dwc3: dwc3-omap: Fix the crash on module removal
  usb: dwc3: dwc3-omap: Disable/Enable core interrupts in Suspend/Resume

 drivers/usb/dwc3/dwc3-omap.c | 211 +++
 1 file changed, 111 insertions(+), 100 deletions(-)

-- 
1.8.3.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 4/5] usb: dwc3: dwc3-omap: Fix the crash on module removal

2014-05-08 Thread George Cherian
Following crash is seen on dwc3_omap removal
Unable to handle kernel NULL pointer dereference at virtual address 0018
pgd = ec098000
[0018] *pgd=ad1f9831, *pte=, *ppte=
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in: usb_f_ss_lb g_zero usb_f_acm u_serial usb_f_ecm u_ether 
libcomposite configfs snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_hwdep 
snd_soc_omap snd_pcm_dmaengine snd_soc_core snd_compress snd_pcm snd_tim]
CPU: 0 PID: 1296 Comm: rmmod Tainted: GW 
3.15.0-rc4-02716-g95c4e18-dirty #10
task: ed05a080 ti: ec368000 task.ti: ec368000
PC is at release_resource+0x14/0x7c
LR is at release_resource+0x10/0x7c
pc : []lr : []psr: 6013
sp : ec369ec0  ip : 6013  fp : 00021008
r10:   r9 : ec368000  r8 : c000e7a4
r7 : 0081  r6 : bf0062c0  r5 : ed7cd000  r4 : ed7d85c0
r3 :   r2 :   r1 : 0011  r0 : c086d08c
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c5387d  Table: ac098059  DAC: 0015
Process rmmod (pid: 1296, stack limit = 0xec368248)
Stack: (0xec369ec0 to 0xec36a000)
9ec0:  0001 ed7cd000 c034de94 ed7cd010 ed7cd000  c034e194
9ee0:  bf0062cc ed7cd010 c03490b0 ed154cc0 ed4c2570 ed2b8410 ed156810
ed156810 bf006d24 c034db9c c034db84 c034c518
9f20: bf006d24 ed156810 bf006d24 c034cd2c bf006d24 bf006d68 0800 c034c340
9f40:  c00a9e5c 0020  bf006d68 0800 ec369f4c 33637764
9f60: 616d6f5f 0070 0001 ec368000 ed05a080 c000e670 0001 c0084010
9f80: 00021088 0800 00021088 0081 8010 e6f4 00021088 0800
9fa0: 00021088 c000e5e0 00021088 0800 000210b8 0800 e04f6d00 e04f6d00
9fc0: 00021088 0800 00021088 0081 0001  be91de08 00021008
9fe0: 4d768880 be91dbb4 b6fc5984 4d76888c 8010 000210b8  
[] (release_resource) from [] 
(platform_device_del+0x6c/0x9c)
[] (platform_device_del) from [] 
(platform_device_unregister+0xc/0x18)
[] (platform_device_unregister) from [] 
(dwc3_omap_remove_core+0xc/0x14 [dwc3_omap])
[] (dwc3_omap_remove_core [dwc3_omap]) from [] 
(device_for_each_child+0x34/0x74)
[] (device_for_each_child) from [] 
(dwc3_omap_remove+0x6c/0x78 [dwc3_omap])
[] (dwc3_omap_remove [dwc3_omap]) from [] 
(platform_drv_remove+0x18/0x1c)
[] (platform_drv_remove) from [] 
(__device_release_driver+0x70/0xc8)
[] (__device_release_driver) from [] 
(driver_detach+0xb4/0xb8)
[] (driver_detach) from [] (bus_remove_driver+0x4c/0x90)
[] (bus_remove_driver) from [] 
(SyS_delete_module+0x10c/0x198)
[] (SyS_delete_module) from [] (ret_fast_syscall+0x0/0x48)
Code: e1a04000 e59f0068 eb14505e e5943010 (e5932018)
---[ end trace 7e2a8746ff4fc811 ]---
Segmentation fault

Signed-off-by: George Cherian 
---
 drivers/usb/dwc3/dwc3-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 0b9b1d8..82b20d8f 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -322,7 +322,7 @@ static int dwc3_omap_remove_core(struct device *dev, void 
*c)
 {
struct platform_device *pdev = to_platform_device(dev);
 
-   platform_device_unregister(pdev);
+   of_device_unregister(pdev);
 
return 0;
 }
-- 
1.8.3.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 1/5] usb: dwc3: dwc3-omap: Add dwc3_omap_map_offset function

2014-05-08 Thread George Cherian
Calculate the wrapper register offsets in a seperate function.
Improve code readability, decrease the dwc3_probe() size.

Signed-off-by: George Cherian 
---
 drivers/usb/dwc3/dwc3-omap.c | 80 
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 1160ff4..872f065 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -383,6 +383,49 @@ static int dwc3_omap_vbus_notifier(struct notifier_block 
*nb,
return NOTIFY_DONE;
 }
 
+static void dwc3_omap_map_offset(struct dwc3_omap *omap)
+{
+   u32 reg;
+   struct device_node  *node = omap->dev->of_node;
+   int x_major;
+
+   reg = dwc3_omap_readl(omap->base, USBOTGSS_REVISION);
+   omap->revision = reg;
+   x_major = USBOTGSS_REVISION_XMAJOR(reg);
+
+   /* Differentiate between OMAP5 and AM437x */
+   switch (x_major) {
+   case USBOTGSS_REVISION_XMAJOR1:
+   case USBOTGSS_REVISION_XMAJOR2:
+   omap->irq_eoi_offset = 0;
+   omap->irq0_offset = 0;
+   omap->irqmisc_offset = 0;
+   omap->utmi_otg_offset = 0;
+   omap->debug_offset = 0;
+   break;
+   default:
+   /* Default to the latest revision */
+   omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
+   omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
+   omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
+   omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
+   omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
+   break;
+   }
+
+   /* For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
+* changes in wrapper registers, Using dt compatible for aegis
+*/
+
+   if (of_device_is_compatible(node, "ti,am437x-dwc3")) {
+   omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
+   omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
+   omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
+   omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
+   omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
+   }
+}
+
 static int dwc3_omap_probe(struct platform_device *pdev)
 {
struct device_node  *node = pdev->dev.of_node;
@@ -397,7 +440,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
int irq;
 
int utmi_mode = 0;
-   int x_major;
 
u32 reg;
 
@@ -448,41 +490,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
goto err0;
}
 
-   reg = dwc3_omap_readl(omap->base, USBOTGSS_REVISION);
-   omap->revision = reg;
-   x_major = USBOTGSS_REVISION_XMAJOR(reg);
-
-   /* Differentiate between OMAP5 and AM437x */
-   switch (x_major) {
-   case USBOTGSS_REVISION_XMAJOR1:
-   case USBOTGSS_REVISION_XMAJOR2:
-   omap->irq_eoi_offset = 0;
-   omap->irq0_offset = 0;
-   omap->irqmisc_offset = 0;
-   omap->utmi_otg_offset = 0;
-   omap->debug_offset = 0;
-   break;
-   default:
-   /* Default to the latest revision */
-   omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
-   omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
-   omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
-   omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
-   omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
-   break;
-   }
-
-   /* For OMAP5(ES2.0) and AM437x x_major is 2 even though there are
-* changes in wrapper registers, Using dt compatible for aegis
-*/
-
-   if (of_device_is_compatible(node, "ti,am437x-dwc3")) {
-   omap->irq_eoi_offset = USBOTGSS_EOI_OFFSET;
-   omap->irq0_offset = USBOTGSS_IRQ0_OFFSET;
-   omap->irqmisc_offset = USBOTGSS_IRQMISC_OFFSET;
-   omap->utmi_otg_offset = USBOTGSS_UTMI_OTG_OFFSET;
-   omap->debug_offset = USBOTGSS_DEBUG_OFFSET;
-   }
+   dwc3_omap_map_offset(omap);
 
reg = dwc3_omap_read_utmi_status(omap);
 
-- 
1.8.3.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/5] usb: dwc3: dwc3-omap: Add dwc3_omap_extcon_register function

2014-05-08 Thread George Cherian
Move the extcon related code to its own function.
Improve code readability, decrease the dwc3_probe() size.

Signed-off-by: George Cherian 
---
 drivers/usb/dwc3/dwc3-omap.c | 65 ++--
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index b739a24..0b9b1d8 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -450,6 +450,42 @@ static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap)
dwc3_omap_write_utmi_status(omap, reg);
 }
 
+static int dwc3_omap_extcon_register(struct dwc3_omap *omap)
+{
+   u32 ret;
+   struct device_node  *node = omap->dev->of_node;
+   struct extcon_dev   *edev;
+
+   if (of_property_read_bool(node, "extcon")) {
+   edev = extcon_get_edev_by_phandle(omap->dev, 0);
+   if (IS_ERR(edev)) {
+   dev_vdbg(omap->dev, "couldn't get extcon device\n");
+   return -EPROBE_DEFER;
+   }
+
+   omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
+   ret = extcon_register_interest(&omap->extcon_vbus_dev,
+  edev->name, "USB",
+  &omap->vbus_nb);
+   if (ret < 0)
+   dev_vdbg(omap->dev, "failed to register notifier for 
USB\n");
+
+   omap->id_nb.notifier_call = dwc3_omap_id_notifier;
+   ret = extcon_register_interest(&omap->extcon_id_dev,
+  edev->name, "USB-HOST",
+  &omap->id_nb);
+   if (ret < 0)
+   dev_vdbg(omap->dev, "failed to register notifier for 
USB-HOST\n");
+
+   if (extcon_get_cable_state(edev, "USB") == true)
+   dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+   if (extcon_get_cable_state(edev, "USB-HOST") == true)
+   dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+   }
+
+   return 0;
+}
+
 static int dwc3_omap_probe(struct platform_device *pdev)
 {
struct device_node  *node = pdev->dev.of_node;
@@ -457,7 +493,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
struct dwc3_omap*omap;
struct resource *res;
struct device   *dev = &pdev->dev;
-   struct extcon_dev   *edev;
struct regulator*vbus_reg = NULL;
 
int ret = -ENOMEM;
@@ -529,31 +564,9 @@ static int dwc3_omap_probe(struct platform_device *pdev)
 
dwc3_omap_enable_irqs(omap);
 
-   if (of_property_read_bool(node, "extcon")) {
-   edev = extcon_get_edev_by_phandle(dev, 0);
-   if (IS_ERR(edev)) {
-   dev_vdbg(dev, "couldn't get extcon device\n");
-   ret = -EPROBE_DEFER;
-   goto err2;
-   }
-
-   omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
-   ret = extcon_register_interest(&omap->extcon_vbus_dev,
-   edev->name, "USB", &omap->vbus_nb);
-   if (ret < 0)
-   dev_vdbg(dev, "failed to register notifier for USB\n");
-   omap->id_nb.notifier_call = dwc3_omap_id_notifier;
-   ret = extcon_register_interest(&omap->extcon_id_dev, edev->name,
-"USB-HOST", &omap->id_nb);
-   if (ret < 0)
-   dev_vdbg(dev,
-   "failed to register notifier for USB-HOST\n");
-
-   if (extcon_get_cable_state(edev, "USB") == true)
-   dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
-   if (extcon_get_cable_state(edev, "USB-HOST") == true)
-   dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
-   }
+   ret = dwc3_omap_extcon_register(omap);
+   if (ret < 0)
+   goto err2;
 
ret = of_platform_populate(node, NULL, NULL, dev);
if (ret) {
-- 
1.8.3.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 5/5] usb: dwc3: dwc3-omap: Disable/Enable core interrupts in Suspend/Resume

2014-05-08 Thread George Cherian
Enabling the core interrupts in complete is too late for XHCI, and stops
xhci from proper operation. So remove prepare and complete and disable/enable
interrupts in suspend/resume

Signed-off-by: George Cherian 
---
 drivers/usb/dwc3/dwc3-omap.c | 20 ++--
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 82b20d8f..0916c4b 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -622,27 +622,12 @@ static const struct of_device_id of_dwc3_match[] = {
 MODULE_DEVICE_TABLE(of, of_dwc3_match);
 
 #ifdef CONFIG_PM_SLEEP
-static int dwc3_omap_prepare(struct device *dev)
-{
-   struct dwc3_omap*omap = dev_get_drvdata(dev);
-
-   dwc3_omap_disable_irqs(omap);
-
-   return 0;
-}
-
-static void dwc3_omap_complete(struct device *dev)
-{
-   struct dwc3_omap*omap = dev_get_drvdata(dev);
-
-   dwc3_omap_enable_irqs(omap);
-}
-
 static int dwc3_omap_suspend(struct device *dev)
 {
struct dwc3_omap*omap = dev_get_drvdata(dev);
 
omap->utmi_otg_status = dwc3_omap_read_utmi_status(omap);
+   dwc3_omap_disable_irqs(omap);
 
return 0;
 }
@@ -652,6 +637,7 @@ static int dwc3_omap_resume(struct device *dev)
struct dwc3_omap*omap = dev_get_drvdata(dev);
 
dwc3_omap_write_utmi_status(omap, omap->utmi_otg_status);
+   dwc3_omap_enable_irqs(omap);
 
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
@@ -661,8 +647,6 @@ static int dwc3_omap_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops dwc3_omap_dev_pm_ops = {
-   .prepare= dwc3_omap_prepare,
-   .complete   = dwc3_omap_complete,
 
SET_SYSTEM_SLEEP_PM_OPS(dwc3_omap_suspend, dwc3_omap_resume)
 };
-- 
1.8.3.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/5] usb: dwc3: dwc3-omap: Add dwc3_omap_set_utmi_mode() function

2014-05-08 Thread George Cherian
Move find and set the utmi mode to its own seperate function.
Improve code readability, decrease the dwc3_probe() size.

Signed-off-by: George Cherian 
---
 drivers/usb/dwc3/dwc3-omap.c | 44 +---
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 872f065..b739a24 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -426,6 +426,30 @@ static void dwc3_omap_map_offset(struct dwc3_omap *omap)
}
 }
 
+static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap)
+{
+   u32 reg;
+   struct device_node  *node = omap->dev->of_node;
+   int utmi_mode = 0;
+
+   reg = dwc3_omap_read_utmi_status(omap);
+
+   of_property_read_u32(node, "utmi-mode", &utmi_mode);
+
+   switch (utmi_mode) {
+   case DWC3_OMAP_UTMI_MODE_SW:
+   reg |= USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
+   break;
+   case DWC3_OMAP_UTMI_MODE_HW:
+   reg &= ~USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
+   break;
+   default:
+   dev_dbg(omap->dev, "UNKNOWN utmi mode %d\n", utmi_mode);
+   }
+
+   dwc3_omap_write_utmi_status(omap, reg);
+}
+
 static int dwc3_omap_probe(struct platform_device *pdev)
 {
struct device_node  *node = pdev->dev.of_node;
@@ -439,8 +463,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
int ret = -ENOMEM;
int irq;
 
-   int utmi_mode = 0;
-
u32 reg;
 
void __iomem*base;
@@ -491,23 +513,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
}
 
dwc3_omap_map_offset(omap);
-
-   reg = dwc3_omap_read_utmi_status(omap);
-
-   of_property_read_u32(node, "utmi-mode", &utmi_mode);
-
-   switch (utmi_mode) {
-   case DWC3_OMAP_UTMI_MODE_SW:
-   reg |= USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
-   break;
-   case DWC3_OMAP_UTMI_MODE_HW:
-   reg &= ~USBOTGSS_UTMI_OTG_STATUS_SW_MODE;
-   break;
-   default:
-   dev_dbg(dev, "UNKNOWN utmi mode %d\n", utmi_mode);
-   }
-
-   dwc3_omap_write_utmi_status(omap, reg);
+   dwc3_omap_set_utmi_mode(omap);
 
/* check the DMA Status */
reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
-- 
1.8.3.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 3/5] usb: dwc3: dwc3-omap: Add dwc3_omap_extcon_register function

2014-05-08 Thread Kishon Vijay Abraham I


On Thursday 08 May 2014 02:51 PM, George Cherian wrote:
> Move the extcon related code to its own function.
> Improve code readability, decrease the dwc3_probe() size.
> 
> Signed-off-by: George Cherian 
> ---
>  drivers/usb/dwc3/dwc3-omap.c | 65 
> ++--
>  1 file changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index b739a24..0b9b1d8 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -450,6 +450,42 @@ static void dwc3_omap_set_utmi_mode(struct dwc3_omap 
> *omap)
>   dwc3_omap_write_utmi_status(omap, reg);
>  }
>  
> +static int dwc3_omap_extcon_register(struct dwc3_omap *omap)
> +{
> + u32 ret;

'ret' shouldn't be unsigned.

Thanks
Kishon

> + struct device_node  *node = omap->dev->of_node;
> + struct extcon_dev   *edev;
> +
> + if (of_property_read_bool(node, "extcon")) {
> + edev = extcon_get_edev_by_phandle(omap->dev, 0);
> + if (IS_ERR(edev)) {
> + dev_vdbg(omap->dev, "couldn't get extcon device\n");
> + return -EPROBE_DEFER;
> + }
> +
> + omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
> + ret = extcon_register_interest(&omap->extcon_vbus_dev,
> +edev->name, "USB",
> +&omap->vbus_nb);
> + if (ret < 0)
> + dev_vdbg(omap->dev, "failed to register notifier for 
> USB\n");
> +
> + omap->id_nb.notifier_call = dwc3_omap_id_notifier;
> + ret = extcon_register_interest(&omap->extcon_id_dev,
> +edev->name, "USB-HOST",
> +&omap->id_nb);
> + if (ret < 0)
> + dev_vdbg(omap->dev, "failed to register notifier for 
> USB-HOST\n");
> +
> + if (extcon_get_cable_state(edev, "USB") == true)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
> + if (extcon_get_cable_state(edev, "USB-HOST") == true)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
> + }
> +
> + return 0;
> +}
> +
>  static int dwc3_omap_probe(struct platform_device *pdev)
>  {
>   struct device_node  *node = pdev->dev.of_node;
> @@ -457,7 +493,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>   struct dwc3_omap*omap;
>   struct resource *res;
>   struct device   *dev = &pdev->dev;
> - struct extcon_dev   *edev;
>   struct regulator*vbus_reg = NULL;
>  
>   int ret = -ENOMEM;
> @@ -529,31 +564,9 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>  
>   dwc3_omap_enable_irqs(omap);
>  
> - if (of_property_read_bool(node, "extcon")) {
> - edev = extcon_get_edev_by_phandle(dev, 0);
> - if (IS_ERR(edev)) {
> - dev_vdbg(dev, "couldn't get extcon device\n");
> - ret = -EPROBE_DEFER;
> - goto err2;
> - }
> -
> - omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
> - ret = extcon_register_interest(&omap->extcon_vbus_dev,
> - edev->name, "USB", &omap->vbus_nb);
> - if (ret < 0)
> - dev_vdbg(dev, "failed to register notifier for USB\n");
> - omap->id_nb.notifier_call = dwc3_omap_id_notifier;
> - ret = extcon_register_interest(&omap->extcon_id_dev, edev->name,
> -  "USB-HOST", &omap->id_nb);
> - if (ret < 0)
> - dev_vdbg(dev,
> - "failed to register notifier for USB-HOST\n");
> -
> - if (extcon_get_cable_state(edev, "USB") == true)
> - dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
> - if (extcon_get_cable_state(edev, "USB-HOST") == true)
> - dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
> - }
> + ret = dwc3_omap_extcon_register(omap);
> + if (ret < 0)
> + goto err2;
>  
>   ret = of_platform_populate(node, NULL, NULL, dev);
>   if (ret) {
> 
--
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


  1   2   >