Re: [PATCH 2/3] phy: da8xx-usb: rename the ohci device to ohci-da8xx

[Sekhar Nori]

Hi Kishon,

On Thursday 03 November 2016 10:20 PM, Kishon Vijay Abraham I wrote:
> 
> 
> On Wednesday 02 November 2016 06:14 PM, Axel Haslam wrote:
>> There is only one ohci on the da8xx series of chips,
>> so remove the ".0" when creating the phy. Also add
>> the "-da8xx" postfix to be consistent across davinci
>> usb drivers.
>>
>> Signed-off-by: Axel Haslam 
> 
> Acked-by: Kishon Vijay Abraham I 

You will have to carry this patch from your tree. I thought I can carry
the entire series, but the USB patch depends on other patches that Greg
has already queued. So I think its best if the individual patches go
through their respective trees.

Note that there is a v2 already submitted.

Thanks,
Sekhar

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

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Peter Chen]

On Thu, Nov 03, 2016 at 12:48:08PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> >> > Peter Chen  writes:
> >> > > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
> >> > >> From: Peter Chen 
> >> > >> Date: Wed, 2 Nov 2016 14:02:02 +0800
> >> > >> 
> >> > >> > Felipe, it may increase cpu utilization since more interrupts will 
> >> > >> > be there,
> >> > >> > it may affect the SoC which has lower cpu frequency. This code 
> >> > >> > existed
> >> > >> > many years, why this problem has only reported at dwc3 recently?
> >> > >> 
> >> > >> It's a bug, and it's going to cause TCP sockets to potentially hang.
> >> > >> 
> >> > >
> >> > > For some controllers, it is, so we need to add parameter for user
> >> > > to see if interrupt migration is supported or not.
> >> > 
> >> > not for some controller, for ALL networking drivers.
> >> > 
> >> > > But just like some ethernet controllers, some USB controllers support
> >> > > hardware timeout mechanism which interrupt will be triggered after
> >> > > some uFrame occurs if the transaction has completed but not required
> >> > > to interrupt, it is used to support interrupt migration like ethernet.
> >> > 
> >> > you're missing the point. What Dave Miller is saying is that it's ALWAYS
> >> > a bug to delay completion of SKBs. The only thing you're doing with
> >> > chipidea is delaying interrupt by up to 125us; which is still a bug from
> >> > the point of view of the networking layer, but it's more difficult to
> >> > perceive any problems because of the short time where interrupt is
> >> > delayed.
> >> > 
> >> 
> >> If it is ALWAYS a bug to delay completion of SKBs, how the local
> >> ethernet driver designs interrupt migration?
> >> 
> >
> > Just a quick test, I delete dev_kfree_skb_any at tx_complete, not find
> > any problems by using simple "ping test", just free memory is less and
> > less. David, do you really mean free tx skb buffer with limited time,
> > but not return NETDEV_TX_OK by ->ndo_start_xmit with limited time?
> 
> ping *will* work just fine. One easy test to *see* the problem is to SSH
> to a machine using g_ether, then run:
> 
> $ while true; do dmesg; done
> 
> you will notice it is rather laggy. Now remove throttling and the lags
> are gone.
> 

I have ran the test using ncm, the qmult is 10, but not find the laggy 
at the screen, just the pipe will be broken after several minutes.
During this test, the data at rx is much more than tx, but throttling
interrupt we are discussing is at tx.


[1.314390] fec 21b4000.ethernet (unnamed net_device) (uninitialized): 
Invalid MAC addresspacket_write_wait: Connection to 192.168.1.1: Broken pipe
root@imx6qdlsolo:~# ifconfig usb0
usb0  Link encap:Ethernet  HWaddr d6:c9:9a:18:4b:b1  
  inet addr:192.168.1.2  Bcast:192.168.1.255  Mask:255.255.255.0
  inet6 addr: fe80::d4c9:9aff:fe18:4bb1/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:5011 errors:0 dropped:0 overruns:0 frame:0
  TX packets:677 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000 
  RX bytes:3015832 (2.8 MiB)  TX bytes:94532 (92.3 KiB)
-- 

Best Regards,
Peter Chen
--
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()

[John Youn]

On 11/1/2016 4:14 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn  writes:
>> From: Vardan Mikayelyan 
>>
>> Update the dwc2 driver for the new behavior of the usb_endpoint_maxp()
>> and also use the new usb_endpoint_maxp_mult() helper function.
>>
>> This commit fixes failures in high-badwith ISOC transfer tests.
>>
>> Signed-off-by: Vardan Mikayelyan 
>> Signed-off-by: John Youn 
>> ---
>>  drivers/usb/dwc2/gadget.c | 38 --
>>  1 file changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 8a7fd73..a505bbf 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -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().

> 
> Anyway, this is in my testing/next now.
> 

Ok thanks.

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 1/1] usb: gadget: fix request length error for isoc transfer

[Peter Chen]

On Thu, Nov 03, 2016 at 12:50:43PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > On Thu, Nov 03, 2016 at 11:27:40AM +0200, Felipe Balbi wrote:
> >> 
> >> Hi,
> >> 
> >> Peter Chen  writes:
> >> >> Peter Chen  writes:
> >> >> > For isoc endpoint descriptor, the wMaxPacketSize is not real max 
> >> >> > packet
> >> >> > size (see Table 9-13. Standard Endpoint Descriptor, USB 2.0 
> >> >> > specifcation),
> >> >> > it may contain the number of packet, so the real max packet should be
> >> >> > ep->desc->wMaxPacketSize && 0x7ff.
> >> >> >
> >> >> > Cc: Felipe F. Tonello 
> >> >> > Cc: Felipe Balbi 
> >> >> > Fixes: 16b114a6d797 ("usb: gadget: fix usb_ep_align_maybe
> >> >> >   endianness and new usb_ep_aligna")
> >> >> >
> >> >> > Signed-off-by: Peter Chen 
> >> >> > ---
> >> >> >  include/linux/usb/gadget.h | 5 -
> >> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> >> >> > index 8e81f9e..cce2db6 100644
> >> >> > --- a/include/linux/usb/gadget.h
> >> >> > +++ b/include/linux/usb/gadget.h
> >> >> > @@ -429,7 +429,10 @@ static inline struct usb_gadget 
> >> >> > *dev_to_usb_gadget(struct device *dev)
> >> >> >   */
> >> >> >  static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
> >> >> >  {
> >> >> > - return round_up(len, 
> >> >> > (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
> >> >> > + int max_packet_size = 
> >> >> > (size_t)le16_to_cpu(ep->desc->wMaxPacketSize)
> >> >> > + && 0x7ff;
> >> ^^
> >> hehe, I just noticed this bug. Did you really test this patch? :-)
> >> 
> >
> > Any problems you find? Without this patch, the usbtest can't be ran
> > for high bandwidth isoc transfer.
> 
> wMaxPacketSize && 0x7ff is always 1 unless wMaxPacketSize is zero, which
> is invalid anyway.
> 
> >> >> > +
> >> >> > + return round_up(len, max_packet_size);
> >> >> >  }
> >> >> 
> >> >> care to update this to use new usb_endpoint_maxp()?
> >> >> 
> >> >
> >> > But it is a bug existed at v4.9, the usb_ep_align returns wrong request
> >> > length for isoc. I found this bug by running usbtest.
> >> 
> >> keep your & (after fixing it), but use usb_endpoint_maxp(). Later we can
> >> just remove the unnecessary & operation.
> >> 
> >> -- 
> >> balbi
> >
> > I will use usb_endpoint_maxp, but I still not find the problem for this
> > patch.
> 
> Logical vs Bitwise operator
> 

Oh, my careless, I should read my patch again after getting your
comments. Test is ok not stands for code logic is ok.

round_up(3072,1) = 3072, so patch does not trigger error.

-- 

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

[PATCH v2 1/5] usb: dwc3: Add a check for the DWC_usb3 core

[John Youn]

Add a helper function to check if we are running on a DWC_usb3 core.

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

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 2322863..c22b38b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1149,6 +1149,12 @@ struct dwc3_gadget_ep_cmd_params {
 void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
 u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type);
 
+/* check whether we are on the DWC_usb3 core */
+static inline bool dwc3_is_usb3(struct dwc3 *dwc)
+{
+   return !(dwc->revision & DWC3_REVISION_IS_DWC31);
+}
+
 /* check whether we are on the DWC_usb31 core */
 static inline bool dwc3_is_usb31(struct dwc3 *dwc)
 {
-- 
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 0/5] usb: dwc3: Interrupt moderation

[John Youn]

This patch series implements interrupt moderation and also uses it in
implementing a workaround for STAR 9000961433.

v2:
* Remove the devicetree binding


John Youn (5):
  usb: dwc3: Add a check for the DWC_usb3 core
  usb: dwc3: Add a function to check properties
  usb: dwc3: gadget: Write the event count in interrupt
  usb: dwc3: Implement interrupt moderation
  usb: dwc3: Workaround for irq mask issue

 drivers/usb/dwc3/core.c   | 87 +--
 drivers/usb/dwc3/core.h   | 21 
 drivers/usb/dwc3/gadget.c | 20 +--
 3 files changed, 100 insertions(+), 28 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 5/5] usb: dwc3: Workaround for irq mask issue

[John Youn]

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

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

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b1ec956..046674d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1048,6 +1048,18 @@ static void dwc3_check_params(struct dwc3 *dwc)
dwc->imod_interval = 0;
}
 
+   /*
+* Workaround for STAR 9000961433 which affects only version
+* 3.00a of the DWC_usb3 core. This prevents the controller
+* interrupt from being masked while handling events. IMOD
+* allows us to work around this issue. Enable it for the
+* affected version.
+*/
+   if (!dwc->imod_interval &&
+   (dwc->revision == DWC3_REVISION_300A)) {
+   dwc->imod_interval = 1;
+   }
+
/* Check the maximum_speed parameter */
switch (dwc->maximum_speed) {
case USB_SPEED_LOW:
-- 
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 3/5] usb: dwc3: gadget: Write the event count in interrupt

[John Youn]

Since we are saving the event count and handling the events in the
threaded interrupt handler, we can write and clear out the eventcount in
the hard interrupt handler itself.

This behavior will be required for IP 3.00a cores that need to use
interrupt moderation as a workaround for an RTL issue were the interrupt
line cannot be masked between the hard/soft interrupt handler.

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

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a9c1d75..ac9eb39 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2877,8 +2877,6 @@ static irqreturn_t dwc3_process_event_buf(struct 
dwc3_event_buffer *evt)
 */
evt->lpos = (evt->lpos + 4) % DWC3_EVENT_BUFFERS_SIZE;
left -= 4;
-
-   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 4);
}
 
evt->count = 0;
@@ -2928,6 +2926,8 @@ static irqreturn_t dwc3_check_event_buf(struct 
dwc3_event_buffer *evt)
evt->count = count;
evt->flags |= DWC3_EVENT_PENDING;
 
+   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
+
/* Mask interrupt */
reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0));
reg |= DWC3_GEVNTSIZ_INTMASK;
-- 
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 2/5] usb: dwc3: Add a function to check properties

[John Youn]

Add a function to check properties and call it from probe. This will
allow us to add check code without bloating the probe function. This
needs to be done after dwc3_get_properties() and dwc3_core_init() so
that all the properties and hardware configs are available.

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

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5e61ef6..1a034f1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1028,6 +1028,38 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 
 }
 
+static void dwc3_check_params(struct dwc3 *dwc)
+{
+   struct device *dev = dwc->dev;
+
+   /* Check the maximum_speed parameter */
+   switch (dwc->maximum_speed) {
+   case USB_SPEED_LOW:
+   case USB_SPEED_FULL:
+   case USB_SPEED_HIGH:
+   case USB_SPEED_SUPER:
+   case USB_SPEED_SUPER_PLUS:
+   break;
+   default:
+   dev_err(dev, "invalid maximum_speed parameter %d\n",
+   dwc->maximum_speed);
+   /* fall through */
+   case USB_SPEED_UNKNOWN:
+   /* default to superspeed */
+   dwc->maximum_speed = USB_SPEED_SUPER;
+
+   /*
+* default to superspeed plus if we are capable.
+*/
+   if (dwc3_is_usb31(dwc) &&
+   (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) ==
+DWC3_GHWPARAMS3_SSPHY_IFC_GEN2))
+   dwc->maximum_speed = USB_SPEED_SUPER_PLUS;
+
+   break;
+   }
+}
+
 static int dwc3_probe(struct platform_device *pdev)
 {
struct device   *dev = >dev;
@@ -1119,32 +1151,7 @@ static int dwc3_probe(struct platform_device *pdev)
goto err4;
}
 
-   /* Check the maximum_speed parameter */
-   switch (dwc->maximum_speed) {
-   case USB_SPEED_LOW:
-   case USB_SPEED_FULL:
-   case USB_SPEED_HIGH:
-   case USB_SPEED_SUPER:
-   case USB_SPEED_SUPER_PLUS:
-   break;
-   default:
-   dev_err(dev, "invalid maximum_speed parameter %d\n",
-   dwc->maximum_speed);
-   /* fall through */
-   case USB_SPEED_UNKNOWN:
-   /* default to superspeed */
-   dwc->maximum_speed = USB_SPEED_SUPER;
-
-   /*
-* default to superspeed plus if we are capable.
-*/
-   if (dwc3_is_usb31(dwc) &&
-   (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) ==
-DWC3_GHWPARAMS3_SSPHY_IFC_GEN2))
-   dwc->maximum_speed = USB_SPEED_SUPER_PLUS;
-
-   break;
-   }
+   dwc3_check_params(dwc);
 
ret = dwc3_core_init_mode(dwc);
if (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 v2 4/5] usb: dwc3: Implement interrupt moderation

[John Youn]

Implement interrupt moderation which allows the interrupt rate to be
throttled. To enable this feature the dwc->imod_interval must be set to
1 or greater. This value specifies the minimum inter-interrupt interval,
in 250 ns increments. A value of 0 disables interrupt moderation.

This applies for DWC_usb3 version 3.00a and higher and for DWC_usb31
version 1.20a and higher.

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/core.c   | 16 
 drivers/usb/dwc3/core.h   | 15 +++
 drivers/usb/dwc3/gadget.c | 16 
 3 files changed, 47 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1a034f1..b1ec956 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1026,12 +1026,28 @@ static void dwc3_get_properties(struct dwc3 *dwc)
dwc->hird_threshold = hird_threshold
| (dwc->is_utmi_l1_suspend << 4);
 
+   dwc->imod_interval = 0;
+}
+
+/* check whether the core supports IMOD */
+bool dwc3_has_imod(struct dwc3 *dwc)
+{
+   return ((dwc3_is_usb3(dwc) &&
+dwc->revision >= DWC3_REVISION_300A) ||
+   (dwc3_is_usb31(dwc) &&
+dwc->revision >= DWC3_USB31_REVISION_120A));
 }
 
 static void dwc3_check_params(struct dwc3 *dwc)
 {
struct device *dev = dwc->dev;
 
+   /* Check for proper value of imod_interval */
+   if (dwc->imod_interval && !dwc3_has_imod(dwc)) {
+   dev_warn(dwc->dev, "Interrupt moderation not supported\n");
+   dwc->imod_interval = 0;
+   }
+
/* Check the maximum_speed parameter */
switch (dwc->maximum_speed) {
case USB_SPEED_LOW:
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index c22b38b..495cac0 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -67,6 +67,7 @@
 #define DWC3_DEVICE_EVENT_OVERFLOW 11
 
 #define DWC3_GEVNTCOUNT_MASK   0xfffc
+#define DWC3_GEVNTCOUNT_EHB(1 << 31)
 #define DWC3_GSNPSID_MASK  0x
 #define DWC3_GSNPSREV_MASK 0x
 
@@ -149,6 +150,8 @@
 #define DWC3_DEPCMDPAR00x08
 #define DWC3_DEPCMD0x0c
 
+#define DWC3_DEV_IMOD(n)   (0xca00 + (n * 0x4))
+
 /* OTG Registers */
 #define DWC3_OCFG  0xcc00
 #define DWC3_OCTL  0xcc04
@@ -465,6 +468,11 @@
 #define DWC3_DEPCMD_TYPE_BULK  2
 #define DWC3_DEPCMD_TYPE_INTR  3
 
+#define DWC3_DEV_IMOD_COUNT_SHIFT  16
+#define DWC3_DEV_IMOD_COUNT_MASK   (0x << 16)
+#define DWC3_DEV_IMOD_INTERVAL_SHIFT   0
+#define DWC3_DEV_IMOD_INTERVAL_MASK(0x << 0)
+
 /* Structures */
 
 struct dwc3_trb;
@@ -845,6 +853,8 @@ struct dwc3_scratchpad_array {
  * 1   - -3.5dB de-emphasis
  * 2   - No de-emphasis
  * 3   - Reserved
+ * @imod_interval: set the interrupt moderation interval in 250ns
+ * increments or 0 to disable.
  */
 struct dwc3 {
struct usb_ctrlrequest  *ctrl_req;
@@ -932,6 +942,7 @@ struct dwc3 {
  */
 #define DWC3_REVISION_IS_DWC31 0x8000
 #define DWC3_USB31_REVISION_110A   (0x3131302a | DWC3_REVISION_IS_DWC31)
+#define DWC3_USB31_REVISION_120A   (0x3132302a | DWC3_REVISION_IS_DWC31)
 
enum dwc3_ep0_next  ep0_next_event;
enum dwc3_ep0_state ep0state;
@@ -990,6 +1001,8 @@ struct dwc3 {
 
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
+
+   u16 imod_interval;
 };
 
 /* -- 
*/
@@ -1161,6 +1174,8 @@ static inline bool dwc3_is_usb31(struct dwc3 *dwc)
return !!(dwc->revision & DWC3_REVISION_IS_DWC31);
 }
 
+bool dwc3_has_imod(struct dwc3 *dwc);
+
 #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
 int dwc3_host_init(struct dwc3 *dwc);
 void dwc3_host_exit(struct dwc3 *dwc);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index ac9eb39..05d0fd2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1696,6 +1696,17 @@ static int __dwc3_gadget_start(struct dwc3 *dwc)
int ret = 0;
u32 reg;
 
+   /*
+* Use IMOD if enabled via dwc->imod_interval. Otherwise, if
+* the core supports IMOD, disable it.
+*/
+   if (dwc->imod_interval) {
+   dwc3_writel(dwc->regs, DWC3_DEV_IMOD(0), dwc->imod_interval);
+   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), DWC3_GEVNTCOUNT_EHB);
+   } else if (dwc3_has_imod(dwc)) {
+   dwc3_writel(dwc->regs, DWC3_DEV_IMOD(0), 0);
+   }
+
reg = dwc3_readl(dwc->regs, DWC3_DCFG);
reg &= ~(DWC3_DCFG_SPEED_MASK);
 
@@ -2888,6 +2899,11 @@ static irqreturn_t dwc3_process_event_buf(struct 
dwc3_event_buffer *evt)
reg &= ~DWC3_GEVNTSIZ_INTMASK;
dwc3_writel(dwc->regs, 

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Peter Chen]

On Thu, Nov 03, 2016 at 12:42:11PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> >> Peter Chen  writes:
> >> > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
> >> >> From: Peter Chen 
> >> >> Date: Wed, 2 Nov 2016 14:02:02 +0800
> >> >> 
> >> >> > Felipe, it may increase cpu utilization since more interrupts will be 
> >> >> > there,
> >> >> > it may affect the SoC which has lower cpu frequency. This code existed
> >> >> > many years, why this problem has only reported at dwc3 recently?
> >> >> 
> >> >> It's a bug, and it's going to cause TCP sockets to potentially hang.
> >> >> 
> >> >
> >> > For some controllers, it is, so we need to add parameter for user
> >> > to see if interrupt migration is supported or not.
> >> 
> >> not for some controller, for ALL networking drivers.
> >> 
> >> > But just like some ethernet controllers, some USB controllers support
> >> > hardware timeout mechanism which interrupt will be triggered after
> >> > some uFrame occurs if the transaction has completed but not required
> >> > to interrupt, it is used to support interrupt migration like ethernet.
> >> 
> >> you're missing the point. What Dave Miller is saying is that it's ALWAYS
> >> a bug to delay completion of SKBs. The only thing you're doing with
> >> chipidea is delaying interrupt by up to 125us; which is still a bug from
> >> the point of view of the networking layer, but it's more difficult to
> >> perceive any problems because of the short time where interrupt is
> >> delayed.
> >> 
> >
> > If it is ALWAYS a bug to delay completion of SKBs, how the local
> > ethernet driver designs interrupt migration?
> 
> what driver are you talking about? Which "local" ethernet driver?
> 
> Also, do you mean 'moderation' instead of 'migration?
> 

migration interrupt means throttling interrupt, sorry, I may use 'wrong'
words.

-- 

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

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Peter Chen]

On Thu, Nov 03, 2016 at 12:42:11PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> >> Peter Chen  writes:
> >> > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
> >> >> From: Peter Chen 
> >> >> Date: Wed, 2 Nov 2016 14:02:02 +0800
> >> >> 
> >> >> > Felipe, it may increase cpu utilization since more interrupts will be 
> >> >> > there,
> >> >> > it may affect the SoC which has lower cpu frequency. This code existed
> >> >> > many years, why this problem has only reported at dwc3 recently?
> >> >> 
> >> >> It's a bug, and it's going to cause TCP sockets to potentially hang.
> >> >> 
> >> >
> >> > For some controllers, it is, so we need to add parameter for user
> >> > to see if interrupt migration is supported or not.
> >> 
> >> not for some controller, for ALL networking drivers.
> >> 
> >> > But just like some ethernet controllers, some USB controllers support
> >> > hardware timeout mechanism which interrupt will be triggered after
> >> > some uFrame occurs if the transaction has completed but not required
> >> > to interrupt, it is used to support interrupt migration like ethernet.
> >> 
> >> you're missing the point. What Dave Miller is saying is that it's ALWAYS
> >> a bug to delay completion of SKBs. The only thing you're doing with
> >> chipidea is delaying interrupt by up to 125us; which is still a bug from
> >> the point of view of the networking layer, but it's more difficult to
> >> perceive any problems because of the short time where interrupt is
> >> delayed.
> >> 
> >
> > If it is ALWAYS a bug to delay completion of SKBs, how the local
> > ethernet driver designs interrupt migration?
> 
> what driver are you talking about? Which "local" ethernet driver?
> 

The ethernet driver located at drivers/net/ethernet, its device uses RJ45 
connector
to communicate.

-- 

Best Regards,
Peter Chen
--
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 07/13] usb: dwc2: Remove unnecessary prototypes

[John Youn]

Remove the unnecessary prototypes for all the parameter setting
functions and declare those functions 'static' in the params.c file.

Also remove the duplicate documentation that went along with them. They
are already documented as part of the params structure definition.

Then move the constants that went along with the prototype into the
structure.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   | 227 --
 drivers/usb/dwc2/params.c |  62 +++--
 2 files changed, 54 insertions(+), 235 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index f282742..2abe15f 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -314,7 +314,8 @@ enum dwc2_ep0_state {
  * @enable_dynamic_fifo: 0 - Use coreConsultant-specified FIFO size parameters
  *   1 - Allow dynamic FIFO sizing (default, if available)
  * @en_multiple_tx_fifo: Specifies whether dedicated per-endpoint transmit 
FIFOs
- *  are enabled
+ *  are enabled for non-periodic IN endpoints in device
+ *  mode.
  * @host_rx_fifo_size:  Number of 4-byte words in the Rx FIFO in host mode when
  *  dynamic FIFO sizing is enabled
  *   16 to 32768
@@ -430,11 +431,18 @@ struct dwc2_core_params {
 * dwc2_set_all_params!
 */
int otg_cap;
+#define DWC2_CAP_PARAM_HNP_SRP_CAPABLE 0
+#define DWC2_CAP_PARAM_SRP_ONLY_CAPABLE1
+#define DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE  2
+
int otg_ver;
int dma_enable;
int dma_desc_enable;
int dma_desc_fs_enable;
int speed;
+#define DWC2_SPEED_PARAM_HIGH  0
+#define DWC2_SPEED_PARAM_FULL  1
+
int enable_dynamic_fifo;
int en_multiple_tx_fifo;
int host_rx_fifo_size;
@@ -444,13 +452,23 @@ struct dwc2_core_params {
int max_packet_count;
int host_channels;
int phy_type;
+#define DWC2_PHY_TYPE_PARAM_FS 0
+#define DWC2_PHY_TYPE_PARAM_UTMI   1
+#define DWC2_PHY_TYPE_PARAM_ULPI   2
+
int phy_utmi_width;
int phy_ulpi_ddr;
int phy_ulpi_ext_vbus;
+#define DWC2_PHY_ULPI_INTERNAL_VBUS0
+#define DWC2_PHY_ULPI_EXTERNAL_VBUS1
+
int i2c_enable;
int ulpi_fs_ls;
int host_support_fs_ls_low_power;
int host_ls_low_power_phy_clk;
+#define DWC2_HOST_LS_LOW_POWER_PHY_CLK_PARAM_48MHZ 0
+#define DWC2_HOST_LS_LOW_POWER_PHY_CLK_PARAM_6MHZ  1
+
int ts_dline;
int reload_ctl;
int ahbcfg;
@@ -1048,216 +1066,11 @@ extern irqreturn_t dwc2_handle_common_intr(int irq, 
void *dev);
 /* The device ID match table */
 extern const struct of_device_id dwc2_of_match_table[];
 
-/* OTG Core Parameters */
-
-/*
- * Specifies the OTG capabilities. The driver will automatically
- * detect the value for this parameter if none is specified.
- * 0 - HNP and SRP capable (default)
- * 1 - SRP Only capable
- * 2 - No HNP/SRP capable
- */
-extern void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg, int val);
-#define DWC2_CAP_PARAM_HNP_SRP_CAPABLE 0
-#define DWC2_CAP_PARAM_SRP_ONLY_CAPABLE1
-#define DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE  2
-
-/*
- * Specifies whether to use slave or DMA mode for accessing the data
- * FIFOs. The driver will automatically detect the value for this
- * parameter if none is specified.
- * 0 - Slave
- * 1 - DMA (default, if available)
- */
-extern void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val);
-
-/*
- * When DMA mode is enabled specifies whether to use
- * address DMA or DMA Descritor mode for accessing the data
- * FIFOs in device mode. The driver will automatically detect
- * the value for this parameter if none is specified.
- * 0 - address DMA
- * 1 - DMA Descriptor(default, if available)
- */
-extern void dwc2_set_param_dma_desc_enable(struct dwc2_hsotg *hsotg, int val);
-
-/*
- * When DMA mode is enabled specifies whether to use
- * address DMA or DMA Descritor mode with full speed devices
- * for accessing the data FIFOs in host mode.
- * 0 - address DMA
- * 1 - FS DMA Descriptor(default, if available)
- */
-extern void dwc2_set_param_dma_desc_fs_enable(struct dwc2_hsotg *hsotg,
- int val);
-
-/*
- * Specifies the maximum speed of operation in host and device mode.
- * The actual speed depends on the speed of the attached device and
- * the value of phy_type. The actual speed depends on the speed of the
- * attached device.
- * 0 - High Speed (default)
- * 1 - Full Speed
- */
-extern void dwc2_set_param_speed(struct dwc2_hsotg *hsotg, int val);
-#define DWC2_SPEED_PARAM_HIGH  0
-#define DWC2_SPEED_PARAM_FULL  1
-
-/*
- * Specifies whether low power mode is supported when attached
- * to a Full Speed or Low Speed device in host mode.
- *
- * 0 - Don't support low power mode (default)
- * 1 

[PATCH v2 05/13] usb: dwc2: Move parameter initialization into params.c

[John Youn]

Consolidate and move all the parameter initialization code from the
probe function to params.c.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h |  3 +++
 drivers/usb/dwc2/params.c   | 29 +
 drivers/usb/dwc2/platform.c | 27 ---
 3 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index f3d14f3..8d26847 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1259,6 +1259,9 @@ extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
 extern int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg);
 
+/* Parameters */
+int dwc2_init_params(struct dwc2_hsotg *hsotg);
+
 /*
  * The following functions check the controller's OTG operation mode
  * capability (GHWCFG2.OTG_MODE).
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index ff7c844..d6c92d4 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1123,3 +1123,32 @@ void dwc2_set_all_params(struct dwc2_core_params 
*params, int value)
for (i = 0; i < size; i++)
p[i] = value;
 }
+
+int dwc2_init_params(struct dwc2_hsotg *hsotg)
+{
+   const struct of_device_id *match;
+   const struct dwc2_core_params *params;
+   struct dwc2_core_params defparams;
+
+   match = of_match_device(dwc2_of_match_table, hsotg->dev);
+   if (match && match->data) {
+   params = match->data;
+   } else {
+   /* Default all params to autodetect */
+   dwc2_set_all_params(, -1);
+   params = 
+
+   /*
+* Disable descriptor dma mode by default as the HW can support
+* it, but does not support it for SPLIT transactions.
+* Disable it for FS devices as well.
+*/
+   defparams.dma_desc_enable = 0;
+   defparams.dma_desc_fs_enable = 0;
+   }
+
+   /* Validate parameter values */
+   dwc2_set_parameters(hsotg, params);
+
+   return 0;
+}
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index d335e36..4fbfe09 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -365,30 +365,10 @@ static void dwc2_driver_shutdown(struct platform_device 
*dev)
  */
 static int dwc2_driver_probe(struct platform_device *dev)
 {
-   const struct of_device_id *match;
-   const struct dwc2_core_params *params;
-   struct dwc2_core_params defparams;
struct dwc2_hsotg *hsotg;
struct resource *res;
int retval;
 
-   match = of_match_device(dwc2_of_match_table, >dev);
-   if (match && match->data) {
-   params = match->data;
-   } else {
-   /* Default all params to autodetect */
-   dwc2_set_all_params(, -1);
-   params = 
-
-   /*
-* Disable descriptor dma mode by default as the HW can support
-* it, but does not support it for SPLIT transactions.
-* Disable it for FS devices as well.
-*/
-   defparams.dma_desc_enable = 0;
-   defparams.dma_desc_fs_enable = 0;
-   }
-
hsotg = devm_kzalloc(>dev, sizeof(*hsotg), GFP_KERNEL);
if (!hsotg)
return -ENOMEM;
@@ -453,11 +433,12 @@ static int dwc2_driver_probe(struct platform_device *dev)
if (retval)
goto error;
 
-   /* Validate parameter values */
-   dwc2_set_parameters(hsotg, params);
-
dwc2_force_dr_mode(hsotg);
 
+   retval = dwc2_init_params(hsotg);
+   if (retval)
+   goto error;
+
if (hsotg->dr_mode != USB_DR_MODE_HOST) {
retval = dwc2_gadget_init(hsotg, hsotg->irq);
if (retval)
-- 
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/13] Documentation: devicetree: dwc2: Add host DMA binding

[John Youn]

Add the snps,host-dma-disable binding. This controls whether to disable
DMA in host mode.

Signed-off-by: John Youn 
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 2c30a54..9472111 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -25,6 +25,7 @@ Optional properties:
 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.
-- 
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/13] usb: dwc2: Get host DMA device properties

[John Youn]

The driver will automatically enable host DMA and use it if available.
This is consistent with the behavior of all existing platforms.

Read in the "snps,host-dma-disable" device property to disable it.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  6 +-
 drivers/usb/dwc2/params.c | 48 ++-
 2 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 4b78dde..a1075ad 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -451,7 +451,6 @@ struct dwc2_core_params {
 #define DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE  2
 
int otg_ver;
-   int host_dma;
int dma_desc_enable;
int dma_desc_fs_enable;
int speed;
@@ -495,6 +494,11 @@ struct dwc2_core_params {
 * The following parameters are *only* set via device
 * properties and cannot be set directly in this structure.
 */
+
+   /* Host parameters */
+   bool host_dma;
+
+   /* Gadget parameters */
bool g_dma;
u16 g_rx_fifo_size;
u16 g_np_tx_fifo_size;
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 08b00ca..2eb79e8 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -41,7 +41,6 @@
 static const struct dwc2_core_params params_hi6220 = {
.otg_cap= 2,/* No HNP/SRP capable */
.otg_ver= 0,/* 1.3 */
-   .host_dma   = 1,
.dma_desc_enable= 0,
.dma_desc_fs_enable = 0,
.speed  = 0,/* High Speed */
@@ -73,7 +72,6 @@ static const struct dwc2_core_params params_hi6220 = {
 static const struct dwc2_core_params params_bcm2835 = {
.otg_cap= 0,/* HNP/SRP capable */
.otg_ver= 0,/* 1.3 */
-   .host_dma   = 1,
.dma_desc_enable= 0,
.dma_desc_fs_enable = 0,
.speed  = 0,/* High Speed */
@@ -104,7 +102,6 @@ static const struct dwc2_core_params params_bcm2835 = {
 static const struct dwc2_core_params params_rk3066 = {
.otg_cap= 2,/* non-HNP/non-SRP */
.otg_ver= -1,
-   .host_dma   = -1,
.dma_desc_enable= 0,
.dma_desc_fs_enable = 0,
.speed  = -1,
@@ -136,7 +133,6 @@ static const struct dwc2_core_params params_rk3066 = {
 static const struct dwc2_core_params params_ltq = {
.otg_cap= 2,/* non-HNP/non-SRP */
.otg_ver= -1,
-   .host_dma   = -1,
.dma_desc_enable= -1,
.dma_desc_fs_enable = -1,
.speed  = -1,
@@ -168,7 +164,6 @@ static const struct dwc2_core_params params_ltq = {
 static const struct dwc2_core_params params_amlogic = {
.otg_cap= DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE,
.otg_ver= -1,
-   .host_dma   = 1,
.dma_desc_enable= 0,
.dma_desc_fs_enable = 0,
.speed  = DWC2_SPEED_PARAM_HIGH,
@@ -200,7 +195,6 @@ static const struct dwc2_core_params params_amlogic = {
 static const struct dwc2_core_params params_default = {
.otg_cap= -1,
.otg_ver= -1,
-   .host_dma   = -1,
 
/*
 * Disable descriptor dma mode by default as the HW can support
@@ -486,27 +480,6 @@ static void dwc2_set_param_otg_cap(struct dwc2_hsotg 
*hsotg, int val)
hsotg->params.otg_cap = val;
 }
 
-static void dwc2_set_param_host_dma(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 host_dma parameter. Check HW 
configuration.\n",
-   val);
-   val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
-   dev_dbg(hsotg->dev, "Setting host_dma to %d\n", val);
-   }
-
-   hsotg->params.host_dma = val;
-}
-
 static void dwc2_set_param_dma_desc_enable(struct dwc2_hsotg *hsotg, int val)
 {
int valid = 1;
@@ -1124,11 +1097,27 @@ static void dwc2_set_parameters(struct dwc2_hsotg 
*hsotg,
 {
struct dwc2_hw_params *hw = >hw_params;
struct dwc2_core_params *p = >params;
+   bool dma_capable = !(hw->arch == 

[PATCH v2 10/13] usb: dwc2: Rename the dma_enable parameter to host_dma

[John Youn]

Rename it so that it is more consistent with the gadget dma parameter.
It only affects host-mode operation so prefix it with "host".

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h |  4 ++--
 drivers/usb/dwc2/hcd.c  | 54 ++---
 drivers/usb/dwc2/hcd_intr.c | 14 ++--
 drivers/usb/dwc2/params.c   | 30 -
 4 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 935ef36..4b78dde 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -286,7 +286,7 @@ enum dwc2_ep0_state {
  * @otg_ver:OTG version supported
  *   0 - 1.3 (default)
  *   1 - 2.0
- * @dma_enable: Specifies whether to use slave or DMA mode for 
accessing
+ * @host_dma:   Specifies whether to use slave or DMA mode for 
accessing
  *  the data FIFOs. The driver will automatically detect 
the
  *  value for this parameter if none is specified.
  *   0 - Slave (always available)
@@ -451,7 +451,7 @@ struct dwc2_core_params {
 #define DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE  2
 
int otg_ver;
-   int dma_enable;
+   int host_dma;
int dma_desc_enable;
int dma_desc_fs_enable;
int speed;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 7a86878..3ac0085 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -79,7 +79,7 @@ static void dwc2_enable_common_interrupts(struct dwc2_hsotg 
*hsotg)
/* Enable the interrupts in the GINTMSK */
intmsk = GINTSTS_MODEMIS | GINTSTS_OTGINT;
 
-   if (hsotg->params.dma_enable <= 0)
+   if (hsotg->params.host_dma <= 0)
intmsk |= GINTSTS_RXFLVL;
if (hsotg->params.external_id_pin_ctl <= 0)
intmsk |= GINTSTS_CONIDSTSCHNG;
@@ -285,11 +285,11 @@ static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg)
break;
}
 
-   dev_dbg(hsotg->dev, "dma_enable:%d dma_desc_enable:%d\n",
-   hsotg->params.dma_enable,
+   dev_dbg(hsotg->dev, "host_dma:%d dma_desc_enable:%d\n",
+   hsotg->params.host_dma,
hsotg->params.dma_desc_enable);
 
-   if (hsotg->params.dma_enable > 0) {
+   if (hsotg->params.host_dma > 0) {
if (hsotg->params.dma_desc_enable > 0)
dev_dbg(hsotg->dev, "Using Descriptor DMA mode\n");
else
@@ -299,7 +299,7 @@ static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg)
hsotg->params.dma_desc_enable = 0;
}
 
-   if (hsotg->params.dma_enable > 0)
+   if (hsotg->params.host_dma > 0)
ahbcfg |= GAHBCFG_DMA_EN;
 
dwc2_writel(ahbcfg, hsotg->regs + GAHBCFG);
@@ -774,7 +774,7 @@ static void dwc2_hc_enable_ints(struct dwc2_hsotg *hsotg,
 {
u32 intmsk;
 
-   if (hsotg->params.dma_enable > 0) {
+   if (hsotg->params.host_dma > 0) {
if (dbg_hc(chan))
dev_vdbg(hsotg->dev, "DMA enabled\n");
dwc2_hc_enable_dma_ints(hsotg, chan);
@@ -1004,7 +1004,7 @@ void dwc2_hc_halt(struct dwc2_hsotg *hsotg, struct 
dwc2_host_chan *chan,
}
hcchar |= HCCHAR_CHDIS;
 
-   if (hsotg->params.dma_enable <= 0) {
+   if (hsotg->params.host_dma <= 0) {
if (dbg_hc(chan))
dev_vdbg(hsotg->dev, "DMA not enabled\n");
hcchar |= HCCHAR_CHENA;
@@ -1350,7 +1350,7 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg 
*hsotg,
dev_vdbg(hsotg->dev, "%s()\n", __func__);
 
if (chan->do_ping) {
-   if (hsotg->params.dma_enable <= 0) {
+   if (hsotg->params.host_dma <= 0) {
if (dbg_hc(chan))
dev_vdbg(hsotg->dev, "ping, no DMA\n");
dwc2_hc_do_ping(hsotg, chan);
@@ -1478,7 +1478,7 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg 
*hsotg,
 TSIZ_SC_MC_PID_SHIFT);
}
 
-   if (hsotg->params.dma_enable > 0) {
+   if (hsotg->params.host_dma > 0) {
dwc2_writel((u32)chan->xfer_dma,
hsotg->regs + HCDMA(chan->hc_num));
if (dbg_hc(chan))
@@ -1521,7 +1521,7 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg 
*hsotg,
chan->xfer_started = 1;
chan->requests++;
 
-   if (hsotg->params.dma_enable <= 0 &&
+   if (hsotg->params.host_dma <= 0 &&
!chan->ep_is_in && chan->xfer_len > 0)
/* Load OUT packet into the appropriate Tx FIFO */
dwc2_hc_write_packet(hsotg, chan);
@@ -1804,7 +1804,7 @@ static void dwc2_hcd_cleanup_channels(struct dwc2_hsotg 
*hsotg)
u32 hcchar;
int i;
 
-   if (hsotg->params.dma_enable <= 

[PATCH v2 08/13] usb: dwc2: Rename host_rx_fifo_size hardware parameter

[John Youn]

This hardware parameter is not host specific. It also applies to device
mode. Drop the "host" from the name to make that clear.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  2 +-
 drivers/usb/dwc2/params.c | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2abe15f..b94d808 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -536,7 +536,7 @@ struct dwc2_hw_params {
unsigned dma_desc_enable:1;
unsigned enable_dynamic_fifo:1;
unsigned en_multiple_tx_fifo:1;
-   unsigned host_rx_fifo_size:16;
+   unsigned rx_fifo_size:16;
unsigned host_nperio_tx_fifo_size:16;
unsigned dev_nperio_tx_fifo_size:16;
unsigned host_perio_tx_fifo_size:16;
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 2c6bc3a6..719205a 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -417,7 +417,7 @@ static void dwc2_set_param_host_rx_fifo_size(struct 
dwc2_hsotg *hsotg, int val)
 {
int valid = 1;
 
-   if (val < 16 || val > hsotg->hw_params.host_rx_fifo_size)
+   if (val < 16 || val > hsotg->hw_params.rx_fifo_size)
valid = 0;
 
if (!valid) {
@@ -425,7 +425,7 @@ static void dwc2_set_param_host_rx_fifo_size(struct 
dwc2_hsotg *hsotg, int val)
dev_err(hsotg->dev,
"%d invalid for host_rx_fifo_size. Check HW 
configuration.\n",
val);
-   val = hsotg->hw_params.host_rx_fifo_size;
+   val = hsotg->hw_params.rx_fifo_size;
dev_dbg(hsotg->dev, "Setting host_rx_fifo_size to %d\n", val);
}
 
@@ -1100,7 +1100,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
  GHWCFG4_UTMI_PHY_DATA_WIDTH_SHIFT;
 
/* fifo sizes */
-   hw->host_rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
+   hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
GRXFSIZ_DEPTH_SHIFT;
 
dev_dbg(hsotg->dev, "Detected values from hardware:\n");
@@ -1142,8 +1142,8 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
hw->en_multiple_tx_fifo);
dev_dbg(hsotg->dev, "  total_fifo_size=%d\n",
hw->total_fifo_size);
-   dev_dbg(hsotg->dev, "  host_rx_fifo_size=%d\n",
-   hw->host_rx_fifo_size);
+   dev_dbg(hsotg->dev, "  rx_fifo_size=%d\n",
+   hw->rx_fifo_size);
dev_dbg(hsotg->dev, "  host_nperio_tx_fifo_size=%d\n",
hw->host_nperio_tx_fifo_size);
dev_dbg(hsotg->dev, "  host_perio_tx_fifo_size=%d\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 06/13] usb: dwc2: Remove dwc2_set_all_params function

[John Youn]

Replace this by statically defining a function with defaults, and just
assigning it. This will allow us to use parameters of any type and any
default value.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h |  2 --
 drivers/usb/dwc2/params.c   | 78 +
 drivers/usb/dwc2/platform.c |  2 --
 3 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 8d26847..f282742 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1252,8 +1252,6 @@ extern void dwc2_set_param_otg_ver(struct dwc2_hsotg 
*hsotg, int val);
 extern void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
const struct dwc2_core_params *params);
 
-extern void dwc2_set_all_params(struct dwc2_core_params *params, int value);
-
 extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 
 extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index d6c92d4..360f294 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -197,6 +197,44 @@ static const struct dwc2_core_params params_amlogic = {
.hibernation= -1,
 };
 
+static const struct dwc2_core_params params_default = {
+   .otg_cap= -1,
+   .otg_ver= -1,
+   .dma_enable = -1,
+
+   /*
+* Disable descriptor dma mode by default as the HW can support
+* it, but does not support it for SPLIT transactions.
+* Disable it for FS devices as well.
+*/
+   .dma_desc_enable= 0,
+   .dma_desc_fs_enable = 0,
+
+   .speed  = -1,
+   .enable_dynamic_fifo= -1,
+   .en_multiple_tx_fifo= -1,
+   .host_rx_fifo_size  = -1,
+   .host_nperio_tx_fifo_size   = -1,
+   .host_perio_tx_fifo_size= -1,
+   .max_transfer_size  = -1,
+   .max_packet_count   = -1,
+   .host_channels  = -1,
+   .phy_type   = -1,
+   .phy_utmi_width = -1,
+   .phy_ulpi_ddr   = -1,
+   .phy_ulpi_ext_vbus  = -1,
+   .i2c_enable = -1,
+   .ulpi_fs_ls = -1,
+   .host_support_fs_ls_low_power   = -1,
+   .host_ls_low_power_phy_clk  = -1,
+   .ts_dline   = -1,
+   .reload_ctl = -1,
+   .ahbcfg = -1,
+   .uframe_sched   = -1,
+   .external_id_pin_ctl= -1,
+   .hibernation= -1,
+};
+
 const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = _bcm2835 },
{ .compatible = "hisilicon,hi6220-usb", .data = _hi6220 },
@@ -1109,46 +1147,18 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
return 0;
 }
 
-/*
- * Sets all parameters to the given value.
- *
- * Assumes that the dwc2_core_params struct contains only integers.
- */
-void dwc2_set_all_params(struct dwc2_core_params *params, int value)
-{
-   int *p = (int *)params;
-   size_t size = sizeof(*params) / sizeof(*p);
-   int i;
-
-   for (i = 0; i < size; i++)
-   p[i] = value;
-}
-
 int dwc2_init_params(struct dwc2_hsotg *hsotg)
 {
const struct of_device_id *match;
-   const struct dwc2_core_params *params;
-   struct dwc2_core_params defparams;
+   struct dwc2_core_params params;
 
match = of_match_device(dwc2_of_match_table, hsotg->dev);
-   if (match && match->data) {
-   params = match->data;
-   } else {
-   /* Default all params to autodetect */
-   dwc2_set_all_params(, -1);
-   params = 
-
-   /*
-* Disable descriptor dma mode by default as the HW can support
-* it, but does not support it for SPLIT transactions.
-* Disable it for FS devices as well.
-*/
-   defparams.dma_desc_enable = 0;
-   defparams.dma_desc_fs_enable = 0;
-   }
+   if (match && match->data)
+   params = *((struct dwc2_core_params *)match->data);
+   else
+   params = params_default;
 
-   /* Validate parameter values */
-   dwc2_set_parameters(hsotg, params);
+   dwc2_set_parameters(hsotg, );
 
return 0;
 }
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 4fbfe09..4fc8c60 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -398,8 +398,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
spin_lock_init(>lock);
 
-   dwc2_set_all_params(>params, -1);
-

[PATCH v2 09/13] usb: dwc2: Move gadget settings into core_params

[John Youn]

Move the gadget devicetree settings into the core_params structure and
document them. Then set and check them in params.c, with the addition of
some helper functions, and remove the equivalent code in gadget.c.

Because these parameters came from the standalone s3c driver, they have
a fixed default value rather than an autodetected one. Preserve and
document this behavior to avoid any compatibility issues.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.h   |  32 --
 drivers/usb/dwc2/gadget.c |  93 +++-
 drivers/usb/dwc2/params.c | 262 +-
 3 files changed, 296 insertions(+), 91 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index b94d808..935ef36 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -418,6 +418,21 @@ 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_rx_fifo_size:The periodic rx fifo size for the device, in
+ * DWORDS from 16-32768 (default: 2048 if
+ * possible, otherwise autodetect).
+ * @g_np_tx_fifo_size: The non-periodic tx fifo size for the device in
+ * DWORDS from 16-32768 (default: 1024 if
+ * possible, otherwise autodetect).
+ * @g_tx_fifo_size:An array of TX fifo sizes in dedicated fifo
+ * mode. Each value corresponds to one EP
+ * starting from EP1 (max 15 values). Sizes are
+ * in DWORDS with possible values from from
+ * 16-32768 (default: 256, 256, 256, 256, 768,
+ * 768, 768, 768, 0, 0, 0, 0, 0, 0, 0).
  *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
@@ -475,6 +490,15 @@ struct dwc2_core_params {
int uframe_sched;
int external_id_pin_ctl;
int hibernation;
+
+   /*
+* The following parameters are *only* set via device
+* properties and cannot be set directly in this structure.
+*/
+   bool g_dma;
+   u16 g_rx_fifo_size;
+   u16 g_np_tx_fifo_size;
+   u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
 };
 
 /**
@@ -857,10 +881,6 @@ struct dwc2_hregs_backup {
  * @ep0_state:  EP0 control transfers state
  * @test_mode:  USB test mode requested by the host
  * @eps:The endpoints being supplied to the gadget framework
- * @g_using_dma:  Indicate if dma usage is enabled
- * @g_rx_fifo_sz: Contains rx fifo size value
- * @g_np_g_tx_fifo_sz:  Contains Non-Periodic tx fifo size value
- * @g_tx_fifo_sz: Contains tx fifo size value per endpoints
  */
 struct dwc2_hsotg {
struct device *dev;
@@ -1008,10 +1028,6 @@ struct dwc2_hsotg {
unsigned int connected:1;
struct dwc2_hsotg_ep *eps_in[MAX_EPS_CHANNELS];
struct dwc2_hsotg_ep *eps_out[MAX_EPS_CHANNELS];
-   u32 g_using_dma;
-   u32 g_rx_fifo_sz;
-   u32 g_np_g_tx_fifo_sz;
-   u32 g_tx_fifo_sz[MAX_EPS_CHANNELS];
 #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
 };
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index a70a7c7..a505bbf 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -93,7 +93,7 @@ static void dwc2_hsotg_dump(struct dwc2_hsotg *hsotg);
  */
 static inline bool using_dma(struct dwc2_hsotg *hsotg)
 {
-   return hsotg->g_using_dma;
+   return hsotg->params.g_dma;
 }
 
 /**
@@ -190,16 +190,17 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
unsigned int addr;
int timeout;
u32 val;
+   u32 *txfsz = hsotg->params.g_tx_fifo_size;
 
/* Reset fifo map if not correctly cleared during previous session */
WARN_ON(hsotg->fifo_map);
hsotg->fifo_map = 0;
 
/* set RX/NPTX FIFO sizes */
-   dwc2_writel(hsotg->g_rx_fifo_sz, hsotg->regs + GRXFSIZ);
-   dwc2_writel((hsotg->g_rx_fifo_sz << FIFOSIZE_STARTADDR_SHIFT) |
-   (hsotg->g_np_g_tx_fifo_sz << FIFOSIZE_DEPTH_SHIFT),
-   hsotg->regs + GNPTXFSIZ);
+   dwc2_writel(hsotg->params.g_rx_fifo_size, hsotg->regs + GRXFSIZ);
+   dwc2_writel((hsotg->params.g_rx_fifo_size << FIFOSIZE_STARTADDR_SHIFT) |
+   (hsotg->params.g_np_tx_fifo_size << FIFOSIZE_DEPTH_SHIFT),
+   hsotg->regs + GNPTXFSIZ);
 
/*
 * arange all the rest of the TX FIFOs, as some versions of this
@@ -209,7 +210,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 */
 
/* start at the end of the GNPTXFSIZ, rounded 

[PATCH v2 13/13] usb: dwc2: Add PCI properties

[John Youn]

From: Vahram Aharonyan 

Add device parameters handling in dwc2-pci similar what is done in dwc3.

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

diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index ae41961..b3f3b58 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -62,6 +62,21 @@ struct dwc2_pci_glue {
struct platform_device *phy;
 };
 
+static int dwc2_pci_quirks(struct pci_dev *pdev, struct platform_device *dwc2)
+{
+   if (pdev->vendor == PCI_VENDOR_ID_SYNOPSYS &&
+   pdev->device == PCI_PRODUCT_ID_HAPS_HSOTG) {
+   struct property_entry properties[] = {
+   PROPERTY_ENTRY_BOOL("g-use-dma"),
+   { },
+   };
+
+   return platform_device_add_properties(dwc2, properties);
+   }
+
+   return 0;
+}
+
 static void dwc2_pci_remove(struct pci_dev *pci)
 {
struct dwc2_pci_glue *glue = pci_get_drvdata(pci);
@@ -122,6 +137,10 @@ static int dwc2_pci_probe(struct pci_dev *pci,
return PTR_ERR(phy);
}
 
+   ret = dwc2_pci_quirks(pci, dwc2);
+   if (ret)
+   goto err;
+
ret = platform_device_add(dwc2);
if (ret) {
dev_err(dev, "failed to register dwc2 device\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 00/13] usb: dwc2: Fix up, consolidate, and simplify driver parameters

[John Youn]

This patch series cleans up and simplifies the parameter handling in
the dwc2 driver so that it is easier to set these parameters and
easier to maintain the driver and support more platforms in the long
run.

The long-term goal is to remove all static and legacy parameters in
favor of autodetection and/or devicetree properties.

However, this patch series is mostly a cleanup and refactoring to
allow for this. Then, it adds the current gadget-specific properties.
And for host-mode, it adds the DMA property. Lastly, it adds the
ability to set properties from the PCI driver so that we can perform
IP validation with the HAPS platform.

Later patch series will address the other properties and also allow
for them to be set by the PCI driver via debugfs.

Tested on DWC_hsotg IP version 3.30a on Synopsys HAPS platform.

v2:
* Renamed host-dma to "snps,host-dma-disable".
* Removed params->host_dma from being set statically.
* Reverted descriptor dma params renaming (to be addressed in the
  future).
* Simplified and removed unused code.

Regards,
John


John Youn (12):
  usb: dwc2: Remove unnecessary kfree
  usb: dwc2: Remove unused hardware parameter
  usb: dwc2: Add params.c file
  usb: dwc2: Declare the core params struct statically
  usb: dwc2: Move parameter initialization into params.c
  usb: dwc2: Remove dwc2_set_all_params function
  usb: dwc2: Remove unnecessary prototypes
  usb: dwc2: Rename host_rx_fifo_size hardware parameter
  usb: dwc2: Move gadget settings into core_params
  usb: dwc2: Rename the dma_enable parameter to host_dma
  Documentation: devicetree: dwc2: Add host DMA binding
  usb: dwc2: Get host DMA device properties

Vahram Aharonyan (1):
  usb: dwc2: Add PCI properties

 Documentation/devicetree/bindings/usb/dwc2.txt |1 +
 drivers/usb/dwc2/Makefile  |1 +
 drivers/usb/dwc2/core.c|  930 +---
 drivers/usb/dwc2/core.h|  280 +
 drivers/usb/dwc2/core_intr.c   |6 +-
 drivers/usb/dwc2/gadget.c  |   95 +-
 drivers/usb/dwc2/hcd.c |  193 ++--
 drivers/usb/dwc2/hcd_ddma.c|4 +-
 drivers/usb/dwc2/hcd_intr.c|   48 +-
 drivers/usb/dwc2/hcd_queue.c   |   18 +-
 drivers/usb/dwc2/params.c  | 1412 
 drivers/usb/dwc2/pci.c |   19 +
 drivers/usb/dwc2/platform.c|  207 +---
 13 files changed, 1653 insertions(+), 1561 deletions(-)
 create mode 100644 drivers/usb/dwc2/params.c

-- 
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/13] usb: dwc2: Remove unnecessary kfree

[John Youn]

This shouldn't be freed by the HCD as it is owned by the core and
allocated with devm_kzalloc.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/hcd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index df5a065..1b6f5e1 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5184,7 +5184,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 error2:
usb_put_hcd(hcd);
 error1:
-   kfree(hsotg->core_params);
 
 #ifdef CONFIG_USB_DWC2_TRACK_MISSED_SOFS
kfree(hsotg->last_frame_num_array);
-- 
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/13] usb: dwc2: Remove unused hardware parameter

[John Youn]

The dma_desc_fs_enable does not correspond to any hardware parameter and
is unused. Remove it.

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

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2a21a04..086bbdf 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -516,7 +516,6 @@ struct dwc2_hw_params {
unsigned op_mode:3;
unsigned arch:2;
unsigned dma_desc_enable:1;
-   unsigned dma_desc_fs_enable:1;
unsigned enable_dynamic_fifo:1;
unsigned en_multiple_tx_fifo:1;
unsigned host_rx_fifo_size:16;
-- 
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/13] usb: dwc2: Add params.c file

[John Youn]

Add a params.c file and move all driver parameter code there, including
all the static parameter definitions.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/Makefile   |1 +
 drivers/usb/dwc2/core.c |  918 +--
 drivers/usb/dwc2/core.h |5 +
 drivers/usb/dwc2/params.c   | 1125 +++
 drivers/usb/dwc2/platform.c |  173 ---
 5 files changed, 1133 insertions(+), 1089 deletions(-)
 create mode 100644 drivers/usb/dwc2/params.c

diff --git a/drivers/usb/dwc2/Makefile b/drivers/usb/dwc2/Makefile
index 50fdaac..b9237e1 100644
--- a/drivers/usb/dwc2/Makefile
+++ b/drivers/usb/dwc2/Makefile
@@ -3,6 +3,7 @@ ccflags-$(CONFIG_USB_DWC2_VERBOSE)  += -DVERBOSE_DEBUG
 
 obj-$(CONFIG_USB_DWC2) += dwc2.o
 dwc2-y := core.o core_intr.o platform.o
+dwc2-y += params.o
 
 ifneq ($(filter y,$(CONFIG_USB_DWC2_HOST) $(CONFIG_USB_DWC2_DUAL_ROLE)),)
dwc2-y  += hcd.o hcd_intr.o
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 4c0fa0b..e1f1b19 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -445,7 +445,7 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool 
host)
  * the force mode. We only need to call this once during probe if
  * dr_mode == OTG.
  */
-static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
+void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
 {
u32 gusbcfg;
 
@@ -735,704 +735,13 @@ void dwc2_flush_rx_fifo(struct dwc2_hsotg *hsotg)
udelay(1);
 }
 
-#define DWC2_OUT_OF_BOUNDS(a, b, c)((a) < (b) || (a) > (c))
-
-/* Parameter access functions */
-void dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg, int val)
-{
-   int valid = 1;
-
-   switch (val) {
-   case DWC2_CAP_PARAM_HNP_SRP_CAPABLE:
-   if (hsotg->hw_params.op_mode != GHWCFG2_OP_MODE_HNP_SRP_CAPABLE)
-   valid = 0;
-   break;
-   case DWC2_CAP_PARAM_SRP_ONLY_CAPABLE:
-   switch (hsotg->hw_params.op_mode) {
-   case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE:
-   case GHWCFG2_OP_MODE_SRP_ONLY_CAPABLE:
-   case GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE:
-   case GHWCFG2_OP_MODE_SRP_CAPABLE_HOST:
-   break;
-   default:
-   valid = 0;
-   break;
-   }
-   break;
-   case DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE:
-   /* always valid */
-   break;
-   default:
-   valid = 0;
-   break;
-   }
-
-   if (!valid) {
-   if (val >= 0)
-   dev_err(hsotg->dev,
-   "%d invalid for otg_cap parameter. Check HW 
configuration.\n",
-   val);
-   switch (hsotg->hw_params.op_mode) {
-   case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE:
-   val = DWC2_CAP_PARAM_HNP_SRP_CAPABLE;
-   break;
-   case GHWCFG2_OP_MODE_SRP_ONLY_CAPABLE:
-   case GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE:
-   case GHWCFG2_OP_MODE_SRP_CAPABLE_HOST:
-   val = DWC2_CAP_PARAM_SRP_ONLY_CAPABLE;
-   break;
-   default:
-   val = DWC2_CAP_PARAM_NO_HNP_SRP_CAPABLE;
-   break;
-   }
-   dev_dbg(hsotg->dev, "Setting otg_cap to %d\n", val);
-   }
-
-   hsotg->core_params->otg_cap = val;
-}
-
-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;
-}
-
-void dwc2_set_param_dma_desc_enable(struct dwc2_hsotg *hsotg, int val)
-{
-   int valid = 1;
-
-   if (val > 0 && (hsotg->core_params->dma_enable <= 0 ||
-   !hsotg->hw_params.dma_desc_enable))
-   valid = 0;
-   if (val < 0)
-   valid = 0;
-
-   if (!valid) {
-   if (val >= 0)
-   dev_err(hsotg->dev,
-   "%d invalid for dma_desc_enable parameter. 
Check HW configuration.\n",
-   val);
-   val = (hsotg->core_params->dma_enable > 0 &&
-   

[PATCH v2 04/13] usb: dwc2: Declare the core params struct statically

[John Youn]

This makes it consistent with the hw_params struct and simplifies the
memory management for future refactoring. Fix up usage in all files.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/core.c  |  12 +--
 drivers/usb/dwc2/core.h  |   2 +-
 drivers/usb/dwc2/core_intr.c |   6 +-
 drivers/usb/dwc2/gadget.c|   2 +-
 drivers/usb/dwc2/hcd.c   | 190 +--
 drivers/usb/dwc2/hcd_ddma.c  |   4 +-
 drivers/usb/dwc2/hcd_intr.c  |  48 +--
 drivers/usb/dwc2/hcd_queue.c |  18 ++--
 drivers/usb/dwc2/params.c|  68 
 drivers/usb/dwc2/platform.c  |   7 +-
 10 files changed, 176 insertions(+), 181 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index e1f1b19..11d8ae9 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -135,7 +135,7 @@ int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, bool 
restore)
u32 pcgcctl;
int ret = 0;
 
-   if (!hsotg->core_params->hibernation)
+   if (!hsotg->params.hibernation)
return -ENOTSUPP;
 
pcgcctl = dwc2_readl(hsotg->regs + PCGCTL);
@@ -188,7 +188,7 @@ int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg)
u32 pcgcctl;
int ret = 0;
 
-   if (!hsotg->core_params->hibernation)
+   if (!hsotg->params.hibernation)
return -ENOTSUPP;
 
/* Backup all registers */
@@ -541,7 +541,7 @@ void dwc2_dump_host_registers(struct dwc2_hsotg *hsotg)
addr = hsotg->regs + HAINTMSK;
dev_dbg(hsotg->dev, "HAINTMSK@0x%08lX : 0x%08X\n",
(unsigned long)addr, dwc2_readl(addr));
-   if (hsotg->core_params->dma_desc_enable > 0) {
+   if (hsotg->params.dma_desc_enable > 0) {
addr = hsotg->regs + HFLBADDR;
dev_dbg(hsotg->dev, "HFLBADDR @0x%08lX : 0x%08X\n",
(unsigned long)addr, dwc2_readl(addr));
@@ -551,7 +551,7 @@ void dwc2_dump_host_registers(struct dwc2_hsotg *hsotg)
dev_dbg(hsotg->dev, "HPRT0   @0x%08lX : 0x%08X\n",
(unsigned long)addr, dwc2_readl(addr));
 
-   for (i = 0; i < hsotg->core_params->host_channels; i++) {
+   for (i = 0; i < hsotg->params.host_channels; i++) {
dev_dbg(hsotg->dev, "Host Channel %d Specific Registers\n", i);
addr = hsotg->regs + HCCHAR(i);
dev_dbg(hsotg->dev, "HCCHAR  @0x%08lX : 0x%08X\n",
@@ -571,7 +571,7 @@ void dwc2_dump_host_registers(struct dwc2_hsotg *hsotg)
addr = hsotg->regs + HCDMA(i);
dev_dbg(hsotg->dev, "HCDMA   @0x%08lX : 0x%08X\n",
(unsigned long)addr, dwc2_readl(addr));
-   if (hsotg->core_params->dma_desc_enable > 0) {
+   if (hsotg->params.dma_desc_enable > 0) {
addr = hsotg->regs + HCDMAB(i);
dev_dbg(hsotg->dev, "HCDMAB  @0x%08lX : 0x%08X\n",
(unsigned long)addr, dwc2_readl(addr));
@@ -753,7 +753,7 @@ bool dwc2_force_mode_if_needed(struct dwc2_hsotg *hsotg, 
bool host)
 
 u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg)
 {
-   return hsotg->core_params->otg_ver == 1 ? 0x0200 : 0x0103;
+   return hsotg->params.otg_ver == 1 ? 0x0200 : 0x0103;
 }
 
 bool dwc2_is_controller_alive(struct dwc2_hsotg *hsotg)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 35337ff..f3d14f3 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -850,7 +850,7 @@ struct dwc2_hsotg {
/** Params detected from hardware */
struct dwc2_hw_params hw_params;
/** Params to actually use */
-   struct dwc2_core_params *core_params;
+   struct dwc2_core_params params;
enum usb_otg_state op_state;
enum usb_dr_mode dr_mode;
unsigned int hcd_enabled:1;
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index d85c5c9..5b228ba 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -159,9 +159,9 @@ static void dwc2_handle_otg_intr(struct dwc2_hsotg *hsotg)
" ++OTG Interrupt: Session Request Success Status 
Change++\n");
gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
if (gotgctl & GOTGCTL_SESREQSCS) {
-   if (hsotg->core_params->phy_type ==
+   if (hsotg->params.phy_type ==
DWC2_PHY_TYPE_PARAM_FS
-   && hsotg->core_params->i2c_enable > 0) {
+   && hsotg->params.i2c_enable > 0) {
hsotg->srp_success = 1;
} else {
/* Clear Session Request */
@@ -370,7 +370,7 @@ static void dwc2_handle_wakeup_detected_intr(struct 
dwc2_hsotg *hsotg)
/* Change to L0 state */
hsotg->lx_state = DWC2_L0;

Re: [PATCH 1/1] usb: xhci: cleanup cmd_completion in xhci_virt_device

[Lu Baolu]

Hi,

On 11/03/2016 07:36 PM, Mathias Nyman wrote:
> On 03.11.2016 12:22, Sergei Shtylyov wrote:
>> On 11/3/2016 9:48 AM, Lu Baolu wrote:
>>
>>> cmd_completion in struct xhci_virt_device is legacy. With command
>>> strutcture and command queue introduced in xhci, cmd_completion is
>>
>> Structure.
>>
>
> I'll fix that while applying the patch, no need to resend

Thank you.

Best regards,
Lu Baolu

>
> -Mathias
>
>

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

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

[Ladislav Michl]

On Thu, Nov 03, 2016 at 03:55:32PM -0700, Tony Lindgren wrote:
> * Tony Lindgren  [161103 13:59]:
> > * Ladislav Michl  [161101 14:14]:
> > > > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > > > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order 
> > > > error")
> > > 
> > > tested with v4.9-rc3 which have these included.
> > 
> > OK thanks.
> 
> So here's something to test, v4.9-rc3 + the PHY patch I
> posted + the patch below.
> 
> > Hmm yeah playing with a hub connected devices don't always enumerate.
> > When that happens, I get this:
> > 
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: USB disconnect, device number 45
> > usb 1-1: new high-speed USB device number 47 using musb-hdrc
> > usb 1-1: new high-speed USB device number 48 using musb-hdrc
> > ...
> > 
> > And that keeps on going until I reconnect the hub.
> 
> The patch below seems to work with PM for me, except I
> the dsps glue layer won't go to idle after disconnecting
> the hub. On 2430 glue layer things idle for me properly
> and I don't seem to get any more vbus errors.

Well, at least musb reacts on hub disconnects now. Devices
get enumerated, but do not work. Also musb does notice
only hub connect/disconnect, but does not react on
disconnection of devices in hub.

Best regards,
ladis
--
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: gadget: f_fs: fix wrong parenthesis in ffs_func_req_match()

[Felix Hädicke]

Properly check the return code of ffs_func_revmap_intf() and
ffs_func_revmap_ep() for a non-negative value.

Instead of checking the return code, the comparison was performed for the last
parameter of the function calls, because of wrong parenthesis.

This also fixes the following static checker warning:
drivers/usb/gadget/function/f_fs.c:3152 ffs_func_req_match()
warn: always true condition '(((creq->wIndex)) >= 0) => (0-u16max >= 0)'

Reported-by: Dan Carpenter 
Signed-off-by: Felix Hädicke 
---
 drivers/usb/gadget/function/f_fs.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 43aeed6..fcfdc6a 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -3028,11 +3028,11 @@ static bool ffs_func_req_match(struct usb_function *f,
 
switch (creq->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_INTERFACE:
-   return ffs_func_revmap_intf(func,
-   le16_to_cpu(creq->wIndex) >= 0);
+   return (ffs_func_revmap_intf(func,
+le16_to_cpu(creq->wIndex)) >= 0);
case USB_RECIP_ENDPOINT:
-   return ffs_func_revmap_ep(func,
- le16_to_cpu(creq->wIndex) >= 0);
+   return (ffs_func_revmap_ep(func,
+  le16_to_cpu(creq->wIndex)) >= 0);
default:
return (bool) (func->ffs->user_flags &
   FUNCTIONFS_ALL_CTRL_RECIP);
-- 
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 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

* Tony Lindgren  [161103 13:59]:
> * Ladislav Michl  [161101 14:14]:
> > > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order 
> > > error")
> > 
> > tested with v4.9-rc3 which have these included.
> 
> OK thanks.

So here's something to test, v4.9-rc3 + the PHY patch I
posted + the patch below.

> Hmm yeah playing with a hub connected devices don't always enumerate.
> When that happens, I get this:
> 
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: USB disconnect, device number 45
> usb 1-1: new high-speed USB device number 47 using musb-hdrc
> usb 1-1: new high-speed USB device number 48 using musb-hdrc
> ...
> 
> And that keeps on going until I reconnect the hub.

The patch below seems to work with PM for me, except I
the dsps glue layer won't go to idle after disconnecting
the hub. On 2430 glue layer things idle for me properly
and I don't seem to get any more vbus errors.

Regards,

Tony

8< --
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -987,7 +987,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 
int_usb,
}
 #endif
 
-   schedule_work(>irq_work);
+   schedule_delayed_work(>irq_work, 0);
 
return handled;
 }
@@ -1864,12 +1864,8 @@ static void musb_pm_runtime_check_session(struct musb 
*musb)
}
break;
case MUSB_QUIRK_A_DISCONNECT_19:
-   if (!musb->session)
-   break;
-   musb_dbg(musb, "Allow PM on possible host mode disconnect");
-   pm_runtime_mark_last_busy(musb->controller);
-   pm_runtime_put_autosuspend(musb->controller);
-   musb->session = false;
+   musb_dbg(musb, "Poll devctl on possible host mode disconnect");
+   schedule_delayed_work(>irq_work, msecs_to_jiffies(1000));
return;
default:
break;
@@ -1900,7 +1896,7 @@ static void musb_pm_runtime_check_session(struct musb 
*musb)
 /* Only used to provide driver mode change events */
 static void musb_irq_work(struct work_struct *data)
 {
-   struct musb *musb = container_of(data, struct musb, irq_work);
+   struct musb *musb = container_of(data, struct musb, irq_work.work);
 
musb_pm_runtime_check_session(musb);
 
@@ -2274,7 +2270,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
musb_generic_disable(musb);
 
/* Init IRQ workqueue before request_irq */
-   INIT_WORK(>irq_work, musb_irq_work);
+   INIT_DELAYED_WORK(>irq_work, musb_irq_work);
INIT_DELAYED_WORK(>deassert_reset_work, musb_deassert_reset);
INIT_DELAYED_WORK(>finish_resume_work, musb_pending_work);
 
@@ -2370,7 +2366,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
musb_host_cleanup(musb);
 
 fail3:
-   cancel_work_sync(>irq_work);
+   cancel_delayed_work_sync(>irq_work);
musb_cancel_resume_work(musb);
cancel_delayed_work_sync(>deassert_reset_work);
if (musb->dma_controller)
@@ -2437,7 +2433,7 @@ static int musb_remove(struct platform_device *pdev)
 */
musb_exit_debugfs(musb);
 
-   cancel_work_sync(>irq_work);
+   cancel_delayed_work_sync(>irq_work);
musb_cancel_resume_work(musb);
cancel_delayed_work_sync(>deassert_reset_work);
pm_runtime_get_sync(musb->controller);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -310,7 +310,7 @@ struct musb {
struct musb_context_registers context;
 
irqreturn_t (*isr)(int, void *);
-   struct work_struct  irq_work;
+   struct delayed_work irq_work;
struct delayed_work deassert_reset_work;
struct delayed_work finish_resume_work;
u16 hwvers;
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1114,7 +1114,7 @@ static int musb_gadget_enable(struct usb_ep *ep,
musb_ep->dma ? "dma, " : "",
musb_ep->packet_sz);
 
-   schedule_work(>irq_work);
+   schedule_delayed_work(>irq_work, 0);
 
 fail:
spin_unlock_irqrestore(>lock, flags);
@@ -1158,7 +1158,7 @@ static int musb_gadget_disable(struct usb_ep *ep)
musb_ep->desc = NULL;
musb_ep->end_point.desc = NULL;
 
-   schedule_work(>irq_work);
+   schedule_delayed_work(>irq_work, 0);
 

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

[Ladislav Michl]

On Thu, Nov 03, 2016 at 02:26:35PM -0700, Tony Lindgren wrote:
[snip]
> Here's the patch updated to use the existing finish_resume_work.
> Is that along the lines you were thinking?
> 
> Adding also Ladis to Cc too.

Just gave it a try. Sure it wasn't supposed to fix "musb with hub"
issue, but at least it doesn not make things worse :-)

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

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

[Tony Lindgren]

* Johan Hovold  [161031 04:50]:
> On Fri, Oct 28, 2016 at 11:13:19AM -0700, Tony Lindgren wrote:
> > * Johan Hovold  [161028 02:45]:
> > > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote:
> > > > * Johan Hovold  [161027 11:46]:
> > > > > But then this looks like it could trigger an ABBA deadlock as 
> > > > > musb->lock
> > > > > is held while queue_on_resume() takes musb->list_lock, and
> > > > > musb_run_pending() would take the same locks in the reverse order.
> > > > 
> > > > It seems we can avoid that by locking only list_add_tail() and 
> > > > list_del():
> > > > 
> > > > list_for_each_entry_safe(w, _w, >resume_work, node) {
> > > > spin_lock_irqsave(>list_lock, flags);
> > > > list_del(>node);
> > > > spin_unlock_irqrestore(>list_lock, flags);
> > > > if (w->callback)
> > > > w->callback(musb, w->data);
> > > > devm_kfree(musb->controller, w);
> > > > }
> > > 
> > > I think you still need to hold the lock while traversing the list (even
> > > if you temporarily release it during the callback).
> > 
> > Hmm yeah we need iterate through the list again to avoid missing new
> > elements being added. I've updated the patch to use a the common
> > while (!list_empty(>resume_work)) loop. Does that solve the
> > concern you had or did you also had some other concern there?
> 
> Yeah, while that minimises the race window it is still possible that the
> timer callback checks pm_runtime_active() after the queue has been
> processed but before the rpm status is updated. 

OK. Sorry for the delay responding, had my motherboard fail
over the weekend..

> How about using a work struct and synchronous get for the deferred case?

Here's the patch updated to use the existing finish_resume_work.
Is that along the lines you were thinking?

Adding also Ladis to Cc too.

Regards,

Tony

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. We can use the existing finish_resume_work for that. Note that
we want to do this only when both musb core and it's parent devices are
awake as noted by Johan Hovold . This allows us also
to get rid of musb_gadget_work and need_finish_resume flag.

Note that we now also need to get rid of static int first as that
won't work right on devices with two musb instances like am335x.

Note that we don't want to mess with deassert_reset_work as that's
more time sensitive and USB spec related instead of PM runtime related.

Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS
glue layer")
Reported-by: Johan Hovold 
Signed-off-by: Tony Lindgren 
---
 drivers/usb/musb/musb_core.c| 89 +
 drivers/usb/musb/musb_core.h|  9 -
 drivers/usb/musb/musb_dsps.c| 24 +++
 drivers/usb/musb/musb_gadget.c  | 31 +-
 drivers/usb/musb/musb_host.h|  6 ++-
 drivers/usb/musb/musb_virthub.c |  5 +--
 6 files changed, 123 insertions(+), 41 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -578,8 +578,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 
int_usb,
| MUSB_PORT_STAT_RESUME;
musb->rh_timer = jiffies
+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
-   musb->need_finish_resume = 1;
-
+   musb_queue_resume_work(musb,
+  musb_host_finish_resume,
+  NULL);
musb->xceiv->otg->state = OTG_STATE_A_HOST;
musb->is_active = 1;
musb_host_resume_root_hub(musb);
@@ 

Re: Multiple chatty devices on Intel 5 Series/3400 USB2 EHCI controller act erratic

[sonofagun]

Hello guys! Did you have any progress on this? Do you still have the 
affected machine and devices?


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

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

[Tony Lindgren]

* Ladislav Michl  [161101 14:14]:
> Hi,
> 
> On Mon, Oct 24, 2016 at 11:07:08AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > * Tony Lindgren  [161021 00:18]:
> > > * Ladislav Michl  [161020 12:37]:
> > > > [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon 
> > > > (90,  > > > 
> > > > And that's the end, since now it does not react on hub plug/unplug.
> > > > 
> > > > Also all that VBUS_ERROR conditions are strange as hub is powered 
> > > > separately
> > > > and power lines from phy are not used.
> > > 
> > > Hmm yeah. I'd like to be able to reproduce this. Can you email me
> > > your .config (again)? You have things in host mode with a powered
> > > hub plus few devices with no USB gadgets configured?
> > 
> > Well I found your earlier .config so presumably that did not change.
> > Below patch seems to do the trick for me, but I need to test more.
> > 
> > Care to test if it helps for you? Please test with v4.9-rc2 and the
> > following two fixes heading in Greg's usb-linus branch:
> > 
> > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")
> 
> tested with v4.9-rc3 which have these included.

OK thanks.

> > I'll send a proper patch if that works for you.
> 
> Unfortunately it's still the same. Direct connection (without hub) remains
> untested as there's not enough power to supply display:
> usb 2-1: USB disconnect, device number 2
> usb 2-1: new high-speed USB device number 3 using musb-hdrc
> usb 2-1: New USB device found, idVendor=17e9, idProduct=0335
> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 2-1: Product: MIMO
> usb 2-1: Manufacturer: DisplayLink
> usb 2-1: SerialNumber: 1071007195
> usb 2-1: rejected 1 configuration due to insufficient available bus power
> usb 2-1: no configuration chosen from 1 choice

Hmm yeah playing with a hub connected devices don't always enumerate.
When that happens, I get this:

usb 1-1: reset high-speed USB device number 45 using musb-hdrc
usb 1-1: reset high-speed USB device number 45 using musb-hdrc
usb 1-1: reset high-speed USB device number 45 using musb-hdrc
usb 1-1: USB disconnect, device number 45
usb 1-1: new high-speed USB device number 47 using musb-hdrc
usb 1-1: new high-speed USB device number 48 using musb-hdrc
...

And that keeps on going until I reconnect the hub.

Regards,

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

Re: [BUG] cdc-acm: no data available after port open

[Ladislav Michl]

On Thu, Nov 03, 2016 at 02:12:58PM +0100, Oliver Neukum wrote:
[snip]
> I think the way usbnet handles -EPIPE is the best. We are a bit on thin
> ice because the CDC spec does not list under which conditions we should
> see a stall, thus by implication: never.
> But in general you cannot ignore a stall. You need to clear the halt.

This still cannot recover from "usb 3-1.4: clear tt 1 (9061) error -71",
but does recover from stall. If I got it wrong, please, let me know.
Thank you,
ladis

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 4c931d9..60e148d 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -28,8 +28,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 
-#undef DEBUG
-#undef VERBOSE_DEBUG
+#define DEBUG
+#define VERBOSE_DEBUG
 
 #include 
 #include 
@@ -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);
@@ -430,18 +429,22 @@ static void acm_read_bulk_callback(struct urb *urb)
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 -ECONNRESET:
case -ENOENT:
case -ESHUTDOWN:
-   dev_dbg(>control->dev,
-   "%s - urb shutting down with status: %d\n",
-   __func__, status);
+   dev_dbg(>data->dev,
+   "%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);
-   break;
+   dev_dbg(>data->dev,
+   "%s - nonzero urb status received: %d\n",
+   __func__, status);
+   return;
}
 
/*
@@ -479,6 +482,7 @@ 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);
 }
 
@@ -486,7 +490,30 @@ static void acm_softint(struct work_struct *work)
 {
struct acm *acm = container_of(work, struct acm, work);
 
-   tty_port_tty_wakeup(>port);
+   dev_vdbg(>data->dev, "scheduled work\n");
+
+   if (test_bit(EVENT_RX_STALL, >flags)) {
+   int i, status;
+
+   for (i = 0; i < acm->rx_buflimit; i++) {
+   usb_kill_urb(acm->read_urbs[i]);
+   set_bit(i, >read_urbs_free);
+   }
+
+   status = usb_autopm_get_interface(acm->data);
+   if (!status) {
+   status = usb_clear_halt(acm->dev, acm->in);
+   usb_autopm_put_interface(acm->data);
+   }
+   if (!status)
+   acm_submit_read_urbs(acm, GFP_KERNEL);
+   clear_bit(EVENT_RX_STALL, >flags);
+   }
+
+   if (test_bit(EVENT_TTY_WAKEUP, >flags)) {
+   tty_port_tty_wakeup(>port);
+   clear_bit(EVENT_TTY_WAKEUP, >flags);
+   }
 }
 
 /*
@@ -1098,19 +1125,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);
 }
 
@@ -1354,8 +1379,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 = 

Re: [PATCH] USB: phy: am335x-control: fix device and of_node leaks

[Bin Liu]

On Thu, Nov 03, 2016 at 10:54:33AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Johan Hovold  writes:
> > Make sure to drop the references taken by of_parse_phandle() and
> > bus_find_device() before returning from am335x_get_phy_control().
> >
> > Note that there is no guarantee that the devres-managed struct
> > phy_control will be valid for the lifetime of the sibling phy device
> > regardless of this change.
> >
> > Fixes: 3bb869c8b3f1 ("usb: phy: Add AM335x PHY driver")
> > Signed-off-by: Johan Hovold 
> > ---
> >  drivers/usb/phy/phy-am335x-control.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/phy/phy-am335x-control.c 
> > b/drivers/usb/phy/phy-am335x-control.c
> > index 42a1afe36a90..5f5f19813fde 100644
> > --- a/drivers/usb/phy/phy-am335x-control.c
> > +++ b/drivers/usb/phy/phy-am335x-control.c
> > @@ -134,10 +134,12 @@ struct phy_control *am335x_get_phy_control(struct 
> > device *dev)
> > return NULL;
> >  
> > dev = bus_find_device(_bus_type, NULL, node, match);
> > +   of_node_put(node);
> > if (!dev)
> > return NULL;
> >  
> > ctrl_usb = dev_get_drvdata(dev);
> > +   put_device(dev);
> > if (!ctrl_usb)
> > return NULL;
> > return _usb->phy_ctrl;
> 
> Bin, Roger, does this look okay to you?

Looks good to me. Thanks for catching this.

Acked-by: Bin Liu 

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

Re: [PATCH] usbhid: add ATEN CS962 to list of quirky devices

[Jiri Kosina]

On Thu, 3 Nov 2016, Oliver Neukum wrote:

> Like many similar devices it needs a quirk to work.
> Issuing the request gets the device into an irrecoverable state.

Applied to hid.git#for-4.9/upstream-fixes.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error

[Alexey Khoroshilov]

On 03.11.2016 16:34, Felipe Balbi wrote:
> 
> Hi,
> 
> Alexey Khoroshilov  writes:
>> mv_u3d_req_to_trb() does not check for dma mapping errors.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> v2: split fix and clenup to separate patches.
> 
> I'll fix this time when applying, but keep in mind we don't want these
> notes in the commit log. They should come after the tearline (---)
> below, together with the diffstat ;-)

ok, thank you

--
Alexey

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

Re: [PATCH v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[David Lechner]

On 11/03/2016 12:33 PM, Alexandre Bailon wrote:

On 11/03/2016 05:53 PM, David Lechner wrote:

On 11/03/2016 10:26 AM, Alexandre Bailon wrote:

The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

Add the usb20_force_mode property to force the PHY to operate
in host or peripheral mode.


Device tree describes the hardware, not the configuration, so this is
not acceptable.

I think that is really hardware (and board) dependent.
We will only need to set it for unusual hardware that doesn't
let the otg phy automatically find the it's role.
What do you think I should do?


I am staring to think that maybe it was a mistake for me to add the mode 
setting to the phy driver. It is not very clear to me where the division 
is between what is the responsibility of the phy and what is the 
responsibility of the musb.


Looking at the bigger picture, forcing the PHY mode and never changing 
it is not useful. It is the transition from not forced to forced or vice 
versa that is important because it is at that moment that interrupts are 
triggered.


The forcing of a PHY mode is really being used to simulate the plugging 
or unplugging of a USB device. And the only reason you would need to do 
this is that the hardware lacks the proper circuitry between the USB 
connector and the SoC in order to detect such an event.


So, what I think you should do is try to get MUSB working with the PHY 
in OTG mode without worrying about forcing other PHY modes. Then once 
that has been sorted out, we can talk about how to handle quirks for 
hardware that needs it.




Besides, this setting should not be fixed to one value anyway.

Actually, a bool is enough. If we need to force the phy in a specific
mode, we can reuse dr_mode of controller.



When usb20_force_mode is used, dr_mode should also be configured
to host or peripheral.
The controller uses dr_mode to configure itself, but the phy use
it to get the mode to use to configure the PHY mode.

Signed-off-by: Alexandre Bailon 
---
 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
index c26478b..9fc87fb 100644
--- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
+++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
@@ -4,6 +4,11 @@ Required properties:
  - compatible: must be "ti,da830-usb-phy".
  - #phy-cells: must be 1.

+Optional properties:
+- usb20-force-mode: Force the phy to operate in same mode than the
USB OTG controller.
+It should only be defined if the hardware is not capable
correctly
+detect the role of USB by using VBUS and ID pin.
+
 This device controls the PHY for both the USB 1.1 OHCI and USB 2.0 OTG
 controllers on DA8xx SoCs. Consumers of this device should use index
0 for
 the USB 2.0 phy device and index 1 for the USB 1.1 phy device.







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

Re: [PATCH v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[Kishon Vijay Abraham I]

Hi,

On Thursday 03 November 2016 10:56 PM, Alexandre Bailon wrote:
> On 11/03/2016 05:34 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 03 November 2016 08:56 PM, Alexandre Bailon wrote:
>>> The USB PHY is able to operate in OTG, host or peripheral.
>>> Some board may be wired to work act only as host or peripheral.
>>> In such case, the dr_mode property of controller must be set to
>>> host or peripheral. But doing that will also configure the PHY
>>> in host or peripheral mode whereas OTG is able to detect which
>>> role the USB controller should take.
>>> The PHY's host or peripheral mode are actually only useful when
>>> hardware doesn't allow OTG to detect it's role.
>>>
>>> Add the usb20_force_mode property to force the PHY to operate
>>> in host or peripheral mode.
>>
>> I think we do just that if we populate dr_mode with host or peripheral. Why 
>> do
>> we need another property to control dr_mode property?
> Because the phy doesn't work correctly when it is in host or
> device mode.

That would be the same even with usb20_force_mode property. How does
usb20_force_mode property help?

Thanks
Kishon

>>> When usb20_force_mode is used, dr_mode should also be configured
>>> to host or peripheral.
>>> The controller uses dr_mode to configure itself, but the phy use
>>> it to get the mode to use to configure the PHY mode.
>>>
>>> Signed-off-by: Alexandre Bailon 
>>> ---
>>>  Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt 
>>> b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>>> index c26478b..9fc87fb 100644
>>> --- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>>> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>>> @@ -4,6 +4,11 @@ Required properties:
>>>   - compatible: must be "ti,da830-usb-phy".
>>>   - #phy-cells: must be 1.
>>>  
>>> +Optional properties:
>>> +- usb20-force-mode: Force the phy to operate in same mode than the USB OTG 
>>> controller.
>>> +   It should only be defined if the hardware is not capable 
>>> correctly
>>> +   detect the role of USB by using VBUS and ID pin.
>>
>> From what I understand from the previous patch, if VBUS sense and the session
>> end comparator is enabled, the controller can work in host mode or device 
>> mode.
> I but VBUS sense and and session end comparator only seems to work when
> the phy is in otg mode.
> In host mode, the phy stop to work after the first disconnect.
> In device mode, the phy never detect a disconnect.
> In otg mode, these issues go away.
> I'm working on workaround for both of them but I think it is
> better to keep the phy in otg when it is possible.
>>
>> Thanks
>> Kishon
>>
> Thanks,
> Alexandre
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] usb: separate out sysdev pointer from usb_bus

[Grygorii Strashko]

On 11/02/2016 12:38 AM, Sriram Dash wrote:
> From: Arnd Bergmann 
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices.
> 
> The idea here is that you pass in the parent of_node along with
> the child device pointer, so it would behave exactly like the
> parent already does. The difference is that it also handles all
> the other attributes besides the mask.
> 
> sysdev will represent the physical device, as seen from firmware
> or bus.Splitting the usb_bus->controller field into the
> Linux-internal device (used for the sysfs hierarchy, for printks
> and for power management) and a new pointer (used for DMA,
> DT enumeration and phy lookup) probably covers all that we really
> need.
> 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Sriram Dash 
> Cc: Felipe Balbi 
> Cc: Grygorii Strashko 
> Cc: Sinjan Kumar 
> Cc: David Fisher 
> Cc: Catalin Marinas 
> Cc: "Thang Q. Nguyen" 
> Cc: Yoshihiro Shimoda 
> Cc: Stephen Boyd 
> Cc: Bjorn Andersson 
> Cc: Ming Lei 
> Cc: Jon Masters 
> Cc: Dann Frazier 
> Cc: Peter Chen 
> Cc: Leo Li 
> ---
> Changes in v2:
>   - Split the patch wrt driver
> 
>  drivers/usb/core/buffer.c | 12 ++--
>  drivers/usb/core/hcd.c| 48 
> ---
>  drivers/usb/core/usb.c| 18 +-
>  include/linux/usb.h   |  1 +
>  include/linux/usb/hcd.h   |  3 +++
>  5 files changed, 48 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index 98e39f9..1e41ef7 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -63,7 +63,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
>   int i, size;
>  
>   if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> - (!hcd->self.controller->dma_mask &&
> + (!hcd->self.sysdev->dma_mask &&

I think code shouldn't access DMA props directly, so may be
is_device_dma_capable() is right API to use here (and other places).

>!(hcd->driver->flags & HCD_LOCAL_MEM)))
>   return 0;
>  
> @@ -72,7 +72,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
>   if (!size)
>   continue;
>   snprintf(name, sizeof(name), "buffer-%d", size);
> - hcd->pool[i] = dma_pool_create(name, hcd->self.controller,
> + hcd->pool[i] = dma_pool_create(name, hcd->self.sysdev,
>   size, size, 0);
>   if (!hcd->pool[i]) {
>   hcd_buffer_destroy(hcd);
> @@ -127,7 +127,7 @@ void *hcd_buffer_alloc(
>  

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

Re: [PATCH v3 5/5] usb: musb: da8xx: Remove set_mode callback

[Alexandre Bailon]

On 11/03/2016 06:27 PM, Bin Liu wrote:
> On Thu, Nov 03, 2016 at 12:18:53PM -0500, David Lechner wrote:
>> On 11/03/2016 10:26 AM, Alexandre Bailon wrote:
>>> The USB PHY is able to operate in OTG, host or peripheral.
>>> Some board may be wired to work act only as host or peripheral.
>>> In such case, the dr_mode property of controller must be set to
>>> host or peripheral. But doing that will also configure the PHY
>>> in host or peripheral mode whereas OTG is able to detect which
>>> role the USB controller should take.
>>> The PHY's host or peripheral mode are actually only useful when
>>> hardware doesn't allow OTG to detect it's role.
>>>
>>> The set_mode callback is used by the musb driver to set mode
>>> of the PHY. But in the case of DA8xx, the PHY have some issues.
>>> The OTG mode work correctly but the host and peripheral don't.
>>> In host mode, the PHY stops to work after the first disconnect.
>>> In device mode, the PHY doesn't detect any disconnect.
>>> As the OTG mode is working properly, let the PHY in OTG mode,
>>> whatever is the controller mode.
>>>
>>> Signed-off-by: Alexandre Bailon 
>>> ---
>>> drivers/usb/musb/da8xx.c | 23 ---
>>> 1 file changed, 23 deletions(-)
>>>
>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>>> index 6749aa1..581f830 100644
>>> --- a/drivers/usb/musb/da8xx.c
>>> +++ b/drivers/usb/musb/da8xx.c
>>> @@ -335,28 +335,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void 
>>> *hci)
>>> return ret;
>>> }
>>>
>>> -static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
>>
>> Bin suggested using some sort of quirks flag. So instead of removing
>> this callback, I think this is where to incorporate the quirks
>> flags.
>>
>> I suppose the quirks could be kernel config options. Perhaps someone
>> else has a better idea?
> 
> I didn't closely follow this thread, but I was thinking about to reuse
> musb->io.quirks, and define the quirks in device tree...
I have understood that.
It seems to be a better solution than what I did.
> 
> I am debugging an issue in dsps, and might need a quirk for the
> solution...
> 
> Regards,
> -Bin.
> 
Thanks,
Alexandre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[Alexandre Bailon]

On 11/03/2016 05:53 PM, David Lechner wrote:
> On 11/03/2016 10:26 AM, Alexandre Bailon wrote:
>> The USB PHY is able to operate in OTG, host or peripheral.
>> Some board may be wired to work act only as host or peripheral.
>> In such case, the dr_mode property of controller must be set to
>> host or peripheral. But doing that will also configure the PHY
>> in host or peripheral mode whereas OTG is able to detect which
>> role the USB controller should take.
>> The PHY's host or peripheral mode are actually only useful when
>> hardware doesn't allow OTG to detect it's role.
>>
>> Add the usb20_force_mode property to force the PHY to operate
>> in host or peripheral mode.
> 
> Device tree describes the hardware, not the configuration, so this is
> not acceptable.
I think that is really hardware (and board) dependent.
We will only need to set it for unusual hardware that doesn't
let the otg phy automatically find the it's role.
What do you think I should do?
> 
> Besides, this setting should not be fixed to one value anyway.
Actually, a bool is enough. If we need to force the phy in a specific
mode, we can reuse dr_mode of controller.
> 
>> When usb20_force_mode is used, dr_mode should also be configured
>> to host or peripheral.
>> The controller uses dr_mode to configure itself, but the phy use
>> it to get the mode to use to configure the PHY mode.
>>
>> Signed-off-by: Alexandre Bailon 
>> ---
>>  Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> index c26478b..9fc87fb 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> @@ -4,6 +4,11 @@ Required properties:
>>   - compatible: must be "ti,da830-usb-phy".
>>   - #phy-cells: must be 1.
>>
>> +Optional properties:
>> +- usb20-force-mode: Force the phy to operate in same mode than the
>> USB OTG controller.
>> +It should only be defined if the hardware is not capable
>> correctly
>> +detect the role of USB by using VBUS and ID pin.
>> +
>>  This device controls the PHY for both the USB 1.1 OHCI and USB 2.0 OTG
>>  controllers on DA8xx SoCs. Consumers of this device should use index
>> 0 for
>>  the USB 2.0 phy device and index 1 for the USB 1.1 phy device.
>>
> 

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

Re: [PATCH v3 5/5] usb: musb: da8xx: Remove set_mode callback

[Bin Liu]

On Thu, Nov 03, 2016 at 12:18:53PM -0500, David Lechner wrote:
> On 11/03/2016 10:26 AM, Alexandre Bailon wrote:
> >The USB PHY is able to operate in OTG, host or peripheral.
> >Some board may be wired to work act only as host or peripheral.
> >In such case, the dr_mode property of controller must be set to
> >host or peripheral. But doing that will also configure the PHY
> >in host or peripheral mode whereas OTG is able to detect which
> >role the USB controller should take.
> >The PHY's host or peripheral mode are actually only useful when
> >hardware doesn't allow OTG to detect it's role.
> >
> >The set_mode callback is used by the musb driver to set mode
> >of the PHY. But in the case of DA8xx, the PHY have some issues.
> >The OTG mode work correctly but the host and peripheral don't.
> >In host mode, the PHY stops to work after the first disconnect.
> >In device mode, the PHY doesn't detect any disconnect.
> >As the OTG mode is working properly, let the PHY in OTG mode,
> >whatever is the controller mode.
> >
> >Signed-off-by: Alexandre Bailon 
> >---
> > drivers/usb/musb/da8xx.c | 23 ---
> > 1 file changed, 23 deletions(-)
> >
> >diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> >index 6749aa1..581f830 100644
> >--- a/drivers/usb/musb/da8xx.c
> >+++ b/drivers/usb/musb/da8xx.c
> >@@ -335,28 +335,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void 
> >*hci)
> > return ret;
> > }
> >
> >-static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
> 
> Bin suggested using some sort of quirks flag. So instead of removing
> this callback, I think this is where to incorporate the quirks
> flags.
> 
> I suppose the quirks could be kernel config options. Perhaps someone
> else has a better idea?

I didn't closely follow this thread, but I was thinking about to reuse
musb->io.quirks, and define the quirks in device tree...

I am debugging an issue in dsps, and might need a quirk for the
solution...

Regards,
-Bin.

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

Re: [PATCH v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[Alexandre Bailon]

On 11/03/2016 05:34 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 03 November 2016 08:56 PM, Alexandre Bailon wrote:
>> The USB PHY is able to operate in OTG, host or peripheral.
>> Some board may be wired to work act only as host or peripheral.
>> In such case, the dr_mode property of controller must be set to
>> host or peripheral. But doing that will also configure the PHY
>> in host or peripheral mode whereas OTG is able to detect which
>> role the USB controller should take.
>> The PHY's host or peripheral mode are actually only useful when
>> hardware doesn't allow OTG to detect it's role.
>>
>> Add the usb20_force_mode property to force the PHY to operate
>> in host or peripheral mode.
> 
> I think we do just that if we populate dr_mode with host or peripheral. Why do
> we need another property to control dr_mode property?
Because the phy doesn't work correctly when it is in host or
device mode.
>> When usb20_force_mode is used, dr_mode should also be configured
>> to host or peripheral.
>> The controller uses dr_mode to configure itself, but the phy use
>> it to get the mode to use to configure the PHY mode.
>>
>> Signed-off-by: Alexandre Bailon 
>> ---
>>  Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt 
>> b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> index c26478b..9fc87fb 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
>> @@ -4,6 +4,11 @@ Required properties:
>>   - compatible: must be "ti,da830-usb-phy".
>>   - #phy-cells: must be 1.
>>  
>> +Optional properties:
>> +- usb20-force-mode: Force the phy to operate in same mode than the USB OTG 
>> controller.
>> +It should only be defined if the hardware is not capable 
>> correctly
>> +detect the role of USB by using VBUS and ID pin.
> 
> From what I understand from the previous patch, if VBUS sense and the session
> end comparator is enabled, the controller can work in host mode or device 
> mode.
I but VBUS sense and and session end comparator only seems to work when
the phy is in otg mode.
In host mode, the phy stop to work after the first disconnect.
In device mode, the phy never detect a disconnect.
In otg mode, these issues go away.
I'm working on workaround for both of them but I think it is
better to keep the phy in otg when it is possible.
> 
> Thanks
> Kishon
> 
Thanks,
Alexandre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/5] usb: musb: da8xx: Remove set_mode callback

[David Lechner]

On 11/03/2016 10:26 AM, Alexandre Bailon wrote:

The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

The set_mode callback is used by the musb driver to set mode
of the PHY. But in the case of DA8xx, the PHY have some issues.
The OTG mode work correctly but the host and peripheral don't.
In host mode, the PHY stops to work after the first disconnect.
In device mode, the PHY doesn't detect any disconnect.
As the OTG mode is working properly, let the PHY in OTG mode,
whatever is the controller mode.

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

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 6749aa1..581f830 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -335,28 +335,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
return ret;
 }

-static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)


Bin suggested using some sort of quirks flag. So instead of removing 
this callback, I think this is where to incorporate the quirks flags.


I suppose the quirks could be kernel config options. Perhaps someone 
else has a better idea?



-{
-   struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
-   enum phy_mode phy_mode;
-
-   switch (musb_mode) {
-   case MUSB_HOST: /* Force VBUS valid, ID = 0 */
-   phy_mode = PHY_MODE_USB_HOST;


this should be something like...

phy_mode = force_host_mode_quirk ? PHY_MODE_USB_HOST
 : PHY_MODE_USB_OTG;



-   break;
-   case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
-   phy_mode = PHY_MODE_USB_DEVICE;


and...

phy_mode = force_peripheral_mode_quirk ?
PHY_MODE_USB_DEVICE : PHY_MODE_USB_OTG;



-   break;
-   case MUSB_OTG:  /* Don't override the VBUS/ID comparators */
-   phy_mode = PHY_MODE_USB_OTG;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   return phy_set_mode(glue->phy, phy_mode);
-}
-
 static int da8xx_musb_init(struct musb *musb)
 {
struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -445,7 +423,6 @@ static const struct musb_platform_ops da8xx_ops = {
.enable = da8xx_musb_enable,
.disable= da8xx_musb_disable,

-   .set_mode   = da8xx_musb_set_mode,
.try_idle   = da8xx_musb_try_idle,

.set_vbus   = da8xx_musb_set_vbus,



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

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[David Miller]

From: Felipe Balbi 
Date: Thu, 03 Nov 2016 09:04:54 +0200

> What Dave Miller is saying is that it's ALWAYS a bug to delay
> completion of SKBs. The only thing you're doing with chipidea is
> delaying interrupt by up to 125us; which is still a bug from the
> point of view of the networking layer, but it's more difficult to
> perceive any problems because of the short time where interrupt is
> delayed.

I didn't say delaying was illegal.

I said that the SKB free must occur in a reasonable, finite, amount of
time.

And if these timeout events ensure that, then it's fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround

[David Lechner]

On 11/03/2016 10:26 AM, Alexandre Bailon wrote:

If we configure the da8xx OTG phy in OTG mode, neither device or host
mode will work. That is because the PHY is not able to detect and notify
the driver that value of ID pin changed.
To work despite this hardware limitation, the da8xx glue implement a
workaround.
But to work, the workaround require the VBUS sense and the session end
comparator to enabled.
Enable them if the phy is configured in OTG mode.

Signed-off-by: Alexandre Bailon 
---
 drivers/phy/phy-da8xx-usb.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index 32ae78c..fd39292 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -93,24 +93,31 @@ static int da8xx_usb20_phy_power_off(struct phy *phy)
 static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)
 {
struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);
+   int ret;
u32 val;

+   ret = regmap_read(d_phy->regmap, CFGCHIP(2), );
+   if (ret)
+   return ret;
+
+   val &= ~CFGCHIP2_OTGMODE_MASK;
+
switch (mode) {
case PHY_MODE_USB_HOST: /* Force VBUS valid, ID = 0 */
-   val = CFGCHIP2_OTGMODE_FORCE_HOST;
+   val |= CFGCHIP2_OTGMODE_FORCE_HOST;
break;
case PHY_MODE_USB_DEVICE:   /* Force VBUS valid, ID = 1 */
-   val = CFGCHIP2_OTGMODE_FORCE_DEVICE;
+   val |= CFGCHIP2_OTGMODE_FORCE_DEVICE;
break;
case PHY_MODE_USB_OTG:  /* Don't override the VBUS/ID comparators */
-   val = CFGCHIP2_OTGMODE_NO_OVERRIDE;
+   val |= CFGCHIP2_OTGMODE_NO_OVERRIDE |
+   CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;


I still think this is wrong that you set these bits but never clear 
them. For example, if you start in host mode, these bits will not bit 
set, but if you start in otg mode and switch to host mode, these bits 
will still be set.


And I still think that these bits (CFGCHIP2_SESENDEN | 
CFGCHIP2_VBDTCTEN) should just be enabled during driver probe rather 
than here since I don't know of a reason to turn them off.



break;
default:
return -EINVAL;
}

-   regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_OTGMODE_MASK,
- val);
+   regmap_write(d_phy->regmap, CFGCHIP(2), val);

return 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 v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[David Lechner]

On 11/03/2016 10:26 AM, Alexandre Bailon wrote:

The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

Add the usb20_force_mode property to force the PHY to operate
in host or peripheral mode.


Device tree describes the hardware, not the configuration, so this is 
not acceptable.


Besides, this setting should not be fixed to one value anyway.


When usb20_force_mode is used, dr_mode should also be configured
to host or peripheral.
The controller uses dr_mode to configure itself, but the phy use
it to get the mode to use to configure the PHY mode.

Signed-off-by: Alexandre Bailon 
---
 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt 
b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
index c26478b..9fc87fb 100644
--- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
+++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
@@ -4,6 +4,11 @@ Required properties:
  - compatible: must be "ti,da830-usb-phy".
  - #phy-cells: must be 1.

+Optional properties:
+- usb20-force-mode: Force the phy to operate in same mode than the USB OTG 
controller.
+   It should only be defined if the hardware is not capable 
correctly
+   detect the role of USB by using VBUS and ID pin.
+
 This device controls the PHY for both the USB 1.1 OHCI and USB 2.0 OTG
 controllers on DA8xx SoCs. Consumers of this device should use index 0 for
 the USB 2.0 phy device and index 1 for the USB 1.1 phy device.



--
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/3] phy: da8xx-usb: rename the ohci device to ohci-da8xx

[Kishon Vijay Abraham I]

On Wednesday 02 November 2016 06:14 PM, Axel Haslam wrote:
> There is only one ohci on the da8xx series of chips,
> so remove the ".0" when creating the phy. Also add
> the "-da8xx" postfix to be consistent across davinci
> usb drivers.
> 
> Signed-off-by: Axel Haslam 

Acked-by: Kishon Vijay Abraham I 
> ---
>  drivers/phy/phy-da8xx-usb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
> index 32ae78c..c85fb0b 100644
> --- a/drivers/phy/phy-da8xx-usb.c
> +++ b/drivers/phy/phy-da8xx-usb.c
> @@ -198,7 +198,8 @@ static int da8xx_usb_phy_probe(struct platform_device 
> *pdev)
>   } else {
>   int ret;
>  
> - ret = phy_create_lookup(d_phy->usb11_phy, "usb-phy", "ohci.0");
> + ret = phy_create_lookup(d_phy->usb11_phy, "usb-phy",
> + "ohci-da8xx");
>   if (ret)
>   dev_warn(dev, "Failed to create usb11 phy lookup\n");
>   ret = phy_create_lookup(d_phy->usb20_phy, "usb-phy",
> @@ -216,7 +217,7 @@ static int da8xx_usb_phy_remove(struct platform_device 
> *pdev)
>  
>   if (!pdev->dev.of_node) {
>   phy_remove_lookup(d_phy->usb20_phy, "usb-phy", "musb-da8xx");
> - phy_remove_lookup(d_phy->usb11_phy, "usb-phy", "ohci.0");
> + phy_remove_lookup(d_phy->usb11_phy, "usb-phy", "ohci-da8xx");
>   }
>  
>   return 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 v2 0/3] davinci: ohci: fix usb ohci device name

[David Lechner]

On 11/03/2016 11:03 AM, Axel Haslam wrote:

The usb ohci clock match is not working because the usb clock
is registered as "ohci" instead of "ohci.0"

But since there is only a single ohci instance, lets pass -1 to
the platform data id parameter and avoid the extra ".0" matching.

while we are fixing this, rename the driver from "ohci" to
"ohci-da8xx" which  is less generic and consistent with other
usb drivers.

changes form v1 -> v2
*Reword commit messages (David Lechner)

Because of the recently accepted patches on the ARM-davinci side,
This patch series is based on:
branch: /v4.10/soc of the linux-davinci tree.

It Depends on two accepted usb patches missing on that branch:
6c21caa USB: OHCI: make ohci-da8xx a separate driver (in next-usb)
6110c42 usb: ohci-da8xx: Remove code that references mach (in linux-next)

A branch with both patches applied + this series can be found here:
https://github.com/axelhaslamx/linux-axel/commits/ti-davinci-ohci-rename


Axel Haslam (3):
  ARM: davinci: da8xx: Fix ohci device name
  phy: da8xx-usb: rename the ohci device to ohci-da8xx
  usb: ohci-da8xx: rename driver to ohci-da8xx

 arch/arm/mach-davinci/da830.c | 2 +-
 arch/arm/mach-davinci/da850.c | 2 +-
 arch/arm/mach-davinci/da8xx-dt.c  | 2 +-
 arch/arm/mach-davinci/usb-da8xx.c | 4 ++--
 drivers/phy/phy-da8xx-usb.c   | 5 +++--
 drivers/usb/host/ohci-da8xx.c | 2 +-
 6 files changed, 9 insertions(+), 8 deletions(-)



Thanks for making the changes. The commit messages make better sense to 
me now. :-)

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

Re: [PATCH v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[Kishon Vijay Abraham I]

Hi,

On Thursday 03 November 2016 08:56 PM, Alexandre Bailon wrote:
> The USB PHY is able to operate in OTG, host or peripheral.
> Some board may be wired to work act only as host or peripheral.
> In such case, the dr_mode property of controller must be set to
> host or peripheral. But doing that will also configure the PHY
> in host or peripheral mode whereas OTG is able to detect which
> role the USB controller should take.
> The PHY's host or peripheral mode are actually only useful when
> hardware doesn't allow OTG to detect it's role.
> 
> Add the usb20_force_mode property to force the PHY to operate
> in host or peripheral mode.

I think we do just that if we populate dr_mode with host or peripheral. Why do
we need another property to control dr_mode property?
> When usb20_force_mode is used, dr_mode should also be configured
> to host or peripheral.
> The controller uses dr_mode to configure itself, but the phy use
> it to get the mode to use to configure the PHY mode.
> 
> Signed-off-by: Alexandre Bailon 
> ---
>  Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt 
> b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> index c26478b..9fc87fb 100644
> --- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
> @@ -4,6 +4,11 @@ Required properties:
>   - compatible: must be "ti,da830-usb-phy".
>   - #phy-cells: must be 1.
>  
> +Optional properties:
> +- usb20-force-mode: Force the phy to operate in same mode than the USB OTG 
> controller.
> + It should only be defined if the hardware is not capable 
> correctly
> + detect the role of USB by using VBUS and ID pin.

>From what I understand from the previous patch, if VBUS sense and the session
end comparator is enabled, the controller can work in host mode or device mode.

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

[PATCH v2 3/3] usb: ohci-da8xx: rename driver to ohci-da8xx

[Axel Haslam]

The davinci ohci driver name (currently "ohci") is too generic.
To be consistent with other usb dirvers, append the "-da8xx" postfix
to the name.

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 30c4878..429d58b 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -27,7 +27,7 @@
 #include "ohci.h"
 
 #define DRIVER_DESC "DA8XX"
-#define DRV_NAME "ohci"
+#define DRV_NAME "ohci-da8xx"
 
 static struct hc_driver __read_mostly ohci_da8xx_hc_driver;
 
-- 
2.10.1

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

[PATCH v2 2/3] phy: da8xx-usb: rename the ohci device to ohci-da8xx

[Axel Haslam]

The ohci device name has changed in the board configuraion files,
hence, change the phy lookup table to match the new name.

Signed-off-by: Axel Haslam 
---
 drivers/phy/phy-da8xx-usb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index 32ae78c..c85fb0b 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -198,7 +198,8 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
} else {
int ret;
 
-   ret = phy_create_lookup(d_phy->usb11_phy, "usb-phy", "ohci.0");
+   ret = phy_create_lookup(d_phy->usb11_phy, "usb-phy",
+   "ohci-da8xx");
if (ret)
dev_warn(dev, "Failed to create usb11 phy lookup\n");
ret = phy_create_lookup(d_phy->usb20_phy, "usb-phy",
@@ -216,7 +217,7 @@ static int da8xx_usb_phy_remove(struct platform_device 
*pdev)
 
if (!pdev->dev.of_node) {
phy_remove_lookup(d_phy->usb20_phy, "usb-phy", "musb-da8xx");
-   phy_remove_lookup(d_phy->usb11_phy, "usb-phy", "ohci.0");
+   phy_remove_lookup(d_phy->usb11_phy, "usb-phy", "ohci-da8xx");
}
 
return 0;
-- 
2.10.1

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

[PATCH v2 1/3] ARM: davinci: da8xx: Fix ohci device name

[Axel Haslam]

While the clk lookup table is making reference to "ohci"
other subsystems (such as phy) are trying to match "ohci.0"

Since there is a single ohci instance, instead of changing
the clk name, change the dev id to -1, and add the "-da8xx"
postfix to match the driver name that will also be changed
in a subsequent patch.

Signed-off-by: Axel Haslam 
---
 arch/arm/mach-davinci/da830.c | 2 +-
 arch/arm/mach-davinci/da850.c | 2 +-
 arch/arm/mach-davinci/da8xx-dt.c  | 2 +-
 arch/arm/mach-davinci/usb-da8xx.c | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 41459bd..073c458 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -420,7 +420,7 @@ static struct clk_lookup da830_clks[] = {
CLK("davinci_mdio.0",   "fck",  _clk),
CLK(NULL,   "gpio", _clk),
CLK("i2c_davinci.2",NULL,   _clk),
-   CLK("ohci", "usb11",_clk),
+   CLK("ohci-da8xx",   "usb11",_clk),
CLK(NULL,   "emif3",_clk),
CLK(NULL,   "arm",  _clk),
CLK(NULL,   "rmii", _clk),
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 196e262..3961556 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -503,7 +503,7 @@ static struct clk_lookup da850_clks[] = {
CLK("da830-mmc.1",  NULL,   _clk),
CLK("ti-aemif", NULL,   _clk),
CLK(NULL,   "aemif",_clk),
-   CLK("ohci", "usb11",_clk),
+   CLK("ohci-da8xx",   "usb11",_clk),
CLK("musb-da8xx",   "usb20",_clk),
CLK("spi_davinci.0",NULL,   _clk),
CLK("spi_davinci.1",NULL,   _clk),
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 92ae093..2afb067 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -39,7 +39,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[] 
__initdata = {
OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d0, "davinci-mcasp.0", 
NULL),
OF_DEV_AUXDATA("ti,da850-aemif", 0x6800, "ti-aemif", NULL),
OF_DEV_AUXDATA("ti,da850-tilcdc", 0x01e13000, "da8xx_lcdc.0", NULL),
-   OF_DEV_AUXDATA("ti,da830-ohci", 0x01e25000, "ohci", NULL),
+   OF_DEV_AUXDATA("ti,da830-ohci", 0x01e25000, "ohci-da8xx", NULL),
OF_DEV_AUXDATA("ti,da830-musb", 0x01e0, "musb-da8xx", NULL),
OF_DEV_AUXDATA("ti,da830-usb-phy", 0x01c1417c, "da8xx-usb-phy", NULL),
{}
diff --git a/arch/arm/mach-davinci/usb-da8xx.c 
b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..c6feecf 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -109,8 +109,8 @@ static struct resource da8xx_usb11_resources[] = {
 static u64 da8xx_usb11_dma_mask = DMA_BIT_MASK(32);
 
 static struct platform_device da8xx_usb11_device = {
-   .name   = "ohci",
-   .id = 0,
+   .name   = "ohci-da8xx",
+   .id = -1,
.dev = {
.dma_mask   = _usb11_dma_mask,
.coherent_dma_mask  = DMA_BIT_MASK(32),
-- 
2.10.1

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

[PATCH v2 0/3] davinci: ohci: fix usb ohci device name

[Axel Haslam]

The usb ohci clock match is not working because the usb clock
is registered as "ohci" instead of "ohci.0"

But since there is only a single ohci instance, lets pass -1 to
the platform data id parameter and avoid the extra ".0" matching.

while we are fixing this, rename the driver from "ohci" to
"ohci-da8xx" which  is less generic and consistent with other
usb drivers.

changes form v1 -> v2
*Reword commit messages (David Lechner)

Because of the recently accepted patches on the ARM-davinci side,
This patch series is based on:
branch: /v4.10/soc of the linux-davinci tree.

It Depends on two accepted usb patches missing on that branch:
6c21caa USB: OHCI: make ohci-da8xx a separate driver (in next-usb)
6110c42 usb: ohci-da8xx: Remove code that references mach (in linux-next)

A branch with both patches applied + this series can be found here:
https://github.com/axelhaslamx/linux-axel/commits/ti-davinci-ohci-rename


Axel Haslam (3):
  ARM: davinci: da8xx: Fix ohci device name
  phy: da8xx-usb: rename the ohci device to ohci-da8xx
  usb: ohci-da8xx: rename driver to ohci-da8xx

 arch/arm/mach-davinci/da830.c | 2 +-
 arch/arm/mach-davinci/da850.c | 2 +-
 arch/arm/mach-davinci/da8xx-dt.c  | 2 +-
 arch/arm/mach-davinci/usb-da8xx.c | 4 ++--
 drivers/phy/phy-da8xx-usb.c   | 5 +++--
 drivers/usb/host/ohci-da8xx.c | 2 +-
 6 files changed, 9 insertions(+), 8 deletions(-)

-- 
2.10.1

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

[PATCH v4 3/4] usb: musb: da8xx: Add DT support for the DA8xx driver

[Alexandre Bailon]

From: Petr Kulhavy 

This adds DT support for TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver

Signed-off-by: Petr Kulhavy 
Signed-off-by: Alexandre Bailon 
Tested-by: David Lechner 
---
 drivers/usb/musb/da8xx.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 210b7e4..51ae3b6 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -6,6 +6,9 @@
  * Based on the DaVinci "glue layer" code.
  * Copyright (C) 2005-2006 by Texas Instruments
  *
+ * DT support
+ * Copyright (c) 2016 Petr Kulhavy 
+ *
  * This file is part of the Inventra Controller Driver for Linux.
  *
  * The Inventra Controller Driver for Linux is free software; you
@@ -433,6 +436,21 @@ static int da8xx_musb_exit(struct musb *musb)
return 0;
 }
 
+static inline u8 get_vbus_power(struct device *dev)
+{
+   struct regulator *vbus_supply;
+   int current_uA;
+
+   vbus_supply = regulator_get_optional(dev, "vbus");
+   if (IS_ERR(vbus_supply))
+   return 255;
+   current_uA = regulator_get_current_limit(vbus_supply);
+   regulator_put(vbus_supply);
+   if (current_uA <= 0 || current_uA > 51)
+   return 255;
+   return current_uA / 1000 / 2;
+}
+
 static const struct musb_platform_ops da8xx_ops = {
.quirks = MUSB_DMA_CPPI | MUSB_INDEXED_EP,
.init   = da8xx_musb_init,
@@ -458,6 +476,12 @@ static const struct platform_device_info da8xx_dev_info = {
.dma_mask   = DMA_BIT_MASK(32),
 };
 
+static const struct musb_hdrc_config da8xx_config = {
+   .ram_bits = 10,
+   .num_eps = 5,
+   .multipoint = 1,
+};
+
 static int da8xx_probe(struct platform_device *pdev)
 {
struct resource musb_resources[2];
@@ -465,6 +489,7 @@ static int da8xx_probe(struct platform_device *pdev)
struct da8xx_glue   *glue;
struct platform_device_info pinfo;
struct clk  *clk;
+   struct device_node  *np = pdev->dev.of_node;
int ret;
 
glue = devm_kzalloc(>dev, sizeof(*glue), GFP_KERNEL);
@@ -486,6 +511,16 @@ static int da8xx_probe(struct platform_device *pdev)
glue->dev   = >dev;
glue->clk   = clk;
 
+   if (IS_ENABLED(CONFIG_OF) && np) {
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return -ENOMEM;
+
+   pdata->config   = _config;
+   pdata->mode = musb_get_mode(>dev);
+   pdata->power= get_vbus_power(>dev);
+   }
+
pdata->platform_ops = _ops;
 
glue->usb_phy = usb_phy_generic_register();
@@ -536,11 +571,22 @@ static int da8xx_remove(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_id_table[] = {
+   {
+   .compatible = "ti,da830-musb",
+   },
+   {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_id_table);
+#endif
+
 static struct platform_driver da8xx_driver = {
.probe  = da8xx_probe,
.remove = da8xx_remove,
.driver = {
.name   = "musb-da8xx",
+   .of_match_table = of_match_ptr(da8xx_id_table),
},
 };
 
-- 
2.7.3

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

[PATCH v4 1/4] dt/bindings: Add binding for the DA8xx MUSB driver

[Alexandre Bailon]

From: Petr Kulhavy 

DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.

Signed-off-by: Petr Kulhavy 
Signed-off-by: Alexandre Bailon 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/usb/da8xx-usb.txt  | 43 ++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt 
b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
new file mode 100644
index 000..ccb844a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -0,0 +1,43 @@
+TI DA8xx MUSB
+~
+For DA8xx/OMAP-L1x/AM17xx/AM18xx platforms.
+
+Required properties:
+
+ - compatible : Should be set to "ti,da830-musb".
+
+ - reg: Offset and length of the USB controller register set.
+
+ - interrupts: The USB interrupt number.
+
+ - interrupt-names: Should be set to "mc".
+
+ - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or 
"otg".
+
+ - phys: Phandle for the PHY device
+
+ - phy-names: Should be "usb-phy"
+
+Optional properties:
+
+ - vbus-supply: Phandle to a regulator providing the USB bus power.
+
+Example:
+   usb_phy: usb-phy {
+   compatible = "ti,da830-usb-phy";
+   #phy-cells = <0>;
+   status = "okay";
+   };
+   usb0: usb@20 {
+   compatible = "ti,da830-musb";
+   reg =   <0x0020 0x1>;
+   interrupts = <58>;
+   interrupt-names = "mc";
+
+   dr_mode = "host";
+   vbus-supply = <_vbus>;
+   phys = <_phy 0>;
+   phy-names = "usb-phy";
+
+   status = "okay";
+   };
-- 
2.7.3

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

[PATCH v4 2/4] usb: musb: core: added helper function for parsing DT

[Alexandre Bailon]

From: Petr Kulhavy 

This adds the function musb_get_mode() to get the DT property "dr_mode"

Signed-off-by: Petr Kulhavy 
Acked-by: Sergei Shtylyov 
Signed-off-by: Alexandre Bailon 
Tested-by: David Lechner 
Reviewed-by: Kevin Hilman 
---
 drivers/usb/musb/musb_core.c | 19 +++
 drivers/usb/musb/musb_core.h |  6 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 27dadc0..bba07e7 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -100,6 +100,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "musb_core.h"
 #include "musb_trace.h"
@@ -130,6 +131,24 @@ static inline struct musb *dev_to_musb(struct device *dev)
return dev_get_drvdata(dev);
 }
 
+enum musb_mode musb_get_mode(struct device *dev)
+{
+   enum usb_dr_mode mode;
+
+   mode = usb_get_dr_mode(dev);
+   switch (mode) {
+   case USB_DR_MODE_HOST:
+   return MUSB_HOST;
+   case USB_DR_MODE_PERIPHERAL:
+   return MUSB_PERIPHERAL;
+   case USB_DR_MODE_OTG:
+   case USB_DR_MODE_UNKNOWN:
+   default:
+   return MUSB_OTG;
+   }
+}
+EXPORT_SYMBOL_GPL(musb_get_mode);
+
 /*-*/
 
 #ifndef CONFIG_BLACKFIN
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 2cb88a49..76f00f6 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -617,4 +617,10 @@ static inline void 
musb_platform_post_root_reset_end(struct musb *musb)
musb->ops->post_root_reset_end(musb);
 }
 
+/*
+ * gets the "dr_mode" property from DT and converts it into musb_mode
+ * if the property is not found or not recognized returns MUSB_OTG
+ */
+extern enum musb_mode musb_get_mode(struct device *dev);
+
 #endif /* __MUSB_CORE_H__ */
-- 
2.7.3

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

[PATCH v4 4/4] ARM: dts: da850: Add the usb otg device node

[Alexandre Bailon]

This adds the device tree node for the usb otg
controller present in the da850 family of SoC's.
This also enables the otg usb controller for the lcdk board.

Signed-off-by: Alexandre Bailon 
---
 arch/arm/boot/dts/da850-lcdk.dts |  8 
 arch/arm/boot/dts/da850.dtsi | 15 +++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..9f5040c 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -158,6 +158,14 @@
rx-num-evt = <32>;
 };
 
+_phy {
+   status = "okay";
+   };
+
+ {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index f79e1b9..322a31a 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -372,6 +372,21 @@
>;
status = "disabled";
};
+   usb_phy: usb-phy {
+   compatible = "ti,da830-usb-phy";
+   #phy-cells = <1>;
+   status = "disabled";
+   };
+   usb0: usb@20 {
+   compatible = "ti,da830-musb";
+   reg = <0x20 0x1>;
+   interrupts = <58>;
+   interrupt-names = "mc";
+   dr_mode = "otg";
+   phys = <_phy 0>;
+   phy-names = "usb-phy";
+   status = "disabled";
+   };
gpio: gpio@226000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
-- 
2.7.3

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

[PATCH v4 0/4] Add DT support for DA8xx

[Alexandre Bailon]

Changes in v2:
* Remove unrelated changes in patch 3
* Rename the device node in patch 4

Changes in v3:
* Fix few mistakes in DT binding sample
* Only build the device table if DT is enabled

Change in v4:
* Fix a nit

Alexandre Bailon (1):
  ARM: dts: da850: Add the usb otg device node

Petr Kulhavy (3):
  dt/bindings: Add binding for the DA8xx MUSB driver
  usb: musb: core: added helper function for parsing DT
  usb: musb: da8xx: Add DT support for the DA8xx driver

 .../devicetree/bindings/usb/da8xx-usb.txt  | 43 
 arch/arm/boot/dts/da850-lcdk.dts   |  8 
 arch/arm/boot/dts/da850.dtsi   | 15 +++
 drivers/usb/musb/da8xx.c   | 46 ++
 drivers/usb/musb/musb_core.c   | 19 +
 drivers/usb/musb/musb_core.h   |  6 +++
 6 files changed, 137 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt

-- 
2.7.3

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

Re: [PATCH] usb: musb: da8xx: Don't print phy error on -EPROBE_DEFER

[Bin Liu]

On Wed, Nov 02, 2016 at 10:45:59PM +0100, Ladislav Michl wrote:
> Hi,
> 
> On Tue, Oct 25, 2016 at 02:02:50PM -0500, David Lechner wrote:
> > This suppresses printing the error message "failed to get phy" in the
> > kernel log when the error is -EPROBE_DEFER. This prevents usless noise
> > in the kernel log.
> > 
> > Signed-off-by: David Lechner 
> > ---
> >  drivers/usb/musb/da8xx.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> > index 481d786..f8a1591 100644
> > --- a/drivers/usb/musb/da8xx.c
> > +++ b/drivers/usb/musb/da8xx.c
> > @@ -516,7 +516,8 @@ static int da8xx_probe(struct platform_device *pdev)
> >  
> > glue->phy = devm_phy_get(>dev, "usb-phy");
> > if (IS_ERR(glue->phy)) {
> > -   dev_err(>dev, "failed to get phy\n");
> > +   if (PTR_ERR(glue->phy) != -EPROBE_DEFER)
> > +   dev_err(>dev, "failed to get phy\n");
> 
> What about something like this?
> 
> dev_printk(PTR_ERR(glue->phy) == -EPROBE_DEFER ? KERN_DEBUG : KERN_ERR, ...
> 
> At least it outputs something if debug is enabled, making debugging easier.

It is unnecessary, when probe failed, kernel prints a probe failure log
with error -517, and da8xx_probe() has only this one place returning
-EPROBE_DEFER.

And this change makes the code a little hard to read.

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

Re: [BUG] cdc-acm: no data available after port open

[Ladislav Michl]

Hi,

On Thu, Nov 03, 2016 at 02:12:58PM +0100, Oliver Neukum wrote:
> On Thu, 2016-11-03 at 13:44 +0100, Ladislav Michl wrote:
> > On Thu, Nov 03, 2016 at 01:03:56PM +0100, Ladislav Michl wrote:
> 
> Hi,
> 
> > > now I'm really confused. Just looked at drivers/usb/serial/generic.c
> > > which just stops submitting urbs on -EPIPE. On contrary
> > > drivers/usb/serial/ti_usb_3410_5052.c does just the opposite.
> > > I'm assuming generic function does it right, but what do you
> > > mean by "clearing a halt"?
> > 
> > This really feels like opening Pandora's box... So usb generic is submitting
> > read urbs on all error except -EPIPE (and -ENOENT, -ECONNRESET, -ESHUTDOWN
> > of course). cypress_m8 stops submitting on all errors and adds comment
> > "Can't call usb_clear_halt while in_interrupt" to -EPIPE case.
> 
> True. That driver is so old that the error was hard to handle.
> 
> > Edgeport and TI 3410/5052 drivers has special case for -EPIPE where they
> > continue submitting read urbs. There's pretty clear pattern showing which
> > driver is derived from other, but all of them simply cannot be right.
> 
> They are for different hardware.

Yes, they are, but those which do continue submiting urbs after -EPIPE
do not bother with usb_clear_halt.

> > Also usb_clear_halt is called only from open in all drivers which do care
> > about clearing a halt at all.
> 
> No. For example look at usbnet.

I see. I was looking only for serial drivers.

> > Any pointers how should be such a error condition solved properly?
> > (I didn't find any documentation and existing drivers do just the opposite)
> 
> I think the way usbnet handles -EPIPE is the best. We are a bit on thin
> ice because the CDC spec does not list under which conditions we should
> see a stall, thus by implication: never.
> But in general you cannot ignore a stall. You need to clear the halt.

So, you'd like to see normal urb processing and fire completion callback
which does usb_clear_halt on -EPIPE. Is that right?

> We cannot do full error handling for modems because they would drop
> the connection if we reset the device.

ladis
--
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 v3 2/5] phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround

[Alexandre Bailon]

If we configure the da8xx OTG phy in OTG mode, neither device or host
mode will work. That is because the PHY is not able to detect and notify
the driver that value of ID pin changed.
To work despite this hardware limitation, the da8xx glue implement a
workaround.
But to work, the workaround require the VBUS sense and the session end
comparator to enabled.
Enable them if the phy is configured in OTG mode.

Signed-off-by: Alexandre Bailon 
---
 drivers/phy/phy-da8xx-usb.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index 32ae78c..fd39292 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -93,24 +93,31 @@ static int da8xx_usb20_phy_power_off(struct phy *phy)
 static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)
 {
struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);
+   int ret;
u32 val;
 
+   ret = regmap_read(d_phy->regmap, CFGCHIP(2), );
+   if (ret)
+   return ret;
+
+   val &= ~CFGCHIP2_OTGMODE_MASK;
+
switch (mode) {
case PHY_MODE_USB_HOST: /* Force VBUS valid, ID = 0 */
-   val = CFGCHIP2_OTGMODE_FORCE_HOST;
+   val |= CFGCHIP2_OTGMODE_FORCE_HOST;
break;
case PHY_MODE_USB_DEVICE:   /* Force VBUS valid, ID = 1 */
-   val = CFGCHIP2_OTGMODE_FORCE_DEVICE;
+   val |= CFGCHIP2_OTGMODE_FORCE_DEVICE;
break;
case PHY_MODE_USB_OTG:  /* Don't override the VBUS/ID comparators */
-   val = CFGCHIP2_OTGMODE_NO_OVERRIDE;
+   val |= CFGCHIP2_OTGMODE_NO_OVERRIDE |
+   CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;
break;
default:
return -EINVAL;
}
 
-   regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_OTGMODE_MASK,
- val);
+   regmap_write(d_phy->regmap, CFGCHIP(2), val);
 
return 0;
 }
-- 
2.7.3

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

[PATCH v3 4/5] phy: da8xx-usb: Use usb20_force_mode property to configure the phy mode

[Alexandre Bailon]

The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

There is some issues when the PHY is forced in peripheral or host mode,
and it worth to let the PHY in OTG when that is possible.
Only force the PHY in a specific mode if usb20_force_mode is set in DT.

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

diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index fd39292..bf1b3af 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct da8xx_usb_phy {
struct phy_provider *phy_provider;
@@ -109,6 +110,7 @@ static int da8xx_usb20_phy_set_mode(struct phy *phy, enum 
phy_mode mode)
case PHY_MODE_USB_DEVICE:   /* Force VBUS valid, ID = 1 */
val |= CFGCHIP2_OTGMODE_FORCE_DEVICE;
break;
+   case PHY_MODE_INVALID:
case PHY_MODE_USB_OTG:  /* Don't override the VBUS/ID comparators */
val |= CFGCHIP2_OTGMODE_NO_OVERRIDE |
CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;
@@ -152,6 +154,7 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
struct device   *dev = >dev;
struct device_node  *node = dev->of_node;
struct da8xx_usb_phy*d_phy;
+   enum usb_dr_modedr_mode = PHY_MODE_INVALID;
 
d_phy = devm_kzalloc(dev, sizeof(*d_phy), GFP_KERNEL);
if (!d_phy)
@@ -202,6 +205,13 @@ static int da8xx_usb_phy_probe(struct platform_device 
*pdev)
dev_err(dev, "Failed to create phy provider\n");
return PTR_ERR(d_phy->phy_provider);
}
+
+   if (of_find_property(node, "usb20-force-mode", NULL)) {
+   dr_mode = of_usb_get_dr_mode_by_phy(node, 0);
+   if (dr_mode == USB_DR_MODE_UNKNOWN ||
+   dr_mode == USB_DR_MODE_OTG)
+   dev_warn(dev, "dr_mode is not properly set\n");
+   }
} else {
int ret;
 
@@ -213,6 +223,7 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
if (ret)
dev_warn(dev, "Failed to create usb20 phy lookup\n");
}
+   da8xx_usb20_phy_set_mode(d_phy->usb20_phy, dr_mode);
 
return 0;
 }
-- 
2.7.3

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

[PATCH v3 5/5] usb: musb: da8xx: Remove set_mode callback

[Alexandre Bailon]

The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

The set_mode callback is used by the musb driver to set mode
of the PHY. But in the case of DA8xx, the PHY have some issues.
The OTG mode work correctly but the host and peripheral don't.
In host mode, the PHY stops to work after the first disconnect.
In device mode, the PHY doesn't detect any disconnect.
As the OTG mode is working properly, let the PHY in OTG mode,
whatever is the controller mode.

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

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 6749aa1..581f830 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -335,28 +335,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
return ret;
 }
 
-static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
-{
-   struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
-   enum phy_mode phy_mode;
-
-   switch (musb_mode) {
-   case MUSB_HOST: /* Force VBUS valid, ID = 0 */
-   phy_mode = PHY_MODE_USB_HOST;
-   break;
-   case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
-   phy_mode = PHY_MODE_USB_DEVICE;
-   break;
-   case MUSB_OTG:  /* Don't override the VBUS/ID comparators */
-   phy_mode = PHY_MODE_USB_OTG;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   return phy_set_mode(glue->phy, phy_mode);
-}
-
 static int da8xx_musb_init(struct musb *musb)
 {
struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -445,7 +423,6 @@ static const struct musb_platform_ops da8xx_ops = {
.enable = da8xx_musb_enable,
.disable= da8xx_musb_disable,
 
-   .set_mode   = da8xx_musb_set_mode,
.try_idle   = da8xx_musb_try_idle,
 
.set_vbus   = da8xx_musb_set_vbus,
-- 
2.7.3

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

[PATCH v3 3/5] dt/bindings: Add a new property to DA8xx USB PHY

[Alexandre Bailon]

The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

Add the usb20_force_mode property to force the PHY to operate
in host or peripheral mode.
When usb20_force_mode is used, dr_mode should also be configured
to host or peripheral.
The controller uses dr_mode to configure itself, but the phy use
it to get the mode to use to configure the PHY mode.

Signed-off-by: Alexandre Bailon 
---
 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt 
b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
index c26478b..9fc87fb 100644
--- a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
+++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt
@@ -4,6 +4,11 @@ Required properties:
  - compatible: must be "ti,da830-usb-phy".
  - #phy-cells: must be 1.
 
+Optional properties:
+- usb20-force-mode: Force the phy to operate in same mode than the USB OTG 
controller.
+   It should only be defined if the hardware is not capable 
correctly
+   detect the role of USB by using VBUS and ID pin.
+
 This device controls the PHY for both the USB 1.1 OHCI and USB 2.0 OTG
 controllers on DA8xx SoCs. Consumers of this device should use index 0 for
 the USB 2.0 phy device and index 1 for the USB 1.1 phy device.
-- 
2.7.3

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

[PATCH v3 1/5] usb: musb: da8xx: Call earlier clk_prepare_enable()

[Alexandre Bailon]

The first attempt to read a register may fail because the clock may not
be enabled, and then the probe of musb driver will fail.
Call clk_prepare_enable() before the first register read.

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

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 210b7e4..6749aa1 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -366,6 +366,12 @@ static int da8xx_musb_init(struct musb *musb)
 
musb->mregs += DA8XX_MENTOR_CORE_OFFSET;
 
+   ret = clk_prepare_enable(glue->clk);
+   if (ret) {
+   dev_err(glue->dev, "failed to enable clock\n");
+   return ret;
+   }
+
/* Returns zero if e.g. not clocked */
rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG);
if (!rev)
@@ -377,12 +383,6 @@ static int da8xx_musb_init(struct musb *musb)
goto fail;
}
 
-   ret = clk_prepare_enable(glue->clk);
-   if (ret) {
-   dev_err(glue->dev, "failed to enable clock\n");
-   goto fail;
-   }
-
setup_timer(_workaround, otg_timer, (unsigned long)musb);
 
/* Reset the controller */
@@ -392,7 +392,7 @@ static int da8xx_musb_init(struct musb *musb)
ret = phy_init(glue->phy);
if (ret) {
dev_err(glue->dev, "Failed to init phy.\n");
-   goto err_phy_init;
+   goto fail;
}
 
ret = phy_power_on(glue->phy);
@@ -412,9 +412,8 @@ static int da8xx_musb_init(struct musb *musb)
 
 err_phy_power_on:
phy_exit(glue->phy);
-err_phy_init:
-   clk_disable_unprepare(glue->clk);
 fail:
+   clk_disable_unprepare(glue->clk);
return ret;
 }
 
-- 
2.7.3

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

[PATCH v3 0/5] usb: musb: da8xx: Fix few issues

[Alexandre Bailon]

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

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

Changes in v3:
* Remove the host workaround that was not working on every platform
* Add a property to the devicetree node of phy to force the phy in a specific
  mode (host or device).
* Only use dr_mode to configure the controller mode and let the phy in OTG mode.
  The main goal of this change is to prevent the phy to be set in host or
  device mode because these modes have some issues.

Alexandre Bailon (5):
  usb: musb: da8xx: Call earlier clk_prepare_enable()
  phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
  dt/bindings: Add a new property to DA8xx USB PHY
  phy: da8xx-usb: Use usb20_force_mode property to configure the phy
mode
  usb: musb: da8xx: Remove set_mode callback

 .../devicetree/bindings/phy/phy-da8xx-usb.txt  |  5 +++
 drivers/phy/phy-da8xx-usb.c| 28 ---
 drivers/usb/musb/da8xx.c   | 40 +-
 3 files changed, 36 insertions(+), 37 deletions(-)

-- 
2.7.3

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

Re: [PATCH 0/3] fix ohci phy name

[Axel Haslam]

On Thu, Nov 3, 2016 at 1:00 PM, Sekhar Nori  wrote:
> On Thursday 03 November 2016 01:54 PM, Axel Haslam wrote:
>> Hi Sekhar, David,
>>
>> It might make sense to have this patch series,
>> squashed into a single patch, would you agree,
>> or do you prefer it as is: one-per-subsystem?
>
> Patches in the current form are okay. Some coordination is required in
> getting them merged though. I am happy to take the driver patches
> through ARM-SoC with ack from respective maintainers.
>
> I will need to carry the platform patch through my tree because it
> conflicts with other changes I have already queued.
>
> That said, I am unable to review 3/3 since I am unable to find its baseline.
>

ok, ill send v2 fixing Davids comments on the commit messages
and referencing the missing patch that is queued on usb-next.

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

Re: [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error

[Felipe Balbi]

Hi,

Alexey Khoroshilov  writes:
> mv_u3d_req_to_trb() does not check for dma mapping errors.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> v2: split fix and clenup to separate patches.

I'll fix this time when applying, but keep in mind we don't want these
notes in the commit log. They should come after the tearline (---)
below, together with the diffstat ;-)

> Signed-off-by: Alexey Khoroshilov 
> ---
>  drivers/usb/gadget/udc/mv_u3d_core.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c 
> b/drivers/usb/gadget/udc/mv_u3d_core.c
> index b9e19a591322..6f3be0ba9ac8 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
>   req->trb_head->trb_hw,
>   trb_num * sizeof(*trb_hw),
>   DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(u3d->gadget.dev.parent,
> + req->trb_head->trb_dma)) {
> + kfree(req->trb_head->trb_hw);
> + kfree(req->trb_head);
> + return -EFAULT;
> + }
>  
>   req->chain = 1;
>   }
> -- 
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi


signature.asc
Description: PGP signature

[PATCH 2/2] usb: dwc3: trace: purge dwc3_trace()

[Felipe Balbi]

Finally get rid of dwc3_trace() hack. If any other
message is truly needed, we should add proper
tracepoints for them instead of hacking around with
dwc3_trace() or similar.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/Makefile |  2 +-
 drivers/usb/dwc3/core.c   | 15 +++--
 drivers/usb/dwc3/debug.c  | 32 --
 drivers/usb/dwc3/debug.h  |  7 --
 drivers/usb/dwc3/ep0.c| 46 +-
 drivers/usb/dwc3/gadget.c | 57 +--
 6 files changed, 24 insertions(+), 135 deletions(-)
 delete mode 100644 drivers/usb/dwc3/debug.c

diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 84de1e4151c4..ffca34029b21 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_USB_DWC3)  += dwc3.o
 dwc3-y := core.o
 
 ifneq ($(CONFIG_FTRACE),)
-   dwc3-y  += debug.o trace.o
+   dwc3-y  += trace.o
 endif
 
 ifneq ($(filter y,$(CONFIG_USB_DWC3_HOST) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5e61ef6a378d..037052db50ef 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -305,13 +305,7 @@ static int dwc3_event_buffers_setup(struct dwc3 *dwc)
struct dwc3_event_buffer*evt;
 
evt = dwc->ev_buf;
-   dwc3_trace(trace_dwc3_core,
-   "Event buf %p dma %08llx length %d\n",
-   evt->buf, (unsigned long long) evt->dma,
-   evt->length);
-
evt->lpos = 0;
-
dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(0),
lower_32_bits(evt->dma));
dwc3_writel(dwc->regs, DWC3_GEVNTADRHI(0),
@@ -428,9 +422,6 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
 
dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps;
-
-   dwc3_trace(trace_dwc3_core, "found %d IN and %d OUT endpoints",
-   dwc->num_in_eps, dwc->num_out_eps);
 }
 
 static void dwc3_cache_hwparams(struct dwc3 *dwc)
@@ -656,13 +647,13 @@ static void dwc3_core_setup_global_control(struct dwc3 
*dwc)
reg |= DWC3_GCTL_GBLHIBERNATIONEN;
break;
default:
-   dwc3_trace(trace_dwc3_core, "No power optimization 
available\n");
+   /* nothing */
+   break;
}
 
/* check if current dwc3 is on simulation board */
if (dwc->hwparams.hwparams6 & DWC3_GHWPARAMS6_EN_FPGA) {
-   dwc3_trace(trace_dwc3_core,
-   "running on FPGA platform\n");
+   dev_info(dwc->dev, "Running with FPGA optmizations\n");
dwc->is_fpga = true;
}
 
diff --git a/drivers/usb/dwc3/debug.c b/drivers/usb/dwc3/debug.c
deleted file mode 100644
index 0be6885bc370..
--- a/drivers/usb/dwc3/debug.c
+++ /dev/null
@@ -1,32 +0,0 @@
-/**
- * debug.c - DesignWare USB3 DRD Controller Debug/Trace Support
- *
- * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com
- *
- * Author: Felipe Balbi 
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2  of
- * the License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include "debug.h"
-
-void dwc3_trace(void (*trace)(struct va_format *), const char *fmt, ...)
-{
-   struct va_format vaf;
-   va_list args;
-
-   va_start(args, fmt);
-   vaf.fmt = fmt;
-   vaf.va = 
-
-   trace();
-
-   va_end(args);
-}
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index d93780e84f07..eeed4ffd8131 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -345,13 +345,6 @@ static inline const char 
*dwc3_gadget_generic_cmd_status_string(int status)
 }
 
 
-#if IS_ENABLED(CONFIG_FTRACE)
-void dwc3_trace(void (*trace)(struct va_format *), const char *fmt, ...);
-#else
-static inline void dwc3_trace(void (*trace)(struct va_format *), const char 
*fmt, ...)
-{  }
-#endif
-
 #ifdef CONFIG_DEBUG_FS
 extern void dwc3_debugfs_init(struct dwc3 *);
 extern void dwc3_debugfs_exit(struct dwc3 *);
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 15b62a5aaff8..2b22ea7263d8 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -49,10 +49,8 @@ static int dwc3_ep0_start_trans(struct dwc3 *dwc, u8 epnum, 
dma_addr_t buf_dma,
int ret;
 
dep = dwc->eps[epnum];
-   if 

[PATCH 1/2] usb: dwc3: trace: add a tracepoint for ep enable/disable

[Felipe Balbi]

instead of using a simple trace_printk() wrapper,
let's add an actual tracepoint and print further
details about the endpoint being operated upon.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/gadget.c | 10 ++
 drivers/usb/dwc3/trace.h  | 51 +++
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a9c1d75ba7d5..7e39f0cf4436 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -587,8 +587,6 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
u32 reg;
int ret;
 
-   dwc3_trace(trace_dwc3_gadget, "Enabling %s", dep->name);
-
if (!(dep->flags & DWC3_EP_ENABLED)) {
ret = dwc3_gadget_start_config(dwc, dep);
if (ret)
@@ -617,7 +615,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
init_waitqueue_head(>wait_end_transfer);
 
if (usb_endpoint_xfer_control(desc))
-   return 0;
+   goto out;
 
/* Initialize the TRB ring */
dep->trb_dequeue = 0;
@@ -664,6 +662,10 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
WARN_ON_ONCE(!dep->resource_index);
}
 
+
+out:
+   trace_dwc3_gadget_ep_enable(dep);
+
return 0;
 }
 
@@ -701,7 +703,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
struct dwc3 *dwc = dep->dwc;
u32 reg;
 
-   dwc3_trace(trace_dwc3_gadget, "Disabling %s", dep->name);
+   trace_dwc3_gadget_ep_disable(dep);
 
dwc3_remove_requests(dwc, dep);
 
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index b2153f231cf0..2b124f94d858 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -341,6 +341,57 @@ DEFINE_EVENT(dwc3_log_trb, dwc3_complete_trb,
TP_ARGS(dep, trb)
 );
 
+DECLARE_EVENT_CLASS(dwc3_log_ep,
+   TP_PROTO(struct dwc3_ep *dep),
+   TP_ARGS(dep),
+   TP_STRUCT__entry(
+   __dynamic_array(char, name, DWC3_MSG_MAX)
+   __field(unsigned, maxpacket)
+   __field(unsigned, maxpacket_limit)
+   __field(unsigned, max_streams)
+   __field(unsigned, maxburst)
+   __field(unsigned, flags)
+   __field(unsigned, direction)
+   __field(u8, trb_enqueue)
+   __field(u8, trb_dequeue)
+   ),
+   TP_fast_assign(
+   snprintf(__get_str(name), DWC3_MSG_MAX, "%s", dep->name);
+   __entry->maxpacket = dep->endpoint.maxpacket;
+   __entry->maxpacket_limit = dep->endpoint.maxpacket_limit;
+   __entry->max_streams = dep->endpoint.max_streams;
+   __entry->maxburst = dep->endpoint.maxburst;
+   __entry->flags = dep->flags;
+   __entry->direction = dep->direction;
+   __entry->trb_enqueue = dep->trb_enqueue;
+   __entry->trb_dequeue = dep->trb_dequeue;
+   ),
+   TP_printk("%s: mps %d/%d streams %d burst %d ring %d/%d flags 
%c:%c%c%c%c%c:%c:%c",
+   __get_str(name), __entry->maxpacket,
+   __entry->maxpacket_limit, __entry->max_streams,
+   __entry->maxburst, __entry->trb_enqueue,
+   __entry->trb_dequeue,
+   __entry->flags & DWC3_EP_ENABLED ? 'E' : 'e',
+   __entry->flags & DWC3_EP_STALL ? 'S' : 's',
+   __entry->flags & DWC3_EP_WEDGE ? 'W' : 'w',
+   __entry->flags & DWC3_EP_BUSY ? 'B' : 'b',
+   __entry->flags & DWC3_EP_PENDING_REQUEST ? 'P' : 'p',
+   __entry->flags & DWC3_EP_MISSED_ISOC ? 'M' : 'm',
+   __entry->flags & DWC3_EP_END_TRANSFER_PENDING ? 'E' : 'e',
+   __entry->direction ? '<' : '>'
+   )
+);
+
+DEFINE_EVENT(dwc3_log_ep, dwc3_gadget_ep_enable,
+   TP_PROTO(struct dwc3_ep *dep),
+   TP_ARGS(dep)
+);
+
+DEFINE_EVENT(dwc3_log_ep, dwc3_gadget_ep_disable,
+   TP_PROTO(struct dwc3_ep *dep),
+   TP_ARGS(dep)
+);
+
 #endif /* __DWC3_TRACE_H */
 
 /* this part has to be here */
-- 
2.10.1

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

Re: [BUG] cdc-acm: no data available after port open

[Oliver Neukum]

On Thu, 2016-11-03 at 13:44 +0100, Ladislav Michl wrote:
> On Thu, Nov 03, 2016 at 01:03:56PM +0100, Ladislav Michl wrote:

Hi,

> > now I'm really confused. Just looked at drivers/usb/serial/generic.c
> > which just stops submitting urbs on -EPIPE. On contrary
> > drivers/usb/serial/ti_usb_3410_5052.c does just the opposite.
> > I'm assuming generic function does it right, but what do you
> > mean by "clearing a halt"?
> 
> This really feels like opening Pandora's box... So usb generic is submitting
> read urbs on all error except -EPIPE (and -ENOENT, -ECONNRESET, -ESHUTDOWN
> of course). cypress_m8 stops submitting on all errors and adds comment
> "Can't call usb_clear_halt while in_interrupt" to -EPIPE case.

True. That driver is so old that the error was hard to handle.

> Edgeport and TI 3410/5052 drivers has special case for -EPIPE where they
> continue submitting read urbs. There's pretty clear pattern showing which
> driver is derived from other, but all of them simply cannot be right.

They are for different hardware.

> Also usb_clear_halt is called only from open in all drivers which do care
> about clearing a halt at all.

No. For example look at usbnet.

> Any pointers how should be such a error condition solved properly?
> (I didn't find any documentation and existing drivers do just the opposite)

I think the way usbnet handles -EPIPE is the best. We are a bit on thin
ice because the CDC spec does not list under which conditions we should
see a stall, thus by implication: never.
But in general you cannot ignore a stall. You need to clear the halt.

We cannot do full error handling for modems because they would drop
the connection if we reset the device.

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 v2 2/2] usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring

[Alexey Khoroshilov]

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

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

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

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

[PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error

[Alexey Khoroshilov]

mv_u3d_req_to_trb() does not check for dma mapping errors.

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

v2: split fix and clenup to separate patches.
Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/mv_u3d_core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c 
b/drivers/usb/gadget/udc/mv_u3d_core.c
index b9e19a591322..6f3be0ba9ac8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
req->trb_head->trb_hw,
trb_num * sizeof(*trb_hw),
DMA_BIDIRECTIONAL);
+   if (dma_mapping_error(u3d->gadget.dev.parent,
+   req->trb_head->trb_dma)) {
+   kfree(req->trb_head->trb_hw);
+   kfree(req->trb_head);
+   return -EFAULT;
+   }
 
req->chain = 1;
}
-- 
2.7.4

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

Re: [BUG] cdc-acm: no data available after port open

[Ladislav Michl]

On Thu, Nov 03, 2016 at 01:03:56PM +0100, Ladislav Michl wrote:
> On Thu, Nov 03, 2016 at 12:44:20PM +0100, Oliver Neukum wrote:
> > On Thu, 2016-11-03 at 11:34 +0100, Ladislav Michl wrote:
> > > Hi Oliver,
> > 
> > > Device operates normally after reconnect. So it seems you were right,
> > > there's no handling for EPIPE and EPROTO errors reported to 
> > > acm_read_bulk_callback.
> > > Following patch fixes it for me, if it looks okay for you, I'll resend it
> > > properly.
> > 
> > Hi,
> > 
> > I am afraid we cannot just ignore or report -EPIPE.
> > We need to clear a halt. Maybe your device would work
> > if there were a delay in communication before the halt
> > is cleared. I am not sure your patch is the right thing
> > to do in all cases.
> > 
> > Regards
> > Oliver
> 
> Hi Oliver,
> 
> now I'm really confused. Just looked at drivers/usb/serial/generic.c
> which just stops submitting urbs on -EPIPE. On contrary
> drivers/usb/serial/ti_usb_3410_5052.c does just the opposite.
> I'm assuming generic function does it right, but what do you
> mean by "clearing a halt"?

This really feels like opening Pandora's box... So usb generic is submitting
read urbs on all error except -EPIPE (and -ENOENT, -ECONNRESET, -ESHUTDOWN
of course). cypress_m8 stops submitting on all errors and adds comment
"Can't call usb_clear_halt while in_interrupt" to -EPIPE case.
Edgeport and TI 3410/5052 drivers has special case for -EPIPE where they
continue submitting read urbs. There's pretty clear pattern showing which
driver is derived from other, but all of them simply cannot be right.
Also usb_clear_halt is called only from open in all drivers which do care
about clearing a halt at all.

Any pointers how should be such a error condition solved properly?
(I didn't find any documentation and existing drivers do just the opposite)

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

Re: [PATCH 3/3] usb: ohci-da8xx: rename driver to ohci-da8xx

[Axel Haslam]

Hi Sekhar,

The baseline used was the branch usb-next, in Greg's tree:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git

Linux next is missing this patch[1] which was applied last week,
but not yet pulled into linux-next. it will be there soon.

Sorry, i did not mention it, i thought it would be already
on linux-next.

[1]
6c21caa USB: OHCI: make ohci-da8xx a separate driver
https://lkml.org/lkml/2016/10/27/120

Regards
Axel



On Thu, Nov 3, 2016 at 12:56 PM, Sekhar Nori  wrote:
> On Wednesday 02 November 2016 06:14 PM, Axel Haslam wrote:
>> To be consistent on the usb driver for the davinci
>> platform follow the example of musb, and add the
>> "-da8xx" postfix to the driver name.
>>
>> Signed-off-by: Axel Haslam 
>> ---
>>  drivers/usb/host/ohci-da8xx.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
>> index bd6cf3c..b3de8bc 100644
>> --- a/drivers/usb/host/ohci-da8xx.c
>> +++ b/drivers/usb/host/ohci-da8xx.c
>> @@ -27,7 +27,7 @@
>>  #include "ohci.h"
>>
>>  #define DRIVER_DESC "DA8XX"
>> -#define DRV_NAME "ohci"
>> +#define DRV_NAME "ohci-da8xx"
>
> To which baseline does this patch apply? I don't see this code in
> linux-next.
>
> Thanks,
> Sekhar
--
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: [BUG] cdc-acm: no data available after port open

[Ladislav Michl]

On Thu, Nov 03, 2016 at 12:44:20PM +0100, Oliver Neukum wrote:
> On Thu, 2016-11-03 at 11:34 +0100, Ladislav Michl wrote:
> > Hi Oliver,
> 
> > Device operates normally after reconnect. So it seems you were right,
> > there's no handling for EPIPE and EPROTO errors reported to 
> > acm_read_bulk_callback.
> > Following patch fixes it for me, if it looks okay for you, I'll resend it
> > properly.
> 
> Hi,
> 
> I am afraid we cannot just ignore or report -EPIPE.
> We need to clear a halt. Maybe your device would work
> if there were a delay in communication before the halt
> is cleared. I am not sure your patch is the right thing
> to do in all cases.
> 
>   Regards
>   Oliver

Hi Oliver,

now I'm really confused. Just looked at drivers/usb/serial/generic.c
which just stops submitting urbs on -EPIPE. On contrary
drivers/usb/serial/ti_usb_3410_5052.c does just the opposite.
I'm assuming generic function does it right, but what do you
mean by "clearing a halt"?

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

Re: [PATCH 0/3] fix ohci phy name

[Sekhar Nori]

On Thursday 03 November 2016 01:54 PM, Axel Haslam wrote:
> Hi Sekhar, David,
> 
> It might make sense to have this patch series,
> squashed into a single patch, would you agree,
> or do you prefer it as is: one-per-subsystem?

Patches in the current form are okay. Some coordination is required in
getting them merged though. I am happy to take the driver patches
through ARM-SoC with ack from respective maintainers.

I will need to carry the platform patch through my tree because it
conflicts with other changes I have already queued.

That said, I am unable to review 3/3 since I am unable to find its baseline.

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

Re: [PATCH 3/3] usb: ohci-da8xx: rename driver to ohci-da8xx

[Sekhar Nori]

On Wednesday 02 November 2016 06:14 PM, Axel Haslam wrote:
> To be consistent on the usb driver for the davinci
> platform follow the example of musb, and add the
> "-da8xx" postfix to the driver name.
> 
> Signed-off-by: Axel Haslam 
> ---
>  drivers/usb/host/ohci-da8xx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
> index bd6cf3c..b3de8bc 100644
> --- a/drivers/usb/host/ohci-da8xx.c
> +++ b/drivers/usb/host/ohci-da8xx.c
> @@ -27,7 +27,7 @@
>  #include "ohci.h"
>  
>  #define DRIVER_DESC "DA8XX"
> -#define DRV_NAME "ohci"
> +#define DRV_NAME "ohci-da8xx"

To which baseline does this patch apply? I don't see this code in
linux-next.

Thanks,
Sekhar
--
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: [BUG] cdc-acm: no data available after port open

[Oliver Neukum]

On Thu, 2016-11-03 at 11:34 +0100, Ladislav Michl wrote:
> Hi Oliver,

> Device operates normally after reconnect. So it seems you were right,
> there's no handling for EPIPE and EPROTO errors reported to 
> acm_read_bulk_callback.
> Following patch fixes it for me, if it looks okay for you, I'll resend it
> properly.

Hi,

I am afraid we cannot just ignore or report -EPIPE.
We need to clear a halt. Maybe your device would work
if there were a delay in communication before the halt
is cleared. I am not sure your patch is the right thing
to do in all cases.

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] usbhid: add ATEN CS962 to list of quirky devices

[Oliver Neukum]

Like many similar devices it needs a quirk to work.
Issuing the request gets the device into an irrecoverable state.

Signed-off-by: Oliver Neukum 
CC: sta...@vger.kernel.org
---
 drivers/hid/hid-ids.h   | 1 +
 drivers/hid/usbhid/hid-quirks.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4ed9a4f..f6d1f34 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -176,6 +176,7 @@
 #define USB_DEVICE_ID_ATEN_4PORTKVM0x2205
 #define USB_DEVICE_ID_ATEN_4PORTKVMC   0x2208
 #define USB_DEVICE_ID_ATEN_CS682   0x2213
+#define USB_DEVICE_ID_ATEN_CS692   0x8021
 
 #define USB_VENDOR_ID_ATMEL0x03eb
 #define USB_DEVICE_ID_ATMEL_MULTITOUCH 0x211c
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index b4b8c6a..85d5ff2 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -62,6 +62,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_4PORTKVM, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_4PORTKVMC, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_CS682, HID_QUIRK_NOGET },
+   { USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_CS692, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_CH, USB_DEVICE_ID_CH_FIGHTERSTICK, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_CH, USB_DEVICE_ID_CH_COMBATSTICK, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_CH, USB_DEVICE_ID_CH_FLIGHT_SIM_ECLIPSE_YOKE, 
HID_QUIRK_NOGET },
-- 
2.6.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 1/1] usb: xhci: cleanup cmd_completion in xhci_virt_device

[Mathias Nyman]

On 03.11.2016 12:22, Sergei Shtylyov wrote:

On 11/3/2016 9:48 AM, Lu Baolu wrote:


cmd_completion in struct xhci_virt_device is legacy. With command
strutcture and command queue introduced in xhci, cmd_completion is


Structure.



I'll fix that while applying the patch, no need to resend

-Mathias

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

Re: [PATCH 1/1] usb: xhci: cleanup cmd_completion in xhci_virt_device

[Mathias Nyman]

On 03.11.2016 08:48, Lu Baolu wrote:

cmd_completion in struct xhci_virt_device is legacy. With command
strutcture and command queue introduced in xhci, cmd_completion is
not used any more. This patch removes it.

Signed-off-by: Lu Baolu 
---
  drivers/usb/host/xhci-mem.c | 1 -
  drivers/usb/host/xhci.h | 1 -
  2 files changed, 2 deletions(-)



Thanks, I'll apply it.

-Mathias

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

Re: [PATCH 1/1] usb: gadget: fix request length error for isoc transfer

[Felipe Balbi]

Hi,

Peter Chen  writes:
> On Thu, Nov 03, 2016 at 11:27:40AM +0200, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Peter Chen  writes:
>> >> Peter Chen  writes:
>> >> > For isoc endpoint descriptor, the wMaxPacketSize is not real max packet
>> >> > size (see Table 9-13. Standard Endpoint Descriptor, USB 2.0 
>> >> > specifcation),
>> >> > it may contain the number of packet, so the real max packet should be
>> >> > ep->desc->wMaxPacketSize && 0x7ff.
>> >> >
>> >> > Cc: Felipe F. Tonello 
>> >> > Cc: Felipe Balbi 
>> >> > Fixes: 16b114a6d797 ("usb: gadget: fix usb_ep_align_maybe
>> >> >   endianness and new usb_ep_aligna")
>> >> >
>> >> > Signed-off-by: Peter Chen 
>> >> > ---
>> >> >  include/linux/usb/gadget.h | 5 -
>> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> >> > index 8e81f9e..cce2db6 100644
>> >> > --- a/include/linux/usb/gadget.h
>> >> > +++ b/include/linux/usb/gadget.h
>> >> > @@ -429,7 +429,10 @@ static inline struct usb_gadget 
>> >> > *dev_to_usb_gadget(struct device *dev)
>> >> >   */
>> >> >  static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
>> >> >  {
>> >> > -   return round_up(len, 
>> >> > (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
>> >> > +   int max_packet_size = 
>> >> > (size_t)le16_to_cpu(ep->desc->wMaxPacketSize)
>> >> > +   && 0x7ff;
>> ^^
>> hehe, I just noticed this bug. Did you really test this patch? :-)
>> 
>
> Any problems you find? Without this patch, the usbtest can't be ran
> for high bandwidth isoc transfer.

wMaxPacketSize && 0x7ff is always 1 unless wMaxPacketSize is zero, which
is invalid anyway.

>> >> > +
>> >> > +   return round_up(len, max_packet_size);
>> >> >  }
>> >> 
>> >> care to update this to use new usb_endpoint_maxp()?
>> >> 
>> >
>> > But it is a bug existed at v4.9, the usb_ep_align returns wrong request
>> > length for isoc. I found this bug by running usbtest.
>> 
>> keep your & (after fixing it), but use usb_endpoint_maxp(). Later we can
>> just remove the unnecessary & operation.
>> 
>> -- 
>> balbi
>
> I will use usb_endpoint_maxp, but I still not find the problem for this
> patch.

Logical vs Bitwise operator

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Felipe Balbi]

Hi,

Peter Chen  writes:
>> > Peter Chen  writes:
>> > > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
>> > >> From: Peter Chen 
>> > >> Date: Wed, 2 Nov 2016 14:02:02 +0800
>> > >> 
>> > >> > Felipe, it may increase cpu utilization since more interrupts will be 
>> > >> > there,
>> > >> > it may affect the SoC which has lower cpu frequency. This code existed
>> > >> > many years, why this problem has only reported at dwc3 recently?
>> > >> 
>> > >> It's a bug, and it's going to cause TCP sockets to potentially hang.
>> > >> 
>> > >
>> > > For some controllers, it is, so we need to add parameter for user
>> > > to see if interrupt migration is supported or not.
>> > 
>> > not for some controller, for ALL networking drivers.
>> > 
>> > > But just like some ethernet controllers, some USB controllers support
>> > > hardware timeout mechanism which interrupt will be triggered after
>> > > some uFrame occurs if the transaction has completed but not required
>> > > to interrupt, it is used to support interrupt migration like ethernet.
>> > 
>> > you're missing the point. What Dave Miller is saying is that it's ALWAYS
>> > a bug to delay completion of SKBs. The only thing you're doing with
>> > chipidea is delaying interrupt by up to 125us; which is still a bug from
>> > the point of view of the networking layer, but it's more difficult to
>> > perceive any problems because of the short time where interrupt is
>> > delayed.
>> > 
>> 
>> If it is ALWAYS a bug to delay completion of SKBs, how the local
>> ethernet driver designs interrupt migration?
>> 
>
> Just a quick test, I delete dev_kfree_skb_any at tx_complete, not find
> any problems by using simple "ping test", just free memory is less and
> less. David, do you really mean free tx skb buffer with limited time,
> but not return NETDEV_TX_OK by ->ndo_start_xmit with limited time?

ping *will* work just fine. One easy test to *see* the problem is to SSH
to a machine using g_ether, then run:

$ while true; do dmesg; done

you will notice it is rather laggy. Now remove throttling and the lags
are gone.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Felipe Balbi]

Hi,

Peter Chen  writes:
>> Peter Chen  writes:
>> > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
>> >> From: Peter Chen 
>> >> Date: Wed, 2 Nov 2016 14:02:02 +0800
>> >> 
>> >> > Felipe, it may increase cpu utilization since more interrupts will be 
>> >> > there,
>> >> > it may affect the SoC which has lower cpu frequency. This code existed
>> >> > many years, why this problem has only reported at dwc3 recently?
>> >> 
>> >> It's a bug, and it's going to cause TCP sockets to potentially hang.
>> >> 
>> >
>> > For some controllers, it is, so we need to add parameter for user
>> > to see if interrupt migration is supported or not.
>> 
>> not for some controller, for ALL networking drivers.
>> 
>> > But just like some ethernet controllers, some USB controllers support
>> > hardware timeout mechanism which interrupt will be triggered after
>> > some uFrame occurs if the transaction has completed but not required
>> > to interrupt, it is used to support interrupt migration like ethernet.
>> 
>> you're missing the point. What Dave Miller is saying is that it's ALWAYS
>> a bug to delay completion of SKBs. The only thing you're doing with
>> chipidea is delaying interrupt by up to 125us; which is still a bug from
>> the point of view of the networking layer, but it's more difficult to
>> perceive any problems because of the short time where interrupt is
>> delayed.
>> 
>
> If it is ALWAYS a bug to delay completion of SKBs, how the local
> ethernet driver designs interrupt migration?

what driver are you talking about? Which "local" ethernet driver?

Also, do you mean 'moderation' instead of 'migration?

-- 
balbi


signature.asc
Description: PGP signature

Re: [BUG] cdc-acm: no data available after port open

[Ladislav Michl]

Hi Oliver,

finally got device which stops responding after while on my table...

On Mon, Jun 13, 2016 at 11:02:19AM +0200, Oliver Neukum wrote:
> On Mon, 2016-06-13 at 00:37 +0200, Ladislav Michl wrote:
> > On Sun, Jun 12, 2016 at 11:03:45PM +0200, Ladislav Michl wrote:
> > > Once ttyACM0 starts behave strangely, read() returns only what's in 
> > > buffer before
> > > ttyACM0 was opened and then hangs infinitely. As this bug is hard to 
> > > trigger, has
> > > anyone clue where to start debugging?
> > 
> > Forgot to mention, once this happens "usb 3-1.4: clear tt 1 (9061) error 
> > -75"
> > starts showing in the syslog. Also "cdc_acm 3-1.4.3:1.0: failed to set 
> > dtr/rts",
> > but this one is there normally.
> 
> The translation transactor in the hub is reporting an error.
> That should be reported to the driver, but there is no good
> error handling.
> 
> static void acm_read_bulk_callback(struct urb *urb)
> {
> struct acm_rb *rb = urb->context;
> struct acm *acm = rb->instance;
> unsigned long flags;
> int status = urb->status;
> 
> dev_vdbg(>data->dev, "%s - urb %d, len %d\n", __func__,
> rb->index, urb->actual_length);
> 
> 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);
> dev_dbg(>data->dev, "%s - non-zero urb status: %d\n",
> __func__, status);
> if ((status != -ENOENT) || (urb->actual_length == 0))
> return;
> }
> 
> Can you please switch on dynamic debugging for cdc_acm to see what
> is being reported?

The core of this problem is hardware related. I do not have a scope here
at the moment, but it seems that supply voltage drops too low once usb
modem tries to connect to network. Here's log what's happens before I'm
unable to read from device:
[   45.765411] cdc_acm 3-1.4.3:1.1: submitted urb 0
[   45.848327] PPP generic driver version 2.4.2
[   45.869354] cdc_acm 3-1.4.3:1.1: got urb 5, len 14, status 0
[   45.869445] cdc_acm 3-1.4.3:1.1: submitted urb 5
[   45.870361] cdc_acm 3-1.4.3:1.1: got urb 6, len 7, status 0
[   45.870452] cdc_acm 3-1.4.3:1.1: submitted urb 6
[   45.871337] cdc_acm 3-1.4.3:1.1: got urb 7, len 16, status 0
[   45.871459] cdc_acm 3-1.4.3:1.1: submitted urb 7
[   45.872985] cdc_acm 3-1.4.3:1.1: got urb 8, len 8, status 0
[   45.873077] cdc_acm 3-1.4.3:1.1: submitted urb 8
[   45.873352] cdc_acm 3-1.4.3:1.1: got urb 9, len 17, status 0
[   45.873443] cdc_acm 3-1.4.3:1.1: submitted urb 9
[   45.874359] cdc_acm 3-1.4.3:1.1: got urb 10, len 9, status 0
[   45.874481] cdc_acm 3-1.4.3:1.1: submitted urb 10
[   45.875366] cdc_acm 3-1.4.3:1.1: got urb 11, len 8, status 0
[   45.875457] cdc_acm 3-1.4.3:1.1: submitted urb 11
[   45.876373] cdc_acm 3-1.4.3:1.1: got urb 12, len 10, status 0
[   45.876464] cdc_acm 3-1.4.3:1.1: submitted urb 12
[   45.877349] cdc_acm 3-1.4.3:1.1: got urb 13, len 15, status 0
[   45.877471] cdc_acm 3-1.4.3:1.1: submitted urb 13
[   45.878356] cdc_acm 3-1.4.3:1.1: got urb 14, len 2, status 0
[   45.878479] cdc_acm 3-1.4.3:1.1: submitted urb 14
[snip...]
[   46.344360] cdc_acm 3-1.4.3:1.1: got urb 15, len 14, status 0
[   46.344482] cdc_acm 3-1.4.3:1.1: submitted urb 15
[   46.464355] cdc_acm 3-1.4.3:1.1: got urb 1, len 7, status 0
[   46.464447] cdc_acm 3-1.4.3:1.1: submitted urb 1
[   46.465332] cdc_acm 3-1.4.3:1.1: got urb 2, len 2, status 0
[   46.465423] cdc_acm 3-1.4.3:1.1: submitted urb 2
[   46.628265] PPP BSD Compression module registered
[   46.754638] PPP Deflate Compression module registered
[   46.829711] cdc_acm 3-1.4.3:1.1: got urb 3, len 0, status -32
[   46.994323] cdc_acm 3-1.4.3:1.1: got urb 4, len 0, status -32
[   47.003601] cdc_acm 3-1.4.3:1.1: got urb 0, len 0, status -32
[   47.008178] cdc_acm 3-1.4.3:1.1: got urb 5, len 0, status -32
[   47.012695] cdc_acm 3-1.4.3:1.1: got urb 6, len 0, status -32
[   47.035827] cdc_acm 3-1.4.3:1.1: got urb 7, len 0, status -32
[   47.045074] cdc_acm 3-1.4.3:1.1: got urb 8, len 0, status -32
[   47.049804] cdc_acm 3-1.4.3:1.1: got urb 9, len 0, status -32
[   47.054321] cdc_acm 3-1.4.3:1.1: got urb 10, len 0, status -32
[   47.079650] cdc_acm 3-1.4.3:1.1: got urb 11, len 0, status -71
[   47.086547] cdc_acm 3-1.4.3:1.1: got urb 12, len 0, status -32
[   47.095794] cdc_acm 3-1.4.3:1.1: got urb 13, len 0, status -32
[   47.105163] cdc_acm 3-1.4.3:1.1: got urb 14, len 0, status -32
[   47.111541] cdc_acm 3-1.4.3:1.1: got urb 15, len 8, status 0
[   47.111663] cdc_acm 3-1.4.3:1.1: submitted urb 15
[   47.112335] cdc_acm 3-1.4.3:1.1: got urb 1, len 37, status 0
[   47.112426] cdc_acm 3-1.4.3:1.1: submitted urb 1
[   47.113311] cdc_acm 3-1.4.3:1.1: got urb 2, len 8, status 0
[   

Re: [PATCH 1/1] usb: xhci: cleanup cmd_completion in xhci_virt_device

[Sergei Shtylyov]

On 11/3/2016 9:48 AM, Lu Baolu wrote:


cmd_completion in struct xhci_virt_device is legacy. With command
strutcture and command queue introduced in xhci, cmd_completion is


   Structure.


not used any more. This patch removes it.

Signed-off-by: Lu Baolu 

[...]

MBR, Sergei

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

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Peter Chen]

On Thu, Nov 03, 2016 at 05:03:10PM +0800, Peter Chen wrote:
> On Thu, Nov 03, 2016 at 09:04:54AM +0200, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Peter Chen  writes:
> > > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
> > >> From: Peter Chen 
> > >> Date: Wed, 2 Nov 2016 14:02:02 +0800
> > >> 
> > >> > Felipe, it may increase cpu utilization since more interrupts will be 
> > >> > there,
> > >> > it may affect the SoC which has lower cpu frequency. This code existed
> > >> > many years, why this problem has only reported at dwc3 recently?
> > >> 
> > >> It's a bug, and it's going to cause TCP sockets to potentially hang.
> > >> 
> > >
> > > For some controllers, it is, so we need to add parameter for user
> > > to see if interrupt migration is supported or not.
> > 
> > not for some controller, for ALL networking drivers.
> > 
> > > But just like some ethernet controllers, some USB controllers support
> > > hardware timeout mechanism which interrupt will be triggered after
> > > some uFrame occurs if the transaction has completed but not required
> > > to interrupt, it is used to support interrupt migration like ethernet.
> > 
> > you're missing the point. What Dave Miller is saying is that it's ALWAYS
> > a bug to delay completion of SKBs. The only thing you're doing with
> > chipidea is delaying interrupt by up to 125us; which is still a bug from
> > the point of view of the networking layer, but it's more difficult to
> > perceive any problems because of the short time where interrupt is
> > delayed.
> > 
> 
> If it is ALWAYS a bug to delay completion of SKBs, how the local
> ethernet driver designs interrupt migration?
> 

Just a quick test, I delete dev_kfree_skb_any at tx_complete, not find
any problems by using simple "ping test", just free memory is less and
less. David, do you really mean free tx skb buffer with limited time,
but not return NETDEV_TX_OK by ->ndo_start_xmit with limited time?

-- 

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

Re: [PATCH 1/1] usb: gadget: fix request length error for isoc transfer

[Peter Chen]

On Thu, Nov 03, 2016 at 11:27:40AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> >> Peter Chen  writes:
> >> > For isoc endpoint descriptor, the wMaxPacketSize is not real max packet
> >> > size (see Table 9-13. Standard Endpoint Descriptor, USB 2.0 
> >> > specifcation),
> >> > it may contain the number of packet, so the real max packet should be
> >> > ep->desc->wMaxPacketSize && 0x7ff.
> >> >
> >> > Cc: Felipe F. Tonello 
> >> > Cc: Felipe Balbi 
> >> > Fixes: 16b114a6d797 ("usb: gadget: fix usb_ep_align_maybe
> >> >   endianness and new usb_ep_aligna")
> >> >
> >> > Signed-off-by: Peter Chen 
> >> > ---
> >> >  include/linux/usb/gadget.h | 5 -
> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> >> > index 8e81f9e..cce2db6 100644
> >> > --- a/include/linux/usb/gadget.h
> >> > +++ b/include/linux/usb/gadget.h
> >> > @@ -429,7 +429,10 @@ static inline struct usb_gadget 
> >> > *dev_to_usb_gadget(struct device *dev)
> >> >   */
> >> >  static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
> >> >  {
> >> > -return round_up(len, 
> >> > (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
> >> > +int max_packet_size = 
> >> > (size_t)le16_to_cpu(ep->desc->wMaxPacketSize)
> >> > +&& 0x7ff;
> ^^
> hehe, I just noticed this bug. Did you really test this patch? :-)
> 

Any problems you find? Without this patch, the usbtest can't be ran
for high bandwidth isoc transfer.

> >> > +
> >> > +return round_up(len, max_packet_size);
> >> >  }
> >> 
> >> care to update this to use new usb_endpoint_maxp()?
> >> 
> >
> > But it is a bug existed at v4.9, the usb_ep_align returns wrong request
> > length for isoc. I found this bug by running usbtest.
> 
> keep your & (after fixing it), but use usb_endpoint_maxp(). Later we can
> just remove the unnecessary & operation.
> 
> -- 
> balbi

I will use usb_endpoint_maxp, but I still not find the problem for this
patch.

-- 

Best Regards,
Peter Chen
--
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: gadget: f_hid add super speed support

[Janusz Dziedzic]

Add super speed descriptors to f_hid.

Signed-off-by: Janusz Dziedzic 
---
 drivers/usb/gadget/function/f_hid.c | 67 -
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index e2966f8..7abd70b 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -98,6 +98,60 @@ static inline struct f_hidg *func_to_hidg(struct 
usb_function *f)
/*.desc[0].wDescriptorLenght= DYNAMIC */
 };
 
+/* Super-Speed Support */
+
+static struct usb_endpoint_descriptor hidg_ss_in_ep_desc = {
+   .bLength= USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType= USB_DT_ENDPOINT,
+   .bEndpointAddress   = USB_DIR_IN,
+   .bmAttributes   = USB_ENDPOINT_XFER_INT,
+   /*.wMaxPacketSize   = DYNAMIC */
+   .bInterval  = 4, /* FIXME: Add this field in the
+ * HID gadget configuration?
+ * (struct hidg_func_descriptor)
+ */
+};
+
+static struct usb_ss_ep_comp_descriptor hidg_ss_in_comp_desc = {
+   .bLength= sizeof(hidg_ss_in_comp_desc),
+   .bDescriptorType= USB_DT_SS_ENDPOINT_COMP,
+
+   /* .bMaxBurst   = 0, */
+   /* .bmAttributes= 0, */
+   /* .wBytesPerInterval   = DYNAMIC */
+};
+
+static struct usb_endpoint_descriptor hidg_ss_out_ep_desc = {
+   .bLength= USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType= USB_DT_ENDPOINT,
+   .bEndpointAddress   = USB_DIR_OUT,
+   .bmAttributes   = USB_ENDPOINT_XFER_INT,
+   /*.wMaxPacketSize   = DYNAMIC */
+   .bInterval  = 4, /* FIXME: Add this field in the
+ * HID gadget configuration?
+ * (struct hidg_func_descriptor)
+ */
+};
+
+static struct usb_ss_ep_comp_descriptor hidg_ss_out_comp_desc = {
+   .bLength= sizeof(hidg_ss_out_comp_desc),
+   .bDescriptorType= USB_DT_SS_ENDPOINT_COMP,
+
+   /* .bMaxBurst   = 0, */
+   /* .bmAttributes= 0, */
+   /* .wBytesPerInterval   = DYNAMIC */
+};
+
+static struct usb_descriptor_header *hidg_ss_descriptors[] = {
+   (struct usb_descriptor_header *)_interface_desc,
+   (struct usb_descriptor_header *)_desc,
+   (struct usb_descriptor_header *)_ss_in_ep_desc,
+   (struct usb_descriptor_header *)_ss_in_comp_desc,
+   (struct usb_descriptor_header *)_ss_out_ep_desc,
+   (struct usb_descriptor_header *)_ss_out_comp_desc,
+   NULL,
+};
+
 /* High-Speed Support */
 
 static struct usb_endpoint_descriptor hidg_hs_in_ep_desc = {
@@ -624,8 +678,14 @@ static int hidg_bind(struct usb_configuration *c, struct 
usb_function *f)
/* set descriptor dynamic values */
hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;
hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
+   hidg_ss_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
+   hidg_ss_in_comp_desc.wBytesPerInterval =
+   cpu_to_le16(hidg->report_length);
hidg_hs_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
hidg_fs_in_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
+   hidg_ss_out_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
+   hidg_ss_out_comp_desc.wBytesPerInterval =
+   cpu_to_le16(hidg->report_length);
hidg_hs_out_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
hidg_fs_out_ep_desc.wMaxPacketSize = cpu_to_le16(hidg->report_length);
/*
@@ -641,8 +701,13 @@ static int hidg_bind(struct usb_configuration *c, struct 
usb_function *f)
hidg_hs_out_ep_desc.bEndpointAddress =
hidg_fs_out_ep_desc.bEndpointAddress;
 
+   hidg_ss_in_ep_desc.bEndpointAddress =
+   hidg_fs_in_ep_desc.bEndpointAddress;
+   hidg_ss_out_ep_desc.bEndpointAddress =
+   hidg_fs_out_ep_desc.bEndpointAddress;
+
status = usb_assign_descriptors(f, hidg_fs_descriptors,
-   hidg_hs_descriptors, NULL, NULL);
+   hidg_hs_descriptors, hidg_ss_descriptors, NULL);
if (status)
goto fail;
 
-- 
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 1/1] usb: gadget: fix request length error for isoc transfer

[Felipe Balbi]

Hi,

Peter Chen  writes:
>> Peter Chen  writes:
>> > For isoc endpoint descriptor, the wMaxPacketSize is not real max packet
>> > size (see Table 9-13. Standard Endpoint Descriptor, USB 2.0 specifcation),
>> > it may contain the number of packet, so the real max packet should be
>> > ep->desc->wMaxPacketSize && 0x7ff.
>> >
>> > Cc: Felipe F. Tonello 
>> > Cc: Felipe Balbi 
>> > Fixes: 16b114a6d797 ("usb: gadget: fix usb_ep_align_maybe
>> >   endianness and new usb_ep_aligna")
>> >
>> > Signed-off-by: Peter Chen 
>> > ---
>> >  include/linux/usb/gadget.h | 5 -
>> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> > index 8e81f9e..cce2db6 100644
>> > --- a/include/linux/usb/gadget.h
>> > +++ b/include/linux/usb/gadget.h
>> > @@ -429,7 +429,10 @@ static inline struct usb_gadget 
>> > *dev_to_usb_gadget(struct device *dev)
>> >   */
>> >  static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
>> >  {
>> > -  return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
>> > +  int max_packet_size = (size_t)le16_to_cpu(ep->desc->wMaxPacketSize)
>> > +  && 0x7ff;
^^
hehe, I just noticed this bug. Did you really test this patch? :-)

>> > +
>> > +  return round_up(len, max_packet_size);
>> >  }
>> 
>> care to update this to use new usb_endpoint_maxp()?
>> 
>
> But it is a bug existed at v4.9, the usb_ep_align returns wrong request
> length for isoc. I found this bug by running usbtest.

keep your & (after fixing it), but use usb_endpoint_maxp(). Later we can
just remove the unnecessary & operation.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/1] usb: gadget: f_uac2: fix error handling at afunc_bind

[Peter Chen]

On Thu, Nov 03, 2016 at 10:50:23AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > The current error handling flow uses incorrect goto label, fix it
> >
> > Cc: 
> > Fixes: d12a8727171c ("usb: gadget: function: Remove
> > redundant usb_free_all_descriptors")
> > Signed-off-by: Peter Chen 
> 
> seems like there's more than one fix here.
> 
> > ---
> >  drivers/usb/gadget/function/f_uac2.c | 20 +++-
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uac2.c 
> > b/drivers/usb/gadget/function/f_uac2.c
> > index cd214ec8..3f4e478 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -1067,13 +1067,13 @@ afunc_bind(struct usb_configuration *cfg, struct 
> > usb_function *fn)
> > agdev->out_ep = usb_ep_autoconfig(gadget, _epout_desc);
> > if (!agdev->out_ep) {
> > dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > -   goto err;
> > +   return ret;
> > }
> >  
> > agdev->in_ep = usb_ep_autoconfig(gadget, _epin_desc);
> > if (!agdev->in_ep) {
> > dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > -   goto err;
> > +   return ret;
> > }
> >  
> > uac2->p_prm.uac2 = uac2;
> > @@ -1091,13 +1091,14 @@ afunc_bind(struct usb_configuration *cfg, struct 
> > usb_function *fn)
> > ret = usb_assign_descriptors(fn, fs_audio_desc, hs_audio_desc, NULL,
> >  NULL);
> > if (ret)
> > -   goto err;
> > +   return ret;
> 
> this is one fix
> 
> >  
> > prm = >uac2.c_prm;
> > prm->max_psize = hs_epout_desc.wMaxPacketSize;
> > prm->rbuf = kzalloc(prm->max_psize * USB_XFERS, GFP_KERNEL);
> > if (!prm->rbuf) {
> > prm->max_psize = 0;
> > +   ret = -ENOMEM;
> 
> but initializing ret to -ENOMEM is a separate fix altogether. 
> 

Ok, I will have a bug-fix just fixing incorrect label, and have 
another patch to change return value for -next.

-- 

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

Re: [PATCH 1/1] usb: gadget: fix request length error for isoc transfer

[Peter Chen]

On Thu, Nov 03, 2016 at 10:47:58AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > For isoc endpoint descriptor, the wMaxPacketSize is not real max packet
> > size (see Table 9-13. Standard Endpoint Descriptor, USB 2.0 specifcation),
> > it may contain the number of packet, so the real max packet should be
> > ep->desc->wMaxPacketSize && 0x7ff.
> >
> > Cc: Felipe F. Tonello 
> > Cc: Felipe Balbi 
> > Fixes: 16b114a6d797 ("usb: gadget: fix usb_ep_align_maybe
> >   endianness and new usb_ep_aligna")
> >
> > Signed-off-by: Peter Chen 
> > ---
> >  include/linux/usb/gadget.h | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> > index 8e81f9e..cce2db6 100644
> > --- a/include/linux/usb/gadget.h
> > +++ b/include/linux/usb/gadget.h
> > @@ -429,7 +429,10 @@ static inline struct usb_gadget 
> > *dev_to_usb_gadget(struct device *dev)
> >   */
> >  static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
> >  {
> > -   return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
> > +   int max_packet_size = (size_t)le16_to_cpu(ep->desc->wMaxPacketSize)
> > +   && 0x7ff;
> > +
> > +   return round_up(len, max_packet_size);
> >  }
> 
> care to update this to use new usb_endpoint_maxp()?
> 

But it is a bug existed at v4.9, the usb_ep_align returns wrong request
length for isoc. I found this bug by running usbtest.

-- 

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

Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling

[Peter Chen]

On Thu, Nov 03, 2016 at 09:04:54AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > On Wed, Nov 02, 2016 at 11:22:54AM -0400, David Miller wrote:
> >> From: Peter Chen 
> >> Date: Wed, 2 Nov 2016 14:02:02 +0800
> >> 
> >> > Felipe, it may increase cpu utilization since more interrupts will be 
> >> > there,
> >> > it may affect the SoC which has lower cpu frequency. This code existed
> >> > many years, why this problem has only reported at dwc3 recently?
> >> 
> >> It's a bug, and it's going to cause TCP sockets to potentially hang.
> >> 
> >
> > For some controllers, it is, so we need to add parameter for user
> > to see if interrupt migration is supported or not.
> 
> not for some controller, for ALL networking drivers.
> 
> > But just like some ethernet controllers, some USB controllers support
> > hardware timeout mechanism which interrupt will be triggered after
> > some uFrame occurs if the transaction has completed but not required
> > to interrupt, it is used to support interrupt migration like ethernet.
> 
> you're missing the point. What Dave Miller is saying is that it's ALWAYS
> a bug to delay completion of SKBs. The only thing you're doing with
> chipidea is delaying interrupt by up to 125us; which is still a bug from
> the point of view of the networking layer, but it's more difficult to
> perceive any problems because of the short time where interrupt is
> delayed.
> 

If it is ALWAYS a bug to delay completion of SKBs, how the local
ethernet driver designs interrupt migration?

-- 

Best Regards,
Peter Chen
--
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: phy: am335x-control: fix device and of_node leaks

[Felipe Balbi]

Hi,

Johan Hovold  writes:
> Make sure to drop the references taken by of_parse_phandle() and
> bus_find_device() before returning from am335x_get_phy_control().
>
> Note that there is no guarantee that the devres-managed struct
> phy_control will be valid for the lifetime of the sibling phy device
> regardless of this change.
>
> Fixes: 3bb869c8b3f1 ("usb: phy: Add AM335x PHY driver")
> Signed-off-by: Johan Hovold 
> ---
>  drivers/usb/phy/phy-am335x-control.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-am335x-control.c 
> b/drivers/usb/phy/phy-am335x-control.c
> index 42a1afe36a90..5f5f19813fde 100644
> --- a/drivers/usb/phy/phy-am335x-control.c
> +++ b/drivers/usb/phy/phy-am335x-control.c
> @@ -134,10 +134,12 @@ struct phy_control *am335x_get_phy_control(struct 
> device *dev)
>   return NULL;
>  
>   dev = bus_find_device(_bus_type, NULL, node, match);
> + of_node_put(node);
>   if (!dev)
>   return NULL;
>  
>   ctrl_usb = dev_get_drvdata(dev);
> + put_device(dev);
>   if (!ctrl_usb)
>   return NULL;
>   return _usb->phy_ctrl;

Bin, Roger, does this look okay to you?


-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/1] usb: phy: phy-generic: add the implementation of .set_suspend

[Felipe Balbi]

Hi,

Peter Chen  writes:
> Add clock operation at .set_suspend if the PHY has
> suspend requirement, it can be benefit of power saving for
> phy and the whole system (parent clock may also be disabled).
>
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/phy/phy-generic.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index 8311ba2..89d6e7a 100644
> --- a/drivers/usb/phy/phy-generic.c
> +++ b/drivers/usb/phy/phy-generic.c
> @@ -59,6 +59,15 @@ EXPORT_SYMBOL_GPL(usb_phy_generic_unregister);
>  
>  static int nop_set_suspend(struct usb_phy *x, int suspend)
>  {
> + struct usb_phy_generic *nop = dev_get_drvdata(x->dev);
> +
> + if (!IS_ERR(nop->clk)) {
> + if (suspend)
> + clk_disable_unprepare(nop->clk);
> + else
> + clk_prepare_enable(nop->clk);
> + }
> +

Bin, Roger, can you make sure this causes no regressions for AM335x
devices?

-- 
balbi


signature.asc
Description: PGP signature

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

[Felipe Balbi]

Hi,

Alexey Khoroshilov  writes:
> mv_u3d_req_to_trb() does not check for dma mapping errors.
>
> By the way, the patch improves readability of mv_u3d_start_queue()
> by rearranging its code with two semantic modifications:
> - assignment zero to ep->processing if usb_gadget_map_request() fails;
> - propagation of error code from mv_u3d_req_to_trb() instead of 
>   hardcoded -ENOMEM.

cleanups and fixes should be done separately.

> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov 
> ---
>  drivers/usb/gadget/udc/mv_u3d_core.c | 34 +-
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c 
> b/drivers/usb/gadget/udc/mv_u3d_core.c
> index b9e19a591322..8d726bd767fd 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
>   req->trb_head->trb_hw,
>   trb_num * sizeof(*trb_hw),
>   DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(u3d->gadget.dev.parent,
> + req->trb_head->trb_dma)) {
> + kfree(req->trb_head->trb_hw);
> + kfree(req->trb_head);
> + return -EFAULT;
> + }
>  
>   req->chain = 1;
>   }

this is one patch: add dma_mapping_error() check

AKA $subject :-p

> @@ -487,30 +493,32 @@ mv_u3d_start_queue(struct mv_u3d_ep *ep)
>   ret = usb_gadget_map_request(>gadget, >req,
>   mv_u3d_ep_dir(ep));
>   if (ret)
> - return ret;
> + goto break_processing;
>  
>   req->req.status = -EINPROGRESS;
>   req->req.actual = 0;
>   req->trb_count = 0;
>  
> - /* build trbs and push them to device queue */
> - if (!mv_u3d_req_to_trb(req)) {
> - ret = mv_u3d_queue_trb(ep, req);
> - if (ret) {
> - ep->processing = 0;
> - return ret;
> - }
> - } else {
> - ep->processing = 0;
> + /* build trbs */
> + ret = mv_u3d_req_to_trb(req);
> + if (ret) {
>   dev_err(u3d->dev, "%s, mv_u3d_req_to_trb fail\n", __func__);
> - return -ENOMEM;
> + goto break_processing;
>   }
>  
> + /* and push them to device queue */
> + ret = mv_u3d_queue_trb(ep, req);
> + if (ret)
> + goto break_processing;
> +
>   /* irq handler advances the queue */
> - if (req)
> - list_add_tail(>queue, >queue);
> + list_add_tail(>queue, >queue);
>  
>   return 0;
> +
> +break_processing:
> + ep->processing = 0;
> + return ret;
>  }
>  
>  static int mv_u3d_ep_enable(struct usb_ep *_ep,

this is another, unrelated patch. Please split

-- 
balbi


signature.asc
Description: PGP signature