[PATCH 4/5] usb: dwc3: isoc clean DWC3_EP_PENDING_REQUEST flag

2016-11-09 Thread Janusz Dziedzic
After we kick_transfer we should clean
DWC3_EP_PENDING_REQUEST endpoint flag.

Signed-off-by: Janusz Dziedzic 
---
 drivers/usb/dwc3/gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0cd98c0..e40d58e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1196,6 +1196,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
struct dwc3_request *req)
 
cur_uf = __dwc3_gadget_get_frame(dwc);
__dwc3_gadget_start_isoc(dwc, dep, cur_uf);
+   dep->flags &= ~DWC3_EP_PENDING_REQUEST;
}
}
return 0;
-- 
1.9.1

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


[PATCH 3/5] usb: dwc3: fix post-increment

2016-11-09 Thread Janusz Dziedzic
Use pre-increment and set -ETIMEDOUT correctly.

Signed-off-by: Janusz Dziedzic 
---
 drivers/usb/dwc3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 19bea3b..0cd98c0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -217,7 +217,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc, 
unsigned cmd, u32 param)
ret = -EINVAL;
break;
}
-   } while (timeout--);
+   } while (--timeout);
 
if (!timeout) {
ret = -ETIMEDOUT;
-- 
1.9.1

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


[PATCH 5/5] usb: dwc3: warn on once when no trbs

2016-11-09 Thread Janusz Dziedzic
Seems last time we hit few issues where
we get trb_left = 0, mainly because of
HWO bit still set in previous TRB.
Add warn on once to catch/fix such
problems much faster.

Signed-off-by: Janusz Dziedzic 
---
 drivers/usb/dwc3/gadget.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e40d58e..153491e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -938,6 +938,7 @@ static struct dwc3_trb *dwc3_ep_prev_trb(struct dwc3_ep 
*dep, u8 index)
 static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
 {
struct dwc3_trb *tmp;
+   struct dwc3 *dwc = dep->dwc;
u8  trbs_left;
 
/*
@@ -949,7 +950,8 @@ static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
 */
if (dep->trb_enqueue == dep->trb_dequeue) {
tmp = dwc3_ep_prev_trb(dep, dep->trb_enqueue);
-   if (tmp->ctrl & DWC3_TRB_CTRL_HWO)
+   if (dev_WARN_ONCE(dwc->dev, tmp->ctrl & DWC3_TRB_CTRL_HWO,
+ "%s No TRBS left\n", dep->name))
return 0;
 
return DWC3_TRB_NUM - 1;
-- 
1.9.1

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


[PATCH 1/5] usb: dwc3: decrement queued_requests

2016-11-09 Thread Janusz Dziedzic
In case we will fail to STARTTRANSFER we should
also decrement queued_requests.

Signed-off-by: Janusz Dziedzic 
---
 drivers/usb/dwc3/gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a9c1d75..840e312 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1085,6 +1085,7 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep 
*dep, u16 cmd_param)
 * here and stop, unmap, free and del each of the linked
 * requests instead of what we do now.
 */
+   dep->queued_requests--;
dwc3_gadget_giveback(dep, req, ret);
return ret;
}
-- 
1.9.1

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


[PATCH 2/5] usb: dwc3: clean TRB if STARTTRANSFER fail

2016-11-09 Thread Janusz Dziedzic
In case STARTTRANSFER will fail, clean TRB.
Seems HW in such case don't clean HWO bit.
So, without this cleanup prev_trb still have
HWO bit set.

In my case (without patch), after first START failed:
- dep->enqueue == 1
- dep->dequeue == 1
- prev_trb still have HWO set
- left_trb() == 0
No way to send more data.

Signed-off-by: Janusz Dziedzic 
---
 drivers/usb/dwc3/gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 840e312..19bea3b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1085,6 +1085,8 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep 
*dep, u16 cmd_param)
 * here and stop, unmap, free and del each of the linked
 * requests instead of what we do now.
 */
+   if (req->trb)
+   memset(req->trb, 0, sizeof(struct dwc3_trb));
dep->queued_requests--;
dwc3_gadget_giveback(dep, req, ret);
return ret;
-- 
1.9.1

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


Re: [PATCH] usb: dwc2: gadget: Update for new usb_endpoint_maxp()

2016-11-09 Thread Felipe Balbi

Hi,

John Youn  writes:
>> John Youn  writes:
 John Youn  writes:
>>> @@ -1812,17 +1812,17 @@ static u32 dwc2_hsotg_ep0_mps(unsigned int mps)
>>>   * @hsotg: The driver state.
>>>   * @ep: The index number of the endpoint
>>>   * @mps: The maximum packet size in bytes
>>> + * @mc: The multicount value
>>>   *
>>>   * Configure the maximum packet size for the given endpoint, updating
>>>   * the hardware control registers to reflect this.
>>>   */
>>>  static void dwc2_hsotg_set_ep_maxpacket(struct dwc2_hsotg *hsotg,
>>> -   unsigned int ep, unsigned int mps, unsigned int 
>>> dir_in)
>>> +   unsigned int ep, unsigned int 
>>> mps,
>>> +   unsigned int mc, unsigned int 
>>> dir_in)
>>
>> this has an odd set of arguments. You pass the ep index, mps, direction
>> and mult value, when you could just pass hsotg_ep and descriptor instead.
>
> Yes looks like we can do some simplification here. And you probably
> don't need to pass a descriptor either since it must be set in the
> usb_ep before enable.
>
> However this is also called in some contexts where a descriptor is not
> available (initialization and ep0). So we have to think about this a
> bit.
>
> I think dwc3 can make similar simplification on the
> __dwc3_gadget_ep_enable().

 __dwc3_gadget_ep_enable() takes the actual descriptors as arguments:

 static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
const struct usb_endpoint_descriptor *desc,
const struct usb_ss_ep_comp_descriptor *comp_desc,
bool modify, bool restore)

 I fail to see how much simpler we can make this. Perhaps we can turn
 bool and restore into a single argument if we use a bitfield instead of
 a bool.

>>>
>>> Hi Felipe,
>>>
>>> I mean that there shouldn't be any need to pass in descriptors with
>>> usb_ep since the ones in usb_ep should be set properly from ep enable
>>> to ep disable.

Oh, I see what you mean now :-)

>>> I think that's the case anyways.
>> 
>> __dwc3_gadget_ep_enable() is the one assigning the descriptor to the ep:
>> 
>> static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>>  const struct usb_endpoint_descriptor *desc,
>>  const struct usb_ss_ep_comp_descriptor *comp_desc,
>>  bool modify, bool restore)
>> {
>>  struct dwc3 *dwc = dep->dwc;
>>  u32 reg;
>>  int ret;
>> 
>>  if (!(dep->flags & DWC3_EP_ENABLED)) {
>>  ret = dwc3_gadget_start_config(dwc, dep);
>>  if (ret)
>>  return ret;
>>  }
>> 
>>  ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify,
>>  restore);
>>  if (ret)
>>  return ret;
>> 
>>  if (!(dep->flags & DWC3_EP_ENABLED)) {
>>  struct dwc3_trb *trb_st_hw;
>>  struct dwc3_trb *trb_link;
>> 
>>  dep->endpoint.desc = desc;
>>  dep->comp_desc = comp_desc;
>> 
>> [...]
>> 
>
> Right, but that's redundant for non-ep0 case. And the EP0 case can be
> handled globally elsewhere.
>
> The usb_ep_enable() API function takes only the usb_ep and requires
> that the descriptors are set in the usb_ep beforehand. So 'desc'
> argument in the callback function is probably not required either.
>
> I think this is correct to make it consistent. Since we don't pass the

makes sense to me :-)

> SS comp descriptor, and we don't want to keep passing more descriptors
> every time a new standard descriptor is added, such as the SSP ISO EP
> comp descriptor for 3.1.
>
> See below for a proposed change to see what I mean (not tested).
>
> Regards,
> John
>
>
> >8
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 2322863..b9903c6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -539,7 +539,6 @@ struct dwc3_ep {
>  
>   struct dwc3_trb *trb_pool;
>   dma_addr_t  trb_pool_dma;
> - const struct usb_ss_ep_comp_descriptor *comp_desc;
>   struct dwc3 *dwc;
>  
>   u32 saved_state;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index e47cba5..0fc55e89 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -576,13 +576,12 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 
> *dwc, struct dwc3_ep *dep)
>   * Caller should take care of locking
>   */
>  static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> - const struct usb_endpoint_descriptor *desc,
> - const struct usb_ss_ep_comp_descriptor *comp_desc,
>   bool modify, bool restore)
>  {

Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

2016-11-09 Thread Thomas Gleixner
On Tue, 1 Nov 2016, Lu Baolu wrote:
> +static int __init xdbc_init(void)
> +{
...
> + base = ioremap_nocache(xdbc.xhci_start, xdbc.xhci_length);
> + if (!base) {
> + xdbc_trace("failed to remap the io address\n");
> + ret = -ENOMEM;
> + goto free_and_quit;
> + }
> +
> + early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
> + xdbc_trace("early mapped IO address released\n");
> +
> + xdbc.xhci_base = base;
> + offset = xhci_find_next_ext_cap(xdbc.xhci_base, 0, XHCI_EXT_CAPS_DEBUG);
> + xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);

This is broken. What prevents that 

 - a printk is in progress on another cpu?

 - a printk happens between the unmap and storing the new base ?

Nothing AFAICT. So this needs to be done in a safe way. And just making it

oldbase = xdbc.xhci_base;
base = ioremap();
xdbc.xhci_base = base;
early_iounmap(oldbase);

does not work either because the compiler can rightfully cache
xdbc.xhci_base in the write related functions. The same issue with
xdbc.xdbc_reg.

Thanks,

tglx
--
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 v4 1/4] usb: dbc: early driver for xhci debug capability

2016-11-09 Thread Thomas Gleixner
On Tue, 1 Nov 2016, Lu Baolu wrote:
> +static void early_xdbc_write(struct console *con, const char *str, u32 n)
> +{
> + int chunk, ret;
> + static char buf[XDBC_MAX_PACKET];
> + int use_cr = 0;
> +
> + if (!xdbc.xdbc_reg)
> + return;
> + memset(buf, 0, XDBC_MAX_PACKET);

How is that dealing with reentrancy?

early_printk() does not protect against it. Peter has a patch to prevent
concurrent access from different cpus, but it cannot and will never prevent
reentrancy on the same cpu (interrupt, nmi).

Thanks,

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


Re: [PATCH 1/2] USB: cdc-acm: fix invalid user-pointer check

2016-11-09 Thread Oliver Neukum
On Tue, 2016-11-08 at 13:28 +0100, Johan Hovold wrote:
> Drop invalid user-pointer check from TIOCGSERIAL handler.
> 
> A NULL-pointer can be valid in user space and copy_to_user() takes
> care
> of sanity checking.
> 
> Signed-off-by: Johan Hovold 
Acked-by: Oliver Neukum 

Regards
Oliver


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


Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

2016-11-09 Thread Bjørn Mork
Oliver Neukum  writes:

> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>
>> These problems could very well be caused by running at SuperSpeed
>> (USB-3) instead of high speed (USB-2).
>> 
>> Is there any way to test what happens when the device is attached to 
>> the computer by a USB-2 cable?  That would prevent it from operating at 
>> SuperSpeed.
>> 
>> The main point, however, is that the proposed patch doesn't seem to
>> address the true problem, which is that the device gets suspended
>> between probes.  The patch only tries to prevent it from being
>> suspended during a probe -- which is already prevented by the USB core.
>
> But why doesn't it fail during normal operation?
>
> I suspect that its firmware requires the altsetting
>
> /* should we change control altsetting on a NCM/MBIM function? */
> if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
> data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
> ret = cdc_mbim_set_ctrlalt(dev, intf, 
> CDC_NCM_COMM_ALTSETTING_MBIM);
>
> to be set before it accepts a suspension.

Could be, but I don't think so.  The above code is effectively a noop
unless the function is a combined NCM/MBIM function.  Something I've
never seen on a Sierra Wireless device (ignoring the infamous EM7345,
which really is an Intel device).

This is a typical example of a Sierra Wireless modem configured for
MBIM:

P:  Vendor=1199 ProdID=9079 Rev= 0.06
S:  Manufacturer=Sierra Wireless, Incorporated
S:  Product=Sierra Wireless EM7455 Qualcomm Snapdragon X7 LTE-A
S:  SerialNumber=LF615126xxx
C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=500mA
A:  FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
I:* If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=(none)
E:  Ad=82(I) Atr=03(Int.) MxPS=  64 Ivl=32ms
I:* If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
I:  If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms


The control interface of plain MBIM functions will always have a single
altsetting, like the example above. So cdc_ncm_select_altsetting(intf)
returns "0", while CDC_NCM_COMM_ALTSETTING_MBIM is "1".


Just for reference, using the Intel^H^H^H^H^HEM7345 as example, this is
what a combined NCM/MBIM function looks like:


P:  Vendor=1199 ProdID=a001 Rev=17.29
S:  Manufacturer=Sierra Wireless Inc.
S:  Product=Sierra Wireless EM7345 4G LTE
S:  SerialNumber=013937000xx
C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=100mA
A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=0d Prot=00
A:  FirstIf#= 2 IfCount= 2 Cls=02(comm.) Sub=02 Prot=01
I:  If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=cdc_mbim
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
I:* If#= 0 Alt= 1 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
I:  If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 1 Alt= 2 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 2 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=01 Driver=(none)
E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
I:* If#= 3 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms


And this is what the code you quote is trying to deal with.  Note the
different subclass of altsetting 0 and 1 This is incredibly ugly.

FWIW, the modem in question cannot be an EM7345. That modem does not
have the static interface numbering oddity.  Another sign that it isn't
a true Sierra device.




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


Re: [PATCH] USB: serial: fix invalid user-pointer checks

2016-11-09 Thread Johan Hovold
On Wed, Nov 09, 2016 at 11:58:31AM +0100, Oliver Neukum wrote:
> On Tue, 2016-11-08 at 16:41 +0100, Johan Hovold wrote:
> > On Tue, Nov 08, 2016 at 03:13:13PM +0100, Oliver Neukum wrote:
> > > On Tue, 2016-11-08 at 13:26 +0100, Johan Hovold wrote:
> > > > Drop invalid user-pointer checks from ioctl handlers.
> > > > 
> > > > A NULL-pointer can be valid in user space and copy_to_user() takes
> > > > care
> > > > of sanity checking.
> > > 
> > > Shouldn't we bail out early in these cases?
> > 
> > I don't think it's worth it, and this is also the general pattern for
> > such ioctls. The added overhead for an error case like this is really
> > negligible.
> 
> OK, as you say.

Also remember that access_ok() is not a sufficient sanity check, and
would specifically fail to catch the NULL-pointer case.

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


[PATCH] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

2016-11-09 Thread Ladislav Michl
Don't complain on -EPROBE_DEFER when when no PHY found, the driver
probe will be retried later.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/host/ehci-omap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 94ea9ff..44945eb 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -181,8 +181,9 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
continue;
 
ret = PTR_ERR(phy);
-   dev_err(dev, "Can't get PHY device for port %d: %d\n",
-   i, ret);
+   if (ret != -EPROBE_DEFER)
+   dev_err(dev, "Can't get PHY device for port "
+   "%d: %d\n", i, ret);
goto err_phy;
}
 
-- 
2.1.4

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


[PATCH] usb: musb: omap2430: use dev_err

2016-11-09 Thread Ladislav Michl
Replace pr_err with dev_err to print also device name.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/musb/omap2430.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index cc12254..451b372 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -277,12 +277,12 @@ static int omap2430_musb_init(struct musb *musb)
if (status == -ENXIO)
return status;
 
-   pr_err("HS USB OTG: no transceiver configured\n");
+   dev_err(dev, "HS USB OTG: no transceiver configured\n");
return -EPROBE_DEFER;
}
 
if (IS_ERR(musb->phy)) {
-   pr_err("HS USB OTG: no PHY configured\n");
+   dev_err(dev, "HS USB OTG: no PHY configured\n");
return PTR_ERR(musb->phy);
}
musb->isr = omap2430_musb_interrupt;
@@ -301,7 +301,7 @@ static int omap2430_musb_init(struct musb *musb)
 
musb_writel(musb->mregs, OTG_INTERFSEL, l);
 
-   pr_debug("HS USB OTG: revision 0x%x, sysconfig 0x%02x, "
+   dev_dbg(dev, "HS USB OTG: revision 0x%x, sysconfig 0x%02x, "
"sysstatus 0x%x, intrfsel 0x%x, simenable  0x%x\n",
musb_readl(musb->mregs, OTG_REVISION),
musb_readl(musb->mregs, OTG_SYSCONFIG),
-- 
2.1.4

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


[PATCH 5/5] cdc-acm: handle read pipe errors

2016-11-09 Thread Ladislav Michl
Read urbs are submitted back only on success, causing read pipe
running out of urbs after few errors. No more characters can
be read from tty device then until it is reopened and no errors
are reported.
Fix that by always submitting urbs back and clearing stall on
-EPIPE.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/class/cdc-acm.c | 60 +
 drivers/usb/class/cdc-acm.h |  3 +++
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index db7bfb8..e170cc1 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -429,27 +429,41 @@ static void acm_read_bulk_callback(struct urb *urb)
dev_vdbg(>data->dev, "got urb %d, len %d, status %d\n",
rb->index, urb->actual_length, status);
 
+   set_bit(rb->index, >read_urbs_free);
+
if (!acm->dev) {
-   set_bit(rb->index, >read_urbs_free);
dev_dbg(>data->dev, "%s - disconnected\n", __func__);
return;
}
 
-   if (status) {
-   set_bit(rb->index, >read_urbs_free);
-   if ((status != -ENOENT) || (urb->actual_length == 0))
-   return;
+   switch (status) {
+   case 0:
+   usb_mark_last_busy(acm->dev);
+   acm_process_read_urb(acm, urb);
+   break;
+   case -EPIPE:
+   set_bit(EVENT_RX_STALL, >flags);
+   schedule_work(>work);
+   return;
+   case -ENOENT:
+   case -ECONNRESET:
+   case -ESHUTDOWN:
+   dev_dbg(>data->dev,
+   "%s - urb shutting down with status: %d\n",
+   __func__, status);
+   return;
+   default:
+   dev_dbg(>data->dev,
+   "%s - nonzero urb status received: %d\n",
+   __func__, status);
+   break;
}
 
-   usb_mark_last_busy(acm->dev);
-
-   acm_process_read_urb(acm, urb);
/*
 * Unthrottle may run on another CPU which needs to see events
 * in the same order. Submission has an implict barrier
 */
smp_mb__before_atomic();
-   set_bit(rb->index, >read_urbs_free);
 
/* throttle device if requested by tty */
spin_lock_irqsave(>read_lock, flags);
@@ -479,14 +493,30 @@ static void acm_write_bulk(struct urb *urb)
spin_lock_irqsave(>write_lock, flags);
acm_write_done(acm, wb);
spin_unlock_irqrestore(>write_lock, flags);
+   set_bit(EVENT_TTY_WAKEUP, >flags);
schedule_work(>work);
 }
 
 static void acm_softint(struct work_struct *work)
 {
+   int i;
struct acm *acm = container_of(work, struct acm, work);
 
-   tty_port_tty_wakeup(>port);
+   if (test_bit(EVENT_RX_STALL, >flags)) {
+   if (!(usb_autopm_get_interface(acm->data))) {
+   for (i = 0; i < acm->rx_buflimit; i++)
+   usb_kill_urb(acm->read_urbs[i]);
+   usb_clear_halt(acm->dev, acm->in);
+   acm_submit_read_urbs(acm, GFP_KERNEL);
+   usb_autopm_put_interface(acm->data);
+   }
+   clear_bit(EVENT_RX_STALL, >flags);
+   }
+
+   if (test_bit(EVENT_TTY_WAKEUP, >flags)) {
+   tty_port_tty_wakeup(>port);
+   clear_bit(EVENT_TTY_WAKEUP, >flags);
+   }
 }
 
 /*
@@ -1643,6 +1673,15 @@ static int acm_reset_resume(struct usb_interface *intf)
 
 #endif /* CONFIG_PM */
 
+static int acm_pre_reset(struct usb_interface *intf)
+{
+   struct acm *acm = usb_get_intfdata(intf);
+
+   clear_bit(EVENT_RX_STALL, >flags);
+
+   return 0;
+}
+
 #define NOKIA_PCSUITE_ACM_INFO(x) \
USB_DEVICE_AND_INTERFACE_INFO(0x0421, x, \
USB_CLASS_COMM, USB_CDC_SUBCLASS_ACM, \
@@ -1884,6 +1923,7 @@ static struct usb_driver acm_driver = {
.resume =   acm_resume,
.reset_resume = acm_reset_resume,
 #endif
+   .pre_reset =acm_pre_reset,
.id_table = acm_ids,
 #ifdef CONFIG_PM
.supports_autosuspend = 1,
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 58ddd25..1db974d 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -103,6 +103,9 @@ struct acm {
spinlock_t write_lock;
struct mutex mutex;
bool disconnected;
+   unsigned long flags;
+#  define EVENT_TTY_WAKEUP 0
+#  define EVENT_RX_STALL   1
struct usb_cdc_line_coding line;/* bits, stop, parity */
struct work_struct work;/* work queue entry for 
line discipline waking up */
unsigned int ctrlin;/* input control lines 
(DCD, DSR, RI, break, overruns) */
-- 
2.1.4

--
To unsubscribe from this list: 

[PATCH 4/5] cdc-acm: store in and out pipes in acm structure

2016-11-09 Thread Ladislav Michl
Clearing stall needs pipe descriptor, store it in acm structure.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/class/cdc-acm.c | 27 ++-
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 4a87fd8..db7bfb8 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1349,8 +1349,15 @@ static int acm_probe(struct usb_interface *intf,
spin_lock_init(>read_lock);
mutex_init(>mutex);
acm->is_int_ep = usb_endpoint_xfer_int(epread);
-   if (acm->is_int_ep)
+   if (acm->is_int_ep) {
acm->bInterval = epread->bInterval;
+   acm->in = usb_rcvintpipe(usb_dev, epread->bEndpointAddress);
+   } else
+   acm->in = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress);
+   if (usb_endpoint_xfer_int(epwrite))
+   acm->out = usb_sndintpipe(usb_dev, epwrite->bEndpointAddress);
+   else
+   acm->out = usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress);
tty_port_init(>port);
acm->port.ops = _port_ops;
init_usb_anchor(>delayed);
@@ -1386,16 +1393,12 @@ static int acm_probe(struct usb_interface *intf,
urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
urb->transfer_dma = rb->dma;
if (acm->is_int_ep) {
-   usb_fill_int_urb(urb, acm->dev,
-usb_rcvintpipe(usb_dev, 
epread->bEndpointAddress),
-rb->base,
+   usb_fill_int_urb(urb, acm->dev, acm->in, rb->base,
 acm->readsize,
 acm_read_bulk_callback, rb,
 acm->bInterval);
} else {
-   usb_fill_bulk_urb(urb, acm->dev,
- usb_rcvbulkpipe(usb_dev, 
epread->bEndpointAddress),
- rb->base,
+   usb_fill_bulk_urb(urb, acm->dev, acm->in, rb->base,
  acm->readsize,
  acm_read_bulk_callback, rb);
}
@@ -1411,12 +1414,10 @@ static int acm_probe(struct usb_interface *intf,
goto alloc_fail7;
 
if (usb_endpoint_xfer_int(epwrite))
-   usb_fill_int_urb(snd->urb, usb_dev,
-   usb_sndintpipe(usb_dev, 
epwrite->bEndpointAddress),
+   usb_fill_int_urb(snd->urb, usb_dev, acm->out,
NULL, acm->writesize, acm_write_bulk, snd, 
epwrite->bInterval);
else
-   usb_fill_bulk_urb(snd->urb, usb_dev,
-   usb_sndbulkpipe(usb_dev, 
epwrite->bEndpointAddress),
+   usb_fill_bulk_urb(snd->urb, usb_dev, acm->out,
NULL, acm->writesize, acm_write_bulk, snd);
snd->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
if (quirks & SEND_ZERO_PACKET)
@@ -1488,8 +1489,8 @@ static int acm_probe(struct usb_interface *intf,
}
 
if (quirks & CLEAR_HALT_CONDITIONS) {
-   usb_clear_halt(usb_dev, usb_rcvbulkpipe(usb_dev, 
epread->bEndpointAddress));
-   usb_clear_halt(usb_dev, usb_sndbulkpipe(usb_dev, 
epwrite->bEndpointAddress));
+   usb_clear_halt(usb_dev, acm->in);
+   usb_clear_halt(usb_dev, acm->out);
}
 
return 0;
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 1f1eabf..58ddd25 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -83,6 +83,7 @@ struct acm {
struct usb_device *dev; /* the corresponding 
usb device */
struct usb_interface *control;  /* control interface */
struct usb_interface *data; /* data interface */
+   unsigned in, out;   /* i/o pipes */
struct tty_port port;   /* our tty port data */
struct urb *ctrlurb;/* urbs */
u8 *ctrl_buffer;/* buffers of urbs */
-- 
2.1.4

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


RE: [PATCH 02/30] usb: dwc2: Update DMA descriptor structure

2016-11-09 Thread David Laight
From: John Youn
> Sent: 08 November 2016 22:30
...
> >> Also make it __packed.
> >
> > why are you making it packed? Does it _have_ to be packed? If it must,
> > why wasn't it before?
> 
> We want to guarantee that it reflects the exact descriptor structure
> in memory, so probably yes. It was overlooked before and worked
> because the compiler likely packed it anyways. This is just to make
> sure it does.
...

That isn't all that 'packed' means.

'packed' also tells the compiler to access the structure with instructions that
will work if it isn't on the 'natural' boundary.

If you want to ensure the structure is the right size (pretty pointless here)
then add a compile time assert against the structure size.

David



[PATCH] usb: musb: don't complain on -EPROBE_DEFER when initializing controller

2016-11-09 Thread Ladislav Michl
Don't complain on -EPROBE_DEFER when initializing controller,
the driver probe will be retried later.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/musb/musb_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 27dadc0..77fd97b 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2329,8 +2329,9 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
musb_platform_exit(musb);
 
 fail1:
-   dev_err(musb->controller,
-   "musb_init_controller failed with status %d\n", status);
+   if (status != -EPROBE_DEFER)
+   dev_err(musb->controller,
+   "%s failed with status %d\n", __func__, status);
 
musb_free(musb);
 
-- 
2.1.4

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


[PATCH] usb: musb: omap2430: make complain on -EPROBE_DEFER dev_dbg

2016-11-09 Thread Ladislav Michl
There is no point having this complaint to be dev_err as it is just adding
noise to bootlog.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/musb/omap2430.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 451b372..7c9aa5e 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -277,7 +277,7 @@ static int omap2430_musb_init(struct musb *musb)
if (status == -ENXIO)
return status;
 
-   dev_err(dev, "HS USB OTG: no transceiver configured\n");
+   dev_dbg(dev, "HS USB OTG: no transceiver configured\n");
return -EPROBE_DEFER;
}
 
-- 
2.1.4


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


Re: [PATCH] USB: serial: fix invalid user-pointer checks

2016-11-09 Thread Oliver Neukum
On Tue, 2016-11-08 at 16:41 +0100, Johan Hovold wrote:
> On Tue, Nov 08, 2016 at 03:13:13PM +0100, Oliver Neukum wrote:
> > On Tue, 2016-11-08 at 13:26 +0100, Johan Hovold wrote:
> > > Drop invalid user-pointer checks from ioctl handlers.
> > > 
> > > A NULL-pointer can be valid in user space and copy_to_user() takes
> > > care
> > > of sanity checking.
> > 
> > Shouldn't we bail out early in these cases?
> 
> I don't think it's worth it, and this is also the general pattern for
> such ioctls. The added overhead for an error case like this is really
> negligible.

OK, as you say.

Regards
Oliver


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


[PATCH 2/5] cdc-acm: avoid interface_to_usbdev call

2016-11-09 Thread Ladislav Michl
Pointer to usb_device is already stored in acm structure.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/class/cdc-acm.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b511289..8e2d82c 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1089,19 +1089,17 @@ static void acm_write_buffers_free(struct acm *acm)
 {
int i;
struct acm_wb *wb;
-   struct usb_device *usb_dev = interface_to_usbdev(acm->control);
 
for (wb = >wb[0], i = 0; i < ACM_NW; i++, wb++)
-   usb_free_coherent(usb_dev, acm->writesize, wb->buf, wb->dmah);
+   usb_free_coherent(acm->dev, acm->writesize, wb->buf, wb->dmah);
 }
 
 static void acm_read_buffers_free(struct acm *acm)
 {
-   struct usb_device *usb_dev = interface_to_usbdev(acm->control);
int i;
 
for (i = 0; i < acm->rx_buflimit; i++)
-   usb_free_coherent(usb_dev, acm->readsize,
+   usb_free_coherent(acm->dev, acm->readsize,
  acm->read_buffers[i].base, acm->read_buffers[i].dma);
 }
 
@@ -1535,7 +1533,6 @@ static void stop_data_traffic(struct acm *acm)
 static void acm_disconnect(struct usb_interface *intf)
 {
struct acm *acm = usb_get_intfdata(intf);
-   struct usb_device *usb_dev = interface_to_usbdev(intf);
struct tty_struct *tty;
int i;
 
@@ -1573,7 +1570,7 @@ static void acm_disconnect(struct usb_interface *intf)
for (i = 0; i < acm->rx_buflimit; i++)
usb_free_urb(acm->read_urbs[i]);
acm_write_buffers_free(acm);
-   usb_free_coherent(usb_dev, acm->ctrlsize, acm->ctrl_buffer, 
acm->ctrl_dma);
+   usb_free_coherent(acm->dev, acm->ctrlsize, acm->ctrl_buffer, 
acm->ctrl_dma);
acm_read_buffers_free(acm);
 
if (!acm->combined_interfaces)
-- 
2.1.4

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


[PATCH 3/5] cdc-acm: refactor killing urbs

2016-11-09 Thread Ladislav Michl
Move urb killing code into separate function and use it
instead of copying that code pattern over.

Signed-off-by: Ladislav Michl 
---
 drivers/usb/class/cdc-acm.c | 43 ---
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 8e2d82c..4a87fd8 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -158,6 +158,17 @@ static inline int acm_set_control(struct acm *acm, int 
control)
 #define acm_send_break(acm, ms) \
acm_ctrl_msg(acm, USB_CDC_REQ_SEND_BREAK, ms, NULL, 0)
 
+static void acm_kill_urbs(struct acm *acm)
+{
+   int i;
+
+   usb_kill_urb(acm->ctrlurb);
+   for (i = 0; i < ACM_NW; i++)
+   usb_kill_urb(acm->wb[i].urb);
+   for (i = 0; i < acm->rx_buflimit; i++)
+   usb_kill_urb(acm->read_urbs[i]);
+}
+
 /*
  * Write buffer management.
  * All of these assume proper locks taken by the caller.
@@ -607,7 +618,6 @@ static void acm_port_shutdown(struct tty_port *port)
struct acm *acm = container_of(port, struct acm, port);
struct urb *urb;
struct acm_wb *wb;
-   int i;
 
/*
 * Need to grab write_lock to prevent race with resume, but no need to
@@ -629,11 +639,7 @@ static void acm_port_shutdown(struct tty_port *port)
usb_autopm_put_interface_async(acm->control);
}
 
-   usb_kill_urb(acm->ctrlurb);
-   for (i = 0; i < ACM_NW; i++)
-   usb_kill_urb(acm->wb[i].urb);
-   for (i = 0; i < acm->rx_buflimit; i++)
-   usb_kill_urb(acm->read_urbs[i]);
+   acm_kill_urbs(acm);
 }
 
 static void acm_tty_cleanup(struct tty_struct *tty)
@@ -1517,24 +1523,10 @@ static int acm_probe(struct usb_interface *intf,
return rv;
 }
 
-static void stop_data_traffic(struct acm *acm)
-{
-   int i;
-
-   usb_kill_urb(acm->ctrlurb);
-   for (i = 0; i < ACM_NW; i++)
-   usb_kill_urb(acm->wb[i].urb);
-   for (i = 0; i < acm->rx_buflimit; i++)
-   usb_kill_urb(acm->read_urbs[i]);
-
-   cancel_work_sync(>work);
-}
-
 static void acm_disconnect(struct usb_interface *intf)
 {
struct acm *acm = usb_get_intfdata(intf);
struct tty_struct *tty;
-   int i;
 
/* sibling interface is already cleaning up */
if (!acm)
@@ -1560,15 +1552,11 @@ static void acm_disconnect(struct usb_interface *intf)
tty_kref_put(tty);
}
 
-   stop_data_traffic(acm);
+   acm_kill_urbs(acm);
+   cancel_work_sync(>work);
 
tty_unregister_device(acm_tty_driver, acm->minor);
 
-   usb_free_urb(acm->ctrlurb);
-   for (i = 0; i < ACM_NW; i++)
-   usb_free_urb(acm->wb[i].urb);
-   for (i = 0; i < acm->rx_buflimit; i++)
-   usb_free_urb(acm->read_urbs[i]);
acm_write_buffers_free(acm);
usb_free_coherent(acm->dev, acm->ctrlsize, acm->ctrl_buffer, 
acm->ctrl_dma);
acm_read_buffers_free(acm);
@@ -1599,7 +1587,8 @@ static int acm_suspend(struct usb_interface *intf, 
pm_message_t message)
if (cnt)
return 0;
 
-   stop_data_traffic(acm);
+   acm_kill_urbs(acm);
+   cancel_work_sync(>work);
 
return 0;
 }
-- 
2.1.4

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


Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

2016-11-09 Thread Oliver Neukum
On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:

> These problems could very well be caused by running at SuperSpeed
> (USB-3) instead of high speed (USB-2).
> 
> Is there any way to test what happens when the device is attached to 
> the computer by a USB-2 cable?  That would prevent it from operating at 
> SuperSpeed.
> 
> The main point, however, is that the proposed patch doesn't seem to
> address the true problem, which is that the device gets suspended
> between probes.  The patch only tries to prevent it from being
> suspended during a probe -- which is already prevented by the USB core.

But why doesn't it fail during normal operation?

I suspect that its firmware requires the altsetting

/* should we change control altsetting on a NCM/MBIM function? */
if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
ret = cdc_mbim_set_ctrlalt(dev, intf, 
CDC_NCM_COMM_ALTSETTING_MBIM);

to be set before it accepts a suspension.

Regards
Oliver


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


[PATCH 1/5] cdc-acm: reindent log messages

2016-11-09 Thread Ladislav Michl
Signed-off-by: Ladislav Michl 
---
 drivers/usb/class/cdc-acm.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 78f0f85..b511289 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -133,8 +133,8 @@ static int acm_ctrl_msg(struct acm *acm, int request, int 
value,
buf, len, 5000);
 
dev_dbg(>control->dev,
-   "%s - rq 0x%02x, val %#x, len %#x, result %d\n",
-   __func__, request, value, len, retval);
+   "%s - rq 0x%02x, val %#x, len %#x, result %d\n",
+   __func__, request, value, len, retval);
 
usb_autopm_put_interface(acm->control);
 
@@ -291,13 +291,13 @@ static void acm_ctrl_irq(struct urb *urb)
case -ESHUTDOWN:
/* this urb is terminated, clean up */
dev_dbg(>control->dev,
-   "%s - urb shutting down with status: %d\n",
-   __func__, status);
+   "%s - urb shutting down with status: %d\n",
+   __func__, status);
return;
default:
dev_dbg(>control->dev,
-   "%s - nonzero urb status received: %d\n",
-   __func__, status);
+   "%s - nonzero urb status received: %d\n",
+   __func__, status);
goto exit;
}
 
@@ -306,16 +306,16 @@ static void acm_ctrl_irq(struct urb *urb)
data = (unsigned char *)(dr + 1);
switch (dr->bNotificationType) {
case USB_CDC_NOTIFY_NETWORK_CONNECTION:
-   dev_dbg(>control->dev, "%s - network connection: %d\n",
-   __func__, dr->wValue);
+   dev_dbg(>control->dev,
+   "%s - network connection: %d\n", __func__, dr->wValue);
break;
 
case USB_CDC_NOTIFY_SERIAL_STATE:
newctrl = get_unaligned_le16(data);
 
if (!acm->clocal && (acm->ctrlin & ~newctrl & ACM_CTRL_DCD)) {
-   dev_dbg(>control->dev, "%s - calling hangup\n",
-   __func__);
+   dev_dbg(>control->dev,
+   "%s - calling hangup\n", __func__);
tty_port_tty_hangup(>port, false);
}
 
@@ -357,8 +357,8 @@ static void acm_ctrl_irq(struct urb *urb)
 exit:
retval = usb_submit_urb(urb, GFP_ATOMIC);
if (retval && retval != -EPERM)
-   dev_err(>control->dev, "%s - usb_submit_urb failed: %d\n",
-   __func__, retval);
+   dev_err(>control->dev,
+   "%s - usb_submit_urb failed: %d\n", __func__, retval);
 }
 
 static int acm_submit_read_urb(struct acm *acm, int index, gfp_t mem_flags)
@@ -372,8 +372,8 @@ static int acm_submit_read_urb(struct acm *acm, int index, 
gfp_t mem_flags)
if (res) {
if (res != -EPERM) {
dev_err(>data->dev,
-   "urb %d failed submission with %d\n",
-   index, res);
+   "urb %d failed submission with %d\n",
+   index, res);
}
set_bit(index, >read_urbs_free);
return res;
@@ -416,8 +416,7 @@ static void acm_read_bulk_callback(struct urb *urb)
int status = urb->status;
 
dev_vdbg(>data->dev, "got urb %d, len %d, status %d\n",
-   rb->index, urb->actual_length,
-   status);
+   rb->index, urb->actual_length, status);
 
if (!acm->dev) {
set_bit(rb->index, >read_urbs_free);
@@ -837,8 +836,8 @@ static int acm_tty_break_ctl(struct tty_struct *tty, int 
state)
 
retval = acm_send_break(acm, state ? 0x : 0);
if (retval < 0)
-   dev_dbg(>control->dev, "%s - send break failed\n",
-   __func__);
+   dev_dbg(>control->dev,
+   "%s - send break failed\n", __func__);
return retval;
 }
 
-- 
2.1.4

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


Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue

2016-11-09 Thread Tony Lindgren
* Tony Lindgren  [161108 18:26]:
> * Johan Hovold  [161108 12:03]:
> +int musb_queue_resume_work(struct musb *musb,
> +int (*callback)(struct musb *musb, void *data),
> +void *data)
> +{
> + struct musb_pending_work *w;
> + unsigned long flags;
> + int error;
> +
> + if (WARN_ON(!callback))
> + return -EINVAL;
> +
> + if (pm_runtime_active(musb->controller))
> + return callback(musb, data);
> +
> + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC);
> + if (!w)
> + return -ENOMEM;
> +
> + w->callback = callback;
> + w->data = data;
> + spin_lock_irqsave(>list_lock, flags);
> + if (musb->is_runtime_suspended) {
> + list_add_tail(>node, >pending_list);
> + error = 0;
> + } else {
> + dev_err(musb->controller, "could not add resume work %p\n",
> + callback);
> + devm_kfree(musb->controller, w);
> + error = -EINPROGRESS;
> + }
> + spin_unlock_irqrestore(>list_lock, flags);
> +
> + if (pm_runtime_active(musb->controller))
> + return musb_run_resume_work(musb);
> +
> + return error;
> +}

Hmm I think we can also leave out musb_run_resume_work() at the end,
we should not hit that case any longer.

Regards,

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


[PATCH v4] USB hub_probe: rework ugly goto-into-compound-statement

2016-11-09 Thread Eugene Korenevsky
Rework smelling code (goto inside compound statement). Perhaps this is
legacy. Anyway such code is not appropriate for Linux kernel.

Changes since v3: fix typo
Changes since v2: extract the code to static function
Changes since v1: fix spaces instead of tab, add missing 'Signed-off-by'

Signed-off-by: Eugene Korenevsky 
---
 drivers/usb/core/hub.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cbb1467..b770c8d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1722,10 +1722,25 @@ static void hub_disconnect(struct usb_interface *intf)
kref_put(>kref, hub_release);
 }
 
+static int hub_check_descriptor_sanity(struct usb_host_interface *desc)
+{
+   /* Some hubs have a subclass of 1, which AFAICT according to the */
+   /*  specs is not defined, but it works */
+   if (desc->desc.bInterfaceSubClass != 0 &&
+   desc->desc.bInterfaceSubClass != 1)
+   return 0;
+
+   /* Multiple endpoints? What kind of mutant ninja-hub is this? */
+   if (desc->desc.bNumEndpoints != 1)
+   return 0;
+
+   /* If it's not an interrupt in endpoint, we'd better punt! */
+   return usb_endpoint_is_int_in(>endpoint[0].desc);
+}
+
 static int hub_probe(struct usb_interface *intf, const struct usb_device_id 
*id)
 {
struct usb_host_interface *desc;
-   struct usb_endpoint_descriptor *endpoint;
struct usb_device *hdev;
struct usb_hub *hub;
 
@@ -1800,25 +1815,11 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
}
 #endif
 
-   /* Some hubs have a subclass of 1, which AFAICT according to the */
-   /*  specs is not defined, but it works */
-   if ((desc->desc.bInterfaceSubClass != 0) &&
-   (desc->desc.bInterfaceSubClass != 1)) {
-descriptor_error:
+   if (!hub_check_descriptor_sanity(desc)) {
dev_err(>dev, "bad descriptor, ignoring hub\n");
return -EIO;
}
 
-   /* Multiple endpoints? What kind of mutant ninja-hub is this? */
-   if (desc->desc.bNumEndpoints != 1)
-   goto descriptor_error;
-
-   endpoint = >endpoint[0].desc;
-
-   /* If it's not an interrupt in endpoint, we'd better punt! */
-   if (!usb_endpoint_is_int_in(endpoint))
-   goto descriptor_error;
-
/* We found a hub */
dev_info(>dev, "USB hub found\n");
 
@@ -1845,7 +1846,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
hub->quirk_check_port_auto_suspend = 1;
 
-   if (hub_configure(hub, endpoint) >= 0)
+   if (hub_configure(hub, >endpoint[0].desc) >= 0)
return 0;
 
hub_disconnect(intf);
-- 
2.10.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: Multiple chatty devices on Intel 5 Series/3400 USB2 EHCI controller act erratic

2016-11-09 Thread Alan Stern
On Tue, 8 Nov 2016 sonofa...@openmailbox.org wrote:

> 
> > I never had them in the first place.
> Yes I know, the question was asked to the thread creator!

Why didn't you say so (given that you were replying to a message that I
sent)?

> Why did the messages move to a new subject and were not added to the 
> original subject?

I don't know.  What new subject are you referring to?

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 v2 3/5] usb: dwc3: gadget: Write the event count in interrupt

2016-11-09 Thread Janusz Dziedzic
On 9 November 2016 at 09:05, Felipe Balbi  wrote:
>
> Hi,
>
> John Youn  writes:
>>> +   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
>>> +
> After that evt->buf[lpos, lpos + count] seems goes back to HW, so
> thread should not rely on this?
>
> Or I miss something?

 Hi,

 Yes, you're right. That's a possibility and to be safe we can cache
 those.

 We're relying on the same mechanism that keeps the event buffer from
 becoming full. Since it is just as (un)likely a possibility. That's
 why you must size the event buffer appropriately taking into account
 your system's latency, etc.

 And if the event buffer becomes full, that indicates something really
 wrong happened in the controller. You shouldn't be getting 100's of
 events at a time.

 But yes, we can address the overwriting issue.

 Either:

 1) Cache all incoming events. Requires double the event buffer space.

 2) Cache just one event and write back only '4' during hard
interrupt. Then in threaded interrupt read the one event from
cache, and process the remaining events from buffer as before.
Doesn't require a large cache, but a bit messier.

 Any other thoughts or ideas?
>>>
>>> do you really need to clear at least one event for this?
>>>
>>
>> Unfortunately yes. That's how the workaround works. We need to write
>> this during IMOD to de-assert the interrupt since the mask bit doesn't
>> work.
>
> okay. Then we should cache and clear a single event.
>
Cache all incoming events looks better for me, while we don't need
care of Vendor Test Event (12Bytes) here - and we will always handle
this correctly and can add simple implementation for that in bottom
half.
In case of single event cache, VTE handling will be much harder/ugly
... - while we have to check if cache 4 or 12 bytes.
Anyway is the VTE case at all? We don't support this currently and
don't have an issues ...

BR
Janusz

>> We could do this only for 3.00a as well.
>
> if we do this only for 3.00a then the code will look odd :-) It
> shouldn't cause any problems for any other revisions
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: C-Media USB speakers do not work on AMD USB 2.0 port

2016-11-09 Thread Alan Stern
On Tue, 8 Nov 2016 sonofa...@openmailbox.org wrote:

> My C-Media USB speakers do not work on Ubuntu 16.10 with 4.8.0 on my AMD 
> KABINI USB 2.0 port. Using Ubuntu 16.04 with 4.4.0 on the same machine 
> at the same port with the same cable fixes the issue.
> If I put the speakers on any USB 3.0 connector then it works both in 
> 16.10 and 16.04!!
> 
> Why do I get a different driver on the same port with a different 
> kernel?

Good question.  I don't know the answer; it may be hidden in a part 
of the system log that you didn't post.

> Here is what gets logged normally,
> 
> on Ubuntu 16.10:
> [   50.252683] usb 4-1: new full-speed USB device number 2 using 
> ohci-pci
> [   50.438712] usb 4-1: New USB device found, idVendor=0d8c, 
> idProduct=0103
> [   50.438719] usb 4-1: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=0
> [   50.438725] usb 4-1: Product: C-Media USB Audio
> [   50.438731] usb 4-1: Manufacturer: C-Media INC.
> [   50.542693] usb 4-1: Warning! Unlikely big volume range (=9472), 
> cval->res is probably wrong.
> [   50.542700] usb 4-1: [13] FU [PCM Playback Volume] ch = 2, val = 
> -9473/-1/1
> [   50.543246] usbcore: registered new interface driver snd-usb-audio
> [   62.867422] ohci-pci :00:13.0: HcDoneHead not written back; 
> disabled
> [   62.867435] ohci-pci :00:13.0: HC died; cleaning up
> [   62.867489] usb 4-1: USB disconnect, device number 2

This is a known bug in the 4.8 kernel, caused by changes to the timer 
code.  A workaround and a proper fix have both been added to the 4.9-rc 
series and will be part of the next 4.8.stable release.

Alan Stern


> on Ubuntu 16.04:
> [ 1168.662575] usb 7-4: new full-speed USB device number 4 using
> xhci_hcd
> [ 1168.833911] usb 7-4: New USB device found, idVendor=0d8c,
> idProduct=0103
> [ 1168.833921] usb 7-4: New USB device strings: Mfr=1, Product=2,
> SerialNumber=0
> [ 1168.833927] usb 7-4: Product: C-Media USB Audio
> [ 1168.833931] usb 7-4: Manufacturer: C-Media INC.
> [ 1168.867900] usb 7-4: Warning! Unlikely big volume range (=9472),
> cval->res is probably wrong.
> [ 1168.867910] usb 7-4: [13] FU [PCM Playback Volume] ch = 2, val =
> -9473/-1/1

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


Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue

2016-11-09 Thread Johan Hovold
On Tue, Nov 08, 2016 at 06:26:07PM -0700, Tony Lindgren wrote:
> * Johan Hovold  [161108 12:03]:
> > On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote:
> > > * Johan Hovold  [161108 10:09]:
> > > > On Mon, Nov 07, 2016 at 02:50:18PM -0700, Tony Lindgren wrote:
> > > > > @@ -2604,6 +2669,9 @@ static int musb_resume(struct device *dev)
> > > > >   mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | 
> > > > > MUSB_DEVCTL_LSDEV;
> > > > >   if ((devctl & mask) != (musb->context.devctl & mask))
> > > > >   musb->port1_status = 0;
> > > > > +
> > > > > + schedule_delayed_work(>pending_resume_work, 0);
> > > > > +
> > > > 
> > > > The interactions with system suspend looks a bit suspicious. It seems
> > > > you need to drain any pending resume work on suspend for example.
> > > > 
> > > > And then the above should not be needed, right?
> > > 
> > > Hmm that's an interesting idea. I think that would allow us to get
> > > rid of the delayed work if we check the list both on resume and
> > > suspend :)
> > 
> > Note that I was referring to draining the runtime-resume work on system
> > suspend above. I think we still want the delayed work considering that
> > we could stay active for long periods of time.
> 
> It seems it's the delayed work causing these race issues, so I'd like
> to avoid it. I think we can do it all from PM runtime resume and
> checking at the end of musb_queue_resume_work(). We need to introduce
> is_runtime_suspended though. This is along the lines of what we have
> in Documentation/power/runtime_pm.txt if you search for completion
> there.

Sure, if you can avoid processing the deferred work also at
runtime-suspend then getting rid of the delayed work would be preferred.

> Below is a version doing things without delayed work, care to check
> again if you still see races there?

I believe there are still some issues however.

> > > > In fact, the dsps timer must also be cancelled on suspend, or you could
> > > > end up calling dsps_check_status() while suspended (it is currently not
> > > > cancelled until the parent device is suspended, which could be too
> > > > late).
> > > 
> > > And then this should no longer be an issue either.
> > 
> > It would still be an issue as a system-suspending device could already
> > have been runtime-resumed so that dsps_check_status() would be called
> > directly from the timer function.
> 
> The glue layers should do pm_runtime_get_sync(musb->controller) which
> dsps glue already does. So that's the musb_core.c device instance. And
> looks like we have dsps_suspend() call del_timer_sync(>timer)
> already. I think we're safe here.

But the point is that the controller might be RPM_ACTIVE if the
controller was already runtime resumed when it is system suspended.

Since this (and the previous) patch run the work directly from the timer
callback if active, it could end up accessing the controller after it
has been system suspended. Specifically, stopping the timer in the glue
(parent) suspend callback is too late to avoid this.

pm_runtime_get_sync(musb->controller);
musb_runtime_resume()
musb_restore_context();

...

musb_suspend()
musb_save_context();

otg_timer()
pm_runtime_get();
if (pm_runtime_active(musb->controller))
dsps_check_status();
pm_runtime_put_autosuspend();

dsps_suspend()
del_timer_sync();

> 8< --
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren 
> Date: Wed, 2 Nov 2016 19:59:05 -0700
> Subject: [PATCH] usb: musb: Fix sleeping function called from invalid
>  context for hdrc glue
> 
> Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
> glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer
> that runs in softirq context. That causes a "BUG: sleeping function called
> from invalid context" every time when polling the cable status:
> 
> [] (__might_sleep) from [] (__pm_runtime_resume+0x9c/0xa0)
> [] (__pm_runtime_resume) from [] (otg_timer+0x3c/0x254)
> [] (otg_timer) from [] (call_timer_fn+0xfc/0x41c)
> [] (call_timer_fn) from [] (expire_timers+0x120/0x210)
> [] (expire_timers) from [] (run_timer_softirq+0xa4/0xdc)
> [] (run_timer_softirq) from [] (__do_softirq+0x12c/0x594)
> 
> I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled.
> And looks like also musb_gadget_queue() suffers from the same problem.
> 
> Let's fix the issue by using a list of delayed work then call it on
> resume. Note that we want to do this only when musb core and it's
> parent devices are awake as noted by Johan Hovold .
> 
> Later on we may be able to remove other delayed work in the musb driver
> and just do it from pending_resume_work. But this should be done only
> for delayed work that 

Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue

2016-11-09 Thread Johan Hovold
On Wed, Nov 09, 2016 at 08:34:10AM -0700, Tony Lindgren wrote:
> * Tony Lindgren  [161108 18:26]:
> > * Johan Hovold  [161108 12:03]:
> > +int musb_queue_resume_work(struct musb *musb,
> > +  int (*callback)(struct musb *musb, void *data),
> > +  void *data)
> > +{
> > +   struct musb_pending_work *w;
> > +   unsigned long flags;
> > +   int error;
> > +
> > +   if (WARN_ON(!callback))
> > +   return -EINVAL;
> > +
> > +   if (pm_runtime_active(musb->controller))
> > +   return callback(musb, data);
> > +
> > +   w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC);
> > +   if (!w)
> > +   return -ENOMEM;
> > +
> > +   w->callback = callback;
> > +   w->data = data;
> > +   spin_lock_irqsave(>list_lock, flags);
> > +   if (musb->is_runtime_suspended) {
> > +   list_add_tail(>node, >pending_list);
> > +   error = 0;
> > +   } else {
> > +   dev_err(musb->controller, "could not add resume work %p\n",
> > +   callback);
> > +   devm_kfree(musb->controller, w);
> > +   error = -EINPROGRESS;
> > +   }
> > +   spin_unlock_irqrestore(>list_lock, flags);
> > +
> > +   if (pm_runtime_active(musb->controller))
> > +   return musb_run_resume_work(musb);
> > +
> > +   return error;
> > +}
> 
> Hmm I think we can also leave out musb_run_resume_work() at the end,
> we should not hit that case any longer.

That seems to be the case, yes. But we still need to process that work
somehow also when racing with runtime_resume() (e.g. here or at every
call site).

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


Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue

2016-11-09 Thread Johan Hovold
On Wed, Nov 09, 2016 at 04:39:53PM +0100, Johan Hovold wrote:
> On Tue, Nov 08, 2016 at 06:26:07PM -0700, Tony Lindgren wrote:

> > @@ -2065,6 +2147,7 @@ musb_init_controller(struct device *dev, int nIrq, 
> > void __iomem *ctrl)
> > }
> >  
> > spin_lock_init(>lock);
> > +   spin_lock_init(>list_lock);
> > musb->board_set_power = plat->set_power;
> > musb->min_power = plat->min_power;
> > musb->ops = plat->platform_ops;
> > @@ -2556,6 +2639,7 @@ static int musb_suspend(struct device *dev)
> > struct musb *musb = dev_to_musb(dev);
> > unsigned long   flags;
> >  
> > +   WARN_ON(!list_empty(>pending_list));
> 
> And this also depends on anyone attempting to queue work having first
> gotten an RPM reference (so that driver core runtime resumes the device
> before calling suspend()).
> 
> But no, there's actually still a window were this could be false when
> work is queued while runtime resuming (and eventually is executed from
> musb_queue_resume_work()).

As you just pointed out that can actually never happen and corresponding
code can be removed from musb_queue_resume_work(), so scrap this last
paragraph too. :)

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


[PATCH 1/2] dma: cppi41: Fix list not empty warning on module removal

2016-11-09 Thread Tony Lindgren
If musb controller is configured with USB peripherals and we have
enumerated with a USB host, we can get warnings on removal of the
modules:

WARNING: CPU: 0 PID: 1269 at drivers/dma/cppi41.c:391
cppi41_dma_free_chan_resources

Fix the issue by adding the missing pm_runtime_get to
cppi41_dma_free_chan_resources to make sure the pending work
list is cleared on removal.

Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Signed-off-by: Tony Lindgren 
---
 drivers/dma/cppi41.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -1072,7 +1072,12 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 static int cppi41_dma_remove(struct platform_device *pdev)
 {
struct cppi41_dd *cdd = platform_get_drvdata(pdev);
+   int error;
 
+   error = pm_runtime_get_sync(>dev);
+   if (error < 0)
+   dev_err(>dev, "%s could not pm_runtime_get: %i\n",
+   __func__, error);
of_dma_controller_free(pdev->dev.of_node);
dma_async_device_unregister(>ddev);
 
-- 
2.10.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/2] Two cpp41 pm runtime found when testing with usb

2016-11-09 Thread Tony Lindgren
Hi,

I found two pm runtime issues when testing with usb on beaglebone.

In the am335x case cppi41 and two instances of musb controller share
the same interconnect wrapper module, so any pm issues with musb or
cppi41 will keep the interconnect wrapper module busy.

Regards,

Tony


Tony Lindgren (2):
  dma: cppi41: Fix list not empty warning on module removal
  dma: cppi41: Fix unpaired pm runtime when only a USB hub is connected

 drivers/dma/cppi41.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

-- 
2.10.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/2] dma: cppi41: Fix unpaired pm runtime when only a USB hub is connected

2016-11-09 Thread Tony Lindgren
On am335x with musb host we can end up with unpaired pm runtime calls
if a hub with no devices is connected and disconnected.

This is because of the conditional pm runtime calls which are always
a bad idea. Let's fix the issue by making them unconditional and
paired in each function.

Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Signed-off-by: Tony Lindgren 
---
 drivers/dma/cppi41.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -318,6 +318,11 @@ static irqreturn_t cppi41_irq(int irq, void *data)
while (val) {
u32 desc, len;
 
+   status = pm_runtime_get(cdd->ddev.dev);
+   if (status < 0)
+   dev_err(cdd->ddev.dev, "%s pm runtime get: 
%i\n",
+   __func__, status);
+
q_num = __fls(val);
val &= ~(1 << q_num);
q_num += 32 * i;
@@ -338,7 +343,6 @@ static irqreturn_t cppi41_irq(int irq, void *data)
dma_cookie_complete(>txd);
dmaengine_desc_get_callback_invoke(>txd, NULL);
 
-   /* Paired with cppi41_dma_issue_pending */
pm_runtime_mark_last_busy(cdd->ddev.dev);
pm_runtime_put_autosuspend(cdd->ddev.dev);
}
@@ -460,7 +464,6 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
struct cppi41_dd *cdd = c->cdd;
int error;
 
-   /* PM runtime paired with dmaengine_desc_get_callback_invoke */
error = pm_runtime_get(cdd->ddev.dev);
if ((error != -EINPROGRESS) && error < 0) {
dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
@@ -473,6 +476,9 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
push_desc_queue(c);
else
pending_desc(c);
+
+   pm_runtime_mark_last_busy(cdd->ddev.dev);
+   pm_runtime_put_autosuspend(cdd->ddev.dev);
 }
 
 static u32 get_host_pd0(u32 length)
-- 
2.10.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 02/30] usb: dwc2: Update DMA descriptor structure

2016-11-09 Thread John Youn
On 11/9/2016 4:18 AM, David Laight wrote:
> From: John Youn
>> Sent: 08 November 2016 22:30
> ...
 Also make it __packed.
>>>
>>> why are you making it packed? Does it _have_ to be packed? If it must,
>>> why wasn't it before?
>>
>> We want to guarantee that it reflects the exact descriptor structure
>> in memory, so probably yes. It was overlooked before and worked
>> because the compiler likely packed it anyways. This is just to make
>> sure it does.
> ...
> 
> That isn't all that 'packed' means.
> 
> 'packed' also tells the compiler to access the structure with instructions 
> that
> will work if it isn't on the 'natural' boundary.

Good to know.

> 
> If you want to ensure the structure is the right size (pretty pointless here)
> then add a compile time assert against the structure size.

Doesn't 'packed' make that unnecessary as well?

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


Re: [PATCH] usb: musb: don't complain on -EPROBE_DEFER when initializing controller

2016-11-09 Thread Tony Lindgren
* Ladislav Michl  [161109 07:08]:
> Don't complain on -EPROBE_DEFER when initializing controller,
> the driver probe will be retried later.
> 
> Signed-off-by: Ladislav Michl 

Acked-by: Tony Lindgren 

> ---
>  drivers/usb/musb/musb_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 27dadc0..77fd97b 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2329,8 +2329,9 @@ musb_init_controller(struct device *dev, int nIrq, void 
> __iomem *ctrl)
>   musb_platform_exit(musb);
>  
>  fail1:
> - dev_err(musb->controller,
> - "musb_init_controller failed with status %d\n", status);
> + if (status != -EPROBE_DEFER)
> + dev_err(musb->controller,
> + "%s failed with status %d\n", __func__, status);
>  
>   musb_free(musb);
>  
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

2016-11-09 Thread John Youn
On 11/8/2016 11:54 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn  writes:
>> On 11/8/2016 1:12 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> John Youn  writes:
 Add a vendor prefix and make the name more consistent by renaming it to
 "snps,gadget-dma-enable".

 Signed-off-by: John Youn 
 ---
  Documentation/devicetree/bindings/usb/dwc2.txt | 5 -
  arch/arm/boot/dts/rk3036.dtsi  | 2 +-
  arch/arm/boot/dts/rk3288.dtsi  | 2 +-
  arch/arm/boot/dts/rk3xxx.dtsi  | 2 +-
  arch/arm64/boot/dts/hisilicon/hi6220.dtsi  | 2 +-
  arch/arm64/boot/dts/rockchip/rk3368.dtsi   | 2 +-
  drivers/usb/dwc2/params.c  | 9 -
  drivers/usb/dwc2/pci.c | 2 +-
  8 files changed, 18 insertions(+), 8 deletions(-)

 diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
 b/Documentation/devicetree/bindings/usb/dwc2.txt
 index 9472111..389a461 100644
 --- a/Documentation/devicetree/bindings/usb/dwc2.txt
 +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
 @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer 
 properties
  - dr_mode: shall be one of "host", "peripheral" and "otg"
Refer to usb/generic.txt
  - snps,host-dma-disable: disable host DMA mode.
 -- g-use-dma: enable dma usage in gadget driver.
 +- snps,gadget-dma-enable: enable gadget DMA mode.
>>>
>>> I don't see why you even have this binding. Looking through the code,
>>> you have:
>>>
>>> #define GHWCFG2_SLAVE_ONLY_ARCH 0
>>> #define GHWCFG2_EXT_DMA_ARCH1
>>> #define GHWCFG2_INT_DMA_ARCH2
>>>
>>> void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
>>> {
>>> int valid = 1;
>>>
>>> if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
>>> valid = 0;
>>> if (val < 0)
>>> valid = 0;
>>>
>>> if (!valid) {
>>> if (val >= 0)
>>> dev_err(hsotg->dev,
>>> "%d invalid for dma_enable parameter. Check HW 
>>> configuration.\n",
>>> val);
>>> val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
>>> dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
>>> }
>>>
>>> hsotg->core_params->dma_enable = val;
>>> }
>>>
>>> which seems to hint that DMA support is discoverable. If there is DMA,
>>> why would disable it?
>>>
>>
>> Yes that's the case and I would prefer to make it discoverable and
>> enabled by default.
>>
>> But the legacy behavior has always been like this because DMA was
>> never fully implemented in the gadget driver and it was an opt-in
>> feature. Periodic support was only added recently.
> 
> legacy behavior can be changed if another 'policy' makes more
> sense. IMHO, whatever can be discovered in runtime, should be enabled by
> default. That way, we force people to use it and find bugs in certain
> features.

Sounds good to me. I'll make the changes.

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


Re: [PATCH] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

2016-11-09 Thread Tony Lindgren
* Ladislav Michl  [161109 07:03]:
> Don't complain on -EPROBE_DEFER when when no PHY found, the driver
> probe will be retried later.

Please send this directly to Alan and Greg:

Acked-by: Tony Lindgren 

> Signed-off-by: Ladislav Michl 
> ---
>  drivers/usb/host/ehci-omap.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 94ea9ff..44945eb 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -181,8 +181,9 @@ static int ehci_hcd_omap_probe(struct platform_device 
> *pdev)
>   continue;
>  
>   ret = PTR_ERR(phy);
> - dev_err(dev, "Can't get PHY device for port %d: %d\n",
> - i, ret);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Can't get PHY device for port "
> + "%d: %d\n", i, ret);
>   goto err_phy;
>   }
>  
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: musb: omap2430: make complain on -EPROBE_DEFER dev_dbg

2016-11-09 Thread Tony Lindgren
* Ladislav Michl  [161109 07:11]:
> There is no point having this complaint to be dev_err as it is just adding
> noise to bootlog.

Acked-by: Tony Lindgren 

> Signed-off-by: Ladislav Michl 
> ---
>  drivers/usb/musb/omap2430.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 451b372..7c9aa5e 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -277,7 +277,7 @@ static int omap2430_musb_init(struct musb *musb)
>   if (status == -ENXIO)
>   return status;
>  
> - dev_err(dev, "HS USB OTG: no transceiver configured\n");
> + dev_dbg(dev, "HS USB OTG: no transceiver configured\n");
>   return -EPROBE_DEFER;
>   }
>  
> -- 
> 2.1.4
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: musb: omap2430: use dev_err

2016-11-09 Thread Tony Lindgren
* Ladislav Michl  [161109 07:09]:
> Replace pr_err with dev_err to print also device name.
> 
> Signed-off-by: Ladislav Michl 

Acked-by: Tony Lindgren 

> ---
>  drivers/usb/musb/omap2430.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index cc12254..451b372 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -277,12 +277,12 @@ static int omap2430_musb_init(struct musb *musb)
>   if (status == -ENXIO)
>   return status;
>  
> - pr_err("HS USB OTG: no transceiver configured\n");
> + dev_err(dev, "HS USB OTG: no transceiver configured\n");
>   return -EPROBE_DEFER;
>   }
>  
>   if (IS_ERR(musb->phy)) {
> - pr_err("HS USB OTG: no PHY configured\n");
> + dev_err(dev, "HS USB OTG: no PHY configured\n");
>   return PTR_ERR(musb->phy);
>   }
>   musb->isr = omap2430_musb_interrupt;
> @@ -301,7 +301,7 @@ static int omap2430_musb_init(struct musb *musb)
>  
>   musb_writel(musb->mregs, OTG_INTERFSEL, l);
>  
> - pr_debug("HS USB OTG: revision 0x%x, sysconfig 0x%02x, "
> + dev_dbg(dev, "HS USB OTG: revision 0x%x, sysconfig 0x%02x, "
>   "sysstatus 0x%x, intrfsel 0x%x, simenable  0x%x\n",
>   musb_readl(musb->mregs, OTG_REVISION),
>   musb_readl(musb->mregs, OTG_SYSCONFIG),
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" 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 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue

2016-11-09 Thread Tony Lindgren
* Johan Hovold  [161109 08:40]:
> On Tue, Nov 08, 2016 at 06:26:07PM -0700, Tony Lindgren wrote:
> > * Johan Hovold  [161108 12:03]:
> > > On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote:
> > > > * Johan Hovold  [161108 10:09]:
> > > > > On Mon, Nov 07, 2016 at 02:50:18PM -0700, Tony Lindgren wrote:
> > > > > > @@ -2604,6 +2669,9 @@ static int musb_resume(struct device *dev)
> > > > > > mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | 
> > > > > > MUSB_DEVCTL_LSDEV;
> > > > > > if ((devctl & mask) != (musb->context.devctl & mask))
> > > > > > musb->port1_status = 0;
> > > > > > +
> > > > > > +   schedule_delayed_work(>pending_resume_work, 0);
> > > > > > +
> > > > > 
> > > > > The interactions with system suspend looks a bit suspicious. It seems
> > > > > you need to drain any pending resume work on suspend for example.
> > > > > 
> > > > > And then the above should not be needed, right?
> > > > 
> > > > Hmm that's an interesting idea. I think that would allow us to get
> > > > rid of the delayed work if we check the list both on resume and
> > > > suspend :)
> > > 
> > > Note that I was referring to draining the runtime-resume work on system
> > > suspend above. I think we still want the delayed work considering that
> > > we could stay active for long periods of time.
> > 
> > It seems it's the delayed work causing these race issues, so I'd like
> > to avoid it. I think we can do it all from PM runtime resume and
> > checking at the end of musb_queue_resume_work(). We need to introduce
> > is_runtime_suspended though. This is along the lines of what we have
> > in Documentation/power/runtime_pm.txt if you search for completion
> > there.
> 
> Sure, if you can avoid processing the deferred work also at
> runtime-suspend then getting rid of the delayed work would be preferred.
> 
> > Below is a version doing things without delayed work, care to check
> > again if you still see races there?
> 
> I believe there are still some issues however.
> 
> > > > > In fact, the dsps timer must also be cancelled on suspend, or you 
> > > > > could
> > > > > end up calling dsps_check_status() while suspended (it is currently 
> > > > > not
> > > > > cancelled until the parent device is suspended, which could be too
> > > > > late).
> > > > 
> > > > And then this should no longer be an issue either.
> > > 
> > > It would still be an issue as a system-suspending device could already
> > > have been runtime-resumed so that dsps_check_status() would be called
> > > directly from the timer function.
> > 
> > The glue layers should do pm_runtime_get_sync(musb->controller) which
> > dsps glue already does. So that's the musb_core.c device instance. And
> > looks like we have dsps_suspend() call del_timer_sync(>timer)
> > already. I think we're safe here.
> 
> But the point is that the controller might be RPM_ACTIVE if the
> controller was already runtime resumed when it is system suspended.
> 
> Since this (and the previous) patch run the work directly from the timer
> callback if active, it could end up accessing the controller after it
> has been system suspended. Specifically, stopping the timer in the glue
> (parent) suspend callback is too late to avoid this.
> 
>   pm_runtime_get_sync(musb->controller);
>   musb_runtime_resume()
>   musb_restore_context();
>   
>   ...
> 
>   musb_suspend()
>   musb_save_context();
> 
>   otg_timer()
>   pm_runtime_get();
>   if (pm_runtime_active(musb->controller))
>   dsps_check_status();
>   pm_runtime_put_autosuspend();
> 
>   dsps_suspend()
>   del_timer_sync();
> 

OK so we need to return without doing anything from otg_timer() on
pm_runtime_get() to avoid that.

In the long run it would be nice to make whatever optional state polling
musb generic with just a glue layer callback.

> > +/*
> > + * Called to run work if device is active or else queue the work to happen
> > + * on resume. Caller must take musb->lock.
> 
> Caller must also hold an RPM reference.

Good point, will add.

> > +   if (musb->is_runtime_suspended) {
> > +   list_add_tail(>node, >pending_list);
> > +   error = 0;
> > +   } else {
> > +   dev_err(musb->controller, "could not add resume work %p\n",
> > +   callback);
> > +   devm_kfree(musb->controller, w);
> > +   error = -EINPROGRESS;
> 
> But this means you should be able to run the callback below, right? It
> has to be run from somewhere so otherwise the caller needs to retry
> instead.

Well there's no longer need to run the callback at that point any longer
and with that removed that should not be an issue.

Anyways, musb->is_runtime_suspended is needed to protect anything from
being queued between runtime resume having called musb_run_resume_work()
and before pm_runtime_active() 

Re: [PATCH] usb: dwc2: gadget: Update for new usb_endpoint_maxp()

2016-11-09 Thread John Youn
On 11/9/2016 12:02 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn  writes:
>>> John Youn  writes:
> John Youn  writes:
 @@ -1812,17 +1812,17 @@ static u32 dwc2_hsotg_ep0_mps(unsigned int mps)
   * @hsotg: The driver state.
   * @ep: The index number of the endpoint
   * @mps: The maximum packet size in bytes
 + * @mc: The multicount value
   *
   * Configure the maximum packet size for the given endpoint, updating
   * the hardware control registers to reflect this.
   */
  static void dwc2_hsotg_set_ep_maxpacket(struct dwc2_hsotg *hsotg,
 -  unsigned int ep, unsigned int mps, unsigned int 
 dir_in)
 +  unsigned int ep, unsigned int 
 mps,
 +  unsigned int mc, unsigned int 
 dir_in)
>>>
>>> this has an odd set of arguments. You pass the ep index, mps, direction
>>> and mult value, when you could just pass hsotg_ep and descriptor 
>>> instead.
>>
>> Yes looks like we can do some simplification here. And you probably
>> don't need to pass a descriptor either since it must be set in the
>> usb_ep before enable.
>>
>> However this is also called in some contexts where a descriptor is not
>> available (initialization and ep0). So we have to think about this a
>> bit.
>>
>> I think dwc3 can make similar simplification on the
>> __dwc3_gadget_ep_enable().
>
> __dwc3_gadget_ep_enable() takes the actual descriptors as arguments:
>
> static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>   const struct usb_endpoint_descriptor *desc,
>   const struct usb_ss_ep_comp_descriptor *comp_desc,
>   bool modify, bool restore)
>
> I fail to see how much simpler we can make this. Perhaps we can turn
> bool and restore into a single argument if we use a bitfield instead of
> a bool.
>

 Hi Felipe,

 I mean that there shouldn't be any need to pass in descriptors with
 usb_ep since the ones in usb_ep should be set properly from ep enable
 to ep disable.
> 
> Oh, I see what you mean now :-)
> 
 I think that's the case anyways.
>>>
>>> __dwc3_gadget_ep_enable() is the one assigning the descriptor to the ep:
>>>
>>> static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>>> const struct usb_endpoint_descriptor *desc,
>>> const struct usb_ss_ep_comp_descriptor *comp_desc,
>>> bool modify, bool restore)
>>> {
>>> struct dwc3 *dwc = dep->dwc;
>>> u32 reg;
>>> int ret;
>>>
>>> if (!(dep->flags & DWC3_EP_ENABLED)) {
>>> ret = dwc3_gadget_start_config(dwc, dep);
>>> if (ret)
>>> return ret;
>>> }
>>>
>>> ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify,
>>> restore);
>>> if (ret)
>>> return ret;
>>>
>>> if (!(dep->flags & DWC3_EP_ENABLED)) {
>>> struct dwc3_trb *trb_st_hw;
>>> struct dwc3_trb *trb_link;
>>>
>>> dep->endpoint.desc = desc;
>>> dep->comp_desc = comp_desc;
>>>
>>> [...]
>>>
>>
>> Right, but that's redundant for non-ep0 case. And the EP0 case can be
>> handled globally elsewhere.
>>
>> The usb_ep_enable() API function takes only the usb_ep and requires
>> that the descriptors are set in the usb_ep beforehand. So 'desc'
>> argument in the callback function is probably not required either.
>>
>> I think this is correct to make it consistent. Since we don't pass the
> 
> makes sense to me :-)
> 
>> SS comp descriptor, and we don't want to keep passing more descriptors
>> every time a new standard descriptor is added, such as the SSP ISO EP
>> comp descriptor for 3.1.
>>
>> See below for a proposed change to see what I mean (not tested).
>>
>> Regards,
>> John
>>
>>
>> >8
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 2322863..b9903c6 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -539,7 +539,6 @@ struct dwc3_ep {
>>  
>>  struct dwc3_trb *trb_pool;
>>  dma_addr_t  trb_pool_dma;
>> -const struct usb_ss_ep_comp_descriptor *comp_desc;
>>  struct dwc3 *dwc;
>>  
>>  u32 saved_state;
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index e47cba5..0fc55e89 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -576,13 +576,12 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 
>> *dwc, struct dwc3_ep *dep)
>>   * Caller should take care of locking
>>   */
>>  static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>> -

Re: C-Media USB speakers do not work on AMD USB 2.0 port

2016-11-09 Thread sonofagun



Good question.  I don't know the answer; it may be hidden in a part
of the system log that you didn't post.
Nothing unusual was found on the log. If you need more information, just 
tell me what you need.



This is a known bug in the 4.8 kernel, caused by changes to the timer
code.  A workaround and a proper fix have both been added to the 4.9-rc
series and will be part of the next 4.8.stable release.
Thank you Alan for analyzing the problem. At least it is already fixed 
so I will just wait for 4.9 or newer 4.8. Since I use those speakers 
only for testing, waiting is not a problem. Once they work I will inform 
you.

--
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: Multiple chatty devices on Intel 5 Series/3400 USB2 EHCI controller act erratic

2016-11-09 Thread sonofagun



Why didn't you say so (given that you were replying to a message that I
sent)?
Because I thought it would go to the correct existing subject on the 
first place.



I don't know.  What new subject are you referring to?

Some days ago, the first two messages I have sent were not linked there:
http://marc.info/?t=14655804433=1=2
but to another page. Today everything looks correct but I am curious why 
it does. Maybe the server needs some time to sync new and old messages.


Back to our subject, I think there is something we can do for intel dual 
EHCI chipsets so that it is easier to debug similar issues in the 
future. PCI dumps from the machine that had the issue are needed.

--
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 v4 1/4] usb: dbc: early driver for xhci debug capability

2016-11-09 Thread Lu Baolu
Hi,

On 11/09/2016 05:23 PM, Thomas Gleixner wrote:
> On Tue, 1 Nov 2016, Lu Baolu wrote:
>> +static int __init xdbc_init(void)
>> +{
> ...
>> +base = ioremap_nocache(xdbc.xhci_start, xdbc.xhci_length);
>> +if (!base) {
>> +xdbc_trace("failed to remap the io address\n");
>> +ret = -ENOMEM;
>> +goto free_and_quit;
>> +}
>> +
>> +early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
>> +xdbc_trace("early mapped IO address released\n");
>> +
>> +xdbc.xhci_base = base;
>> +offset = xhci_find_next_ext_cap(xdbc.xhci_base, 0, XHCI_EXT_CAPS_DEBUG);
>> +xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);
> This is broken. What prevents that 
>
>  - a printk is in progress on another cpu?
>
>  - a printk happens between the unmap and storing the new base ?
>
> Nothing AFAICT. So this needs to be done in a safe way. And just making it
>
>   oldbase = xdbc.xhci_base;
>   base = ioremap();
>   xdbc.xhci_base = base;
>   early_iounmap(oldbase);
>
> does not work either because the compiler can rightfully cache
> xdbc.xhci_base in the write related functions. The same issue with
> xdbc.xdbc_reg.

If there isn't a good solution, I will remove the ioremap code and let
it use the early mapped one.

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


[PATCH 2/2] usb: dwc3: gadget: Remove descriptor arguments to ep_config

2016-11-09 Thread John Youn
This function has access to the descriptors via the usb_ep.

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/gadget.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0e73383..4914182 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -488,16 +488,19 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc, 
struct dwc3_ep *dep)
 }
 
 static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
-   const struct usb_endpoint_descriptor *desc,
-   const struct usb_ss_ep_comp_descriptor *comp_desc,
bool modify, bool restore)
 {
struct dwc3_gadget_ep_cmd_params params;
+   const struct usb_endpoint_descriptor *desc;
+   const struct usb_ss_ep_comp_descriptor *comp_desc;
 
if (dev_WARN_ONCE(dwc->dev, modify && restore,
"Can't modify and restore\n"))
return -EINVAL;
 
+   desc = dep->endpoint.desc;
+   comp_desc = dep->endpoint.comp_desc;
+
memset(, 0x00, sizeof(params));
 
params.param0 = DWC3_DEPCFG_EP_TYPE(usb_endpoint_type(desc))
@@ -592,7 +595,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
 
ep = >endpoint;
desc = ep->desc;
-   ret = dwc3_gadget_set_ep_config(dwc, dep, desc, ep->comp_desc,
+   ret = dwc3_gadget_set_ep_config(dwc, dep,
modify, restore);
if (ret)
return ret;
-- 
2.10.0

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


[PATCH 1/2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable

2016-11-09 Thread John Youn
The __dwc3_gadget_endpoint_enable() function has access to the endpoint
descriptors via the usb_ep. So we don't need to pass them in as
arguments. The descriptors should be set by the caller prior to calling
usb_ep_enable().

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/core.h   |  1 -
 drivers/usb/dwc3/gadget.c | 40 +++-
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 2322863..b9903c6 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -539,7 +539,6 @@ struct dwc3_ep {
 
struct dwc3_trb *trb_pool;
dma_addr_t  trb_pool_dma;
-   const struct usb_ss_ep_comp_descriptor *comp_desc;
struct dwc3 *dwc;
 
u32 saved_state;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 7e465ea..0e73383 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -576,13 +576,13 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 
*dwc, struct dwc3_ep *dep)
  * Caller should take care of locking
  */
 static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
-   const struct usb_endpoint_descriptor *desc,
-   const struct usb_ss_ep_comp_descriptor *comp_desc,
bool modify, bool restore)
 {
struct dwc3 *dwc = dep->dwc;
u32 reg;
int ret;
+   struct usb_ep   *ep;
+   const struct usb_endpoint_descriptor *desc;
 
if (!(dep->flags & DWC3_EP_ENABLED)) {
ret = dwc3_gadget_start_config(dwc, dep);
@@ -590,8 +590,10 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
return ret;
}
 
-   ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify,
-   restore);
+   ep = >endpoint;
+   desc = ep->desc;
+   ret = dwc3_gadget_set_ep_config(dwc, dep, desc, ep->comp_desc,
+   modify, restore);
if (ret)
return ret;
 
@@ -599,8 +601,6 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
struct dwc3_trb *trb_st_hw;
struct dwc3_trb *trb_link;
 
-   dep->endpoint.desc = desc;
-   dep->comp_desc = comp_desc;
dep->type = usb_endpoint_type(desc);
dep->flags |= DWC3_EP_ENABLED;
dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
@@ -713,11 +713,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
 
dep->stream_capable = false;
-   dep->endpoint.desc = NULL;
-   dep->comp_desc = NULL;
dep->type = 0;
dep->flags &= DWC3_EP_END_TRANSFER_PENDING;
 
+   /* Clear out the ep descriptors for non-ep0 */
+   if (dep->number >> 1) {
+   dep->endpoint.desc = NULL;
+   dep->endpoint.comp_desc = NULL;
+   }
+
return 0;
 }
 
@@ -763,7 +767,7 @@ static int dwc3_gadget_ep_enable(struct usb_ep *ep,
return 0;
 
spin_lock_irqsave(>lock, flags);
-   ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc, false, false);
+   ret = __dwc3_gadget_ep_enable(dep, false, false);
spin_unlock_irqrestore(>lock, flags);
 
return ret;
@@ -1741,16 +1745,14 @@ static int __dwc3_gadget_start(struct dwc3 *dwc)
dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 
dep = dwc->eps[0];
-   ret = __dwc3_gadget_ep_enable(dep, _gadget_ep0_desc, NULL, false,
-   false);
+   ret = __dwc3_gadget_ep_enable(dep, false, false);
if (ret) {
dev_err(dwc->dev, "failed to enable %s\n", dep->name);
goto err0;
}
 
dep = dwc->eps[1];
-   ret = __dwc3_gadget_ep_enable(dep, _gadget_ep0_desc, NULL, false,
-   false);
+   ret = __dwc3_gadget_ep_enable(dep, false, false);
if (ret) {
dev_err(dwc->dev, "failed to enable %s\n", dep->name);
goto err1;
@@ -1891,6 +1893,12 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 
*dwc,
(epnum & 1) ? "in" : "out");
 
dep->endpoint.name = dep->name;
+
+   if (!(dep->number >> 1)) {
+   dep->endpoint.desc = _gadget_ep0_desc;
+   dep->endpoint.comp_desc = NULL;
+   }
+
spin_lock_init(>lock);
 
if (epnum == 0 || epnum == 1) {
@@ -2579,16 +2587,14 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 
*dwc)
}
 
dep = dwc->eps[0];
-   ret = __dwc3_gadget_ep_enable(dep, _gadget_ep0_desc, NULL, true,
-   false);
+   ret = __dwc3_gadget_ep_enable(dep, true, false);
if (ret) {
  

Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

2016-11-09 Thread Lu Baolu
Hi,

On 11/09/2016 05:37 PM, Thomas Gleixner wrote:
> On Tue, 1 Nov 2016, Lu Baolu wrote:
>> +static void early_xdbc_write(struct console *con, const char *str, u32 n)
>> +{
>> +int chunk, ret;
>> +static char buf[XDBC_MAX_PACKET];
>> +int use_cr = 0;
>> +
>> +if (!xdbc.xdbc_reg)
>> +return;
>> +memset(buf, 0, XDBC_MAX_PACKET);
> How is that dealing with reentrancy?
>
> early_printk() does not protect against it. Peter has a patch to prevent
> concurrent access from different cpus, but it cannot and will never prevent
> reentrancy on the same cpu (interrupt, nmi).

I can use a spinlock_irq to protect reentrancy of interrupt on the same
cpu. But I have no idea about the nmi one. This seems to be a common
issue for all early printk drivers.

Peter, any idea?

Best regards,
Lu Baolu

--
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 V11 1/1] usb:serial: Add Fintek F81532/534 driver

2016-11-09 Thread Ji-Ze Hong (Peter Hong)

Hi Johan,

Johan Hovold 於 2016/11/2 下午 08:37 寫道:

On Fri, Oct 14, 2016 at 04:20:46PM +0800, Ji-Ze Hong (Peter Hong) wrote:


Reviewed-by: Johan Hovold 


You must never add other peoples' Reviewed-by tags unless you've
explicitly been given permission to do so (e.g. "fix this minor thing up
and then you can add...").

Please make sure to read the section about Reviewed-by tags in
Documentation/SubmittingPatches.


Sorry for misusing "Reviewed-by" tags.


Changelog:
V11
1. Reduce F81534_MAX_BUS_RETRY from 2000 to 20. We are only using
   internal SPI bus to read flash when attach() & calc_num_ports()
   and never read flash when the F81532/534 in full loading, so we
   can reduce retry count.


Does this mean you can remove that retry mechanism completely?


The mechanism is for checking SPI Bus busy status. We need to perform
read/write operation only when the bus idle. So we need remain the check
mechanism.




2. Modify attach() & calc_num_ports() read default value only when
   can't find the custom setting.
3. Change tx_empty protect method from spinlock to set_bit()/
   clear_bit()/test_and_clear_bit().
4. Move calculate tty_idx[i] from port_probe() to attach().
5. Add f81534_tx_empty()



+#define DRIVER_DESC"Fintek F81532/F81534"
+#define FINTEK_VENDOR_ID_1 0x1934
+#define FINTEK_VENDOR_ID_2 0x2C42
+#define FINTEK_DEVICE_ID   0x1202
+#define F81534_MAX_TX_SIZE 100


You never replies to my question about why this is not 124 as for rx.


The limit for TX with 100 bytes is tuned for high-speed TX performance.
But this patch is not contained for high-baudrate, so we'll change it
for 124 byte with next patch.






+static int f81534_set_normal_register(struct usb_serial *serial, u16 reg,


What do mean by "normal" here? Could you give this a more descriptive
name?

Perhaps just call this one f81534_set_register and add a "port" or
"uart" infix to the current f81534_set_register below (e.g. rename it
f81534_set_uart_register, and similar for get).
Or simply replace "normal" with "generic" above.


OK, I'll rename such functions with following:

Generic read/write USB:
  f81534_set/get_normal_register -> f81534_set/get_register

UART port read/write:
  f81534_set/get_register -> f81534_set/get_port_register

SPI bus read/write
  f81534_set_normal_register_with_delay -> f81534_set/get_spi_register




+{
+   int status;
+
+   status = f81534_get_normal_register(serial, reg, data);
+   if (status)
+   return status;
+
+   status = f81534_command_delay(serial);
+   if (status)
+   return status;


Why do you need a delay after reading?


Sorry for misleading.
I'll rename f81534_command_delay to f81534_wait_for_spi_idle.



+static void f81534_prepare_write_buffer(struct usb_serial_port *port, u8 *buf,
+   size_t size)


You never use size in this function. You need to make sure you never
overwrite the provided buffer using some sanity checks.


OK, I'll add probe() to check bulk_in/out endpoints count & packet size.




+   /* Check H/W is TXEMPTY */
+   if (!test_and_clear_bit(F81534_TX_EMPTY_BIT, _priv->tx_empty))
+   return 0;
+
+   urb = port->write_urbs[0];
+   f81534_prepare_write_buffer(port, port->bulk_out_buffers[0],
+   port->bulk_out_size);
+   urb->transfer_buffer_length = F81534_WRITE_BUFFER_SIZE;


You need to make sure the buffers have the expected size. They are
currently set to the endpoint size, but you can you can make sure they
are never smaller than F81534_WRITE_BUFFER_SIZE by setting bulk_out_size
in the usb_serial_driver struct.


Thanks for your notice. We had tested it on USB 2.0 Full-speed, the
packet size will reduce to 64, it will cause buffer overflow. So I'll
reconfirm the packet size in probe().


Thanks for your comments.
--
With Best Regards,
Peter Hong
--
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 29/30] usb: dwc2: gadget: Program ep0_mps for LS

2016-11-09 Thread John Youn
From: Vardan Mikayelyan 

When device is enumerated in LS we should program ep0_mps accordingly.
USB2 spec says that in LS mode, control ep mps must be 8.

Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 74f0a5e..e672832 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3007,6 +3007,8 @@ static void dwc2_hsotg_irq_enumdone(struct dwc2_hsotg 
*hsotg)
 
case DSTS_ENUMSPD_LS:
hsotg->gadget.speed = USB_SPEED_LOW;
+   ep0_mps = 8;
+   ep_mps = 8;
/*
 * note, we don't actually support LS in this driver at the
 * moment, and the documentation seems to imply that it isn't
-- 
2.10.0

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


[PATCH v2 19/30] usb: dwc2: gadget: Add completions for DDMA isoc transfers

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

For DDMA mode in case of isochronous transfers completion performed
differently than other transfer types. This is because each usb request
was mapped to one descriptor in the chain and SW gets xfercomplete
interrupt on all descriptors. The endpoint remains enabled until HW
processes last descriptor with "L" bit set or BNA interrupt gets
asserted for IN and OUT endpoints correspondingly.

Add function dwc2_gadget_complete_isoc_request_ddma() - completes one
isochronous request taken from endpoint's queue.

Add function dwc2_gadget_start_next_isoc_ddma() - tries to restart
isochronous endpoint if requests are pending. Check for EPENA. If the
endpoint was disabled, try to restart it after programming descriptor
chain prepared by SW earlier, switch SW to fill the other half of chain.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 105 ++
 1 file changed, 105 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6351b3c..696529a 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1930,6 +1930,111 @@ static void dwc2_hsotg_complete_request(struct 
dwc2_hsotg *hsotg,
}
 }
 
+/*
+ * dwc2_gadget_complete_isoc_request_ddma - complete an isoc request in DDMA
+ * @hs_ep: The endpoint the request was on.
+ *
+ * Get first request from the ep queue, determine descriptor on which complete
+ * happened. SW based on isoc_chain_num discovers which half of the descriptor
+ * chain is currently in use by HW, adjusts dma_address and calculates index
+ * of completed descriptor based on the value of DEPDMA register. Update actual
+ * length of request, giveback to gadget.
+ */
+static void dwc2_gadget_complete_isoc_request_ddma(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   struct dwc2_hsotg_req *hs_req;
+   struct usb_request *ureq;
+   int index;
+   dma_addr_t dma_addr;
+   u32 dma_reg;
+   u32 depdma;
+   u32 desc_sts;
+   u32 mask;
+
+   hs_req = get_ep_head(hs_ep);
+   if (!hs_req) {
+   dev_warn(hsotg->dev, "%s: ISOC EP queue empty\n", __func__);
+   return;
+   }
+   ureq = _req->req;
+
+   dma_addr = hs_ep->desc_list_dma;
+
+   /*
+* If lower half of  descriptor chain is currently use by SW,
+* that means higher half is being processed by HW, so shift
+* DMA address to higher half of descriptor chain.
+*/
+   if (!hs_ep->isoc_chain_num)
+   dma_addr += sizeof(struct dwc2_dma_desc) *
+   (MAX_DMA_DESC_NUM_GENERIC / 2);
+
+   dma_reg = hs_ep->dir_in ? DIEPDMA(hs_ep->index) : DOEPDMA(hs_ep->index);
+   depdma = dwc2_readl(hsotg->regs + dma_reg);
+
+   index = (depdma - dma_addr) / sizeof(struct dwc2_dma_desc) - 1;
+   desc_sts = hs_ep->desc_list[index].status;
+
+   mask = hs_ep->dir_in ? DEV_DMA_ISOC_TX_NBYTES_MASK :
+  DEV_DMA_ISOC_RX_NBYTES_MASK;
+   ureq->actual = ureq->length -
+  ((desc_sts & mask) >> DEV_DMA_ISOC_NBYTES_SHIFT);
+
+   dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
+}
+
+/*
+ * dwc2_gadget_start_next_isoc_ddma - start next isoc request, if any.
+ * @hs_ep: The isochronous endpoint to be re-enabled.
+ *
+ * If ep has been disabled due to last descriptor servicing (IN endpoint) or
+ * BNA (OUT endpoint) check the status of other half of descriptor chain that
+ * was under SW control till HW was busy and restart the endpoint if needed.
+ */
+static void dwc2_gadget_start_next_isoc_ddma(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   u32 depctl;
+   u32 dma_reg;
+   u32 ctrl;
+   u32 dma_addr = hs_ep->desc_list_dma;
+   unsigned char index = hs_ep->index;
+
+   dma_reg = hs_ep->dir_in ? DIEPDMA(index) : DOEPDMA(index);
+   depctl = hs_ep->dir_in ? DIEPCTL(index) : DOEPCTL(index);
+
+   ctrl = dwc2_readl(hsotg->regs + depctl);
+
+   /*
+* EP was disabled if HW has processed last descriptor or BNA was set.
+* So restart ep if SW has prepared new descriptor chain in ep_queue
+* routine while HW was busy.
+*/
+   if (!(ctrl & DXEPCTL_EPENA)) {
+   if (!hs_ep->next_desc) {
+   dev_dbg(hsotg->dev, "%s: No more ISOC requests\n",
+   __func__);
+   return;
+   }
+
+   dma_addr += sizeof(struct dwc2_dma_desc) *
+   (MAX_DMA_DESC_NUM_GENERIC / 2) *
+   hs_ep->isoc_chain_num;
+   dwc2_writel(dma_addr, hsotg->regs + dma_reg);
+
+   ctrl |= DXEPCTL_EPENA | DXEPCTL_CNAK;
+   dwc2_writel(ctrl, hsotg->regs + depctl);
+
+

[PATCH v2 30/30] usb: dwc2: gadget: Add new core parameter for low speed

2016-11-09 Thread John Youn
From: Vardan Mikayelyan 

Added new core param for low speed, which can be used only when SNPSID
is equal to DWC2_CORE_FS_IOT. When LS mode is enabled, we are
restricting ep types and providing to upper layer only INTR and CTRL
endpoints.

Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/gadget.c | 27 +--
 drivers/usb/dwc2/hcd.c|  8 +---
 drivers/usb/dwc2/params.c |  8 ++--
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 067e24b..9548d3e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -467,6 +467,7 @@ struct dwc2_core_params {
int speed;
 #define DWC2_SPEED_PARAM_HIGH  0
 #define DWC2_SPEED_PARAM_FULL  1
+#define DWC2_SPEED_PARAM_LOW   2
 
int enable_dynamic_fifo;
int en_multiple_tx_fifo;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e672832..ad0cd0e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3175,7 +3175,8 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
GUSBCFG_HNPCAP);
 
if (hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS &&
-   hsotg->params.speed == DWC2_SPEED_PARAM_FULL) {
+   (hsotg->params.speed == DWC2_SPEED_PARAM_FULL ||
+hsotg->params.speed == DWC2_SPEED_PARAM_LOW)) {
/* FS/LS Dedicated Transceiver Interface */
usbcfg |= GUSBCFG_PHYSEL;
} else {
@@ -3192,14 +3193,21 @@ void dwc2_hsotg_core_init_disconnected(struct 
dwc2_hsotg *hsotg,
__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
 
dcfg |= DCFG_EPMISCNT(1);
-   if (hsotg->params.speed == DWC2_SPEED_PARAM_FULL) {
+
+   switch (hsotg->params.speed) {
+   case DWC2_SPEED_PARAM_LOW:
+   dcfg |= DCFG_DEVSPD_LS;
+   break;
+   case DWC2_SPEED_PARAM_FULL:
if (hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS)
dcfg |= DCFG_DEVSPD_FS48;
else
dcfg |= DCFG_DEVSPD_FS;
-   } else {
+   break;
+   default:
dcfg |= DCFG_DEVSPD_HS;
}
+
dwc2_writel(dcfg,  hsotg->regs + DCFG);
 
/* Clear any pending OTG interrupts */
@@ -4388,14 +4396,21 @@ static void dwc2_hsotg_initep(struct dwc2_hsotg *hsotg,
 
hs_ep->parent = hsotg;
hs_ep->ep.name = hs_ep->name;
-   usb_ep_set_maxpacket_limit(_ep->ep, epnum ? 1024 : EP0_MPS_LIMIT);
+
+   if (hsotg->params.speed == DWC2_SPEED_PARAM_LOW)
+   usb_ep_set_maxpacket_limit(_ep->ep, 8);
+   else
+   usb_ep_set_maxpacket_limit(_ep->ep,
+  epnum ? 1024 : EP0_MPS_LIMIT);
hs_ep->ep.ops = _hsotg_ep_ops;
 
if (epnum == 0) {
hs_ep->ep.caps.type_control = true;
} else {
-   hs_ep->ep.caps.type_iso = true;
-   hs_ep->ep.caps.type_bulk = true;
+   if (hsotg->params.speed != DWC2_SPEED_PARAM_LOW) {
+   hs_ep->ep.caps.type_iso = true;
+   hs_ep->ep.caps.type_bulk = true;
+   }
hs_ep->ep.caps.type_int = true;
}
 
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 393a962..61324d9 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -230,9 +230,10 @@ static int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool 
select_phy)
u32 usbcfg;
int retval = 0;
 
-   if (hsotg->params.speed == DWC2_SPEED_PARAM_FULL &&
+   if ((hsotg->params.speed == DWC2_SPEED_PARAM_FULL ||
+hsotg->params.speed == DWC2_SPEED_PARAM_LOW) &&
hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS) {
-   /* If FS mode with FS PHY */
+   /* If FS/LS mode with FS/LS PHY */
retval = dwc2_fs_phy_init(hsotg, select_phy);
if (retval)
return retval;
@@ -2277,7 +2278,8 @@ static void dwc2_core_host_init(struct dwc2_hsotg *hsotg)
 
/* Initialize Host Configuration Register */
dwc2_init_fs_ls_pclk_sel(hsotg);
-   if (hsotg->params.speed == DWC2_SPEED_PARAM_FULL) {
+   if (hsotg->params.speed == DWC2_SPEED_PARAM_FULL ||
+   hsotg->params.speed == DWC2_SPEED_PARAM_LOW) {
hcfg = dwc2_readl(hsotg->regs + HCFG);
hcfg |= HCFG_FSLSSUPP;
dwc2_writel(hcfg, hsotg->regs + HCFG);
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index c294344..7b84c1e 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -742,14 +742,18 @@ static void dwc2_set_param_speed(struct dwc2_hsotg 
*hsotg, int val)
 {
int valid = 1;
 
-   if 

[PATCH v2 18/30] usb: dwc2: gadget: Fill isoc descriptor and start transfer in DDMA

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add function dwc2_gadget_fill_isoc_desc() - initializes DMA descriptor
for isochronous transfer based on the received request data and endpoint
characteristics.

Added function dwc2_gadget_start_isoc_ddma() - prepare DMA chain for
isochronous transfer in DDMA, programs corresponding DMA address to
DEPDMA, enables the endpoint. This function is called once SW decides to
start isochronous IN or OUT transfer depend on reception of NAK or
OUTTknEPdis interrupts indicating first isochronous token arrival from
host.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 133 ++
 1 file changed, 133 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index c32520d..6351b3c 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -735,6 +735,139 @@ static void dwc2_gadget_config_nonisoc_xfer_ddma(struct 
dwc2_hsotg_ep *hs_ep,
}
 }
 
+/*
+ * dwc2_gadget_fill_isoc_desc - fills next isochronous descriptor in chain.
+ * @hs_ep: The isochronous endpoint.
+ * @dma_buff: usb requests dma buffer.
+ * @len: usb request transfer length.
+ *
+ * Finds out index of first free entry either in the bottom or up half of
+ * descriptor chain depend on which is under SW control and not processed
+ * by HW. Then fills that descriptor with the data of the arrived usb request,
+ * frame info, sets Last and IOC bits increments next_desc. If filled
+ * descriptor is not the first one, removes L bit from the previous descriptor
+ * status.
+ */
+static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep,
+ dma_addr_t dma_buff, unsigned int len)
+{
+   struct dwc2_dma_desc *desc;
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   u32 index;
+   u32 maxsize = 0;
+   u32 mask = 0;
+
+   maxsize = dwc2_gadget_get_desc_params(hs_ep, );
+   if (len > maxsize) {
+   dev_err(hsotg->dev, "wrong len %d\n", len);
+   return -EINVAL;
+   }
+
+   /*
+* If SW has already filled half of chain, then return and wait for
+* the other chain to be processed by HW.
+*/
+   if (hs_ep->next_desc == MAX_DMA_DESC_NUM_GENERIC / 2)
+   return -EBUSY;
+
+   /* Increment frame number by interval for IN */
+   if (hs_ep->dir_in)
+   dwc2_gadget_incr_frame_num(hs_ep);
+
+   index = (MAX_DMA_DESC_NUM_GENERIC / 2) * hs_ep->isoc_chain_num +
+hs_ep->next_desc;
+
+   /* Sanity check of calculated index */
+   if ((hs_ep->isoc_chain_num && index > MAX_DMA_DESC_NUM_GENERIC) ||
+   (!hs_ep->isoc_chain_num && index > MAX_DMA_DESC_NUM_GENERIC / 2)) {
+   dev_err(hsotg->dev, "wrong index %d for iso chain\n", index);
+   return -EINVAL;
+   }
+
+   desc = _ep->desc_list[index];
+
+   /* Clear L bit of previous desc if more than one entries in the chain */
+   if (hs_ep->next_desc)
+   hs_ep->desc_list[index - 1].status &= ~DEV_DMA_L;
+
+   dev_dbg(hsotg->dev, "%s: Filling ep %d, dir %s isoc desc # %d\n",
+   __func__, hs_ep->index, hs_ep->dir_in ? "in" : "out", index);
+
+   desc->status = 0;
+   desc->status |= (DEV_DMA_BUFF_STS_HBUSY << DEV_DMA_BUFF_STS_SHIFT);
+
+   desc->buf = dma_buff;
+   desc->status |= (DEV_DMA_L | DEV_DMA_IOC |
+((len << DEV_DMA_NBYTES_SHIFT) & mask));
+
+   if (hs_ep->dir_in) {
+   desc->status |= ((hs_ep->mc << DEV_DMA_ISOC_PID_SHIFT) &
+DEV_DMA_ISOC_PID_MASK) |
+   ((len % hs_ep->ep.maxpacket) ?
+DEV_DMA_SHORT : 0) |
+   ((hs_ep->target_frame <<
+ DEV_DMA_ISOC_FRNUM_SHIFT) &
+DEV_DMA_ISOC_FRNUM_MASK);
+   }
+
+   desc->status &= ~DEV_DMA_BUFF_STS_MASK;
+   desc->status |= (DEV_DMA_BUFF_STS_HREADY << DEV_DMA_BUFF_STS_SHIFT);
+
+   /* Update index of last configured entry in the chain */
+   hs_ep->next_desc++;
+
+   return 0;
+}
+
+/*
+ * dwc2_gadget_start_isoc_ddma - start isochronous transfer in DDMA
+ * @hs_ep: The isochronous endpoint.
+ *
+ * Prepare first descriptor chain for isochronous endpoints. Afterwards
+ * write DMA address to HW and enable the endpoint.
+ *
+ * Switch between descriptor chains via isoc_chain_num to give SW opportunity
+ * to prepare second descriptor chain while first one is being processed by HW.
+ */
+static void dwc2_gadget_start_isoc_ddma(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   struct dwc2_hsotg_req *hs_req, *treq;
+   int index = hs_ep->index;
+   int ret;
+   u32 dma_reg;
+   u32 depctl;
+   u32 ctrl;
+

[PATCH v2 23/30] usb: dwc2: gadget: Adjust ISOC OUT request's actual len for DDMA

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

In DDMA mode if programmed ISOC OUT transfer length is not DWORD
aligned, after closing descriptor HW leaves value of 4 - (ureq->length %
4) in the RX bytes. This is caused because DMA works using 4B chunks.
Example: if length = 9 and all 9 bytes were received from the bus, after
xfercomplete rx_bytes value is 3. Hence add this value to the amount of
transferred bytes.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 9f28756..d2442f4 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2001,6 +2001,10 @@ static void 
dwc2_gadget_complete_isoc_request_ddma(struct dwc2_hsotg_ep *hs_ep)
ureq->actual = ureq->length -
   ((desc_sts & mask) >> DEV_DMA_ISOC_NBYTES_SHIFT);
 
+   /* Adjust actual length for ISOC Out if length is not align of 4 */
+   if (!hs_ep->dir_in && ureq->length & 0x3)
+   ureq->actual += 4 - (ureq->length & 0x3);
+
dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
 }
 
-- 
2.10.0

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


[PATCH v2 20/30] usb: dwc2: gadget: In DDMA keep incompISOOUT and incompISOIN masked

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

In DDMA mode incompISOOUT should be masked, similar as Bulk Out -
XferCompl and BNA should be handled. incompISOIN is not valid in DDMA
and is not getting asserted.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 696529a..d5f3d60 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3118,8 +3118,10 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
GINTSTS_GOUTNAKEFF | GINTSTS_GINNAKEFF |
GINTSTS_USBRST | GINTSTS_RESETDET |
GINTSTS_ENUMDONE | GINTSTS_OTGINT |
-   GINTSTS_USBSUSP | GINTSTS_WKUPINT |
-   GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT;
+   GINTSTS_USBSUSP | GINTSTS_WKUPINT;
+
+   if (!using_desc_dma(hsotg))
+   intmsk |= GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT;
 
if (hsotg->params.external_id_pin_ctl <= 0)
intmsk |= GINTSTS_CONIDSTSCHNG;
-- 
2.10.0

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


[PATCH] r8152: Fix error path in open function

2016-11-09 Thread Guenter Roeck
If usb_submit_urb() called from the open function fails, the following
crash may be observed.

r8152 8-1:1.0 eth0: intr_urb submit failed: -19
...
r8152 8-1:1.0 eth0: v1.08.3
Unable to handle kernel paging request at virtual address 6b6b6b6b6b6b6b7b
pgd = ffc0e7305000
[6b6b6b6b6b6b6b7b] *pgd=, *pud=
Internal error: Oops: 9604 [#1] PREEMPT SMP
...
PC is at notifier_chain_register+0x2c/0x58
LR is at blocking_notifier_chain_register+0x54/0x70
...
Call trace:
[] notifier_chain_register+0x2c/0x58
[] blocking_notifier_chain_register+0x54/0x70
[] register_pm_notifier+0x24/0x2c
[] rtl8152_open+0x3dc/0x3f8 [r8152]
[] __dev_open+0xac/0x104
[] __dev_change_flags+0xb0/0x148
[] dev_change_flags+0x34/0x70
[] do_setlink+0x2c8/0x888
[] rtnl_newlink+0x328/0x644
[] rtnetlink_rcv_msg+0x1a8/0x1d4
[] netlink_rcv_skb+0x68/0xd0
[] rtnetlink_rcv+0x2c/0x3c
[] netlink_unicast+0x16c/0x234
[] netlink_sendmsg+0x340/0x364
[] sock_sendmsg+0x48/0x60
[] SyS_sendto+0xe0/0x120
[] SyS_send+0x40/0x4c
[] el0_svc_naked+0x24/0x28

Clean up error handling to avoid registering the notifier if the open
function is going to fail.

Signed-off-by: Guenter Roeck 
---
 drivers/net/usb/r8152.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 44d439f50961..677922039548 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3266,10 +3266,8 @@ static int rtl8152_open(struct net_device *netdev)
goto out;
 
res = usb_autopm_get_interface(tp->intf);
-   if (res < 0) {
-   free_all_mem(tp);
-   goto out;
-   }
+   if (res < 0)
+   goto out_free;
 
mutex_lock(>control);
 
@@ -3285,10 +3283,9 @@ static int rtl8152_open(struct net_device *netdev)
netif_device_detach(tp->netdev);
netif_warn(tp, ifup, netdev, "intr_urb submit failed: %d\n",
   res);
-   free_all_mem(tp);
-   } else {
-   napi_enable(>napi);
+   goto out_unlock;
}
+   napi_enable(>napi);
 
mutex_unlock(>control);
 
@@ -3297,7 +3294,13 @@ static int rtl8152_open(struct net_device *netdev)
tp->pm_notifier.notifier_call = rtl_notifier;
register_pm_notifier(>pm_notifier);
 #endif
+   return 0;
 
+out_unlock:
+   mutex_unlock(>control);
+   usb_autopm_put_interface(tp->intf);
+out_free:
+   free_all_mem(tp);
 out:
return res;
 }
-- 
2.5.0

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


[PATCH v2 08/30] usb: dwc2: host: Rename MAX_DMA_DESC_SIZE to HOST_DMA_NBYTES_LIMIT

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Rename MAX_DMA_DESC_SIZE to HOST_DMA_NBYTES_LIMIT as it stores value of
host DMA descriptor transfer bytes' limit. Values are different in case
of gadget DMA descriptors.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/hcd_ddma.c | 4 ++--
 drivers/usb/dwc2/hw.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 70851b9..cf03677 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -693,8 +693,8 @@ static void dwc2_fill_host_dma_desc(struct dwc2_hsotg 
*hsotg,
struct dwc2_dma_desc *dma_desc = >desc_list[n_desc];
int len = chan->xfer_len;
 
-   if (len > MAX_DMA_DESC_SIZE - (chan->max_packet - 1))
-   len = MAX_DMA_DESC_SIZE - (chan->max_packet - 1);
+   if (len > HOST_DMA_NBYTES_LIMIT - (chan->max_packet - 1))
+   len = HOST_DMA_NBYTES_LIMIT - (chan->max_packet - 1);
 
if (chan->ep_is_in) {
int num_packets;
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 5fad5a1..0e5bfb3 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -820,6 +820,7 @@ struct dwc2_dma_desc {
 #define HOST_DMA_ISOC_NBYTES_SHIFT 0
 #define HOST_DMA_NBYTES_MASK   (0x1 << 0)
 #define HOST_DMA_NBYTES_SHIFT  0
+#define HOST_DMA_NBYTES_LIMIT  131071
 
 /* Device Mode DMA descriptor status quadlet */
 
@@ -856,7 +857,6 @@ struct dwc2_dma_desc {
 #define DEV_DMA_NBYTES_SHIFT   0
 #define DEV_DMA_NBYTES_LIMIT   0x
 
-#define MAX_DMA_DESC_SIZE  131071
 #define MAX_DMA_DESC_NUM_GENERIC   64
 #define MAX_DMA_DESC_NUM_HS_ISOC   256
 
-- 
2.10.0

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


[PATCH v2 14/30] usb: dwc2: gadget: Fixes for StsPhseRcvd interrupt

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

The StsPhseRcvd interrupt should not be enabled in slave mode.

Also move the StsPhsRcvd interrupt checking in the endpoint interrupt
handler to the correct order according to the databook. The interrupt
itself will be implemented in a later commit.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6c988c2..bcae58d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2552,9 +2552,6 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
if (idx == 0 && (ints & (DXEPINT_SETUP | DXEPINT_SETUP_RCVD)))
ints &= ~DXEPINT_XFERCOMPL;
 
-   if (ints & DXEPINT_STSPHSERCVD)
-   dev_dbg(hsotg->dev, "%s: StsPhseRcvd asserted\n", __func__);
-
if (ints & DXEPINT_XFERCOMPL) {
dev_dbg(hsotg->dev,
"%s: XferCompl: DxEPCTL=0x%08x, DXEPTSIZ=%08x\n",
@@ -2617,6 +2614,9 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
}
}
 
+   if (ints & DXEPINT_STSPHSERCVD)
+   dev_dbg(hsotg->dev, "%s: StsPhseRcvd\n", __func__);
+
if (ints & DXEPINT_BACK2BACKSETUP)
dev_dbg(hsotg->dev, "%s: B2BSetup/INEPNakEff\n", __func__);
 
@@ -2905,11 +2905,12 @@ void dwc2_hsotg_core_init_disconnected(struct 
dwc2_hsotg *hsotg,
 
/*
 * don't need XferCompl, we get that from RXFIFO in slave mode. In
-* DMA mode we may need this.
+* DMA mode we may need this and StsPhseRcvd.
 */
-   dwc2_writel((using_dma(hsotg) ? (DIEPMSK_XFERCOMPLMSK) : 0) |
+   dwc2_writel((using_dma(hsotg) ? (DIEPMSK_XFERCOMPLMSK |
+   DOEPMSK_STSPHSERCVDMSK) : 0) |
DOEPMSK_EPDISBLDMSK | DOEPMSK_AHBERRMSK |
-   DOEPMSK_SETUPMSK | DOEPMSK_STSPHSERCVDMSK,
+   DOEPMSK_SETUPMSK,
hsotg->regs + DOEPMSK);
 
dwc2_writel(0, hsotg->regs + DAINTMSK);
-- 
2.10.0

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


[PATCH v2 05/30] usb: dwc2: Add bindings to disable gadget DMA modes

2016-11-09 Thread John Youn
Now that the gadget driver automatically detects DMA modes, we need to
provide a way to disable them. Certain platforms may still have issues
with DMA and require it to be disabled. It is also needed for IP
validation purposes.

Signed-off-by: John Youn 
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 2 ++
 drivers/usb/dwc2/params.c  | 9 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 389bb13..10a2a4b 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,6 +26,8 @@ Refer to phy/phy-bindings.txt for generic phy consumer 
properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
 - snps,host-dma-disable: disable host DMA mode.
+- snps,gadget-dma-disable: disable gadget DMA mode.
+- snps,gadget-dma-desc-disable: disable gadget DMA descriptor mode.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget 
mode.
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 2f18a7b..64d5c66 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1091,17 +1091,22 @@ static void dwc2_set_gadget_dma(struct dwc2_hsotg 
*hsotg)
struct dwc2_hw_params *hw = >hw_params;
struct dwc2_core_params *p = >params;
bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
+   bool disable;
 
/* Buffer DMA */
+   disable = device_property_read_bool(hsotg->dev,
+   "snps,gadget-dma-disable");
dwc2_set_param_bool(hsotg, >g_dma,
false, "gadget-dma",
-   true, false,
+   !disable, false,
dma_capable);
 
/* DMA Descriptor */
+   disable = device_property_read_bool(hsotg->dev,
+   "snps,gadget-dma-desc-disable");
dwc2_set_param_bool(hsotg, >g_dma_desc, false,
"gadget-dma-desc",
-   p->g_dma, false,
+   p->g_dma && !disable, false,
!!hw->dma_desc_enable);
 }
 
-- 
2.10.0

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


[PATCH v2 01/30] usb: dwc2: Deprecate g-use-dma binding

2016-11-09 Thread John Youn
This is not needed as the gadget now fully supports DMA and it can
autodetect it. This was initially added because gadget DMA mode was only
partially implemented so could not be automatically enabled.

Signed-off-by: John Youn 
---
 Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++-
 arch/arm/boot/dts/rk3036.dtsi  |  1 -
 arch/arm/boot/dts/rk3288.dtsi  |  1 -
 arch/arm/boot/dts/rk3xxx.dtsi  |  1 -
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi  |  1 -
 arch/arm64/boot/dts/rockchip/rk3368.dtsi   |  1 -
 drivers/usb/dwc2/core.h|  4 +---
 drivers/usb/dwc2/params.c  | 17 ++---
 drivers/usb/dwc2/pci.c |  1 -
 9 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 9472111..389bb13 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,11 +26,13 @@ Refer to phy/phy-bindings.txt for generic phy consumer 
properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
 - snps,host-dma-disable: disable host DMA mode.
-- g-use-dma: enable dma usage in gadget driver.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget 
mode.
 
+Deprecated properties:
+- g-use-dma: gadget DMA mode is automatically detected
+
 Example:
 
 usb@101c {
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index a935523..7c2dc19 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -204,7 +204,6 @@
g-np-tx-fifo-size = <16>;
g-rx-fifo-size = <275>;
g-tx-fifo-size = <256 128 128 64 64 32>;
-   g-use-dma;
status = "disabled";
};
 
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 17ec2e2..74a749c 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -596,7 +596,6 @@
g-np-tx-fifo-size = <16>;
g-rx-fifo-size = <275>;
g-tx-fifo-size = <256 128 128 64 64 32>;
-   g-use-dma;
phys = <>;
phy-names = "usb2-phy";
status = "disabled";
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index e15beb3..8fbd3c8 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -181,7 +181,6 @@
g-np-tx-fifo-size = <16>;
g-rx-fifo-size = <275>;
g-tx-fifo-size = <256 128 128 64 64 32>;
-   g-use-dma;
phys = <>;
phy-names = "usb2-phy";
status = "disabled";
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 17839db..e0ea603 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -747,7 +747,6 @@
clocks = <_ctrl HI6220_USBOTG_HCLK>;
clock-names = "otg";
dr_mode = "otg";
-   g-use-dma;
g-rx-fifo-size = <512>;
g-np-tx-fifo-size = <128>;
g-tx-fifo-size = <128 128 128 128 128 128>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 0fcb214..df231c4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -537,7 +537,6 @@
g-np-tx-fifo-size = <16>;
g-rx-fifo-size = <275>;
g-tx-fifo-size = <256 128 128 64 64 32>;
-   g-use-dma;
status = "disabled";
};
 
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a1075ad..f8c97f5 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -418,9 +418,7 @@ enum dwc2_ep0_state {
  * needed.
  * 0 - No (default)
  * 1 - Yes
- * @g_dma:  If true, enables dma usage on the device. This
- *  setting is not auto-detected. It must be
- *  explicitly enabled (default: false).
+ * @g_dma:  Enables gadget dma usage (default: autodetect).
  * @g_rx_fifo_size:The periodic rx fifo size for the device, in
  * DWORDS from 16-32768 (default: 2048 if
  * possible, otherwise autodetect).
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 2eb79e8..74c3728 100644
--- a/drivers/usb/dwc2/params.c
+++ 

[PATCH v2 10/30] usb: dwc2: gadget: Add DDMA chain pointers to dwc2_hsotg_ep structure

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add DMA descriptor members to the dwc2_hsotg_ep structure.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  7 +++
 drivers/usb/dwc2/gadget.c | 33 +++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 26d4c17..4094d3d 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -172,6 +172,9 @@ struct dwc2_hsotg_req;
  * @periodic: Set if this is a periodic ep, such as Interrupt
  * @isochronous: Set if this is a isochronous ep
  * @send_zlp: Set if we need to send a zero-length packet.
+ * @desc_list_dma: The DMA address of descriptor chain currently in use.
+ * @desc_list: Pointer to descriptor DMA chain head currently in use.
+ * @desc_count: Count of entries within the DMA descriptor chain of EP.
  * @total_data: The total number of data bytes done.
  * @fifo_size: The size of the FIFO (for periodic IN endpoints)
  * @fifo_load: The amount of data loaded into the FIFO (periodic IN)
@@ -219,6 +222,10 @@ struct dwc2_hsotg_ep {
 #define TARGET_FRAME_INITIAL   0x
boolframe_overrun;
 
+   dma_addr_t  desc_list_dma;
+   struct dwc2_dma_desc*desc_list;
+   u8  desc_count;
+
charname[10];
 };
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4c1098f..3264375 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3077,6 +3077,18 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
dev_dbg(hsotg->dev, "%s: read DxEPCTL=0x%08x from 0x%08x\n",
__func__, epctrl, epctrl_reg);
 
+   /* Allocate DMA descriptor chain for non-ctrl endpoints */
+   if (using_desc_dma(hsotg)) {
+   hs_ep->desc_list = dma_alloc_coherent(hsotg->dev,
+   MAX_DMA_DESC_NUM_GENERIC *
+   sizeof(struct dwc2_dma_desc),
+   _ep->desc_list_dma, GFP_KERNEL);
+   if (!hs_ep->desc_list) {
+   ret = -ENOMEM;
+   goto error2;
+   }
+   }
+
spin_lock_irqsave(>lock, flags);
 
epctrl &= ~(DXEPCTL_EPTYPE_MASK | DXEPCTL_MPS_MASK);
@@ -3160,7 +3172,7 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
dev_err(hsotg->dev,
"%s: No suitable fifo found\n", __func__);
ret = -ENOMEM;
-   goto error;
+   goto error1;
}
hsotg->fifo_map |= 1 << fifo_index;
epctrl |= DXEPCTL_TXFNUM(fifo_index);
@@ -3182,8 +3194,17 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
/* enable the endpoint interrupt */
dwc2_hsotg_ctrl_epint(hsotg, index, dir_in, 1);
 
-error:
+error1:
spin_unlock_irqrestore(>lock, flags);
+
+error2:
+   if (ret && using_desc_dma(hsotg) && hs_ep->desc_list) {
+   dma_free_coherent(hsotg->dev, MAX_DMA_DESC_NUM_GENERIC *
+   sizeof(struct dwc2_dma_desc),
+   hs_ep->desc_list, hs_ep->desc_list_dma);
+   hs_ep->desc_list = NULL;
+   }
+
return ret;
 }
 
@@ -3208,6 +3229,14 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
return -EINVAL;
}
 
+   /* Remove DMA memory allocated for non-control Endpoints */
+   if (using_desc_dma(hsotg)) {
+   dma_free_coherent(hsotg->dev, MAX_DMA_DESC_NUM_GENERIC *
+ sizeof(struct dwc2_dma_desc),
+ hs_ep->desc_list, hs_ep->desc_list_dma);
+   hs_ep->desc_list = NULL;
+   }
+
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
 
spin_lock_irqsave(>lock, flags);
-- 
2.10.0

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


[PATCH v2 03/30] usb: dwc2: Make the DMA descriptor structure packed

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Make the DMA descriptor structure packed to guarantee alignment and size
in memory.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/hw.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 6031efe..ee827e8 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -802,7 +802,7 @@
 struct dwc2_dma_desc {
u32 status;
u32 buf;
-};
+} __packed;
 
 #define HOST_DMA_A (1 << 31)
 #define HOST_DMA_STS_MASK  (0x3 << 28)
-- 
2.10.0

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


[PATCH v2 04/30] usb: dwc2: gadget: Add descriptor DMA parameter

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add a parameter for descriptor DMA and set it based on hardware
capabilities. This won't actually be used by the gadget until later,
when the descriptor DMA code is in place.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  2 ++
 drivers/usb/dwc2/gadget.c | 11 +++
 drivers/usb/dwc2/params.c |  6 ++
 3 files changed, 19 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index f8c97f5..32a3cfc 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -419,6 +419,7 @@ enum dwc2_ep0_state {
  * 0 - No (default)
  * 1 - Yes
  * @g_dma:  Enables gadget dma usage (default: autodetect).
+ * @g_dma_desc: Enables gadget descriptor DMA (default: autodetect).
  * @g_rx_fifo_size:The periodic rx fifo size for the device, in
  * DWORDS from 16-32768 (default: 2048 if
  * possible, otherwise autodetect).
@@ -498,6 +499,7 @@ struct dwc2_core_params {
 
/* Gadget parameters */
bool g_dma;
+   bool g_dma_desc;
u16 g_rx_fifo_size;
u16 g_np_tx_fifo_size;
u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 1ba0bfc..4013518 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -96,6 +96,17 @@ static inline bool using_dma(struct dwc2_hsotg *hsotg)
return hsotg->params.g_dma;
 }
 
+/*
+ * using_desc_dma - return the descriptor DMA status of the driver.
+ * @hsotg: The driver state.
+ *
+ * Return true if we're using descriptor DMA.
+ */
+static inline bool using_desc_dma(struct dwc2_hsotg *hsotg)
+{
+   return hsotg->params.g_dma_desc;
+}
+
 /**
  * dwc2_gadget_incr_frame_num - Increments the targeted frame number.
  * @hs_ep: The endpoint
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 74c3728..2f18a7b 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1097,6 +1097,12 @@ static void dwc2_set_gadget_dma(struct dwc2_hsotg *hsotg)
false, "gadget-dma",
true, false,
dma_capable);
+
+   /* DMA Descriptor */
+   dwc2_set_param_bool(hsotg, >g_dma_desc, false,
+   "gadget-dma-desc",
+   p->g_dma, false,
+   !!hw->dma_desc_enable);
 }
 
 /**
-- 
2.10.0

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


[PATCH v2 02/30] usb: dwc2: Update DMA descriptor structure

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Rename DMA descriptor structure from dwc2_hcd_dma_desc to dwc2_dma_desc
as it is applies to both host and gadget.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/hcd.c  |  4 ++--
 drivers/usb/dwc2/hcd.h  |  2 +-
 drivers/usb/dwc2/hcd_ddma.c | 48 ++---
 drivers/usb/dwc2/hw.h   |  5 +++--
 4 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 3ac0085..393a962 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5110,7 +5110,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
if (hsotg->params.dma_desc_enable ||
hsotg->params.dma_desc_fs_enable) {
hsotg->desc_gen_cache = kmem_cache_create("dwc2-gen-desc",
-   sizeof(struct dwc2_hcd_dma_desc) *
+   sizeof(struct dwc2_dma_desc) *
MAX_DMA_DESC_NUM_GENERIC, 512, SLAB_CACHE_DMA,
NULL);
if (!hsotg->desc_gen_cache) {
@@ -5126,7 +5126,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
}
 
hsotg->desc_hsisoc_cache = kmem_cache_create("dwc2-hsisoc-desc",
-   sizeof(struct dwc2_hcd_dma_desc) *
+   sizeof(struct dwc2_dma_desc) *
MAX_DMA_DESC_NUM_HS_ISOC, 512, 0, NULL);
if (!hsotg->desc_hsisoc_cache) {
dev_err(hsotg->dev,
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 7758bfb..d92656d 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -348,7 +348,7 @@ struct dwc2_qh {
struct list_head qtd_list;
struct dwc2_host_chan *channel;
struct list_head qh_list_entry;
-   struct dwc2_hcd_dma_desc *desc_list;
+   struct dwc2_dma_desc *desc_list;
dma_addr_t desc_list_dma;
u32 desc_list_sz;
u32 *n_bytes;
diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 41e0a8d..70851b9 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -95,7 +95,7 @@ static int dwc2_desc_list_alloc(struct dwc2_hsotg *hsotg, 
struct dwc2_qh *qh,
else
desc_cache = hsotg->desc_gen_cache;
 
-   qh->desc_list_sz = sizeof(struct dwc2_hcd_dma_desc) *
+   qh->desc_list_sz = sizeof(struct dwc2_dma_desc) *
dwc2_max_desc_num(qh);
 
qh->desc_list = kmem_cache_zalloc(desc_cache, flags | GFP_DMA);
@@ -322,7 +322,7 @@ static void dwc2_release_channel_ddma(struct dwc2_hsotg 
*hsotg,
qh->ntd = 0;
 
if (qh->desc_list)
-   memset(qh->desc_list, 0, sizeof(struct dwc2_hcd_dma_desc) *
+   memset(qh->desc_list, 0, sizeof(struct dwc2_dma_desc) *
   dwc2_max_desc_num(qh));
 }
 
@@ -542,7 +542,7 @@ static void dwc2_fill_host_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
 struct dwc2_qh *qh, u32 max_xfer_size,
 u16 idx)
 {
-   struct dwc2_hcd_dma_desc *dma_desc = >desc_list[idx];
+   struct dwc2_dma_desc *dma_desc = >desc_list[idx];
struct dwc2_hcd_iso_packet_desc *frame_desc;
 
memset(dma_desc, 0, sizeof(*dma_desc));
@@ -571,8 +571,8 @@ static void dwc2_fill_host_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
 
dma_sync_single_for_device(hsotg->dev,
qh->desc_list_dma +
-   (idx * sizeof(struct dwc2_hcd_dma_desc)),
-   sizeof(struct dwc2_hcd_dma_desc),
+   (idx * sizeof(struct dwc2_dma_desc)),
+   sizeof(struct dwc2_dma_desc),
DMA_TO_DEVICE);
 }
 
@@ -645,8 +645,8 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
qh->desc_list[idx].status |= HOST_DMA_IOC;
dma_sync_single_for_device(hsotg->dev,
   qh->desc_list_dma + (idx *
-  sizeof(struct dwc2_hcd_dma_desc)),
-  sizeof(struct dwc2_hcd_dma_desc),
+  sizeof(struct dwc2_dma_desc)),
+  sizeof(struct dwc2_dma_desc),
   DMA_TO_DEVICE);
}
 #else
@@ -679,8 +679,8 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
qh->desc_list[idx].status |= HOST_DMA_IOC;
dma_sync_single_for_device(hsotg->dev,
   qh->desc_list_dma +
-  (idx * sizeof(struct dwc2_hcd_dma_desc)),
-  sizeof(struct 

[PATCH v2 06/30] usb: dwc2: gadget: Add DMA descriptor status quadlet fields

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add device mode DMA transfer descriptor status quadlet field notations.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/hw.h | 37 +
 1 file changed, 37 insertions(+)

diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index ee827e8..5fad5a1 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -804,6 +804,8 @@ struct dwc2_dma_desc {
u32 buf;
 } __packed;
 
+/* Host Mode DMA descriptor status quadlet */
+
 #define HOST_DMA_A (1 << 31)
 #define HOST_DMA_STS_MASK  (0x3 << 28)
 #define HOST_DMA_STS_SHIFT 28
@@ -819,6 +821,41 @@ struct dwc2_dma_desc {
 #define HOST_DMA_NBYTES_MASK   (0x1 << 0)
 #define HOST_DMA_NBYTES_SHIFT  0
 
+/* Device Mode DMA descriptor status quadlet */
+
+#define DEV_DMA_BUFF_STS_MASK  (0x3 << 30)
+#define DEV_DMA_BUFF_STS_SHIFT 30
+#define DEV_DMA_BUFF_STS_HREADY0
+#define DEV_DMA_BUFF_STS_DMABUSY   1
+#define DEV_DMA_BUFF_STS_DMADONE   2
+#define DEV_DMA_BUFF_STS_HBUSY 3
+#define DEV_DMA_STS_MASK   (0x3 << 28)
+#define DEV_DMA_STS_SHIFT  28
+#define DEV_DMA_STS_SUCC   0
+#define DEV_DMA_STS_BUFF_FLUSH 1
+#define DEV_DMA_STS_BUFF_ERR   3
+#define DEV_DMA_L  (1 << 27)
+#define DEV_DMA_SHORT  (1 << 26)
+#define DEV_DMA_IOC(1 << 25)
+#define DEV_DMA_SR (1 << 24)
+#define DEV_DMA_MTRF   (1 << 23)
+#define DEV_DMA_ISOC_PID_MASK  (0x3 << 23)
+#define DEV_DMA_ISOC_PID_SHIFT 23
+#define DEV_DMA_ISOC_PID_DATA0 0
+#define DEV_DMA_ISOC_PID_DATA2 1
+#define DEV_DMA_ISOC_PID_DATA1 2
+#define DEV_DMA_ISOC_PID_MDATA 3
+#define DEV_DMA_ISOC_FRNUM_MASK(0x7ff << 12)
+#define DEV_DMA_ISOC_FRNUM_SHIFT   12
+#define DEV_DMA_ISOC_TX_NBYTES_MASK(0xfff << 0)
+#define DEV_DMA_ISOC_TX_NBYTES_LIMIT   0xfff
+#define DEV_DMA_ISOC_RX_NBYTES_MASK(0x7ff << 0)
+#define DEV_DMA_ISOC_RX_NBYTES_LIMIT   0x7ff
+#define DEV_DMA_ISOC_NBYTES_SHIFT  0
+#define DEV_DMA_NBYTES_MASK(0x << 0)
+#define DEV_DMA_NBYTES_SHIFT   0
+#define DEV_DMA_NBYTES_LIMIT   0x
+
 #define MAX_DMA_DESC_SIZE  131071
 #define MAX_DMA_DESC_NUM_GENERIC   64
 #define MAX_DMA_DESC_NUM_HS_ISOC   256
-- 
2.10.0

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


[PATCH v2 00/30] usb: dwc2: Gadget descriptor DMA and IOT

2016-11-09 Thread John Youn
This series implements gadget-side descriptor DMA for the DWC_hsotg
controller.

It also includes support for DWC USB IOT controllers which use the
descriptor DMA mode of operation exclusively. These are two new
device-only USB controller IPs based on DWC_hsotg.

Tested on HAPS platform with:
* HSOTG IP version 3.30a
* FS/LS IOT IP version 1.00a
* HS IOT IP version 1.00a


v2:
* Remove the DMA 'enable' bindings and make them autodetected.
* Add DMA 'disable' bindings to override.
* Separate out commit to add '__packed' attribute.
* Don't print errors on -ENOMEM.
* Remove unnecessary GFP_ATOMIC flags.
* Remove unnecessary patch removing a WARN_ON.
* Reorganize and clarify BNA interrupt.
* Fix issue with enabling STSPHSERCVD interrupt.

Regards,
John


John Youn (4):
  usb: dwc2: Deprecate g-use-dma binding
  usb: dwc2: Update DMA descriptor structure
  usb: dwc2: Make the DMA descriptor structure packed
  usb: dwc2: Add bindings to disable gadget DMA modes

Vahram Aharonyan (23):
  usb: dwc2: gadget: Add descriptor DMA parameter
  usb: dwc2: gadget: Add DMA descriptor status quadlet fields
  usb: dwc2: gadget: Add DMA descriptor chains for EP 0
  usb: dwc2: host: Rename MAX_DMA_DESC_SIZE to HOST_DMA_NBYTES_LIMIT
  usb: dwc2: gadget: Transfer length limit checking for DDMA
  usb: dwc2: gadget: Add DDMA chain pointers to dwc2_hsotg_ep structure
  usb: dwc2: gadget: Add DDMA chain fill and parse functions
  usb: dwc2: gadget: EP 0 specific DDMA programming
  usb: dwc2: gadget: DDMA transfer start and complete
  usb: dwc2: gadget: Fixes for StsPhseRcvd interrupt
  usb: dwc2: gadget: Start DDMA IN status phase in StsPhseRcvd handler
  usb: dwc2: gadget: Enable descriptor DMA mode
  usb: dwc2: gadget: Add DDMA isoc related fields to dwc2_hsotg_ep
  usb: dwc2: gadget: Fill isoc descriptor and start transfer in DDMA
  usb: dwc2: gadget: Add completions for DDMA isoc transfers
  usb: dwc2: gadget: In DDMA keep incompISOOUT and incompISOIN masked
  usb: dwc2: gadget: Add start and complete calls for DDMA ISOC
  usb: dwc2: gadget: Enable the BNA interrupt
  usb: dwc2: gadget: Adjust ISOC OUT request's actual len for DDMA
  usb: dwc2: gadget: For DDMA parse setup only after SetUp interrupt
  usb: dwc2: gadget: Correct dwc2_hsotg_ep_stop_xfr() function
  usb: dwc2: gadget: Disable enabled HW endpoint in
dwc2_hsotg_ep_disable
  usb: dwc2: Add support of dedicated full-speed PHY interface

Vardan Mikayelyan (3):
  usb: dwc2: gadget: Add IOT device IDs, configure core accordingly
  usb: dwc2: gadget: Program ep0_mps for LS
  usb: dwc2: gadget: Add new core parameter for low speed

 Documentation/devicetree/bindings/usb/dwc2.txt |   6 +-
 arch/arm/boot/dts/rk3036.dtsi  |   1 -
 arch/arm/boot/dts/rk3288.dtsi  |   1 -
 arch/arm/boot/dts/rk3xxx.dtsi  |   1 -
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi  |   1 -
 arch/arm64/boot/dts/rockchip/rk3368.dtsi   |   1 -
 drivers/usb/dwc2/core.h|  50 +-
 drivers/usb/dwc2/gadget.c  | 968 ++---
 drivers/usb/dwc2/hcd.c |  12 +-
 drivers/usb/dwc2/hcd.h |   2 +-
 drivers/usb/dwc2/hcd_ddma.c|  52 +-
 drivers/usb/dwc2/hw.h  |  48 +-
 drivers/usb/dwc2/params.c  |  45 +-
 drivers/usb/dwc2/pci.c |   1 -
 14 files changed, 1023 insertions(+), 166 deletions(-)

-- 
2.10.0

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


[PATCH v2 11/30] usb: dwc2: gadget: Add DDMA chain fill and parse functions

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add dwc2_gadget_get_desc_params() function to define descriptor entry
parameters based on the endpoint type.

Add dwc2_gadget_config_nonisoc_xfer_ddma() function, which programs DDMA
chain entries with corresponding values based on the received DMA buffer
and transfer length in case of non-isochronous endpoint. Isochronous
endpoints' DMA descriptors will be filled separately.

Add dwc2_gadget_get_xfersize_ddma() function to iterate over DDMA chain
and get info on bytes remaining in descriptor status field after
transfer complete.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 138 ++
 1 file changed, 138 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 3264375..40a6c57 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -627,6 +627,114 @@ static unsigned int dwc2_gadget_get_chain_limit(struct 
dwc2_hsotg_ep *hs_ep)
return maxsize;
 }
 
+/*
+ * dwc2_gadget_get_desc_params - get DMA descriptor parameters.
+ * @hs_ep: The endpoint
+ * @mask: RX/TX bytes mask to be defined
+ *
+ * Returns maximum data payload for one descriptor after analyzing endpoint
+ * characteristics.
+ * DMA descriptor transfer bytes limit depends on EP type:
+ * Control out - MPS,
+ * Isochronous - descriptor rx/tx bytes bitfield limit,
+ * Control In/Bulk/Interrupt - multiple of mps. This will allow to not
+ * have concatenations from various descriptors within one packet.
+ *
+ * Selects corresponding mask for RX/TX bytes as well.
+ */
+static u32 dwc2_gadget_get_desc_params(struct dwc2_hsotg_ep *hs_ep, u32 *mask)
+{
+   u32 mps = hs_ep->ep.maxpacket;
+   int dir_in = hs_ep->dir_in;
+   u32 desc_size = 0;
+
+   if (!hs_ep->index && !dir_in) {
+   desc_size = mps;
+   *mask = DEV_DMA_NBYTES_MASK;
+   } else if (hs_ep->isochronous) {
+   if (dir_in) {
+   desc_size = DEV_DMA_ISOC_TX_NBYTES_LIMIT;
+   *mask = DEV_DMA_ISOC_TX_NBYTES_MASK;
+   } else {
+   desc_size = DEV_DMA_ISOC_RX_NBYTES_LIMIT;
+   *mask = DEV_DMA_ISOC_RX_NBYTES_MASK;
+   }
+   } else {
+   desc_size = DEV_DMA_NBYTES_LIMIT;
+   *mask = DEV_DMA_NBYTES_MASK;
+
+   /* Round down desc_size to be mps multiple */
+   desc_size -= desc_size % mps;
+   }
+
+   return desc_size;
+}
+
+/*
+ * dwc2_gadget_config_nonisoc_xfer_ddma - prepare non ISOC DMA desc chain.
+ * @hs_ep: The endpoint
+ * @dma_buff: DMA address to use
+ * @len: Length of the transfer
+ *
+ * This function will iterate over descriptor chain and fill its entries
+ * with corresponding information based on transfer data.
+ */
+static void dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep,
+dma_addr_t dma_buff,
+unsigned int len)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   int dir_in = hs_ep->dir_in;
+   struct dwc2_dma_desc *desc = hs_ep->desc_list;
+   u32 mps = hs_ep->ep.maxpacket;
+   u32 maxsize = 0;
+   u32 offset = 0;
+   u32 mask = 0;
+   int i;
+
+   maxsize = dwc2_gadget_get_desc_params(hs_ep, );
+
+   hs_ep->desc_count = (len / maxsize) +
+   ((len % maxsize) ? 1 : 0);
+   if (len == 0)
+   hs_ep->desc_count = 1;
+
+   for (i = 0; i < hs_ep->desc_count; ++i) {
+   desc->status = 0;
+   desc->status |= (DEV_DMA_BUFF_STS_HBUSY
+<< DEV_DMA_BUFF_STS_SHIFT);
+
+   if (len > maxsize) {
+   if (!hs_ep->index && !dir_in)
+   desc->status |= (DEV_DMA_L | DEV_DMA_IOC);
+
+   desc->status |= (maxsize <<
+   DEV_DMA_NBYTES_SHIFT & mask);
+   desc->buf = dma_buff + offset;
+
+   len -= maxsize;
+   offset += maxsize;
+   } else {
+   desc->status |= (DEV_DMA_L | DEV_DMA_IOC);
+
+   if (dir_in)
+   desc->status |= (len % mps) ? DEV_DMA_SHORT :
+   ((hs_ep->send_zlp) ? DEV_DMA_SHORT : 0);
+   if (len > maxsize)
+   dev_err(hsotg->dev, "wrong len %d\n", len);
+
+   desc->status |=
+   len << DEV_DMA_NBYTES_SHIFT & mask;
+   desc->buf = dma_buff + offset;
+   }
+
+   desc->status &= ~DEV_DMA_BUFF_STS_MASK;
+   desc->status |= (DEV_DMA_BUFF_STS_HREADY
+ 

[PATCH v2 07/30] usb: dwc2: gadget: Add DMA descriptor chains for EP 0

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Update dwc2_hsotg structure to add descriptor chains for EP 0: two DMA
descriptors for Setup phase, per one for IN/OUT data and status phases.
Add their allocation function dwc2_gadget_alloc_ctrl_desc_chains() and
its call during gadget probe.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   | 13 
 drivers/usb/dwc2/gadget.c | 54 +++
 2 files changed, 67 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 32a3cfc..26d4c17 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -884,6 +884,12 @@ struct dwc2_hregs_backup {
  * @ctrl_req:   Request for EP0 control packets.
  * @ep0_state:  EP0 control transfers state
  * @test_mode:  USB test mode requested by the host
+ * @setup_desc_dma:EP0 setup stage desc chain DMA address
+ * @setup_desc:EP0 setup stage desc chain pointer
+ * @ctrl_in_desc_dma:  EP0 IN data phase desc chain DMA address
+ * @ctrl_in_desc:  EP0 IN data phase desc chain pointer
+ * @ctrl_out_desc_dma: EP0 OUT data phase desc chain DMA address
+ * @ctrl_out_desc: EP0 OUT data phase desc chain pointer
  * @eps:The endpoints being supplied to the gadget framework
  */
 struct dwc2_hsotg {
@@ -1027,6 +1033,13 @@ struct dwc2_hsotg {
enum dwc2_ep0_state ep0_state;
u8 test_mode;
 
+   dma_addr_t setup_desc_dma[2];
+   struct dwc2_dma_desc *setup_desc[2];
+   dma_addr_t ctrl_in_desc_dma;
+   struct dwc2_dma_desc *ctrl_in_desc;
+   dma_addr_t ctrl_out_desc_dma;
+   struct dwc2_dma_desc *ctrl_out_desc;
+
struct usb_gadget gadget;
unsigned int enabled:1;
unsigned int connected:1;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4013518..729bc02d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -324,6 +324,54 @@ static void dwc2_hsotg_unmap_dma(struct dwc2_hsotg *hsotg,
usb_gadget_unmap_request(>gadget, req, hs_ep->dir_in);
 }
 
+/*
+ * dwc2_gadget_alloc_ctrl_desc_chains - allocate DMA descriptor chains
+ * for Control endpoint
+ * @hsotg: The device state.
+ *
+ * This function will allocate 4 descriptor chains for EP 0: 2 for
+ * Setup stage, per one for IN and OUT data/status transactions.
+ */
+static int dwc2_gadget_alloc_ctrl_desc_chains(struct dwc2_hsotg *hsotg)
+{
+   hsotg->setup_desc[0] =
+   dmam_alloc_coherent(hsotg->dev,
+   sizeof(struct dwc2_dma_desc),
+   >setup_desc_dma[0],
+   GFP_KERNEL);
+   if (!hsotg->setup_desc[0])
+   goto fail;
+
+   hsotg->setup_desc[1] =
+   dmam_alloc_coherent(hsotg->dev,
+   sizeof(struct dwc2_dma_desc),
+   >setup_desc_dma[1],
+   GFP_KERNEL);
+   if (!hsotg->setup_desc[1])
+   goto fail;
+
+   hsotg->ctrl_in_desc =
+   dmam_alloc_coherent(hsotg->dev,
+   sizeof(struct dwc2_dma_desc),
+   >ctrl_in_desc_dma,
+   GFP_KERNEL);
+   if (!hsotg->ctrl_in_desc)
+   goto fail;
+
+   hsotg->ctrl_out_desc =
+   dmam_alloc_coherent(hsotg->dev,
+   sizeof(struct dwc2_dma_desc),
+   >ctrl_out_desc_dma,
+   GFP_KERNEL);
+   if (!hsotg->ctrl_out_desc)
+   goto fail;
+
+   return 0;
+
+fail:
+   return -ENOMEM;
+}
+
 /**
  * dwc2_hsotg_write_fifo - write packet Data to the TxFIFO
  * @hsotg: The controller state.
@@ -3857,6 +3905,12 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
if (!hsotg->ep0_buff)
return -ENOMEM;
 
+   if (using_desc_dma(hsotg)) {
+   ret = dwc2_gadget_alloc_ctrl_desc_chains(hsotg);
+   if (ret < 0)
+   return ret;
+   }
+
ret = devm_request_irq(hsotg->dev, irq, dwc2_hsotg_irq, IRQF_SHARED,
dev_name(hsotg->dev), hsotg);
if (ret < 0) {
-- 
2.10.0

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


[PATCH v2 15/30] usb: dwc2: gadget: Start DDMA IN status phase in StsPhseRcvd handler

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

In DDMA mode of operation IN status phase of control write transfer
should start after getting StsPhseRcvd interrupt. This interrupt is
issued by HW once host starts to send IN tokens after data stage.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index bcae58d..e5d0959 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1991,7 +1991,9 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg 
*hsotg, int epnum)
 */
}
 
-   if (epnum == 0 && hsotg->ep0_state == DWC2_EP0_DATA_OUT) {
+   /* DDMA IN status phase will start from StsPhseRcvd interrupt */
+   if (!using_desc_dma(hsotg) && epnum == 0 &&
+   hsotg->ep0_state == DWC2_EP0_DATA_OUT) {
/* Move to STATUS IN */
dwc2_hsotg_ep0_zlp(hsotg, true);
return;
@@ -2614,9 +2616,14 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
}
}
 
-   if (ints & DXEPINT_STSPHSERCVD)
+   if (ints & DXEPINT_STSPHSERCVD) {
dev_dbg(hsotg->dev, "%s: StsPhseRcvd\n", __func__);
 
+   /* Move to STATUS IN for DDMA */
+   if (using_desc_dma(hsotg))
+   dwc2_hsotg_ep0_zlp(hsotg, true);
+   }
+
if (ints & DXEPINT_BACK2BACKSETUP)
dev_dbg(hsotg->dev, "%s: B2BSetup/INEPNakEff\n", __func__);
 
-- 
2.10.0

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


[PATCH v2 12/30] usb: dwc2: gadget: EP 0 specific DDMA programming

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add dwc2_gadget_set_ep0_desc_chain() function to switch between EP0 DDMA
chains depend on the stage of control transfer.

Include EP0 DDMA chain selection during ep_queue called from
dwc2_hsotg_enqueue_setup() for setup stage. Selecting and filling DDMA
chain for status phase as well - add calls of
dwc2_gadget_set_ep0_desc_chain() and
dwc2_gadget_config_nonisoc_xfer_ddma() functions.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 58 +++
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 40a6c57..eba0594 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1034,6 +1034,41 @@ static bool dwc2_gadget_target_frame_elapsed(struct 
dwc2_hsotg_ep *hs_ep)
return false;
 }
 
+/*
+ * dwc2_gadget_set_ep0_desc_chain - Set EP's desc chain pointers
+ * @hsotg: The driver state
+ * @hs_ep: the ep descriptor chain is for
+ *
+ * Called to update EP0 structure's pointers depend on stage of
+ * control transfer.
+ */
+static int dwc2_gadget_set_ep0_desc_chain(struct dwc2_hsotg *hsotg,
+ struct dwc2_hsotg_ep *hs_ep)
+{
+   switch (hsotg->ep0_state) {
+   case DWC2_EP0_SETUP:
+   case DWC2_EP0_STATUS_OUT:
+   hs_ep->desc_list = hsotg->setup_desc[0];
+   hs_ep->desc_list_dma = hsotg->setup_desc_dma[0];
+   break;
+   case DWC2_EP0_DATA_IN:
+   case DWC2_EP0_STATUS_IN:
+   hs_ep->desc_list = hsotg->ctrl_in_desc;
+   hs_ep->desc_list_dma = hsotg->ctrl_in_desc_dma;
+   break;
+   case DWC2_EP0_DATA_OUT:
+   hs_ep->desc_list = hsotg->ctrl_out_desc;
+   hs_ep->desc_list_dma = hsotg->ctrl_out_desc_dma;
+   break;
+   default:
+   dev_err(hsotg->dev, "invalid EP 0 state in queue %d\n",
+   hsotg->ep0_state);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct usb_request *req,
  gfp_t gfp_flags)
 {
@@ -1069,6 +1104,12 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct 
usb_request *req,
if (ret)
return ret;
}
+   /* If using descriptor DMA configure EP0 descriptor chain pointers */
+   if (using_desc_dma(hs) && !hs_ep->index) {
+   ret = dwc2_gadget_set_ep0_desc_chain(hs, hs_ep);
+   if (ret)
+   return ret;
+   }
 
first = list_empty(_ep->queue);
list_add_tail(_req->queue, _ep->queue);
@@ -1637,14 +1678,21 @@ static void dwc2_hsotg_program_zlp(struct dwc2_hsotg 
*hsotg,
 
if (hs_ep->dir_in)
dev_dbg(hsotg->dev, "Sending zero-length packet on ep%d\n",
-   index);
+   index);
else
dev_dbg(hsotg->dev, "Receiving zero-length packet on ep%d\n",
-   index);
+   index);
+   if (using_desc_dma(hsotg)) {
+   /* Not specific buffer needed for ep0 ZLP */
+   dma_addr_t dma = hs_ep->desc_list_dma;
 
-   dwc2_writel(DXEPTSIZ_MC(1) | DXEPTSIZ_PKTCNT(1) |
-   DXEPTSIZ_XFERSIZE(0), hsotg->regs +
-   epsiz_reg);
+   dwc2_gadget_set_ep0_desc_chain(hsotg, hs_ep);
+   dwc2_gadget_config_nonisoc_xfer_ddma(hs_ep, dma, 0);
+   } else {
+   dwc2_writel(DXEPTSIZ_MC(1) | DXEPTSIZ_PKTCNT(1) |
+   DXEPTSIZ_XFERSIZE(0), hsotg->regs +
+   epsiz_reg);
+   }
 
ctrl = dwc2_readl(hsotg->regs + epctl_reg);
ctrl |= DXEPCTL_CNAK;  /* clear NAK set by core */
-- 
2.10.0

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


[PATCH v2 13/30] usb: dwc2: gadget: DDMA transfer start and complete

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Update transfer starting dwc2_hsotg_start_req() routine with call of
function dwc2_gadget_config_nonisoc_xfer_ddma() to fill descriptor
chain.

Add call of dwc2_gadget_get_xfersize_ddma() in
dwc2_hsotg_handle_outdone() and dwc2_hsotg_complete_in() interrupt
handlers for DDMA mode to get information on transferred data from
descriptors instead of DXEPTSIZ.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 66 +--
 1 file changed, 53 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index eba0594..6c988c2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -760,6 +760,7 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
unsigned length;
unsigned packets;
unsigned maxreq;
+   unsigned int dma_reg;
 
if (index != 0) {
if (hs_ep->req && !continuing) {
@@ -774,6 +775,7 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
}
}
 
+   dma_reg = dir_in ? DIEPDMA(index) : DOEPDMA(index);
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
epsize_reg = dir_in ? DIEPTSIZ(index) : DOEPTSIZ(index);
 
@@ -849,22 +851,51 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
/* store the request as the current one we're doing */
hs_ep->req = hs_req;
 
-   /* write size / packets */
-   dwc2_writel(epsize, hsotg->regs + epsize_reg);
-
-   if (using_dma(hsotg) && !continuing) {
-   unsigned int dma_reg;
+   if (using_desc_dma(hsotg)) {
+   u32 offset = 0;
+   u32 mps = hs_ep->ep.maxpacket;
+
+   /* Adjust length: EP0 - MPS, other OUT EPs - multiple of MPS */
+   if (!dir_in) {
+   if (!index)
+   length = mps;
+   else if (length % mps)
+   length += (mps - (length % mps));
+   }
 
/*
-* write DMA address to control register, buffer already
-* synced by dwc2_hsotg_ep_queue().
+* If more data to send, adjust DMA for EP0 out data stage.
+* ureq->dma stays unchanged, hence increment it by already
+* passed passed data count before starting new transaction.
 */
+   if (!index && hsotg->ep0_state == DWC2_EP0_DATA_OUT &&
+   continuing)
+   offset = ureq->actual;
+
+   /* Fill DDMA chain entries */
+   dwc2_gadget_config_nonisoc_xfer_ddma(hs_ep, ureq->dma + offset,
+length);
+
+   /* write descriptor chain address to control register */
+   dwc2_writel(hs_ep->desc_list_dma, hsotg->regs + dma_reg);
 
-   dma_reg = dir_in ? DIEPDMA(index) : DOEPDMA(index);
-   dwc2_writel(ureq->dma, hsotg->regs + dma_reg);
+   dev_dbg(hsotg->dev, "%s: %08x pad => 0x%08x\n",
+   __func__, (u32)hs_ep->desc_list_dma, dma_reg);
+   } else {
+   /* write size / packets */
+   dwc2_writel(epsize, hsotg->regs + epsize_reg);
+
+   if (using_dma(hsotg) && !continuing) {
+   /*
+* write DMA address to control register, buffer
+* already synced by dwc2_hsotg_ep_queue().
+*/
 
-   dev_dbg(hsotg->dev, "%s: %pad => 0x%08x\n",
-   __func__, >dma, dma_reg);
+   dwc2_writel(ureq->dma, hsotg->regs + dma_reg);
+
+   dev_dbg(hsotg->dev, "%s: %pad => 0x%08x\n",
+   __func__, >dma, dma_reg);
+   }
}
 
if (hs_ep->isochronous && hs_ep->interval == 1) {
@@ -1923,6 +1954,9 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg 
*hsotg, int epnum)
return;
}
 
+   if (using_desc_dma(hsotg))
+   size_left = dwc2_gadget_get_xfersize_ddma(hs_ep);
+
if (using_dma(hsotg)) {
unsigned size_done;
 
@@ -2254,8 +2288,14 @@ static void dwc2_hsotg_complete_in(struct dwc2_hsotg 
*hsotg,
 * past the end of the buffer (DMA transfers are always 32bit
 * aligned).
 */
-
-   size_left = DXEPTSIZ_XFERSIZE_GET(epsize);
+   if (using_desc_dma(hsotg)) {
+   size_left = dwc2_gadget_get_xfersize_ddma(hs_ep);
+   if (size_left < 0)
+   dev_err(hsotg->dev, "error parsing DDMA results %d\n",
+   size_left);
+   } else {
+   size_left = DXEPTSIZ_XFERSIZE_GET(epsize);
+   }
 

[PATCH v2 25/30] usb: dwc2: gadget: Correct dwc2_hsotg_ep_stop_xfr() function

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Correct dwc2_hsotg_ep_stop_xfr() function to follow dwc2 programming
guide for setting NAK on specific endpoint, disabling it and flushing
corresponding FIFO.

Current code does not take into account whether core acts in shared or
dedicated FIFO mode, current endpoint is periodic or not. It does not
clear EPDISBLD interrupt after programming of DXEPCTL_EPDIS, does not
flush shared TX FIFO and tries to clear global out NAK in wrong manner
instead of setting DCTL_CGOUTNAK.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 64 ++-
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 5930b1a..143577b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3867,23 +3867,35 @@ static void dwc2_hsotg_ep_stop_xfr(struct dwc2_hsotg 
*hsotg,
DOEPINT(hs_ep->index);
 
dev_dbg(hsotg->dev, "%s: stopping transfer on %s\n", __func__,
-   hs_ep->name);
+   hs_ep->name);
+
if (hs_ep->dir_in) {
-   __orr32(hsotg->regs + epctrl_reg, DXEPCTL_SNAK);
-   /* Wait for Nak effect */
-   if (dwc2_hsotg_wait_bit_set(hsotg, epint_reg,
-   DXEPINT_INEPNAKEFF, 100))
-   dev_warn(hsotg->dev,
-   "%s: timeout DIEPINT.NAKEFF\n", __func__);
+   if (hsotg->dedicated_fifos || hs_ep->periodic) {
+   __orr32(hsotg->regs + epctrl_reg, DXEPCTL_SNAK);
+   /* Wait for Nak effect */
+   if (dwc2_hsotg_wait_bit_set(hsotg, epint_reg,
+   DXEPINT_INEPNAKEFF, 100))
+   dev_warn(hsotg->dev,
+"%s: timeout DIEPINT.NAKEFF\n",
+__func__);
+   } else {
+   __orr32(hsotg->regs + DCTL, DCTL_SGNPINNAK);
+   /* Wait for Nak effect */
+   if (dwc2_hsotg_wait_bit_set(hsotg, GINTSTS,
+   GINTSTS_GINNAKEFF, 100))
+   dev_warn(hsotg->dev,
+"%s: timeout GINTSTS.GINNAKEFF\n",
+__func__);
+   }
} else {
if (!(dwc2_readl(hsotg->regs + GINTSTS) & GINTSTS_GOUTNAKEFF))
__orr32(hsotg->regs + DCTL, DCTL_SGOUTNAK);
 
/* Wait for global nak to take effect */
if (dwc2_hsotg_wait_bit_set(hsotg, GINTSTS,
-   GINTSTS_GOUTNAKEFF, 100))
-   dev_warn(hsotg->dev,
-   "%s: timeout GINTSTS.GOUTNAKEFF\n", __func__);
+   GINTSTS_GOUTNAKEFF, 100))
+   dev_warn(hsotg->dev, "%s: timeout GINTSTS.GOUTNAKEFF\n",
+__func__);
}
 
/* Disable ep */
@@ -3892,23 +3904,29 @@ static void dwc2_hsotg_ep_stop_xfr(struct dwc2_hsotg 
*hsotg,
/* Wait for ep to be disabled */
if (dwc2_hsotg_wait_bit_set(hsotg, epint_reg, DXEPINT_EPDISBLD, 100))
dev_warn(hsotg->dev,
-   "%s: timeout DOEPCTL.EPDisable\n", __func__);
+"%s: timeout DOEPCTL.EPDisable\n", __func__);
+
+   /* Clear EPDISBLD interrupt */
+   __orr32(hsotg->regs + epint_reg, DXEPINT_EPDISBLD);
 
if (hs_ep->dir_in) {
-   if (hsotg->dedicated_fifos) {
-   dwc2_writel(GRSTCTL_TXFNUM(hs_ep->fifo_index) |
-   GRSTCTL_TXFFLSH, hsotg->regs + GRSTCTL);
-   /* Wait for fifo flush */
-   if (dwc2_hsotg_wait_bit_set(hsotg, GRSTCTL,
-   GRSTCTL_TXFFLSH, 100))
-   dev_warn(hsotg->dev,
-   "%s: timeout flushing fifos\n",
-   __func__);
-   }
-   /* TODO: Flush shared tx fifo */
+   unsigned short fifo_index;
+
+   if (hsotg->dedicated_fifos || hs_ep->periodic)
+   fifo_index = hs_ep->fifo_index;
+   else
+   fifo_index = 0;
+
+   /* Flush TX FIFO */
+   dwc2_flush_tx_fifo(hsotg, fifo_index);
+
+   /* Clear Global In NP NAK in Shared FIFO for non periodic ep */
+   if (!hsotg->dedicated_fifos && !hs_ep->periodic)
+   __orr32(hsotg->regs + DCTL, DCTL_CGNPINNAK);
+
} else {

[PATCH v2 09/30] usb: dwc2: gadget: Transfer length limit checking for DDMA

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add dwc2_gadget_get_chain_limit() function and its call in transfer
start routine to correctly estimate one go on transfer size if
descriptor DMA mode is selected.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 729bc02d..4c1098f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -601,6 +601,32 @@ static u32 dwc2_hsotg_read_frameno(struct dwc2_hsotg 
*hsotg)
return dsts;
 }
 
+/**
+ * dwc2_gadget_get_chain_limit - get the maximum data payload value of the
+ * DMA descriptor chain prepared for specific endpoint
+ * @hs_ep: The endpoint
+ *
+ * Return the maximum data that can be queued in one go on a given endpoint
+ * depending on its descriptor chain capacity so that transfers that
+ * are too long can be split.
+ */
+static unsigned int dwc2_gadget_get_chain_limit(struct dwc2_hsotg_ep *hs_ep)
+{
+   int is_isoc = hs_ep->isochronous;
+   unsigned int maxsize;
+
+   if (is_isoc)
+   maxsize = hs_ep->dir_in ? DEV_DMA_ISOC_TX_NBYTES_LIMIT :
+  DEV_DMA_ISOC_RX_NBYTES_LIMIT;
+   else
+   maxsize = DEV_DMA_NBYTES_LIMIT;
+
+   /* Above size of one descriptor was chosen, multiple it */
+   maxsize *= MAX_DMA_DESC_NUM_GENERIC;
+
+   return maxsize;
+}
+
 /**
  * dwc2_hsotg_start_req - start a USB request from an endpoint's queue
  * @hsotg: The controller state.
@@ -658,8 +684,12 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
length = ureq->length - ureq->actual;
dev_dbg(hsotg->dev, "ureq->length:%d ureq->actual:%d\n",
ureq->length, ureq->actual);
+
+   if (!using_desc_dma(hsotg))
+   maxreq = get_ep_limit(hs_ep);
+   else
+   maxreq = dwc2_gadget_get_chain_limit(hs_ep);
 
-   maxreq = get_ep_limit(hs_ep);
if (length > maxreq) {
int round = maxreq % hs_ep->ep.maxpacket;
 
-- 
2.10.0

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


[PATCH v2 24/30] usb: dwc2: gadget: For DDMA parse setup only after SetUp interrupt

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Tests with various hosts show that depend on time difference between
host sending SETUP packet and IN/OUT token SW could get Xfercomplete
interrupt without SetUp interrupt. On the other hand, SW should parse
received SETUP packet only after ensuring that Host has moved to either
Data or Status stage of control transfer.

For this purpose added checking in the dwc2_hsotg_epint() function to
not handle xfercomplete and postpone SETUP packet analysis till SW's
getting of setup phase done interrupt.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d2442f4..5930b1a 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2838,6 +2838,16 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
if (idx == 0 && (ints & (DXEPINT_SETUP | DXEPINT_SETUP_RCVD)))
ints &= ~DXEPINT_XFERCOMPL;
 
+   /*
+* Don't process XferCompl interrupt in DDMA if EP0 is still in SETUP
+* stage and xfercomplete was generated without SETUP phase done
+* interrupt. SW should parse received setup packet only after host's
+* exit from setup phase of control transfer.
+*/
+   if (using_desc_dma(hsotg) && idx == 0 && !hs_ep->dir_in &&
+   hsotg->ep0_state == DWC2_EP0_SETUP && !(ints & DXEPINT_SETUP))
+   ints &= ~DXEPINT_XFERCOMPL;
+
if (ints & DXEPINT_XFERCOMPL) {
dev_dbg(hsotg->dev,
"%s: XferCompl: DxEPCTL=0x%08x, DXEPTSIZ=%08x\n",
-- 
2.10.0

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


[PATCH v2 22/30] usb: dwc2: gadget: Enable the BNA interrupt

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Enable the BNA (Buffer Not Available) interrupt in descriptor DMA mode.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 4 
 drivers/usb/dwc2/hw.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6f74a3f..9f28756 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3227,6 +3227,10 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
DOEPMSK_SETUPMSK,
hsotg->regs + DOEPMSK);
 
+   /* Enable BNA interrupt for DDMA */
+   if (using_desc_dma(hsotg))
+   __orr32(hsotg->regs + DOEPMSK, DOEPMSK_BNAMSK);
+
dwc2_writel(0, hsotg->regs + DAINTMSK);
 
dev_dbg(hsotg->dev, "EP0: DIEPCTL0=0x%08x, DOEPCTL0=0x%08x\n",
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 631396b..5be056b 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -474,6 +474,7 @@
 #define DIEPMSK_XFERCOMPLMSK   (1 << 0)
 
 #define DOEPMSKHSOTG_REG(0x814)
+#define DOEPMSK_BNAMSK (1 << 9)
 #define DOEPMSK_BACK2BACKSETUP (1 << 6)
 #define DOEPMSK_STSPHSERCVDMSK (1 << 5)
 #define DOEPMSK_OUTTKNEPDISMSK (1 << 4)
-- 
2.10.0

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


[PATCH v2 17/30] usb: dwc2: gadget: Add DDMA isoc related fields to dwc2_hsotg_ep

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Preparing for isochronous transfers support adding in DDMA mode. In DDMA
isochronous transfers are handled differently compared to Slave and BDMA
modes. This is caused by fact that isoc requests contain data for one
frame/microframe. HW descriptor should contain data of one
frame/microframe as well. Hence each DMA descriptor in the chain will
correspond to one usb request.

Decided to divide endpoints descriptor chain to two halves - while one
will be processed by HW, other one will be under SW control. First part
will be passed to HW once ISOC traffic needs to be started. In parallel
to HW's work SW will keep creating new entries in the other half of
chain if new requests arrive in ep_queue routine. This will allow
passing of already pre-prepared descriptors to HW immediately after
endpoint gets disabled. The endpoint should be disabled once HW closes
descriptor with "L" bit set. Afterwards SW will switch to use first part
of chain if more requests are arriving.

Add two members to the dwc2_hsotg_ep structure to be used in isochronous
transfers' handling in DDMA mode:

-isoc_chain_num - indicates which half of EP descriptor chain can be
used by SW to add new queued requests while HW is
processing other half.

-next_desc - index which points to next not yet programmed descriptor in
the half of descriptor chain which is under SW control.

Also add initialization of these fields in function
dwc2_hsotg_ep_enable().

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   | 5 +
 drivers/usb/dwc2/gadget.c | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 4094d3d..5fa05a3 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -175,6 +175,8 @@ struct dwc2_hsotg_req;
  * @desc_list_dma: The DMA address of descriptor chain currently in use.
  * @desc_list: Pointer to descriptor DMA chain head currently in use.
  * @desc_count: Count of entries within the DMA descriptor chain of EP.
+ * @isoc_chain_num: Number of ISOC chain currently in use - either 0 or 1.
+ * @next_desc: index of next free descriptor in the ISOC chain under SW 
control.
  * @total_data: The total number of data bytes done.
  * @fifo_size: The size of the FIFO (for periodic IN endpoints)
  * @fifo_load: The amount of data loaded into the FIFO (periodic IN)
@@ -226,6 +228,9 @@ struct dwc2_hsotg_ep {
struct dwc2_dma_desc*desc_list;
u8  desc_count;
 
+   unsigned char   isoc_chain_num;
+   unsigned intnext_desc;
+
charname[10];
 };
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e12b1f0..c32520d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3356,6 +3356,8 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
hs_ep->isochronous = 1;
hs_ep->interval = 1 << (desc->bInterval - 1);
hs_ep->target_frame = TARGET_FRAME_INITIAL;
+   hs_ep->isoc_chain_num = 0;
+   hs_ep->next_desc = 0;
if (dir_in) {
hs_ep->periodic = 1;
mask = dwc2_readl(hsotg->regs + DIEPMSK);
-- 
2.10.0

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


[PATCH v2 21/30] usb: dwc2: gadget: Add start and complete calls for DDMA ISOC

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add dwc2_gadget_start_isoc_ddma() function calls in NAK and OUTTknEPDis
interrupts to start isochronous transfer once Host has sent a token.

Add call of dwc2_gadget_fill_isoc_desc() function in
dwc2_hsotg_ep_queue() to fill descriptor chain's half that is under SW
control by queued usb requests if isochronous transfer has started
before.

Add return in dwc2_hsotg_complete_request() for DDMA isochronous
transfers not to proceed with next request starting routine.

Add call of functions dwc2_gadget_complete_isoc_request_ddma() and
dwc2_gadget_start_next_isoc_ddma() in XferCompl interrupt handler.

Add call of dwc2_gadget_start_next_isoc_ddma() in BNA interrupt handler
to restart OUT endpoint once it gets disabled due to BNA interrupt
assertion.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 71 +++
 1 file changed, 66 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d5f3d60..6f74a3f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1278,6 +1278,22 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct 
usb_request *req,
first = list_empty(_ep->queue);
list_add_tail(_req->queue, _ep->queue);
 
+   /*
+* Handle DDMA isochronous transfers separately - just add new entry
+* to the half of descriptor chain that is not processed by HW.
+* Transfer will be started once SW gets either one of NAK or
+* OutTknEpDis interrupts.
+*/
+   if (using_desc_dma(hs) && hs_ep->isochronous &&
+   hs_ep->target_frame != TARGET_FRAME_INITIAL) {
+   ret = dwc2_gadget_fill_isoc_desc(hs_ep, hs_req->req.dma,
+hs_req->req.length);
+   if (ret)
+   dev_dbg(hs->dev, "%s: ISO desc chain full\n", __func__);
+
+   return 0;
+   }
+
if (first) {
if (!hs_ep->isochronous) {
dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
@@ -1919,6 +1935,10 @@ static void dwc2_hsotg_complete_request(struct 
dwc2_hsotg *hsotg,
spin_lock(>lock);
}
 
+   /* In DDMA don't need to proceed to starting of next ISOC request */
+   if (using_desc_dma(hsotg) && hs_ep->isochronous)
+   return;
+
/*
 * Look to see if there is anything else to do. Note, the completion
 * of the previous request may have caused a new request to be started
@@ -2683,12 +2703,28 @@ static void 
dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
struct dwc2_hsotg *hsotg = ep->parent;
int dir_in = ep->dir_in;
u32 doepmsk;
+   u32 tmp;
 
if (dir_in || !ep->isochronous)
return;
 
+   /*
+* Store frame in which irq was asserted here, as
+* it can change while completing request below.
+*/
+   tmp = dwc2_hsotg_read_frameno(hsotg);
+
dwc2_hsotg_complete_request(hsotg, ep, get_ep_head(ep), -ENODATA);
 
+   if (using_desc_dma(hsotg)) {
+   if (ep->target_frame == TARGET_FRAME_INITIAL) {
+   /* Start first ISO Out */
+   ep->target_frame = tmp;
+   dwc2_gadget_start_isoc_ddma(ep);
+   }
+   return;
+   }
+
if (ep->interval > 1 &&
ep->target_frame == TARGET_FRAME_INITIAL) {
u32 dsts;
@@ -2737,6 +2773,12 @@ static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep 
*hs_ep)
 
if (hs_ep->target_frame == TARGET_FRAME_INITIAL) {
hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+
+   if (using_desc_dma(hsotg)) {
+   dwc2_gadget_start_isoc_ddma(hs_ep);
+   return;
+   }
+
if (hs_ep->interval > 1) {
u32 ctrl = dwc2_readl(hsotg->regs +
  DIEPCTL(hs_ep->index));
@@ -2798,11 +2840,17 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, 
unsigned int idx,
__func__, dwc2_readl(hsotg->regs + epctl_reg),
dwc2_readl(hsotg->regs + epsiz_reg));
 
-   /*
-* we get OutDone from the FIFO, so we only need to look
-* at completing IN requests here
-*/
-   if (dir_in) {
+   /* In DDMA handle isochronous requests separately */
+   if (using_desc_dma(hsotg) && hs_ep->isochronous) {
+   dwc2_gadget_complete_isoc_request_ddma(hs_ep);
+   /* Try to start next isoc request */
+   dwc2_gadget_start_next_isoc_ddma(hs_ep);
+   } else if (dir_in) {
+

[PATCH v2 16/30] usb: dwc2: gadget: Enable descriptor DMA mode

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Add DCFG register field macro for descriptor DMA mode and update core
initialization routine to set that bit accordingly.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 10 --
 drivers/usb/dwc2/hw.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e5d0959..e12b1f0 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2888,15 +2888,21 @@ void dwc2_hsotg_core_init_disconnected(struct 
dwc2_hsotg *hsotg,
 
dwc2_writel(intmsk, hsotg->regs + GINTMSK);
 
-   if (using_dma(hsotg))
+   if (using_dma(hsotg)) {
dwc2_writel(GAHBCFG_GLBL_INTR_EN | GAHBCFG_DMA_EN |
(GAHBCFG_HBSTLEN_INCR4 << GAHBCFG_HBSTLEN_SHIFT),
hsotg->regs + GAHBCFG);
-   else
+
+   /* Set DDMA mode support in the core if needed */
+   if (using_desc_dma(hsotg))
+   __orr32(hsotg->regs + DCFG, DCFG_DESCDMA_EN);
+
+   } else {
dwc2_writel(((hsotg->dedicated_fifos) ?
(GAHBCFG_NP_TXF_EMP_LVL |
 GAHBCFG_P_TXF_EMP_LVL) : 0) |
GAHBCFG_GLBL_INTR_EN, hsotg->regs + GAHBCFG);
+   }
 
/*
 * If INTknTXFEmpMsk is enabled, it's important to disable ep interrupts
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 0e5bfb3..631396b 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -412,6 +412,7 @@
 /* Device mode registers */
 
 #define DCFG   HSOTG_REG(0x800)
+#define DCFG_DESCDMA_EN(1 << 23)
 #define DCFG_EPMISCNT_MASK (0x1f << 18)
 #define DCFG_EPMISCNT_SHIFT18
 #define DCFG_EPMISCNT_LIMIT0x1f
-- 
2.10.0

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


[PATCH v2 26/30] usb: dwc2: gadget: Disable enabled HW endpoint in dwc2_hsotg_ep_disable

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Check if endpoint is enabled during dwc2_hsotg_ep_disable() function
processing and call dwc2_hsotg_ep_stop_xfr() to disable it and flush
associated FIFO.

Move dwc2_hsotg_ep_stop_xfr() and dwc2_hsotg_wait_bit_set() functions
upper before dwc2_hsotg_ep_enable and dwc2_hsotg_ep_disable function
definitions.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 182 +++---
 1 file changed, 93 insertions(+), 89 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 143577b..0b4f0bd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3585,6 +3585,95 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
return IRQ_HANDLED;
 }
 
+static int dwc2_hsotg_wait_bit_set(struct dwc2_hsotg *hs_otg, u32 reg,
+  u32 bit, u32 timeout)
+{
+   u32 i;
+
+   for (i = 0; i < timeout; i++) {
+   if (dwc2_readl(hs_otg->regs + reg) & bit)
+   return 0;
+   udelay(1);
+   }
+
+   return -ETIMEDOUT;
+}
+
+static void dwc2_hsotg_ep_stop_xfr(struct dwc2_hsotg *hsotg,
+  struct dwc2_hsotg_ep *hs_ep)
+{
+   u32 epctrl_reg;
+   u32 epint_reg;
+
+   epctrl_reg = hs_ep->dir_in ? DIEPCTL(hs_ep->index) :
+   DOEPCTL(hs_ep->index);
+   epint_reg = hs_ep->dir_in ? DIEPINT(hs_ep->index) :
+   DOEPINT(hs_ep->index);
+
+   dev_dbg(hsotg->dev, "%s: stopping transfer on %s\n", __func__,
+   hs_ep->name);
+
+   if (hs_ep->dir_in) {
+   if (hsotg->dedicated_fifos || hs_ep->periodic) {
+   __orr32(hsotg->regs + epctrl_reg, DXEPCTL_SNAK);
+   /* Wait for Nak effect */
+   if (dwc2_hsotg_wait_bit_set(hsotg, epint_reg,
+   DXEPINT_INEPNAKEFF, 100))
+   dev_warn(hsotg->dev,
+"%s: timeout DIEPINT.NAKEFF\n",
+__func__);
+   } else {
+   __orr32(hsotg->regs + DCTL, DCTL_SGNPINNAK);
+   /* Wait for Nak effect */
+   if (dwc2_hsotg_wait_bit_set(hsotg, GINTSTS,
+   GINTSTS_GINNAKEFF, 100))
+   dev_warn(hsotg->dev,
+"%s: timeout GINTSTS.GINNAKEFF\n",
+__func__);
+   }
+   } else {
+   if (!(dwc2_readl(hsotg->regs + GINTSTS) & GINTSTS_GOUTNAKEFF))
+   __orr32(hsotg->regs + DCTL, DCTL_SGOUTNAK);
+
+   /* Wait for global nak to take effect */
+   if (dwc2_hsotg_wait_bit_set(hsotg, GINTSTS,
+   GINTSTS_GOUTNAKEFF, 100))
+   dev_warn(hsotg->dev, "%s: timeout GINTSTS.GOUTNAKEFF\n",
+__func__);
+   }
+
+   /* Disable ep */
+   __orr32(hsotg->regs + epctrl_reg, DXEPCTL_EPDIS | DXEPCTL_SNAK);
+
+   /* Wait for ep to be disabled */
+   if (dwc2_hsotg_wait_bit_set(hsotg, epint_reg, DXEPINT_EPDISBLD, 100))
+   dev_warn(hsotg->dev,
+"%s: timeout DOEPCTL.EPDisable\n", __func__);
+
+   /* Clear EPDISBLD interrupt */
+   __orr32(hsotg->regs + epint_reg, DXEPINT_EPDISBLD);
+
+   if (hs_ep->dir_in) {
+   unsigned short fifo_index;
+
+   if (hsotg->dedicated_fifos || hs_ep->periodic)
+   fifo_index = hs_ep->fifo_index;
+   else
+   fifo_index = 0;
+
+   /* Flush TX FIFO */
+   dwc2_flush_tx_fifo(hsotg, fifo_index);
+
+   /* Clear Global In NP NAK in Shared FIFO for non periodic ep */
+   if (!hsotg->dedicated_fifos && !hs_ep->periodic)
+   __orr32(hsotg->regs + DCTL, DCTL_CGNPINNAK);
+
+   } else {
+   /* Remove global NAKs */
+   __orr32(hsotg->regs + DCTL, DCTL_CGOUTNAK);
+   }
+}
+
 /**
  * dwc2_hsotg_ep_enable - enable the given endpoint
  * @ep: The USB endpint to configure
@@ -3803,6 +3892,10 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
spin_lock_irqsave(>lock, flags);
 
ctrl = dwc2_readl(hsotg->regs + epctrl_reg);
+
+   if (ctrl & DXEPCTL_EPENA)
+   dwc2_hsotg_ep_stop_xfr(hsotg, hs_ep);
+
ctrl &= ~DXEPCTL_EPENA;
ctrl &= ~DXEPCTL_USBACTEP;
ctrl |= DXEPCTL_SNAK;
@@ -3841,95 +3934,6 @@ static bool on_list(struct dwc2_hsotg_ep *ep, struct 
dwc2_hsotg_req *test)
return false;
 }
 
-static int dwc2_hsotg_wait_bit_set(struct dwc2_hsotg *hs_otg, u32 

[PATCH v2 28/30] usb: dwc2: gadget: Add IOT device IDs, configure core accordingly

2016-11-09 Thread John Youn
From: Vardan Mikayelyan 

Add new device IDs for IOT gadget. Done changes in probe to
configure core accordingly depending on device ID value.

Signed-off-by: Vardan Mikayelyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   | 18 ++
 drivers/usb/dwc2/params.c |  9 -
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5fa05a3..067e24b 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -950,6 +950,8 @@ struct dwc2_hsotg {
 #define DWC2_CORE_REV_2_94a0x4f54294a
 #define DWC2_CORE_REV_3_00a0x4f54300a
 #define DWC2_CORE_REV_3_10a0x4f54310a
+#define DWC2_FS_IOT_REV_1_00a  0x5531100a
+#define DWC2_HS_IOT_REV_1_00a  0x5532100a
 
 #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
union dwc2_hcd_internal_flags {
@@ -1078,6 +1080,22 @@ enum dwc2_halt_status {
DWC2_HC_XFER_URB_DEQUEUE,
 };
 
+/* Core version information */
+static inline bool dwc2_is_iot(struct dwc2_hsotg *hsotg)
+{
+   return (hsotg->hw_params.snpsid & 0xfff0) == 0x5530;
+}
+
+static inline bool dwc2_is_fs_iot(struct dwc2_hsotg *hsotg)
+{
+   return (hsotg->hw_params.snpsid & 0x) == 0x5531;
+}
+
+static inline bool dwc2_is_hs_iot(struct dwc2_hsotg *hsotg)
+{
+   return (hsotg->hw_params.snpsid & 0x) == 0x5532;
+}
+
 /*
  * The following functions support initialization of the core driver component
  * and the DWC_otg controller
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..c294344 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1286,7 +1286,9 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 */
hw->snpsid = dwc2_readl(hsotg->regs + GSNPSID);
if ((hw->snpsid & 0xf000) != 0x4f542000 &&
-   (hw->snpsid & 0xf000) != 0x4f543000) {
+   (hw->snpsid & 0xf000) != 0x4f543000 &&
+   (hw->snpsid & 0x) != 0x5531 &&
+   (hw->snpsid & 0x) != 0x5532) {
dev_err(hsotg->dev, "Bad value for GSNPSID: 0x%08x\n",
hw->snpsid);
return -ENODEV;
@@ -1428,6 +1430,11 @@ int dwc2_init_params(struct dwc2_hsotg *hsotg)
else
params = params_default;
 
+   if (dwc2_is_fs_iot(hsotg)) {
+   params.speed = DWC2_SPEED_PARAM_FULL;
+   params.phy_type = DWC2_PHY_TYPE_PARAM_FS;
+   }
+
dwc2_set_parameters(hsotg, );
 
return 0;
-- 
2.10.0

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


[PATCH v2 27/30] usb: dwc2: Add support of dedicated full-speed PHY interface

2016-11-09 Thread John Youn
From: Vahram Aharonyan 

Do modifications in dwc2_hsotg_core_init_disconnected() function to
enable USB 1.1 Full-Speed Serial Transceiver Select in GUSBCFG register
if corresponding speed and PHY types are chosen. Adjust device speed
selection in DCFG register as well.

Signed-off-by: Vahram Aharonyan 
Signed-off-by: John Youn 
---
 drivers/usb/dwc2/gadget.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0b4f0bd..74f0a5e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3153,6 +3153,7 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
u32 intmsk;
u32 val;
u32 usbcfg;
+   u32 dcfg = 0;
 
/* Kill any ep0 requests as controller will be reinitialized */
kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
@@ -3171,10 +3172,16 @@ void dwc2_hsotg_core_init_disconnected(struct 
dwc2_hsotg *hsotg,
usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
GUSBCFG_HNPCAP);
 
-   /* set the PLL on, remove the HNP/SRP and set the PHY */
-   val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
-   usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
-   (val << GUSBCFG_USBTRDTIM_SHIFT);
+   if (hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS &&
+   hsotg->params.speed == DWC2_SPEED_PARAM_FULL) {
+   /* FS/LS Dedicated Transceiver Interface */
+   usbcfg |= GUSBCFG_PHYSEL;
+   } else {
+   /* set the PLL on, remove the HNP/SRP and set the PHY */
+   val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
+   usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
+   (val << GUSBCFG_USBTRDTIM_SHIFT);
+   }
dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
 
dwc2_hsotg_init_fifo(hsotg);
@@ -3182,7 +3189,16 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
if (!is_usb_reset)
__orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
 
-   dwc2_writel(DCFG_EPMISCNT(1) | DCFG_DEVSPD_HS,  hsotg->regs + DCFG);
+   dcfg |= DCFG_EPMISCNT(1);
+   if (hsotg->params.speed == DWC2_SPEED_PARAM_FULL) {
+   if (hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS)
+   dcfg |= DCFG_DEVSPD_FS48;
+   else
+   dcfg |= DCFG_DEVSPD_FS;
+   } else {
+   dcfg |= DCFG_DEVSPD_HS;
+   }
+   dwc2_writel(dcfg,  hsotg->regs + DCFG);
 
/* Clear any pending OTG interrupts */
dwc2_writel(0x, hsotg->regs + GOTGINT);
-- 
2.10.0

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


Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

2016-11-09 Thread Kai-Heng Feng
Hi,

On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork  wrote:
> Oliver Neukum  writes:
>
>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>
>>> These problems could very well be caused by running at SuperSpeed
>>> (USB-3) instead of high speed (USB-2).

Yes, it's running at SuperSpeed, on a Kabylake laptop.

It does not have this issue on a Broadwell laptop, also running at SuperSpeed.

>>>
>>> Is there any way to test what happens when the device is attached to
>>> the computer by a USB-2 cable?  That would prevent it from operating at
>>> SuperSpeed.

I recall old Intel PCH can change the USB host from XHCI to EHCI,
newer PCH does not have this option.

Is there a way to force XHCI run at HighSpeed?

>>>
>>> The main point, however, is that the proposed patch doesn't seem to
>>> address the true problem, which is that the device gets suspended
>>> between probes.  The patch only tries to prevent it from being
>>> suspended during a probe -- which is already prevented by the USB core.
>>
>> But why doesn't it fail during normal operation?
>>
>> I suspect that its firmware requires the altsetting
>>
>> /* should we change control altsetting on a NCM/MBIM function? */
>> if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) 
>> {
>> data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
>> ret = cdc_mbim_set_ctrlalt(dev, intf, 
>> CDC_NCM_COMM_ALTSETTING_MBIM);
>>
>> to be set before it accepts a suspension.
>
> Could be, but I don't think so.  The above code is effectively a noop
> unless the function is a combined NCM/MBIM function.  Something I've
> never seen on a Sierra Wireless device (ignoring the infamous EM7345,
> which really is an Intel device).
>
> This is a typical example of a Sierra Wireless modem configured for
> MBIM:
>
> P:  Vendor=1199 ProdID=9079 Rev= 0.06
> S:  Manufacturer=Sierra Wireless, Incorporated
> S:  Product=Sierra Wireless EM7455 Qualcomm Snapdragon X7 LTE-A
> S:  SerialNumber=LF615126xxx
> C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=500mA
> A:  FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
> I:* If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=(none)
> E:  Ad=82(I) Atr=03(Int.) MxPS=  64 Ivl=32ms
> I:* If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
> I:  If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
> E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
>
>
> The control interface of plain MBIM functions will always have a single
> altsetting, like the example above. So cdc_ncm_select_altsetting(intf)
> returns "0", while CDC_NCM_COMM_ALTSETTING_MBIM is "1".
>
>
> Just for reference, using the Intel^H^H^H^H^HEM7345 as example, this is
> what a combined NCM/MBIM function looks like:
>
>
> P:  Vendor=1199 ProdID=a001 Rev=17.29
> S:  Manufacturer=Sierra Wireless Inc.
> S:  Product=Sierra Wireless EM7345 4G LTE
> S:  SerialNumber=013937000xx
> C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=100mA
> A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=0d Prot=00
> A:  FirstIf#= 2 IfCount= 2 Cls=02(comm.) Sub=02 Prot=01
> I:  If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=cdc_mbim
> E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
> I:* If#= 0 Alt= 1 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
> E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
> I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
> I:  If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
> E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> I:* If#= 1 Alt= 2 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
> E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> I:* If#= 2 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=01 Driver=(none)
> E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
> I:* If#= 3 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
> E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
>
>
> And this is what the code you quote is trying to deal with.  Note the
> different subclass of altsetting 0 and 1 This is incredibly ugly.
>
> FWIW, the modem in question cannot be an EM7345. That modem does not
> have the static interface numbering oddity.  Another sign that it isn't
> a true Sierra device.

Yes, this modem is an EM7445.

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


Re: [PATCH v4] USB hub_probe: rework ugly goto-into-compound-statement

2016-11-09 Thread Baolin Wang
Hi,

On 9 November 2016 at 23:35, Eugene Korenevsky  wrote:
> Rework smelling code (goto inside compound statement). Perhaps this is
> legacy. Anyway such code is not appropriate for Linux kernel.
>
> Changes since v3: fix typo
> Changes since v2: extract the code to static function
> Changes since v1: fix spaces instead of tab, add missing 'Signed-off-by'
>
> Signed-off-by: Eugene Korenevsky 
> ---
>  drivers/usb/core/hub.c | 35 ++-
>  1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index cbb1467..b770c8d 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1722,10 +1722,25 @@ static void hub_disconnect(struct usb_interface *intf)
> kref_put(>kref, hub_release);
>  }
>
> +static int hub_check_descriptor_sanity(struct usb_host_interface *desc)
> +{
> +   /* Some hubs have a subclass of 1, which AFAICT according to the */
> +   /*  specs is not defined, but it works */
> +   if (desc->desc.bInterfaceSubClass != 0 &&
> +   desc->desc.bInterfaceSubClass != 1)
> +   return 0;
> +
> +   /* Multiple endpoints? What kind of mutant ninja-hub is this? */
> +   if (desc->desc.bNumEndpoints != 1)
> +   return 0;

We usually return 0 as successful result, can you return kernel error
number if we check the descriptor failed? Or change the return value
of the function as bool type.

> +
> +   /* If it's not an interrupt in endpoint, we'd better punt! */
> +   return usb_endpoint_is_int_in(>endpoint[0].desc);
> +}
> +
>  static int hub_probe(struct usb_interface *intf, const struct usb_device_id 
> *id)
>  {
> struct usb_host_interface *desc;
> -   struct usb_endpoint_descriptor *endpoint;
> struct usb_device *hdev;
> struct usb_hub *hub;
>
> @@ -1800,25 +1815,11 @@ static int hub_probe(struct usb_interface *intf, 
> const struct usb_device_id *id)
> }
>  #endif
>
> -   /* Some hubs have a subclass of 1, which AFAICT according to the */
> -   /*  specs is not defined, but it works */
> -   if ((desc->desc.bInterfaceSubClass != 0) &&
> -   (desc->desc.bInterfaceSubClass != 1)) {
> -descriptor_error:
> +   if (!hub_check_descriptor_sanity(desc)) {
> dev_err(>dev, "bad descriptor, ignoring hub\n");
> return -EIO;
> }
>
> -   /* Multiple endpoints? What kind of mutant ninja-hub is this? */
> -   if (desc->desc.bNumEndpoints != 1)
> -   goto descriptor_error;
> -
> -   endpoint = >endpoint[0].desc;
> -
> -   /* If it's not an interrupt in endpoint, we'd better punt! */
> -   if (!usb_endpoint_is_int_in(endpoint))
> -   goto descriptor_error;
> -
> /* We found a hub */
> dev_info(>dev, "USB hub found\n");
>
> @@ -1845,7 +1846,7 @@ static int hub_probe(struct usb_interface *intf, const 
> struct usb_device_id *id)
> if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
> hub->quirk_check_port_auto_suspend = 1;
>
> -   if (hub_configure(hub, endpoint) >= 0)
> +   if (hub_configure(hub, >endpoint[0].desc) >= 0)
> return 0;
>
> hub_disconnect(intf);
> --
> 2.10.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



-- 
Baolin.wang
Best Regards
--
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: dwc3: gadget: Write the event count in interrupt

2016-11-09 Thread Felipe Balbi

Hi,

John Youn  writes:
>> +   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
>> +
 After that evt->buf[lpos, lpos + count] seems goes back to HW, so
 thread should not rely on this?

 Or I miss something?
>>>
>>> Hi,
>>>
>>> Yes, you're right. That's a possibility and to be safe we can cache
>>> those.
>>>
>>> We're relying on the same mechanism that keeps the event buffer from
>>> becoming full. Since it is just as (un)likely a possibility. That's
>>> why you must size the event buffer appropriately taking into account
>>> your system's latency, etc.
>>>
>>> And if the event buffer becomes full, that indicates something really
>>> wrong happened in the controller. You shouldn't be getting 100's of
>>> events at a time.
>>>
>>> But yes, we can address the overwriting issue.
>>>
>>> Either:
>>>
>>> 1) Cache all incoming events. Requires double the event buffer space.
>>>
>>> 2) Cache just one event and write back only '4' during hard
>>>interrupt. Then in threaded interrupt read the one event from
>>>cache, and process the remaining events from buffer as before.
>>>Doesn't require a large cache, but a bit messier.
>>>
>>> Any other thoughts or ideas?
>> 
>> do you really need to clear at least one event for this?
>> 
>
> Unfortunately yes. That's how the workaround works. We need to write
> this during IMOD to de-assert the interrupt since the mask bit doesn't
> work.

okay. Then we should cache and clear a single event.

> We could do this only for 3.00a as well.

if we do this only for 3.00a then the code will look odd :-) It
shouldn't cause any problems for any other revisions

-- 
balbi


signature.asc
Description: PGP signature