date:20150903

Re: [PATCH] usb/gadget/function/u_ether.c: Allow jumbo frames

2015-09-03 Thread Mike Looijmans


I'd like to bring this to your attention again please.

If there is something wrong about this patch, please tell me so.

And also note that this patch does not enable jumbo frames on itself, it just 
removes the artificial limits in the kernel prohibiting mtus above 1500. The 
MTU can be set from user space, and the impact on performance is quite impressive.


On 05-08-15 08:54, Mike Looijmans wrote:

USB network adapters support Jumbo frames. The only thing blocking
that feature is the code in the gadget driver that disposes of
packets larger than 1518 bytes, and the limit on the ioctl to set
the mtu.

This patch relaxes these limits, and allows up to 15k frames sizes.
The 15k value was chosen because 16k does not work on all platforms,
and usingclose to 16k will result in allocating 5 or 8 4k pages to
store the skb, wasting pages at no measurable performance gain.

On a topic-miami board (Zynq-7000), iperf3 performance reports:
MTU= 1500, PC-to-gadget: 139 Mbps, Gadget-to-PC: 116 Mbps
MTU=15000, PC-to-gadget: 239 Mbps, Gadget-to-PC: 361 Mbps

On boards with slower CPUs the performance improvement will be
relatively much larger, e.g. an OMAP-L138 increased from 40 to
220 Mbps using a similar patch on an  2.6.37 kernel.

Signed-off-by: Mike Looijmans 
---
  drivers/usb/gadget/function/u_ether.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_ether.c 
b/drivers/usb/gadget/function/u_ether.c
index f1fd777..6828ea2 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -48,6 +48,11 @@

  #define UETH__VERSION "29-May-2008"

+/* Experiments show that both Linux and Windows hosts allow up to 16k
+ * frame sizes. Set the max size to 15k+52 to prevent allocating 32k
+ * blocks and still have efficient handling. */
+#define GETHER_MAX_ETH_FRAME_LEN 15412
+
  struct eth_dev {
/* lock is held while accessing port_usb
 */
@@ -146,7 +151,7 @@ static int ueth_change_mtu(struct net_device *net, int 
new_mtu)
spin_lock_irqsave(>lock, flags);
if (dev->port_usb)
status = -EBUSY;
-   else if (new_mtu <= ETH_HLEN || new_mtu > ETH_FRAME_LEN)
+   else if (new_mtu <= ETH_HLEN || new_mtu > GETHER_MAX_ETH_FRAME_LEN)
status = -ERANGE;
else
net->mtu = new_mtu;
@@ -294,7 +299,7 @@ static void rx_complete(struct usb_ep *ep, struct 
usb_request *req)
while (skb2) {
if (status < 0
|| ETH_HLEN > skb2->len
-   || skb2->len > VLAN_ETH_FRAME_LEN) {
+   || skb2->len > 
GETHER_MAX_ETH_FRAME_LEN) {
dev->net->stats.rx_errors++;
dev->net->stats.rx_length_errors++;
DBG(dev, "rx length %d\n", skb2->len);





Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijm...@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail





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

Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq

2015-09-03 Thread Felipe Balbi

Hi,

On Thu, Sep 03, 2015 at 04:52:02PM +0300, Roger Quadros wrote:
> >>if (on) {
> >> -  dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> >> +  /* OCTL.PeriMode = 0 */
> >> +  reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> >> +  reg &= ~DWC3_OCTL_PERIMODE;
> >> +  dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >> +  /* unconditionally turn on VBUS */
> >> +  reg |= DWC3_OCTL_PRTPWRCTL;
> >> +  dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >>/* start the HCD */
> >>usb_otg_start_host(fsm, true);
> >>} else {
> >>/* stop the HCD */
> >>usb_otg_start_host(fsm, false);
> >> +  /* turn off VBUS */
> >> +  reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> >> +  reg &= ~DWC3_OCTL_PRTPWRCTL;
> >> +  dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >> +  /* OCTL.PeriMode = 1 */
> >> +  reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> >> +  reg |= DWC3_OCTL_PERIMODE;
> >> +  dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >>}
> > 
> > it looks like you're not really following the fluxchart from SNPS
> > documentation, see Figure 11-4 on section 11.1.4.5
> 
> Did you mean that I'm ignoring all OTG bits (HNP/SRP/ADP)?

yes and no :-)  There's a rather complex flux chart which details how we
switch from host to peripheral and vice versa. We need to follow that to
the smallest details since that's what IP provider considers to be
correct. If we deviate from that we should have very strong reasons for
doing so and we also want big, fat, long comments in source code
detailing why and how we're deviating :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v4 8/9] usb: dwc3: core: Prevent otg events from disabling themselves

2015-09-03 Thread Roger Quadros

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 02/09/15 17:47, Felipe Balbi wrote:
> On Wed, Sep 02, 2015 at 05:24:23PM +0300, Roger Quadros wrote:
>> There is a race happening during dwc3_drd_init() that causes
>> otg events to get disabled. This is what happens.
>>
>> dwc3_otg_irq() happens immediately when PRTCAP is set to OTG,
>> even though OEVTEN is 0. This is because BIT 31 IRQ of
>> OEVT can't be disabled by OEVTEN.
>> We configure OEVTEN in dwc3_otg_init() but dwc3_otg_irq() has
>> already saved OEVTEN as 0 into dwc->oevten. So finally when
>> dwc3_irq_thread_irq() is called we save 0 into OEVTEN
>> thus disabling OTG irqs forever.
>>
>> We fix this by disabling IRQs when configuring OEVTEN in
>> dwc3_otg_init().
>>
>> Signed-off-by: Roger Quadros 
> 
> can't you just merge this patch into the one which introduced the bug to
> start with ?

Yes, I'll do that.

> 
>> ---
>>  drivers/usb/dwc3/core.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 684010c..654aebf 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -921,6 +921,7 @@ static int dwc3_drd_init(struct dwc3 *dwc)
>>  int ret, id, vbus;
>>  struct usb_otg_caps *otgcaps = >otg_config.otg_caps;
>>  u32 reg;
>> +unsigned long flags;
>>  
>>  otgcaps->otg_rev = 0;
>>  otgcaps->hnp_support = false;
>> @@ -993,6 +994,8 @@ try_otg_core:
>>  goto error;
>>  }
>>  
>> +spin_lock_irqsave(>lock, flags);
>> +
>>  /* we need to set OTG to get events from OTG core */
>>  dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>>  /* GUSB2PHYCFG0.SusPHY=0 */
>> @@ -1018,6 +1021,8 @@ try_otg_core:
>>  /* OCTL.PeriMode = 1 */
>>  dwc3_writel(dwc->regs, DWC3_OCTL, DWC3_OCTL_PERIMODE);
>>  
>> +spin_unlock_irqrestore(>lock, flags);
>> +
>>  dwc3_otg_fsm_sync(dwc);
>>  usb_otg_sync_inputs(dwc->fsm);
>>  
>> -- 
>> 2.1.4
>>
> 
- --
cheers,
- -roger
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJV6FEnAAoJENJaa9O+djCTSjgQALL73OtdfKAWte2JFxsw1PVT
Xeh+ec7FJqNCKb2TwTSgnnWp6BxapbAlkK3kSwhsbgZvZLGtf3IC+xPHWNCs4dZs
uCs9oDPC/oAiYBnN3gYZxBFuhe1fsFEuS7aFIzrvKD/+tnXmd0IQ5Yu3k06VDXIM
jf/034Wep9AK44fyPy0sLKET1J6/UMvjxvUpEyxj9elbvs+GqzD9qR/GF6Ob8q6d
5f7y4ajMBCgyoC/wpIXDIJi7yqH1MUZq8bdC+7BKyc8Fz7kzVSpU2SL0BSRMU7Jn
Mzyx25WbWy1mXkfdgOjXtxx6MH5gV6jIIvqobyXXrK/e0z3kU6OeFprbubsOAvmC
fcgRkOrfr1O7/G5Gm7mhU5kLSZF6kvf6MoDPGCd/TbP4OgfYcPiTj5O020qDbRxZ
Hfuy/5Xk1oxR2aui1q9ycCWrpSA4L+B4fSpDXUznZw755BhhPnVeoYlXUfcb2sI+
YPNs6cLeX95I27AoMpK0wjMptmswaKcDmwv50mjK0kXgUcnVGs6z7YxSIJ2eAxPV
OpYLwVTfJiqoeJMscHvqoipwjlHqq4xS780SbzgZTDFOk2Ik7l0B6mNoL9CgaaXa
LVCBEywdWo+9os4b3JRrjDSqqdOMhBVaO0GvtiZZqfF7mOTmJHF/yFAODBw/CLXi
jAmBC/nM3M32CJphQQ1J
=z7cJ
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

2015-09-03 Thread Roger Quadros

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 02/09/15 17:48, Felipe Balbi wrote:
> On Wed, Sep 02, 2015 at 05:24:24PM +0300, Roger Quadros wrote:
>> We can't rely just on dr_mode to decide if we're in host or gadget
>> mode when we're configured as otg/dual-role. So while dr_mode is
>> OTG, we find out from  the otg state machine if we're in host
>> or gadget mode and take the necessary actions during suspend/resume.
>>
>> Also make sure that we disable OTG irq and events during system suspend
>> so that we don't lockup the system during system suspend/resume.
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/dwc3/core.c | 27 +--
>>  1 file changed, 9 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 654aebf..25891e3 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
>>  dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
>>  dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
>>  dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> +dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
>> +disable_irq(dwc->otg_irq);
> 
> you don't need disable_irq() here. In fact, it causes problems since
> you're calling it with IRQs disabled.

OK. will remove it.

> 
>>  }
>>  
>> -switch (dwc->dr_mode) {
>> -case USB_DR_MODE_PERIPHERAL:
>> -case USB_DR_MODE_OTG:
>> +if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
>> +((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
>> PROTO_GADGET)))
>>  dwc3_gadget_suspend(dwc);
>> -/* FALLTHROUGH */
>> -case USB_DR_MODE_HOST:
>> -default:
>> -dwc3_event_buffers_cleanup(dwc);
>> -break;
>> -}
>> +
>> +dwc3_event_buffers_cleanup(dwc);
>>  
>>  dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
>>  spin_unlock_irqrestore(>lock, flags);
>> @@ -1506,18 +1503,12 @@ static int dwc3_resume(struct device *dev)
>>  dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
>>  dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
>>  dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> +enable_irq(dwc->otg_irq);
>>  }
>>  
>> -switch (dwc->dr_mode) {
>> -case USB_DR_MODE_PERIPHERAL:
>> -case USB_DR_MODE_OTG:
>> +if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
>> +((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
>> PROTO_GADGET)))
>>  dwc3_gadget_resume(dwc);
>> -/* FALLTHROUGH */
>> -case USB_DR_MODE_HOST:
>> -default:
>> -/* do nothing */
>> -break;
>> -}
>>  
>>  spin_unlock_irqrestore(>lock, flags);
>>  
>> -- 
>> 2.1.4
>>
> 

- --
cheers,
- -roger
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJV6FLtAAoJENJaa9O+djCTxnMQANGOTEBjO8E9v32qfrgwL6X+
VyTeGBHhfwv7/u7xOM0im6tfXXaLtjj16mEY8fePC+oXO2Vv3RDxi0HTiF6sePnT
odt8nTOYGq7arPAKrtL8BK/8xyJRDjijXWGdQDAgPT1D5V2y8Ib4AbkM2j5IkL+/
75EGybhxIPNGNEV2eS/OBn2BPWSzn/r6rFMJiuYPY2yoPQqwG1ovdt+K+tvMybvZ
sYdQxu4UPN1z1pKplmCQxmPut9SyCqIAeHXdWGT6kJsleBsv2WNM1ZdM/y4zZmMw
gpyNOs+HYqdd/llfEYFrSRnSM2io2GyKZ73xM+DVfY8ot7Vf/h4x41Dz4V19jqtC
3IoyPzNb+inbsRKs0GhTw3yD9N8b7xRbVF+qZPIvDvn7QGLF2pY8cDiqZE1LiOs2
VpKBRoC0wdmdQ/bjRMt516jFmJiQ2RbR2SenGffIr+PrCb1EQ7Fmwtoya5FPgIz/
nMdL7g1MGHUzWvzXTYO+RH4bHV9cDL66qekOL7PR1yEyAAo09vLc+ds/4z85MGQ6
2JYKYb4Mdtv2EAOzIqNR3+FzcCawpZbhm8bBFz14Kj12pgjwl5A3o+fYaZ4Q07OK
ZfFRAUG4RKFNVcjb7hhiBPdN+mb34/50YreFcNj/MOYwklXwUAmgiTFH7cjO9SA7
XZpbfSRwT4Ec4WWqTeeR
=unNn
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume

2015-09-03 Thread Roger Quadros

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 02/09/15 17:44, Felipe Balbi wrote:
> On Wed, Sep 02, 2015 at 05:24:21PM +0300, Roger Quadros wrote:
>> Without this we loose OTG controller register context and malfunction
>> after a system suspend-resume.
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/dwc3/core.c | 17 +
>>  drivers/usb/dwc3/core.h |  6 +-
>>  2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 632ee53..684010c 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -56,6 +56,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
>>  reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>>  reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
>>  reg |= DWC3_GCTL_PRTCAPDIR(mode);
>> +dwc->current_mode = mode;
>>  dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>>  }
>>  
>> @@ -1443,6 +1444,14 @@ static int dwc3_suspend(struct device *dev)
>>  
>>  spin_lock_irqsave(>lock, flags);
>>  
>> +/* Save OTG state only if we're really using it */
>> +if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
>> +dwc->ocfg = dwc3_readl(dwc->regs, DWC3_OCFG);
>> +dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
>> +dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
> 
> oevt is what you use to clear pending IRQs, which means that ...
> 
>> +dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> +}
>> +
>>  switch (dwc->dr_mode) {
>>  case USB_DR_MODE_PERIPHERAL:
>>  case USB_DR_MODE_OTG:
>> @@ -1486,6 +1495,14 @@ static int dwc3_resume(struct device *dev)
>>  dwc3_event_buffers_setup(dwc);
>>  dwc3_writel(dwc->regs, DWC3_GCTL, dwc->gctl);
>>  
>> +/* Restore OTG state only if we're really using it */
>> +if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
>> +dwc3_writel(dwc->regs, DWC3_OCFG, dwc->ocfg);
>> +dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
>> +dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
> 
> ... you could be clearing pending IRQs right here.

Good catch. So we can't really restore this register.
I'll remove this line.

> 
>> +dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> +}
>> +
>>  switch (dwc->dr_mode) {
>>  case USB_DR_MODE_PERIPHERAL:
>>  case USB_DR_MODE_OTG:
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 129ef37..1115ce0 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -737,6 +737,7 @@ struct dwc3_scratchpad_array {
>>   * @regs: base address for our registers
>>   * @regs_size: address space size
>>   * @oevten: otg interrupt enable mask copy
>> + * @current_mode: current mode of operation written to PRTCAPDIR
>>   * @nr_scratch: number of scratch buffers
>>   * @num_event_buffers: calculated number of event buffers
>>   * @u1u2: only used on revisions <1.83a for workaround
>> @@ -858,9 +859,12 @@ struct dwc3 {
>>  /* used for suspend/resume */
>>  u32 dcfg;
>>  u32 gctl;
>> -
>> +u32 ocfg;
>> +u32 octl;
>> +u32 oevt;
>>  u32 oevten;
>>  
>> +u32 current_mode;
>>  u32 nr_scratch;
>>  u32 num_event_buffers;
>>  u32 u1u2;
>> -- 
>> 2.1.4
>>
> 

- --
cheers,
- -roger
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJV6FEGAAoJENJaa9O+djCTE7gP/0rlQJPL1R11k8hJCM842zjT
SSxfdNQ9kkG008D88As8nLodL79agvD/XBot7iIrTGVFMG/tzzkwQb71gZoCOogb
K0eMMnLNyzLRapNpOe1OJqI1NjO8Yx1o7heoaGkGYERR7m6dwz1LmSPYEtaaveVq
5gb5NbUKfSq0Cs6wM+3+U8CQzAPLf7eRSZwiORkzyn7cWiZDsYHr9Bn35NA24r5v
0YMQvmTp7K/7pFENiFHmOWbKPoY6G8ebkInBSMXQfHcLPtL86l9e+VrlJrIZzBqQ
SJyoQw8JFU9c2ojtKeG9AYn8fSbCQpbsuIFAZtCVy2rK4xMiITmaSsjHF5+KpBt2
cE7eahicYAP3edLqT0R0hYfiI+JrVuOdkiW+W8UWPHrGt/nELQjQxZtIkTOI1b9B
FIxnZbIwp0veEE+vqXiTQizUPdZag0qPG1PZ5Bpu0867vfX6khY2MCNeYTDatzHT
dz9RwWIDH3/K5K7TOug/VZEiqvLY3BYMrMK+W3FreGfagwvYvlLyojsHifrA1x9z
1+zAjkml1kIIP5Jlf3VOYdLsN8mgeMyCNfi5USvVFnzPITAjtwm9+ai7xxxCjSxA
QTvmhQRjAxIPWWZgVuc/Z5PdqgHKFKpCLxG8IE9bqGBQnNSKueZHEhfUCxfPNKV6
ZkkhIq/s6hm6S/YM4WcS
=hUag
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

2015-09-03 Thread Sergei Shtylyov


Hello.

On 09/03/2015 05:01 PM, Roger Quadros wrote:


We can't rely just on dr_mode to decide if we're in host or gadget
mode when we're configured as otg/dual-role. So while dr_mode is
OTG, we find out from  the otg state machine if we're in host
or gadget mode and take the necessary actions during suspend/resume.

Also make sure that we disable OTG irq and events during system suspend
so that we don't lockup the system during system suspend/resume.

Signed-off-by: Roger Quadros 
---
   drivers/usb/dwc3/core.c | 27 +--
   1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 654aebf..25891e3 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
   dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
   dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
   dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+disable_irq(dwc->otg_irq);
   }

-switch (dwc->dr_mode) {
-case USB_DR_MODE_PERIPHERAL:
-case USB_DR_MODE_OTG:
+if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
+((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
PROTO_GADGET)))


Hm, you're not very consistent about your parens. :-)


You're right. Should be

if ((dwc->dr_mode == USB_DR_MODE_PERIPHERAL) ||
 ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
PROTO_GADGET)))


   Parens around == are useless, I'd prefer if you deleted them. But if you'd 
like to keep them, let it be so. :-)



--
cheers,
-roger


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 v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

2015-09-03 Thread Roger Quadros

On 03/09/15 17:05, Sergei Shtylyov wrote:
> Hello.
> 
> On 09/03/2015 05:01 PM, Roger Quadros wrote:
> 
 We can't rely just on dr_mode to decide if we're in host or gadget
 mode when we're configured as otg/dual-role. So while dr_mode is
 OTG, we find out from  the otg state machine if we're in host
 or gadget mode and take the necessary actions during suspend/resume.

 Also make sure that we disable OTG irq and events during system suspend
 so that we don't lockup the system during system suspend/resume.

 Signed-off-by: Roger Quadros 
 ---
drivers/usb/dwc3/core.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)

 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index 654aebf..25891e3 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
 +dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
 +disable_irq(dwc->otg_irq);
}

 -switch (dwc->dr_mode) {
 -case USB_DR_MODE_PERIPHERAL:
 -case USB_DR_MODE_OTG:
 +if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
 +((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
 PROTO_GADGET)))
>>>
>>> Hm, you're not very consistent about your parens. :-)
>>
>> You're right. Should be
>>
>> if ((dwc->dr_mode == USB_DR_MODE_PERIPHERAL) ||
>>  ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
>> PROTO_GADGET)))
> 
>Parens around == are useless, I'd prefer if you deleted them. But if you'd 
> like to keep them, let it be so. :-)

OK, how about this then?

if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
(dwc->dr_mode == USB_DR_MODE_OTG && dwc->fsm->protocol == PROTO_GADGET))

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe 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/function/u_ether.c: Allow jumbo frames

2015-09-03 Thread Greg KH

On Thu, Sep 03, 2015 at 11:36:15AM +0200, Mike Looijmans wrote:
> I'd like to bring this to your attention again please.
> 
> If there is something wrong about this patch, please tell me so.

It's the middle of the merge window and we can't do anything with new
patches at the moment until 4.3-rc1 comes out.  Please be patient.

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

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

2015-09-03 Thread Roger Quadros

On 02/09/15 20:22, Sergei Shtylyov wrote:
> Hello.
> 
> On 09/02/2015 05:24 PM, Roger Quadros wrote:
> 
>> We can't rely just on dr_mode to decide if we're in host or gadget
>> mode when we're configured as otg/dual-role. So while dr_mode is
>> OTG, we find out from  the otg state machine if we're in host
>> or gadget mode and take the necessary actions during suspend/resume.
>>
>> Also make sure that we disable OTG irq and events during system suspend
>> so that we don't lockup the system during system suspend/resume.
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>   drivers/usb/dwc3/core.c | 27 +--
>>   1 file changed, 9 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 654aebf..25891e3 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
>>   dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
>>   dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
>>   dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> +dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
>> +disable_irq(dwc->otg_irq);
>>   }
>>
>> -switch (dwc->dr_mode) {
>> -case USB_DR_MODE_PERIPHERAL:
>> -case USB_DR_MODE_OTG:
>> +if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
>> +((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
>> PROTO_GADGET)))
> 
>Hm, you're not very consistent about your parens. :-)

You're right. Should be

if ((dwc->dr_mode == USB_DR_MODE_PERIPHERAL) ||
((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))


> 
> [...]
>> @@ -1506,18 +1503,12 @@ static int dwc3_resume(struct device *dev)
>>   dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
>>   dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
>>   dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> +enable_irq(dwc->otg_irq);
>>   }
>>
>> -switch (dwc->dr_mode) {
>> -case USB_DR_MODE_PERIPHERAL:
>> -case USB_DR_MODE_OTG:
>> +if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
>> +((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
>> PROTO_GADGET)))
> 
>Same here...

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

Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq

2015-09-03 Thread Roger Quadros

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 02/09/15 17:43, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Sep 02, 2015 at 05:24:20PM +0300, Roger Quadros wrote:
>> If the ID pin event is not available over extcon
>> then we rely on the OTG controller to provide us ID and VBUS
>> information.
>>
>> We still don't support any OTG features but just
>> dual-role operation.
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/dwc3/core.c | 217 
>> 
>>  drivers/usb/dwc3/core.h |   3 +
>>  2 files changed, 205 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 38b31df..632ee53 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -704,6 +704,63 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>>  return 0;
>>  }
>>  
>> +/* Get OTG events and sync it to OTG fsm */
>> +static void dwc3_otg_fsm_sync(struct dwc3 *dwc)
>> +{
>> +u32 reg;
>> +int id, vbus;
>> +
>> +reg = dwc3_readl(dwc->regs, DWC3_OSTS);
>> +dev_dbg(dwc->dev, "otgstatus 0x%x\n", reg);
>> +
>> +id = !!(reg & DWC3_OSTS_CONIDSTS);
>> +vbus = !!(reg & DWC3_OSTS_BSESVLD);
>> +
>> +if (id != dwc->fsm->id || vbus != dwc->fsm->b_sess_vld) {
>> +dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
>> +dwc->fsm->id = id;
>> +dwc->fsm->b_sess_vld = vbus;
>> +usb_otg_sync_inputs(dwc->fsm);
>> +}
> 
> this guy shouldn't try to filter events here. That's what the FSM should
> be doing.

OK. I'll remove the if condition.

> 
>> +}
>> +
>> +static irqreturn_t dwc3_otg_thread_irq(int irq, void *_dwc)
>> +{
>> +struct dwc3 *dwc = _dwc;
>> +unsigned long flags;
>> +irqreturn_t ret = IRQ_NONE;
> 
> this IRQ will be disabled pretty quickly. You *always* return IRQ_NONE
> 
>> +spin_lock_irqsave(>lock, flags);
> 
> if you cache current OSTS in dwc3, you can use that instead and change
> this to a spin_lock() instead of disabling IRQs here. This device's IRQs
> are already masked anyway.

OK.

> 
>> +dwc3_otg_fsm_sync(dwc);
>> +/* unmask interrupts */
>> +dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> +spin_unlock_irqrestore(>lock, flags);
>> +
>> +return ret;
>> +}
>> +
>> +static irqreturn_t dwc3_otg_irq(int irq, void *_dwc)
>> +{
>> +struct dwc3 *dwc = _dwc;
>> +irqreturn_t ret = IRQ_NONE;
>> +u32 reg;
>> +
>> +spin_lock(>lock);
> 
> this seems unnecessary, we're already in hardirq with IRQs disabled.
> What sort of race could we have ? (in fact, this also needs change in
> dwc3/gadget.c).

You're right. Will fix at both places.
> 
>> +
>> +reg = dwc3_readl(dwc->regs, DWC3_OEVT);
>> +if (reg) {
>> +dwc3_writel(dwc->regs, DWC3_OEVT, reg);
>> +/* mask interrupts till processed */
>> +dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> +dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
>> +ret = IRQ_WAKE_THREAD;
>> +}
>> +
>> +spin_unlock(>lock);
>> +
>> +return ret;
>> +}
>> +
>>  /* - Dual-Role management 
>> --- */
>>  
>>  static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
>> @@ -728,15 +785,44 @@ static int dwc3_drd_start_host(struct otg_fsm *fsm, 
>> int on)
>>  {
>>  struct device *dev = usb_otg_fsm_to_dev(fsm);
>>  struct dwc3 *dwc = dev_get_drvdata(dev);
>> +u32 reg;
>>  
>>  dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> +if (dwc->edev) {
>> +if (on) {
>> +dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> +/* start the HCD */
>> +usb_otg_start_host(fsm, true);
>> +} else {
>> +/* stop the HCD */
>> +usb_otg_start_host(fsm, false);
>> +}
> 
>   if (on)
>   dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>   usb_otg_start_host(fsm, on);
> 

OK.

>> +
>> +return 0;
>> +}
>> +
>> +/* switch OTG core */
>>  if (on) {
>> -dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> +/* OCTL.PeriMode = 0 */
>> +reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> +reg &= ~DWC3_OCTL_PERIMODE;
>> +dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> +/* unconditionally turn on VBUS */
>> +reg |= DWC3_OCTL_PRTPWRCTL;
>> +dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>>  /* start the HCD */
>>  usb_otg_start_host(fsm, true);
>>  } else {
>>  /* stop the HCD */
>>  usb_otg_start_host(fsm, false);
>> +/* turn off VBUS */
>> +reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> +reg &= ~DWC3_OCTL_PRTPWRCTL;
>> +dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> +/* OCTL.PeriMode = 1 */
>> +reg =

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

2015-09-03 Thread Sergei Shtylyov


On 09/03/2015 05:10 PM, Roger Quadros wrote:


We can't rely just on dr_mode to decide if we're in host or gadget
mode when we're configured as otg/dual-role. So while dr_mode is
OTG, we find out from  the otg state machine if we're in host
or gadget mode and take the necessary actions during suspend/resume.

Also make sure that we disable OTG irq and events during system suspend
so that we don't lockup the system during system suspend/resume.

Signed-off-by: Roger Quadros 
---
drivers/usb/dwc3/core.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 654aebf..25891e3 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+disable_irq(dwc->otg_irq);
}

-switch (dwc->dr_mode) {
-case USB_DR_MODE_PERIPHERAL:
-case USB_DR_MODE_OTG:
+if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
+((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
PROTO_GADGET)))


 Hm, you're not very consistent about your parens. :-)


You're right. Should be

if ((dwc->dr_mode == USB_DR_MODE_PERIPHERAL) ||
  ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == 
PROTO_GADGET)))


Parens around == are useless, I'd prefer if you deleted them. But if you'd 
like to keep them, let it be so. :-)



OK, how about this then?



if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
 (dwc->dr_mode == USB_DR_MODE_OTG && dwc->fsm->protocol == PROTO_GADGET))


   Strictly speaking, parens around && are also not needed but gcc may 
probably issue a warning without them. Not sure, your call.



cheers,
-roger


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 v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode

2015-09-03 Thread Roman Bacik

Is there anything else we should address in this patch?
Thanks,

Roman

> -Original Message-
> From: Scott Branden [mailto:sbran...@broadcom.com]
> Sent: August-31-15 9:17 AM
> To: John Youn; Greg Kroah-Hartman; linux-usb@vger.kernel.org; Roman
> Bacik
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list; Scott Branden
> Subject: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> 
> From: Roman Bacik 
> 
> USB OTG driver in isochronous mode has to set the parity of the receiving
> microframe. The parity is set to even by default. This causes problems for an
> audio gadget, if the host starts transmitting on odd microframes.
> 
> This fix uses Incomplete Periodic Transfer interrupt to toggle between even
> and odd parity until the Transfer Complete interrupt is received.
> 
> Signed-off-by: Roman Bacik 
> Reviewed-by: Abhinav Ratna 
> Reviewed-by: Srinath Mannam 
> Signed-off-by: Scott Branden 
> ---
>  drivers/usb/dwc2/core.h   |  1 +
>  drivers/usb/dwc2/gadget.c | 51
> ++-
>  drivers/usb/dwc2/hw.h |  1 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index
> 0ed87620..a5634fd 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
>   unsigned intperiodic:1;
>   unsigned intisochronous:1;
>   unsigned intsend_zlp:1;
> + unsigned inthas_correct_parity:1;
> 
>   charname[10];
>  };
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index
> 4d47b7c..fac3e2f 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -1954,6 +1954,7 @@ static void s3c_hsotg_epint(struct dwc2_hsotg
> *hsotg, unsigned int idx,
>   ints &= ~DXEPINT_XFERCOMPL;
> 
>   if (ints & DXEPINT_XFERCOMPL) {
> + hs_ep->has_correct_parity = 1;
>   if (hs_ep->isochronous && hs_ep->interval == 1) {
>   if (ctrl & DXEPCTL_EOFRNUM)
>   ctrl |= DXEPCTL_SETEVENFR;
> @@ -2316,7 +2317,8 @@ void s3c_hsotg_core_init_disconnected(struct
> dwc2_hsotg *hsotg,
>   GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST |
>   GINTSTS_RESETDET | GINTSTS_ENUMDONE |
>   GINTSTS_OTGINT | GINTSTS_USBSUSP |
> - GINTSTS_WKUPINT,
> + GINTSTS_WKUPINT |
> + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT,
>   hsotg->regs + GINTMSK);
> 
>   if (using_dma(hsotg))
> @@ -2581,6 +2583,52 @@ irq_retry:
>   s3c_hsotg_dump(hsotg);
>   }
> 
> + if (gintsts & GINTSTS_INCOMPL_SOIN) {
> + u32 idx, epctl_reg, ctrl;
> + struct s3c_hsotg_ep *hs_ep;
> +
> + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n",
> __func__);
> + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) {
> + hs_ep = hsotg->eps_in[idx];
> +
> + if (!hs_ep->isochronous || hs_ep-
> >has_correct_parity)
> + continue;
> +
> + epctl_reg = DIEPCTL(idx);
> + ctrl = readl(hsotg->regs + epctl_reg);
> +
> + if (ctrl & DXEPCTL_EOFRNUM)
> + ctrl |= DXEPCTL_SETEVENFR;
> + else
> + ctrl |= DXEPCTL_SETODDFR;
> + writel(ctrl, hsotg->regs + epctl_reg);
> + }
> + writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS);
> + }
> +
> + if (gintsts & GINTSTS_INCOMPL_SOOUT) {
> + u32 idx, epctl_reg, ctrl;
> + struct s3c_hsotg_ep *hs_ep;
> +
> + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n",
> __func__);
> + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) {
> + hs_ep = hsotg->eps_out[idx];
> +
> + if (!hs_ep->isochronous || hs_ep-
> >has_correct_parity)
> + continue;
> +
> + epctl_reg = DOEPCTL(idx);
> + ctrl = readl(hsotg->regs + epctl_reg);
> +
> + if (ctrl & DXEPCTL_EOFRNUM)
> + ctrl |= DXEPCTL_SETEVENFR;
> + else
> + ctrl |= DXEPCTL_SETODDFR;
> + writel(ctrl, hsotg->regs + epctl_reg);
> + }
> + writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS);
> + }
> +
>   /*
>* if we've had fifo events, we should try and go around the
>* loop again to see if there's any point in returning yet.
> @@ -2667,6 +2715,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>   hs_ep->periodic = 0;
>   hs_ep->halted = 0;

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

2015-09-03 Thread Alan Stern

On Thu, 3 Sep 2015, Roland Weber wrote:

> Hello Alan,
> 
> > The kernel freezes before the
> > new log statements, or any others in hub.c, are executed.
> 
> Wrong filename there, it's usb.c of course.

I'm not sure what you mean by that.  Everything we have been discussing
is in hub.c, not usb.c.

> I've collected debug output from the boot sequence of the
> bad kernel, with extra entry/exit statements, and of the
> last good kernel, without the extra statements. Below are
> excerpts which hopefully catch all relevant lines. Because
> of its size, I've attached the full output to the bug report
> that I initially opened against my distribution.

The boot sequence isn't enough.  I need to see what happens when the
system freezes.

Also, the log doesn't go all the way back to the initial boot.  
Probably the kernel's log buffer overflowed and wrapped around because
of all the useless systemd output.  Is there any way you can tell
systemd not to write its output to the kernel log buffer?  Or failing
that, can you expand the log buffer's size (increase
CONFIG_LOG_BUF_SHIFT)?

> The "good" kernel apparently has delays of 4 seconds
> related to the "Cannot enable" problem.

That doesn't mean anything.  It's just the interval between retries.

>  The "bad" kernel
> has no such problem at all. Any idea how that difference
> in behavior can be caused by changing the implementation
> of the workqueue?

In the "good" kernel, the 1-1 device is probed _before_ the devices on
bus 2, whereas in the "bad" kernel it is probed _after_ them.  That
could make a difference; the buses are supposed to be independent but
they might not be.  However, this is not relevant to the main problem,
which is the hangs.

(You can test this hypothesis by booting the "good" kernel, and after 
everything settles down, doing this:

echo :00:1d.0 >/sys/bus/pci/drivers/ehci-pci/unbind
echo :00:1d.0 >/sys/bus/pci/drivers/ehci-pci/bind

If the guess is correct, the probes following this bind should 
succeed.)

> There are two exit traces without a matching entry trace:
> [2.990519] hub 2-0:1.0: exit
> [   17.092630] hub 1-0:1.0: exit
> I guess the x-0 device init starts at a different function.

No, the entry traces undoubtedly occurred earlier in the log and got 
overwritten when the log buffer wrapped around.

Alan Stern

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

Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts

2015-09-03 Thread Felipe Balbi

Hi,

On Thu, Sep 03, 2015 at 03:46:43PM +0300, Roger Quadros wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA256
> 
> On 02/09/15 17:34, Felipe Balbi wrote:
> > On Wed, Sep 02, 2015 at 05:24:19PM +0300, Roger Quadros wrote:
> >> From: Felipe Balbi 
> >>
> >> Add support to use interrupt names,
> >>
> >> Following are the interrupt names
> >>
> >> Peripheral Interrupt - peripheral
> >> HOST Interrupt - host
> >> OTG Interrupt - otg
> >>
> >> [Roger Q]
> >> - If any of these are missing we use the
> >> first available IRQ resource so that we don't
> >> break with older DTBs.
> > 
> > this is what original commit did:
> > 
> > commit ecd5f71cfd663bcd4efd2f29824acd8b2ba9715d
> > Author: Felipe Balbi 
> > Date:   Fri Jan 3 13:49:38 2014 -0600
> > 
> > usb: dwc3: cleanup IRQ resources
> > 
> > In order to make it easier for the driver to
> > figure out which modes of operation it has,
> > and because some dwc3 integrations have rather
> > fuzzy IRQ routing, we now require three different
> > IRQ numbers (peripheral, host, otg).
> > 
> > In order to do that and maintain backwards compatibility,
> > we still maintain support for the old IRQ resource name,
> > but if you *really* want to have proper peripheral/host/otg
> > support, you should make sure your resources are correct.
> > 
> > Signed-off-by: Felipe Balbi 
> 
> This is better since we request the resource only if needed and bail out
> if it is not present.
> 
> > 
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 60580a01cdd2..1f01031873b7 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -556,10 +556,22 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> >  static int dwc3_core_init_mode(struct dwc3 *dwc)
> >  {
> > struct device *dev = dwc->dev;
> > +   struct platform_device *pdev = to_platform_device(dev);
> > int ret;
> >  
> > switch (dwc->dr_mode) {
> > case USB_DR_MODE_PERIPHERAL:
> > +   dwc->gadget_irq = platform_get_irq_byname(pdev, 
> > "dwc3_peripheral");
> 
> Shall we just name it just "peripheral"?

sure, why not :-)

> > +   if (dwc->gadget_irq < 0) {
> > +   dwc->gadget_irq = platform_get_irq_byname(pdev, 
> > "dwc_usb3");
> 
> How will this work? Currently we don't have a name for the IRQ in the DT.

we can just add interrupt-names, right ? Or, the fallback could be what
is already done today: just fetch it by index.

> 
> > +   if (dwc->gadget_irq < 0) {
> > +   dev_err(dev, "missing IRQ\n");
> > +   return dwc->gadget_irq;
> > +   } else {
> > +   dev_warn(dev, "dwc_usb3 resource is 
> > deprecated\n");
> 
> Do we want to warn about legacy nodes?

Sure :-)

Now, do you want me to update it, or will you do it ? BTW, if you decide
to do it, we need to patch all users :-)

> > @@ -576,6 +604,28 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> > }
> > break;
> > case USB_DR_MODE_OTG:
> > +   dwc->gadget_irq = platform_get_irq_byname(pdev, 
> > "dwc3_peripheral");
> > +   if (dwc->gadget_irq < 0) {
> > +   dwc->gadget_irq = platform_get_irq_byname(pdev, 
> > "dwc_usb3");
> > +   if (dwc->gadget_irq < 0) {
> > +   dev_err(dev, "missing IRQ\n");
> > +   return dwc->gadget_irq;
> > +   } else {
> > +   dev_warn(dev, "dwc_usb3 resource is 
> > deprecated\n");
> > +   }
> > +
> > +   dwc->xhci_irq = dwc->gadget_irq;
> > +   dwc->otg_irq = dwc->gadget_irq;
> > +   } else {
> > +   dwc->xhci_irq = platform_get_irq_byname(pdev, 
> > "dwc3_host");
> > +   if (dwc->xhci_irq < 0) {
> > +   dev_err(dev, "missing Host IRQ\n");
> > +   return dwc->xhci_irq;
> > +   }
> > +
> > +   dwc->otg_irq = platform_get_irq_byname(pdev, 
> > "dwc3_otg");
> 
> need to check if error?

right

> > +   }
> 
> Need to setup xhci_resource[1]?

isn't it done above ?

-- 
balbi


signature.asc
Description: Digital signature

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

2015-09-03 Thread Roland Weber

Hello Alan,

> The kernel freezes before the
> new log statements, or any others in hub.c, are executed.

Wrong filename there, it's usb.c of course.

I've collected debug output from the boot sequence of the
bad kernel, with extra entry/exit statements, and of the
last good kernel, without the extra statements. Below are
excerpts which hopefully catch all relevant lines. Because
of its size, I've attached the full output to the bug report
that I initially opened against my distribution.

The "good" kernel apparently has delays of 4 seconds
related to the "Cannot enable" problem. The "bad" kernel
has no such problem at all. Any idea how that difference
in behavior can be caused by changing the implementation
of the workqueue?

There are two exit traces without a matching entry trace:
[2.990519] hub 2-0:1.0: exit
[   17.092630] hub 1-0:1.0: exit
I guess the x-0 device init starts at a different function.

cheers and thanks,
  Roland


# bad kernel, full output at
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1485057/+attachment/4456918/+files/dmesg.bad.full.txt
> grep -i -e 'usb\|hub' # bad kernel
[2.672184] usb usb2-port4: status 0101, change , 12 Mb/s
[2.690605] systemd-udevd[105]: seq 1289 queued, 'add' 'usb'
[2.690662] systemd-udevd[105]: seq 1290 queued, 'add' 'usb'
[2.771257] usb 2-2-port1: status 0101 change 0001
[2.786767] usb 2-4: new high-speed USB device number 3 using xhci_hcd
[2.842780] usb usb2-port4: not reset yet, waiting 50ms
[2.878808] hub 2-2:1.0: entry
[2.878811] hub 2-2:1.0: state 7 ports 4 chg 0002 evt 
[2.881573] usb 2-2-port1: status 0101, change , 12 Mb/s
[2.885371] usb 2-2-port1: indicator auto status 0
[2.985325] usb 2-4: udev 3, busnum 2, minor = 130
[2.985328] usb 2-4: New USB device found, idVendor=04f2, idProduct=b469
[2.985330] usb 2-4: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[2.985332] usb 2-4: Product: HD WebCam
[2.985334] usb 2-4: Manufacturer: Chicony Electronics Co.,Ltd.
[2.985868] systemd-udevd[105]: seq 1294 queued, 'add' 'usb'
[2.990519] hub 2-0:1.0: exit
[2.990787] systemd-udevd[119]: handling device node '/dev/bus/usb/002/001', 
devnum=c189:128, mode=0664, uid=0, gid=0
[2.990797] systemd-udevd[119]: set permissions /dev/bus/usb/002/001, 
020664, uid=0, gid=0
[2.990821] systemd-udevd[119]: creating symlink '/dev/char/189:128' to 
'../bus/usb/002/001'
[2.990920] systemd-udevd[119]: created db file '/run/udev/data/c189:128' 
for '/devices/pci:00/:00:14.0/usb2'
[2.991034] systemd-udevd[105]: seq 1296 queued, 'add' 'usb'
[2.991150] systemd-udevd[119]: no db file to read 
/run/udev/data/+usb:2-0:1.0: No such file or directory
[2.991199] systemd-udevd[119]: Execute 'load' 
'usb:v1D6Bp0002d0317dc09dsc00dp01ic09isc00ip00in00'
[2.991244] systemd-udevd[119]: No module matches 
'usb:v1D6Bp0002d0317dc09dsc00dp01ic09isc00ip00in00'
[2.991429] systemd-udevd[123]: IMPORT builtin 'usb_id' 
/lib/udev/rules.d/50-udev-default.rules:9
[2.991618] systemd-udevd[125]: IMPORT builtin 'usb_id' 
/lib/udev/rules.d/50-udev-default.rules:9
[2.991820] systemd-udevd[125]: handling device node '/dev/bus/usb/002/003', 
devnum=c189:130, mode=0664, uid=0, gid=0
[2.991830] systemd-udevd[125]: set permissions /dev/bus/usb/002/003, 
020664, uid=0, gid=0
[2.991851] systemd-udevd[125]: creating symlink '/dev/char/189:130' to 
'../bus/usb/002/003'
[2.991922] systemd-udevd[125]: created db file '/run/udev/data/c189:130' 
for '/devices/pci:00/:00:14.0/usb2/2-4'
[2.992049] systemd-udevd[105]: seq 1297 queued, 'add' 'usb'
[2.992170] systemd-udevd[119]: no db file to read 
/run/udev/data/+usb:2-4:1.0: No such file or directory
[2.992220] systemd-udevd[119]: Execute 'load' 
'usb:v04F2pB469d2857dcEFdsc02dp01ic0Eisc01ip00in00'
[2.992262] systemd-udevd[119]: No module matches 
'usb:v04F2pB469d2857dcEFdsc02dp01ic0Eisc01ip00in00'
[2.992421] systemd-udevd[125]: no db file to read 
/run/udev/data/+usb:2-4:1.1: No such file or directory
[2.992468] systemd-udevd[125]: Execute 'load' 
'usb:v04F2pB469d2857dcEFdsc02dp01ic0Eisc02ip00in01'
[2.992508] systemd-udevd[125]: No module matches 
'usb:v04F2pB469d2857dcEFdsc02dp01ic0Eisc02ip00in01'
[3.043577] usb 2-2.1: new full-speed USB device number 4 using xhci_hcd
[3.133028] usb 2-2.1: udev 4, busnum 2, minor = 131
[3.133031] usb 2-2.1: New USB device found, idVendor=04ca, idProduct=300b
[3.133033] usb 2-2.1: New USB device strings: Mfr=0, Product=0, 
SerialNumber=0
[3.133245] systemd-udevd[105]: seq 1309 queued, 'add' 'usb'
[3.135058] hub 2-2:1.0: exit
[3.135062] hub 2-2:1.0: entry
[3.135064] hub 2-2:1.0: state 7 ports 4 chg  evt 0002
[3.135685] hub 2-2:1.0: exit
[3.135872] systemd-udevd[123]: handling device node '/dev/bus/usb/002/002', 
devnum=c189:129, mode=0664, uid=0, gid=0
[3.135882] systemd-udevd[123]: set permissions

Re: [PATCH v4 1/9] usb: dwc3: add dual-role support

2015-09-03 Thread Felipe Balbi

Hi,

On Thu, Sep 03, 2015 at 03:21:48PM +0300, Roger Quadros wrote:
> >> +  dwc->fsm->id = id;
> >> +  dwc->fsm->b_sess_vld = vbus;
> >> +  usb_otg_sync_inputs(dwc->fsm);
> >> +}
> >> +
> >> +static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
> >> +{
> >> +  struct device *dev = usb_otg_fsm_to_dev(fsm);
> >> +  struct dwc3 *dwc = dev_get_drvdata(dev);
> > 
> > how about adding a usb_otg_get_drvdata(fsm) ?
> 
> You meant for otg core? That can be done.

yeah. BTW, I think otg core needs quite a few changes to become actually
useful. Currently it's just too much pointer ping-pong going back and
forth between phy, otg core, udc and hcd.

Also, I caught a ton of issues with it and suspend/resume. You might
want to fix them before adding more users to it.

It's also rather racy and that needs fixing too. On top of all that, I
think there's too much being added to UDC just to get Dual-Role, let's
see if we can improve that too.

> >> @@ -843,6 +998,16 @@ static int dwc3_probe(struct platform_device *pdev)
> >>hird_threshold = 12;
> >>  
> >>if (node) {
> >> +  if (of_property_read_bool(node, "extcon"))
> >> +  dwc->edev = extcon_get_edev_by_phandle(dev, 0);
> >> +  else if (of_property_read_bool(dev->parent->of_node, "extcon"))
> >> +  dwc->edev = extcon_get_edev_by_phandle(dev->parent, 0);
> > 
> > why do you need to check the parent ? Why isn't that done on the glue
> > layer ?
> 
> On DRA7-evm, the extcon device is defined in the glue layer node. But
> we need the device both at the glue layer and at the core layer.

why do you need extcon here ? Glue updates core via UTMI about the
states, right ? So you should get proper VBUS and ID status via OTG IRQ.
Is that not working ?

> We do get the extcon device in dwc3-omap.c
> 
> Any suggestion how to pass the extcon device from glue layer to core.c?
> Or should I add the extcon property to dwc3 USB node as well in the DT?

GPIO toggles
  dwc3-omap extcon event
update status via UTMI STATUS register
  OTG IRQ on core
Horray!

:-)

> >> +
> >> +  if (IS_ERR(dwc->edev)) {
> >> +  dev_vdbg(dev, "couldn't get extcon device\n");
> > 
> > dev_err() ??
> 
> Is it ok to print it even in EPROBE_DEFER case?

hmm, probably pointless, indeed.

-- 
balbi

signature.asc
Description: Digital signature

RE: [PATCH 2/3][v3] drivers: usb: dwc3: Add frame length adjustment quirk

2015-09-03 Thread Badola Nikhil

> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Thursday, September 03, 2015 9:25 PM
> To: Badola Nikhil-B46172 
> Cc: linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; linux-
> o...@vger.kernel.org; ba...@ti.com; gre...@linuxfoundation.org
> Subject: Re: [PATCH 2/3][v3] drivers: usb: dwc3: Add frame length
> adjustment quirk
> 
> On Thu, Sep 03, 2015 at 11:34:04AM +0530, Nikhil Badola wrote:
> > Add adjust_frame_length_quirk for writing to fladj register which
> > adjusts (micro)frame length to value provided by
> > "snps,quirk-frame-length-adjustment" property thus avoiding USB 2.0
> > devices to time-out over a longer run
> >
> > Signed-off-by: Nikhil Badola 
> > ---
> > Changes for v3:
> > - removed unnecessary if(fladj) condition
> > - removed stray newline
> >
> >  drivers/usb/dwc3/core.c  | 34
> ++
> >  drivers/usb/dwc3/core.h  |  5 +
> >  drivers/usb/dwc3/platform_data.h |  2 ++
> >  3 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> > 064123e..75a17bf 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -143,6 +143,32 @@ static int dwc3_soft_reset(struct dwc3 *dwc)
> > return 0;
> >  }
> >
> > +/*
> > + * dwc3_frame_length_adjustment - Adjusts frame length if required
> > + * @dwc3: Pointer to our controller context structure
> > + * @fladj: Value of GFLADJ_30MHZ to adjust frame length  */ static
> > +void dwc3_frame_length_adjustment(struct dwc3 *dwc, u32 fladj) {
> > +   if (dwc->revision < DWC3_REVISION_250A)
> > +   return;
> > +
> > +   if (fladj == 0)
> > +   return;
> > +
> > +   u32 reg;
> > +   u32 dft;
> 
> are you seriously telling me that GCC didn't warn you about mixing
> declaration and code ?

That's really careless of me.  Somehow this warning skipped my attention.
Apologies for this silly thing. I will resend the patch.

--
To unsubscribe from this list: send the line "unsubscribe 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][v3] drivers: usb: dwc3: Add frame length adjustment quirk

2015-09-03 Thread Felipe Balbi

On Thu, Sep 03, 2015 at 11:34:04AM +0530, Nikhil Badola wrote:
> Add adjust_frame_length_quirk for writing to fladj register
> which adjusts (micro)frame length to value provided by
> "snps,quirk-frame-length-adjustment" property thus avoiding
> USB 2.0 devices to time-out over a longer run
> 
> Signed-off-by: Nikhil Badola 
> ---
> Changes for v3:
>   - removed unnecessary if(fladj) condition
>   - removed stray newline
> 
>  drivers/usb/dwc3/core.c  | 34 ++
>  drivers/usb/dwc3/core.h  |  5 +
>  drivers/usb/dwc3/platform_data.h |  2 ++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 064123e..75a17bf 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -143,6 +143,32 @@ static int dwc3_soft_reset(struct dwc3 *dwc)
>   return 0;
>  }
>  
> +/*
> + * dwc3_frame_length_adjustment - Adjusts frame length if required
> + * @dwc3: Pointer to our controller context structure
> + * @fladj: Value of GFLADJ_30MHZ to adjust frame length
> + */
> +static void dwc3_frame_length_adjustment(struct dwc3 *dwc, u32 fladj)
> +{
> + if (dwc->revision < DWC3_REVISION_250A)
> + return;
> +
> + if (fladj == 0)
> + return;
> +
> + u32 reg;
> + u32 dft;

are you seriously telling me that GCC didn't warn you about mixing
declaration and code ?

-- 
balbi


signature.asc
Description: Digital signature

[PATCH] usb: musb: Disable interrupts on suspend, enable them on resume

2015-09-03 Thread pascal . huerst

From: Pascal Huerst 

In certain situations, an interrupt triggers on resume, before musb_start()
has been called. This has been observed to cause enumeration issues after
suspend/resume cycles with AM335x.

Signed-off-by: Pascal Huerst 
---
 drivers/usb/musb/musb_core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 68bf6d8..d861c21 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2421,6 +2421,9 @@ static int musb_suspend(struct device *dev)
struct musb *musb = dev_to_musb(dev);
unsigned long   flags;

+   musb_platform_disable(musb);
+   musb_generic_disable(musb);
+
spin_lock_irqsave(>lock, flags);

if (is_peripheral_active(musb)) {
@@ -2474,6 +2477,9 @@ static int musb_resume(struct device *dev)
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
+
+   musb_start(musb);
+
return 0;
 }

--
2.4.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: usb: musb: host: enumeration issues

2015-09-03 Thread Pascal Huerst



On 02.09.2015 22:50, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Sep 02, 2015 at 06:33:11PM +0200, Pascal Huerst wrote:
>> Hey,
>>
>> On 02.09.2015 14:28, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Wed, Sep 02, 2015 at 11:35:45AM +0200, Pascal Huerst wrote:
 Hey Felipe,

 on a custom built, am335x based board, running v4.0, I'm facing an issue
 with usb:

 After suspend, storage devices are not being enumerated correctly. I
>>>
>>> hmm, mainline kernel doesn't support suspend/resume on AM335x devices so
>>> I'm gonna say you need to ask for support from whoever gave you this
>>> kernel. If that's TI, then you need to either ask for help on TI's e2e
>>> forums or talk to your FAE...
>>
>> Well, that would be me. It's a vanilla kernel, except for some ASoC
>> patches and v5 of Dave's PM series, which was for v3.19-rc5, if I
>> remember that correctly, which I rebased on v4.0.0 later.
>>
 have seen and applied your recent patch:

 usb: musb: host: rely on port_mode to call musb_start()

 Now, the issue only happens once. So if the device resumes the first
 time, I face the same enumeration problem, but all following times, it
 seems to work properly.

 I figureed out, that in a working situation:

 void musb_start(...)

 -> http://lxr.free-electrons.com/source/drivers/usb/musb/musb_core.c#L1027

 is called before the isr:

 static irqreturn_t musb_stage0_irq(...)

 -> http://lxr.free-electrons.com/source/drivers/usb/musb/musb_core.c#L539

 And in a non working situation, its the other way arround. (Which should
 not happen!?) I'm not too familiar with usb. Do you have any ideas, what
 could cause that behavior?
>>>
>>> ... with that said, you didn't give me logs of the problem happening nor
>>> gave me any instructions on how to reproduce.
>>>
>>> based on the description alone, I think making sure MUSB IRQs are masked
>>> on suspen and unmask on resume might be enough, though you'd have to
>>> patch that up and check.
>>
>> To reproduce, I just have an usb block device inserted, trigger suspend by:
>>
>> echo mem > /sys/power/state
>>
>> wake it up by GPIO on bank0 and then I get:
>>
>> usb 1-1: new high-speed USB device number 3 using musb-hdrc
>> usb 1-1: new high-speed USB device number 4 using musb-hdrc
>> usb 1-1: new high-speed USB device number 5 using musb-hdrc
>> usb 1-1: new high-speed USB device number 6 using musb-hdrc
>>
>> Since I applied your patch mentioned above, this only happens once
>> (first suspend).  So I had to reboot every time, to provoke it. Without
>> your patch applied, it happened on almost all suspend/resume cycles.
>>
>> If I disable the interrupt in musb_suspend() and reactivate them in
>> musb_resume() everything seems to works fine. (At least the for ~20
>> times I tried to reproduce afterwards)
>>
>> I guess this is the case on other hw was well, such as BBB. If you
>> think, that this is worth further investigation in order to fix it
>> upstream, I can do some testing on BBB and provide you with more detail?
>>
>> Just for clarification about my changes, I applied a patch.
>>
>> regards
>> pascal
>>
>>
>> ---
>>  drivers/usb/musb/musb_core.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 68bf6d8..d861c21 100644
>> --- a/drivers/usb/musb/musb_core.c
>> +++ b/drivers/usb/musb/musb_core.c
>> @@ -2421,6 +2421,9 @@ static int musb_suspend(struct device *dev)
>> struct musb *musb = dev_to_musb(dev);
>> unsigned long   flags;
>>
>> +   musb_platform_disable(musb);
>> +   musb_generic_disable(musb);
>> +
>> spin_lock_irqsave(>lock, flags);
>>
>> if (is_peripheral_active(musb)) {
>> @@ -2474,6 +2477,9 @@ static int musb_resume(struct device *dev)
>> pm_runtime_disable(dev);
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>> +
>> +   musb_start(musb);
> 
> this looks correct to me. Can you send it as a proper patch ?

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

Re: [RFC PATCH] media: uvcvideo: handle urb completion in a work queue

2015-09-03 Thread Laurent Pinchart

Hi Mian Yousaf,

(CC'ing linux-usb for tips regarding proper handling of URBs in work queues)

On Tuesday 01 September 2015 13:49:31 Kaukab, Yousaf wrote:
> On Tuesday, September 1, 2015 2:45 PM Laurent Pinchart wrote:
> > On Tuesday 01 September 2015 11:45:11 Mian Yousaf Kaukab wrote:
> >> urb completion callback is executed in host controllers interrupt
> >> context. To keep preempt disable time short, add an ordered work-
> >> queue. Associate a work_struct with each urb and queue work using it
> >> on urb completion.
> >> 
> >> In uvc_uninit_video, usb_kill_urb and usb_free_urb are separated in
> >> different loops so that workqueue can be destroyed without a lock.
> > 
> > This will change the timing of the uvc_video_clock_decode() call. Have you
> > double-checked that it won't cause any issue ? It will also increase the
> > delay between end of frame reception and timestamp sampling in
> > uvc_video_decode_start(), which I'd like to avoid.
> 
> Can this be fixed by saving the timestamp from uvc_video_get_ts() in
> uvc_urb_complete() and use it in both uvc_video_decode_start() and
> uvc_video_clock_decode()?

Yes, I think that would work. I think it's especially important in 
uvc_video_decode_start(). For uvc_video_clock_decode() it might not matter (I 
won't mind if you investigate whether it's needed ;-)), but if you use the 
saved timestamp there, you should also save the USB frame number along with 
the timestamp as they must match.

> >> Signed-off-by: Mian Yousaf Kaukab 
> >> ---
> >> 
> >>  drivers/media/usb/uvc/uvc_video.c | 63 ++---
> >> 
> >>  drivers/media/usb/uvc/uvcvideo.h  |  9 +-
> >>  2 files changed, 60 insertions(+), 12 deletions(-)
> >> 
> >> diff --git a/drivers/media/usb/uvc/uvc_video.c
> >> b/drivers/media/usb/uvc/uvc_video.c index f839654..943dbd6 100644
> >> --- a/drivers/media/usb/uvc/uvc_video.c
> >> +++ b/drivers/media/usb/uvc/uvc_video.c
> >> @@ -1317,9 +1317,23 @@ static void uvc_video_encode_bulk(struct urb
> >> *urb, struct uvc_streaming *stream, urb->transfer_buffer_length =
> >> stream->urb_size - len;
> >> 
> >>  }
> >> 
> >> -static void uvc_video_complete(struct urb *urb)
> >> +static void uvc_urb_complete(struct urb *urb)
> >>  {
> >> -  struct uvc_streaming *stream = urb->context;
> >> +  struct uvc_urb_work *uw = urb->context;
> >> +  struct uvc_streaming *stream = uw->stream;
> >> +  /* stream->urb_wq can be set to NULL without lock */
> > 
> > That's sound racy. If stream->urb_wq can be set to NULL and the work queue
> > destroyed by uvc_uninit_video() in parallel to the URB completion handler,
> > the work queue could be destroyed between the if (wq) check and the call
> > to queue_work().
> 
> steam->urb_wq is set to NULL after killing all urbs. There should be
> no completion callback when its NULL. This is the reason for two for-
> loops in uvc_uninit_video()

Indeed, I've missed that.

There's still at least one race condition though. The URB completion handler 
uvc_video_complete() is now called from the work queue. It could thus race 
usb_kill_urb(), which will make resubmission of the URB with usb_submit_urb() 
return -EPERM. The driver will then print an error message to the kernel log 
that could worry the user unnecessarily.

I'm in general a bit wary regarding race conditions, and especially when a 
complex function that used to run synchronously is moved to a work queue. I'm 
wondering whether it wouldn't be better to use a lock, as contention would 
only occur at stream stop time.

Could you please double-check possible race conditions ? Keeping the work 
queue around for the whole duration of the device life time might also help 
simplifying the code, but I haven't investigated that.

Another idea that just came to my mind, wouldn't it be better to add URBs to a 
list in their synchronous completion handler and use a normal work queue ? If 
several URBs complete in a row we could possibly avoid some scheduling context 
switches.

> >> +  struct workqueue_struct *wq = stream->urb_wq;
> >> +
> >> +  if (wq)
> >> +  queue_work(wq, >work);
> >> +}

[snip]

> >> @@ -1445,17 +1459,34 @@ static void uvc_uninit_video(struct
> >> uvc_streaming *stream, int free_buffers)
> >>  {
> >>struct urb *urb;
> >>unsigned int i;
> >> +  struct workqueue_struct *wq;
> >>
> >>uvc_video_stats_stop(stream);
> >>
> >> +  /* Kill all URB first so that urb_wq can be destroyed without a
> >> lock
> >> +*/
> >>for (i = 0; i < UVC_URBS; ++i) {
> >> -  urb = stream->urb[i];
> >> +  urb = stream->uw[i].urb;
> >>if (urb == NULL)
> >>continue;
> >>
> >>usb_kill_urb(urb);
> >> +  }
> >> +
> >> +  if (stream->urb_wq) {
> >> +  wq = stream->urb_wq;
> >> +  /* Since all URBs are killed set urb_wq to NULL */
> >> +  stream->urb_wq = NULL;
> >> +  flush_workqueue(wq);
> >> +

Re: [PATCH v4 1/9] usb: dwc3: add dual-role support

2015-09-03 Thread Roger Quadros

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 02/09/15 17:31, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Sep 02, 2015 at 05:24:16PM +0300, Roger Quadros wrote:
>> Register with the USB OTG core. Since we don't support
>> OTG yet we just work as a dual-role device even
>> if device tree says "otg".
>>
>> Use extcon framework to get VBUS/ID cable events and
>> kick the OTG state machine.
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/dwc3/core.c  | 174 
>> ++-
>>  drivers/usb/dwc3/core.h  |   7 ++
>>  drivers/usb/dwc3/platform_data.h |   1 +
>>  3 files changed, 181 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 064123e..2e36a9b 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -704,6 +704,152 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>>  return 0;
>>  }
>>  
>> +/* - Dual-Role management 
>> --- */
>> +
>> +static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
>> +{
>> +int id, vbus;
>> +
>> +/* get ID */
>> +id = extcon_get_cable_state(dwc->edev, "USB-HOST");
>> +/* Host means ID == 0 */
>> +id = !id;
>> +
>> +/* get VBUS */
>> +vbus = extcon_get_cable_state(dwc->edev, "USB");
>> +dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
> 
> tracepoint please. We don't want this driver to use dev_(v)?db anymore.
> Ditto to all others.

OK.

> 
>> +
>> +dwc->fsm->id = id;
>> +dwc->fsm->b_sess_vld = vbus;
>> +usb_otg_sync_inputs(dwc->fsm);
>> +}
>> +
>> +static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
>> +{
>> +struct device *dev = usb_otg_fsm_to_dev(fsm);
>> +struct dwc3 *dwc = dev_get_drvdata(dev);
> 
> how about adding a usb_otg_get_drvdata(fsm) ?

You meant for otg core? That can be done.

> 
>> +dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> +if (on) {
>> +dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> +/* start the HCD */
>> +usb_otg_start_host(fsm, true);
>> +} else {
>> +/* stop the HCD */
>> +usb_otg_start_host(fsm, false);
>> +}
> 
> This can be simplified.
> 
>   if (on)
>   dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> 
>   usb_otg_start_host(fsm, on);

Indeed.

> 
>> +
>> +return 0;
>> +}
>> +
>> +static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> +struct device *dev = usb_otg_fsm_to_dev(fsm);
>> +struct dwc3 *dwc = dev_get_drvdata(dev);
>> +
>> +dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> +if (on) {
>> +dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> +dwc3_event_buffers_setup(dwc);
>> +
>> +/* start the UDC */
>> +usb_otg_start_gadget(fsm, true);
>> +} else {
>> +/* stop the UDC */
>> +usb_otg_start_gadget(fsm, false);
>> +}
> 
> likewise:
> 
>   if (on) {
>   dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>   dwc3_event_buffers_setup(dwc);
>   }
> 
>   usb_otg_start_gadget(fsm, on);

OK.
> 
>> +return 0;
>> +}
>> +
>> +static struct otg_fsm_ops dwc3_drd_ops = {
>> +.start_host = dwc3_drd_start_host,
>> +.start_gadget = dwc3_drd_start_gadget,
>> +};
>> +
>> +static int dwc3_drd_notifier(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> +struct dwc3 *dwc = container_of(nb, struct dwc3, otg_nb);
>> +
>> +dwc3_drd_fsm_sync(dwc);
>> +
>> +return NOTIFY_DONE;
>> +}
>> +
>> +static int dwc3_drd_init(struct dwc3 *dwc)
>> +{
>> +int ret, id, vbus;
>> +struct usb_otg_caps *otgcaps = >otg_config.otg_caps;
>> +
>> +otgcaps->otg_rev = 0;
>> +otgcaps->hnp_support = false;
>> +otgcaps->srp_support = false;
>> +otgcaps->adp_support = false;
>> +dwc->otg_config.fsm_ops = _drd_ops;
>> +
>> +if (!dwc->edev) {
>> +dev_err(dwc->dev, "No extcon device found for OTG mode\n");
>> +return -ENODEV;
>> +}
>> +
>> +dwc->otg_nb.notifier_call = dwc3_drd_notifier;
>> +ret = extcon_register_notifier(dwc->edev, EXTCON_USB, >otg_nb);
>> +if (ret < 0) {
>> +dev_err(dwc->dev, "Couldn't register USB cable notifier\n");
>> +return -ENODEV;
>> +}
>> +
>> +ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
>> +   >otg_nb);
>> +if (ret < 0) {
>> +dev_err(dwc->dev, "Couldn't register USB-HOST cable 
>> notifier\n");
>> +ret = -ENODEV;
>> +goto extcon_fail;
>> +}
>> +
>> +/* sanity check id & vbus states */
>> +id = extcon_get_cable_state(dwc->edev, "USB-HOST");
>> +vbus = extcon_get_cable_state(dwc->edev, "USB");
>> +if (id < 0 || vbus < 0) {
>> +dev_err(dwc->dev, "Invalid USB cable state. id %d, vbus %d\n",
>> +

[PATCH 1/3][v4] Documentation: dt: dwc3: Add snps,quirk-frame-length-adjustment property

2015-09-03 Thread Nikhil Badola

Add snps,quirk-frame-length-adjustment property which provides value
for post silicon frame length adjustment

Signed-off-by: Nikhil Badola 
---
Changes for v4: None

 Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 0815eac..8c7d585 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -40,6 +40,9 @@ Optional properties:
  - snps,hird-threshold: HIRD threshold
  - snps,hsphy_interface: High-Speed PHY interface selection between "utmi" for
UTMI+ and "ulpi" for ULPI when the DWC_USB3_HSPHY_INTERFACE has value 3.
+ - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
+   register for post-silicon frame length adjustment when the
+   fladj_30mhz_sdbnd signal is invalid or incorrect.
 
 This is usually a subnode to DWC3 glue to which it is connected.
 
-- 
2.1.0

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

[PATCH 2/3][v4] drivers: usb: dwc3: Add frame length adjustment quirk

2015-09-03 Thread Nikhil Badola

Add adjust_frame_length_quirk for writing to fladj register
which adjusts (micro)frame length to value provided by
"snps,quirk-frame-length-adjustment" property thus avoiding
USB 2.0 devices to time-out over a longer run

Signed-off-by: Nikhil Badola 
---
Changes for v4: Removed mixed declerations and code

 drivers/usb/dwc3/core.c  | 34 ++
 drivers/usb/dwc3/core.h  |  5 +
 drivers/usb/dwc3/platform_data.h |  2 ++
 3 files changed, 41 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 064123e..6270581 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -143,6 +143,32 @@ static int dwc3_soft_reset(struct dwc3 *dwc)
return 0;
 }
 
+/*
+ * dwc3_frame_length_adjustment - Adjusts frame length if required
+ * @dwc3: Pointer to our controller context structure
+ * @fladj: Value of GFLADJ_30MHZ to adjust frame length
+ */
+static void dwc3_frame_length_adjustment(struct dwc3 *dwc, u32 fladj)
+{
+   u32 reg;
+   u32 dft;
+
+   if (dwc->revision < DWC3_REVISION_250A)
+   return;
+
+   if (fladj == 0)
+   return;
+
+   reg = dwc3_readl(dwc->regs, DWC3_GFLADJ);
+   dft = reg & DWC3_GFLADJ_30MHZ_MASK;
+   if (!dev_WARN_ONCE(dwc->dev, dft == fladj,
+   "request value same as default, ignoring\n")) {
+   reg &= ~DWC3_GFLADJ_30MHZ_MASK;
+   reg |= DWC3_GFLADJ_30MHZ_SDBND_SEL | fladj;
+   dwc3_writel(dwc->regs, DWC3_GFLADJ, reg);
+   }
+}
+
 /**
  * dwc3_free_one_event_buffer - Frees one event buffer
  * @dwc: Pointer to our controller context structure
@@ -779,6 +805,7 @@ static int dwc3_probe(struct platform_device *pdev)
u8  lpm_nyet_threshold;
u8  tx_de_emphasis;
u8  hird_threshold;
+   u32 fladj = 0;
 
int ret;
 
@@ -886,6 +913,9 @@ static int dwc3_probe(struct platform_device *pdev)
_de_emphasis);
of_property_read_string(node, "snps,hsphy_interface",
>hsphy_interface);
+   of_property_read_u32(node,
+"snps,quirk-frame-length-adjustment",
+);
} else if (pdata) {
dwc->maximum_speed = pdata->maximum_speed;
dwc->has_lpm_erratum = pdata->has_lpm_erratum;
@@ -915,6 +945,7 @@ static int dwc3_probe(struct platform_device *pdev)
tx_de_emphasis = pdata->tx_de_emphasis;
 
dwc->hsphy_interface = pdata->hsphy_interface;
+   fladj = pdata->fladj_value;
}
 
/* default to superspeed if no maximum_speed passed */
@@ -971,6 +1002,9 @@ static int dwc3_probe(struct platform_device *pdev)
goto err1;
}
 
+   /* Adjust Frame Length */
+   dwc3_frame_length_adjustment(dwc, fladj);
+
usb_phy_set_suspend(dwc->usb2_phy, 0);
usb_phy_set_suspend(dwc->usb3_phy, 0);
ret = phy_power_on(dwc->usb2_generic_phy);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0447788..9188745 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -124,6 +124,7 @@
 #define DWC3_GEVNTCOUNT(n) (0xc40c + (n * 0x10))
 
 #define DWC3_GHWPARAMS80xc600
+#define DWC3_GFLADJ0xc630
 
 /* Device Registers */
 #define DWC3_DCFG  0xc700
@@ -234,6 +235,10 @@
 /* Global HWPARAMS6 Register */
 #define DWC3_GHWPARAMS6_EN_FPGA(1 << 7)
 
+/* Global Frame Length Adjustment Register */
+#define DWC3_GFLADJ_30MHZ_SDBND_SEL(1 << 7)
+#define DWC3_GFLADJ_30MHZ_MASK 0x3f
+
 /* Device Configuration Register */
 #define DWC3_DCFG_DEVADDR(addr)((addr) << 3)
 #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index d3614ec..400b197 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -46,5 +46,7 @@ struct dwc3_platform_data {
unsigned tx_de_emphasis_quirk:1;
unsigned tx_de_emphasis:2;
 
+   u32 fladj_value;
+
const char *hsphy_interface;
 };
-- 
2.1.0

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

[PATCH 3/3][v4] arm: dts: ls1021a: Add quirk for Erratum A009116

2015-09-03 Thread Nikhil Badola

Add "snps,quirk-frame-length-adjustment" property to
USB3 node for erratum A009116. This property provides
value of GFLADJ_30MHZ for post silicon frame length
adjustment.

Signed-off-by: Nikhil Badola 
---
Changes for v4: None

 arch/arm/boot/dts/ls1021a.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index c70bb27..50ac0d4 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -404,6 +404,7 @@
reg = <0x0 0x310 0x0 0x1>;
interrupts = ;
dr_mode = "host";
+   snps,quirk-frame-length-adjustment = <0x20>;
};
};
 };
-- 
2.1.0

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

Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts

2015-09-03 Thread Roger Quadros

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 02/09/15 17:34, Felipe Balbi wrote:
> On Wed, Sep 02, 2015 at 05:24:19PM +0300, Roger Quadros wrote:
>> From: Felipe Balbi 
>>
>> Add support to use interrupt names,
>>
>> Following are the interrupt names
>>
>> Peripheral Interrupt - peripheral
>> HOST Interrupt - host
>> OTG Interrupt - otg
>>
>> [Roger Q]
>> - If any of these are missing we use the
>> first available IRQ resource so that we don't
>> break with older DTBs.
> 
> this is what original commit did:
> 
> commit ecd5f71cfd663bcd4efd2f29824acd8b2ba9715d
> Author: Felipe Balbi 
> Date:   Fri Jan 3 13:49:38 2014 -0600
> 
> usb: dwc3: cleanup IRQ resources
> 
> In order to make it easier for the driver to
> figure out which modes of operation it has,
> and because some dwc3 integrations have rather
> fuzzy IRQ routing, we now require three different
> IRQ numbers (peripheral, host, otg).
> 
> In order to do that and maintain backwards compatibility,
> we still maintain support for the old IRQ resource name,
> but if you *really* want to have proper peripheral/host/otg
> support, you should make sure your resources are correct.
> 
> Signed-off-by: Felipe Balbi 

This is better since we request the resource only if needed and bail out
if it is not present.

> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 60580a01cdd2..1f01031873b7 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -556,10 +556,22 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>  static int dwc3_core_init_mode(struct dwc3 *dwc)
>  {
>   struct device *dev = dwc->dev;
> + struct platform_device *pdev = to_platform_device(dev);
>   int ret;
>  
>   switch (dwc->dr_mode) {
>   case USB_DR_MODE_PERIPHERAL:
> + dwc->gadget_irq = platform_get_irq_byname(pdev, 
> "dwc3_peripheral");

Shall we just name it just "peripheral"?

> + if (dwc->gadget_irq < 0) {
> + dwc->gadget_irq = platform_get_irq_byname(pdev, 
> "dwc_usb3");

How will this work? Currently we don't have a name for the IRQ in the DT.

> + if (dwc->gadget_irq < 0) {
> + dev_err(dev, "missing IRQ\n");
> + return dwc->gadget_irq;
> + } else {
> + dev_warn(dev, "dwc_usb3 resource is 
> deprecated\n");

Do we want to warn about legacy nodes?

> + }
> + }
> +
>   dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>   ret = dwc3_gadget_init(dwc);
>   if (ret) {
> @@ -568,6 +580,22 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>   }
>   break;
>   case USB_DR_MODE_HOST:
> + dwc->xhci_irq = platform_get_irq_byname(pdev, "dwc3_host");
> + if (dwc->xhci_irq < 0) {
> + dwc->xhci_irq = platform_get_irq_byname(pdev, 
> "dwc_usb3");
> + if (dwc->xhci_irq < 0) {
> + dev_err(dev, "missing Host IRQ\n");
> + return dwc->xhci_irq;
> + } else {
> + dev_warn(dev, "dwc_usb3 resource is 
> deprecated\n");
> + }
> +
> + dwc->xhci_resources[1].start = dwc->xhci_irq;
> + dwc->xhci_resources[1].end = dwc->xhci_irq;
> + dwc->xhci_resources[1].flags = IORESOURCE_IRQ;
> + dwc->xhci_resources[1].name = "xhci";
> + }
> +
>   dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>   ret = dwc3_host_init(dwc);
>   if (ret) {
> @@ -576,6 +604,28 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>   }
>   break;
>   case USB_DR_MODE_OTG:
> + dwc->gadget_irq = platform_get_irq_byname(pdev, 
> "dwc3_peripheral");
> + if (dwc->gadget_irq < 0) {
> + dwc->gadget_irq = platform_get_irq_byname(pdev, 
> "dwc_usb3");
> + if (dwc->gadget_irq < 0) {
> + dev_err(dev, "missing IRQ\n");
> + return dwc->gadget_irq;
> + } else {
> + dev_warn(dev, "dwc_usb3 resource is 
> deprecated\n");
> + }
> +
> + dwc->xhci_irq = dwc->gadget_irq;
> + dwc->otg_irq = dwc->gadget_irq;
> + } else {
> + dwc->xhci_irq = platform_get_irq_byname(pdev, 
> "dwc3_host");
> + if (dwc->xhci_irq < 0) {
> + dev_err(dev, "missing Host IRQ\n");
> + return dwc->xhci_irq;
> + }
> +
> + dwc->otg_irq = platform_get_irq_byname(pdev, 
>

[PATCH] USB: serial: add Moxa UPORT 11x0 driver

2015-09-03 Thread Mathieu OTHACEHE

Add a driver which supports :

- UPort 1110  : 1 port RS-232 USB to Serial Hub.
- UPort 1130  : 1 port RS-422/485 USB to Serial Hub.
- UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation.
- UPort 1150  : 1 port RS-232/422/485 USB to Serial Hub.
- UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation.

This driver is based on GPL MOXA driver written by Hen Huang and available
on MOXA website. The original driver was based on io_ti serial driver.

Signed-off-by: Mathieu OTHACEHE 
---
 drivers/usb/serial/Kconfig   |   16 +
 drivers/usb/serial/Makefile  |1 +
 drivers/usb/serial/mxu11x0.c | 1500 ++
 drivers/usb/serial/mxu11x0.h |  199 ++
 4 files changed, 1716 insertions(+)
 create mode 100644 drivers/usb/serial/mxu11x0.c
 create mode 100644 drivers/usb/serial/mxu11x0.h

diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 56ecb8b..f612dda 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -475,6 +475,22 @@ config USB_SERIAL_MOS7840
  To compile this driver as a module, choose M here: the
  module will be called mos7840.  If unsure, choose N.
 
+config USB_SERIAL_MXUPORT11
+   tristate "USB Moxa UPORT 11x0 Serial Driver"
+   ---help---
+ Say Y here if you want to use a MOXA UPort 11x0 Serial hub.
+
+ This driver supports:
+
+ - UPort 1110  : 1 port RS-232 USB to Serial Hub.
+ - UPort 1130  : 1 port RS-422/485 USB to Serial Hub.
+ - UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation.
+ - UPort 1150  : 1 port RS-232/422/485 USB to Serial Hub.
+ - UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with 
Isolation.
+
+ To compile this driver as a module, choose M here: the
+ module will be called mxu11x0.
+
 config USB_SERIAL_MXUPORT
tristate "USB Moxa UPORT Serial Driver"
---help---
diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile
index 349d9df..f3fa5e5 100644
--- a/drivers/usb/serial/Makefile
+++ b/drivers/usb/serial/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_USB_SERIAL_METRO)+= 
metro-usb.o
 obj-$(CONFIG_USB_SERIAL_MOS7720)   += mos7720.o
 obj-$(CONFIG_USB_SERIAL_MOS7840)   += mos7840.o
 obj-$(CONFIG_USB_SERIAL_MXUPORT)   += mxuport.o
+obj-$(CONFIG_USB_SERIAL_MXUPORT11) += mxu11x0.o
 obj-$(CONFIG_USB_SERIAL_NAVMAN)+= navman.o
 obj-$(CONFIG_USB_SERIAL_OMNINET)   += omninet.o
 obj-$(CONFIG_USB_SERIAL_OPTICON)   += opticon.o
diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
new file mode 100644
index 000..20fa15d
--- /dev/null
+++ b/drivers/usb/serial/mxu11x0.c
@@ -0,0 +1,1500 @@
+/*
+ *
+ * USB Moxa UPORT 11x0 Serial Driver
+ *
+ * Copyright (C) 2007 MOXA Technologies Co., Ltd.
+ * Copyright (C) 2015 Mathieu Othacehe 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ *
+ * Supports the following Moxa USB to serial converters:
+ *  UPort 1110,  1 port RS-232 USB to Serial Hub.
+ *  UPort 1130,  1 port RS-422/485 USB to Serial Hub.
+ *  UPort 1130I, 1 port RS-422/485 USB to Serial Hub with isolation
+ *protection.
+ *  UPort 1150,  1 port RS-232/422/485 USB to Serial Hub.
+ *  UPort 1150I, 1 port RS-232/422/485 USB to Serial Hub with isolation
+ *  protection.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "mxu11x0.h"
+
+struct mxu1_port {
+   int mxp_is_open;
+   __u8 mxp_msr;
+   __u8 mxp_lsr;
+   __u8 mxp_shadow_mcr;
+   __u8 mxp_uart_mode; /* 232 or 485 modes */
+   __u8 mxp_user_get_uart_mode;
+   unsigned int mxp_uart_base_addr;
+   int mxp_flags;
+   int mxp_msr_wait_flags;
+   struct async_icount mxp_icount;
+   wait_queue_head_t mxp_msr_wait; /* wait for msr change */
+   struct mxu1_device *mxp_mxdev;
+   struct usb_serial_port *mxp_port;
+   spinlock_t mxp_lock;
+   int mxp_read_urb_state;
+   int mxp_write_urb_in_use;
+   int mxp_send_break;
+   int mxp_set_B0;
+};
+
+struct mxu1_device {
+   struct mutex mxd_lock;
+   int mxd_open_port_count;
+   struct usb_serial *mxd_serial;
+   int mxd_model_name;
+};
+
+/* supported setserial flags */
+#define MXU1_SET_SERIAL_FLAGS  (ASYNC_LOW_LATENCY)
+
+#define MXU1_DEFAULT_LOW_LATENCY1
+
+#define MXU1_TRANSFER_TIMEOUT  2
+#define MXU1_MSR_WAIT_TIMEOUT  (5 * HZ)
+
+/* Configuration ids */
+#define MXU1_BOOT_CONFIG   1
+#define

Re: [PATCH] usb/gadget/function/u_ether.c: Allow jumbo frames

Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq

Re: [PATCH v4 8/9] usb: dwc3: core: Prevent otg events from disabling themselves

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

Re: [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

Re: [PATCH] usb/gadget/function/u_ether.c: Allow jumbo frames

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq

Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role

RE: [PATCH v2 1/1] usb: dwc2: gadget: parity fix in isochronous mode

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

Re: [PATCH v4 1/9] usb: dwc3: add dual-role support

RE: [PATCH 2/3][v3] drivers: usb: dwc3: Add frame length adjustment quirk

Re: [PATCH 2/3][v3] drivers: usb: dwc3: Add frame length adjustment quirk

[PATCH] usb: musb: Disable interrupts on suspend, enable them on resume

Re: usb: musb: host: enumeration issues

Re: [RFC PATCH] media: uvcvideo: handle urb completion in a work queue

Re: [PATCH v4 1/9] usb: dwc3: add dual-role support

[PATCH 1/3][v4] Documentation: dt: dwc3: Add snps,quirk-frame-length-adjustment property

[PATCH 2/3][v4] drivers: usb: dwc3: Add frame length adjustment quirk

[PATCH 3/3][v4] arm: dts: ls1021a: Add quirk for Erratum A009116

Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts

[PATCH] USB: serial: add Moxa UPORT 11x0 driver

27 matches

Site Navigation

Mail list logo

Footer information