Re: [PATCH v2 1/1] usb: chipidea: move the lock initialization to core file

[Peter Chen]

On Tue, Nov 15, 2016 at 07:58:16AM +0100, Greg KH wrote:
> On Tue, Nov 15, 2016 at 02:02:47PM +0800, Peter Chen wrote:
> > This can fix below dump when the lock is accessed at host
> > mode due to it is not initialized.
> > 
> > [   46.119638] INFO: trying to register non-static key.
> > [   46.124643] the code is fine but needs lockdep annotation.
> > [   46.130144] turning off the locking correctness validator.
> > [   46.135659] CPU: 0 PID: 690 Comm: cat Not tainted 
> > 4.9.0-rc3-00079-g4b75f1d #1210
> > [   46.143075] Hardware name: Freescale i.MX6 SoloX (Device Tree)
> > [   46.148923] Backtrace:
> > [   46.151448] [] (dump_backtrace) from [] 
> > (show_stack+0x18/0x1c)
> > [   46.159038]  r7:edf52000
> > [   46.161412]  r6:6193
> > [   46.163967]  r5:
> > [   46.165035]  r4:c0e25c2c
> > 
> > [   46.169109] [] (show_stack) from [] 
> > (dump_stack+0xb4/0xe8)
> > [   46.176362] [] (dump_stack) from [] 
> > (register_lock_class+0x4fc/0x56c)
> > [   46.184554]  r10:c0e25d24
> > [   46.187014]  r9:edf53e70
> > [   46.189569]  r8:c1642444
> > [   46.190637]  r7:ee9da024
> > [   46.193191]  r6:
> > [   46.194258]  r5:
> > [   46.196812]  r4:
> > [   46.199185]  r3:0001
> > 
> > [   46.203259] [] (register_lock_class) from [] 
> > (__lock_acquire+0x80/0x10f0)
> > [   46.211797]  r10:c0e25d24
> > [   46.214257]  r9:edf53e70
> > [   46.216813]  r8:ee9da024
> > [   46.217880]  r7:c1642444
> > [   46.220435]  r6:edcd1800
> > [   46.221502]  r5:6193
> > [   46.224057]  r4:
> > 
> > [   46.227953] [] (__lock_acquire) from [] 
> > (lock_acquire+0x74/0x94)
> > [   46.235710]  r10:0001
> > [   46.238169]  r9:edf53e70
> > [   46.240723]  r8:edf53f80
> > [   46.241790]  r7:0001
> > [   46.244344]  r6:0001
> > [   46.245412]  r5:6193
> > [   46.247966]  r4:
> > 
> > [   46.251866] [] (lock_acquire) from [] 
> > (_raw_spin_lock_irqsave+0x40/0x54)
> > [   46.260319]  r7:ee1c6a00
> > [   46.262691]  r6:c062a570
> > [   46.265247]  r5:2113
> > [   46.266314]  r4:ee9da014
> > 
> > [   46.270393] [] (_raw_spin_lock_irqsave) from [] 
> > (ci_port_test_show+0x2c/0x70)
> > [   46.279280]  r6:eebd2000
> > [   46.281652]  r5:ee9da010
> > [   46.284207]  r4:ee9da014
> > 
> > [   46.286810] [] (ci_port_test_show) from [] 
> > (seq_read+0x1ac/0x4f8)
> > [   46.294655]  r9:edf53e70
> > [   46.297028]  r8:edf53f80
> > [   46.299583]  r7:ee1c6a00
> > [   46.300650]  r6:0001
> > [   46.303205]  r5:
> > [   46.304273]  r4:eebd2000
> > [   46.306850] [] (seq_read) from [] 
> > (full_proxy_read+0x54/0x6c)
> > [   46.314348]  r10:
> > [   46.316808]  r9:c0a6ad30
> > [   46.319363]  r8:edf53f80
> > [   46.320430]  r7:0002
> > [   46.322986]  r6:b6de3000
> > [   46.324053]  r5:ee1c6a00
> > [   46.326607]  r4:c0248b58
> > 
> > [   46.330505] [] (full_proxy_read) from [] 
> > (__vfs_read+0x34/0x118)
> > [   46.338262]  r9:edf52000
> > [   46.340635]  r8:c0107fc4
> > [   46.343190]  r7:0002
> > [   46.344257]  r6:edf53f80
> > [   46.346812]  r5:c039e810
> > [   46.347879]  r4:ee1c6a00
> > [   46.350447] [] (__vfs_read) from [] 
> > (vfs_read+0x8c/0x11c)
> > [   46.357597]  r9:edf52000
> > [   46.359969]  r8:c0107fc4
> > [   46.362524]  r7:edf53f80
> > [   46.363592]  r6:b6de3000
> > [   46.366147]  r5:ee1c6a00
> > [   46.367214]  r4:0002
> > [   46.369782] [] (vfs_read) from [] 
> > (SyS_read+0x4c/0xa8)
> > [   46.376672]  r8:c0107fc4
> > [   46.379045]  r7:0002
> > [   46.381600]  r6:b6de3000
> > [   46.382667]  r5:ee1c6a00
> > [   46.385222]  r4:ee1c6a00
> > 
> > [   46.387817] [] (SyS_read) from [] 
> > (ret_fast_syscall+0x0/0x1c)
> > [   46.395314]  r7:0003
> > [   46.397687]  r6:b6de3000
> > [   46.400243]  r5:0002
> > [   46.401310]  r4:0002
> > 
> > Cc:  #v4.1+
> 
> Any idea what commit caused the problem?
> 

This issue has been existed from this driver has written, and the
structure refined later, this patch can be applied after that
(4 years ago), I think v4.1 is the last stable tree it can be
applied.

-- 

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: [PATHCv10 1/2] usb: USB Type-C connector class

[Greg KH]

On Mon, Nov 14, 2016 at 12:46:50PM -0800, Guenter Roeck wrote:
> On Mon, Nov 14, 2016 at 02:32:35PM +0200, Heikki Krogerus wrote:
> > Hi Greg,
> > 
> > On Mon, Nov 14, 2016 at 10:51:48AM +0100, Greg KH wrote:
> > > On Mon, Sep 19, 2016 at 02:16:56PM +0300, Heikki Krogerus wrote:
> > > > The purpose of USB Type-C connector class is to provide
> > > > unified interface for the user space to get the status and
> > > > basic information about USB Type-C connectors on a system,
> > > > control over data role swapping, and when the port supports
> > > > USB Power Delivery, also control over power role swapping
> > > > and Alternate Modes.
> > > > 
> > > > Reviewed-by: Guenter Roeck 
> > > > Tested-by: Guenter Roeck 
> > > > Signed-off-by: Heikki Krogerus 
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-class-typec |  218 ++
> > > >  Documentation/usb/typec.txt |  103 +++
> > > >  MAINTAINERS |9 +
> > > >  drivers/usb/Kconfig |2 +
> > > >  drivers/usb/Makefile|2 +
> > > >  drivers/usb/typec/Kconfig   |7 +
> > > >  drivers/usb/typec/Makefile  |1 +
> > > >  drivers/usb/typec/typec.c   | 1075 
> > > > +++
> > > >  include/linux/usb/typec.h   |  252 +++
> > > >  9 files changed, 1669 insertions(+)
> > > >  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
> > > >  create mode 100644 Documentation/usb/typec.txt
> > > >  create mode 100644 drivers/usb/typec/Kconfig
> > > >  create mode 100644 drivers/usb/typec/Makefile
> > > >  create mode 100644 drivers/usb/typec/typec.c
> > > >  create mode 100644 include/linux/usb/typec.h
> > > 
> [ ... ]
> 
> > > > +
> > > > +int typec_connect(struct typec_port *port, struct typec_connection 
> > > > *con)
> > > > +{
> > > > +   int ret;
> > > > +
> > > > +   if (!con->partner && !con->cable)
> > > > +   return -EINVAL;
> > > > +
> > > > +   port->connected = 1;
> > > > +   port->data_role = con->data_role;
> > > > +   port->pwr_role = con->pwr_role;
> > > > +   port->vconn_role = con->vconn_role;
> > > > +   port->pwr_opmode = con->pwr_opmode;
> > > > +
> > > > +   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> > > 
> > > This worries me.  Who is listening for it?  What will you do with it?
> > > Shouldn't you just poll on an attribute file instead?
> > 
> > Oliver! Did you need this or can we remove it?
> > 
> > I remember I removed the "connected" attribute because you did not see
> > any use for it at one point. I don't remember the reason exactly why?
> > 
> 
> The Android team tells me that they are currently using the udev events
> to track port role changes, and to detect presence of port partner.
> 
> Also, there are plans to track changes on usbc*cable to differentiate
> between cable attach vs. device being attached on the remote end. 
> 
> What is the problem with using kobject_uevent() and thus presumably
> udev events ?

It's not a "normal" thing to do and is pretty "heavy" to do.  What does
userspace do with that change event?  Does it read specific attributes?
What causes the event to happen in the kernel, is it really just a
change in the specific object, or do new ones get added/removed?

In short, document the heck out of this please so people know how to use
it, and what is happening when the event happens.

thanks,

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: [GIT PULL] USB: chipidea fixes for v4.9

[Peter Chen]

On Tue, Nov 15, 2016 at 07:59:51AM +0100, Greg KH wrote:
> On Tue, Nov 15, 2016 at 02:05:49PM +0800, Peter Chen wrote:
> > On Mon, Nov 14, 2016 at 12:48:45PM +0100, Greg KH wrote:
> > > On Mon, Nov 14, 2016 at 10:01:53AM +0800, Peter Chen wrote:
> > > > The following changes since commit 
> > > > 18266403f3fe507f0246faa1d5432333a2f139ca:
> > > > 
> > > >   USB: cdc-acm: fix TIOCMIWAIT (2016-11-10 13:12:59 +0100)
> > > > 
> > > > are available in the git repository at:
> > > > 
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ 
> > > > tags/usb-ci-v4.9-rc6
> > > > 
> > > > for you to fetch changes up to 62ee9585fed19e4de9c3941b9c74044205ae13bd:
> > > > 
> > > >   usb: chipidea: move the lock initialization to core file (2016-11-14 
> > > > 09:42:25 +0800)
> > > > 
> > > > 
> > > > One patch to fix kernel dump for non-initialized lock
> > > 
> > > Is this a regression, or fixing something new?  Can you just post the
> > > one patch?  That's easier than a git pull request...
> > > 
> > 
> > It is a new fix. Patch v2 has sent.
> 
> Oh, I didn't realize that was the fix you were trying to send.  Please
> be more explicit.
> 
> > What's criterion for sending patch and pull request? The number of Patch? 
> > How
> > many?
> 
> Given that I found a problem with your first version, maybe we should
> switch back to patches?  There is no hard-and-fast rule as to which is
> better here, I can handle either just as well, with patches usually
> working quicker than pull requests as they fit into my workflow easier.
> 

I see, thanks.

Please omit this git pull request

-- 

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: [GIT PULL] USB: chipidea fixes for v4.9

[Greg KH]

On Tue, Nov 15, 2016 at 02:05:49PM +0800, Peter Chen wrote:
> On Mon, Nov 14, 2016 at 12:48:45PM +0100, Greg KH wrote:
> > On Mon, Nov 14, 2016 at 10:01:53AM +0800, Peter Chen wrote:
> > > The following changes since commit 
> > > 18266403f3fe507f0246faa1d5432333a2f139ca:
> > > 
> > >   USB: cdc-acm: fix TIOCMIWAIT (2016-11-10 13:12:59 +0100)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ 
> > > tags/usb-ci-v4.9-rc6
> > > 
> > > for you to fetch changes up to 62ee9585fed19e4de9c3941b9c74044205ae13bd:
> > > 
> > >   usb: chipidea: move the lock initialization to core file (2016-11-14 
> > > 09:42:25 +0800)
> > > 
> > > 
> > > One patch to fix kernel dump for non-initialized lock
> > 
> > Is this a regression, or fixing something new?  Can you just post the
> > one patch?  That's easier than a git pull request...
> > 
> 
> It is a new fix. Patch v2 has sent.

Oh, I didn't realize that was the fix you were trying to send.  Please
be more explicit.

> What's criterion for sending patch and pull request? The number of Patch? How
> many?

Given that I found a problem with your first version, maybe we should
switch back to patches?  There is no hard-and-fast rule as to which is
better here, I can handle either just as well, with patches usually
working quicker than pull requests as they fit into my workflow easier.

thanks,

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 v2 1/1] usb: chipidea: move the lock initialization to core file

[Greg KH]

On Tue, Nov 15, 2016 at 02:02:47PM +0800, Peter Chen wrote:
> This can fix below dump when the lock is accessed at host
> mode due to it is not initialized.
> 
> [   46.119638] INFO: trying to register non-static key.
> [   46.124643] the code is fine but needs lockdep annotation.
> [   46.130144] turning off the locking correctness validator.
> [   46.135659] CPU: 0 PID: 690 Comm: cat Not tainted 4.9.0-rc3-00079-g4b75f1d 
> #1210
> [   46.143075] Hardware name: Freescale i.MX6 SoloX (Device Tree)
> [   46.148923] Backtrace:
> [   46.151448] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [   46.159038]  r7:edf52000
> [   46.161412]  r6:6193
> [   46.163967]  r5:
> [   46.165035]  r4:c0e25c2c
> 
> [   46.169109] [] (show_stack) from [] 
> (dump_stack+0xb4/0xe8)
> [   46.176362] [] (dump_stack) from [] 
> (register_lock_class+0x4fc/0x56c)
> [   46.184554]  r10:c0e25d24
> [   46.187014]  r9:edf53e70
> [   46.189569]  r8:c1642444
> [   46.190637]  r7:ee9da024
> [   46.193191]  r6:
> [   46.194258]  r5:
> [   46.196812]  r4:
> [   46.199185]  r3:0001
> 
> [   46.203259] [] (register_lock_class) from [] 
> (__lock_acquire+0x80/0x10f0)
> [   46.211797]  r10:c0e25d24
> [   46.214257]  r9:edf53e70
> [   46.216813]  r8:ee9da024
> [   46.217880]  r7:c1642444
> [   46.220435]  r6:edcd1800
> [   46.221502]  r5:6193
> [   46.224057]  r4:
> 
> [   46.227953] [] (__lock_acquire) from [] 
> (lock_acquire+0x74/0x94)
> [   46.235710]  r10:0001
> [   46.238169]  r9:edf53e70
> [   46.240723]  r8:edf53f80
> [   46.241790]  r7:0001
> [   46.244344]  r6:0001
> [   46.245412]  r5:6193
> [   46.247966]  r4:
> 
> [   46.251866] [] (lock_acquire) from [] 
> (_raw_spin_lock_irqsave+0x40/0x54)
> [   46.260319]  r7:ee1c6a00
> [   46.262691]  r6:c062a570
> [   46.265247]  r5:2113
> [   46.266314]  r4:ee9da014
> 
> [   46.270393] [] (_raw_spin_lock_irqsave) from [] 
> (ci_port_test_show+0x2c/0x70)
> [   46.279280]  r6:eebd2000
> [   46.281652]  r5:ee9da010
> [   46.284207]  r4:ee9da014
> 
> [   46.286810] [] (ci_port_test_show) from [] 
> (seq_read+0x1ac/0x4f8)
> [   46.294655]  r9:edf53e70
> [   46.297028]  r8:edf53f80
> [   46.299583]  r7:ee1c6a00
> [   46.300650]  r6:0001
> [   46.303205]  r5:
> [   46.304273]  r4:eebd2000
> [   46.306850] [] (seq_read) from [] 
> (full_proxy_read+0x54/0x6c)
> [   46.314348]  r10:
> [   46.316808]  r9:c0a6ad30
> [   46.319363]  r8:edf53f80
> [   46.320430]  r7:0002
> [   46.322986]  r6:b6de3000
> [   46.324053]  r5:ee1c6a00
> [   46.326607]  r4:c0248b58
> 
> [   46.330505] [] (full_proxy_read) from [] 
> (__vfs_read+0x34/0x118)
> [   46.338262]  r9:edf52000
> [   46.340635]  r8:c0107fc4
> [   46.343190]  r7:0002
> [   46.344257]  r6:edf53f80
> [   46.346812]  r5:c039e810
> [   46.347879]  r4:ee1c6a00
> [   46.350447] [] (__vfs_read) from [] 
> (vfs_read+0x8c/0x11c)
> [   46.357597]  r9:edf52000
> [   46.359969]  r8:c0107fc4
> [   46.362524]  r7:edf53f80
> [   46.363592]  r6:b6de3000
> [   46.366147]  r5:ee1c6a00
> [   46.367214]  r4:0002
> [   46.369782] [] (vfs_read) from [] (SyS_read+0x4c/0xa8)
> [   46.376672]  r8:c0107fc4
> [   46.379045]  r7:0002
> [   46.381600]  r6:b6de3000
> [   46.382667]  r5:ee1c6a00
> [   46.385222]  r4:ee1c6a00
> 
> [   46.387817] [] (SyS_read) from [] 
> (ret_fast_syscall+0x0/0x1c)
> [   46.395314]  r7:0003
> [   46.397687]  r6:b6de3000
> [   46.400243]  r5:0002
> [   46.401310]  r4:0002
> 
> Cc:  #v4.1+

Any idea what commit caused the problem?

thanks,

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

[PATCH v2 1/1] usb: chipidea: move the lock initialization to core file

[Peter Chen]

This can fix below dump when the lock is accessed at host
mode due to it is not initialized.

[   46.119638] INFO: trying to register non-static key.
[   46.124643] the code is fine but needs lockdep annotation.
[   46.130144] turning off the locking correctness validator.
[   46.135659] CPU: 0 PID: 690 Comm: cat Not tainted 4.9.0-rc3-00079-g4b75f1d 
#1210
[   46.143075] Hardware name: Freescale i.MX6 SoloX (Device Tree)
[   46.148923] Backtrace:
[   46.151448] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[   46.159038]  r7:edf52000
[   46.161412]  r6:6193
[   46.163967]  r5:
[   46.165035]  r4:c0e25c2c

[   46.169109] [] (show_stack) from [] 
(dump_stack+0xb4/0xe8)
[   46.176362] [] (dump_stack) from [] 
(register_lock_class+0x4fc/0x56c)
[   46.184554]  r10:c0e25d24
[   46.187014]  r9:edf53e70
[   46.189569]  r8:c1642444
[   46.190637]  r7:ee9da024
[   46.193191]  r6:
[   46.194258]  r5:
[   46.196812]  r4:
[   46.199185]  r3:0001

[   46.203259] [] (register_lock_class) from [] 
(__lock_acquire+0x80/0x10f0)
[   46.211797]  r10:c0e25d24
[   46.214257]  r9:edf53e70
[   46.216813]  r8:ee9da024
[   46.217880]  r7:c1642444
[   46.220435]  r6:edcd1800
[   46.221502]  r5:6193
[   46.224057]  r4:

[   46.227953] [] (__lock_acquire) from [] 
(lock_acquire+0x74/0x94)
[   46.235710]  r10:0001
[   46.238169]  r9:edf53e70
[   46.240723]  r8:edf53f80
[   46.241790]  r7:0001
[   46.244344]  r6:0001
[   46.245412]  r5:6193
[   46.247966]  r4:

[   46.251866] [] (lock_acquire) from [] 
(_raw_spin_lock_irqsave+0x40/0x54)
[   46.260319]  r7:ee1c6a00
[   46.262691]  r6:c062a570
[   46.265247]  r5:2113
[   46.266314]  r4:ee9da014

[   46.270393] [] (_raw_spin_lock_irqsave) from [] 
(ci_port_test_show+0x2c/0x70)
[   46.279280]  r6:eebd2000
[   46.281652]  r5:ee9da010
[   46.284207]  r4:ee9da014

[   46.286810] [] (ci_port_test_show) from [] 
(seq_read+0x1ac/0x4f8)
[   46.294655]  r9:edf53e70
[   46.297028]  r8:edf53f80
[   46.299583]  r7:ee1c6a00
[   46.300650]  r6:0001
[   46.303205]  r5:
[   46.304273]  r4:eebd2000
[   46.306850] [] (seq_read) from [] 
(full_proxy_read+0x54/0x6c)
[   46.314348]  r10:
[   46.316808]  r9:c0a6ad30
[   46.319363]  r8:edf53f80
[   46.320430]  r7:0002
[   46.322986]  r6:b6de3000
[   46.324053]  r5:ee1c6a00
[   46.326607]  r4:c0248b58

[   46.330505] [] (full_proxy_read) from [] 
(__vfs_read+0x34/0x118)
[   46.338262]  r9:edf52000
[   46.340635]  r8:c0107fc4
[   46.343190]  r7:0002
[   46.344257]  r6:edf53f80
[   46.346812]  r5:c039e810
[   46.347879]  r4:ee1c6a00
[   46.350447] [] (__vfs_read) from [] (vfs_read+0x8c/0x11c)
[   46.357597]  r9:edf52000
[   46.359969]  r8:c0107fc4
[   46.362524]  r7:edf53f80
[   46.363592]  r6:b6de3000
[   46.366147]  r5:ee1c6a00
[   46.367214]  r4:0002
[   46.369782] [] (vfs_read) from [] (SyS_read+0x4c/0xa8)
[   46.376672]  r8:c0107fc4
[   46.379045]  r7:0002
[   46.381600]  r6:b6de3000
[   46.382667]  r5:ee1c6a00
[   46.385222]  r4:ee1c6a00

[   46.387817] [] (SyS_read) from [] 
(ret_fast_syscall+0x0/0x1c)
[   46.395314]  r7:0003
[   46.397687]  r6:b6de3000
[   46.400243]  r5:0002
[   46.401310]  r4:0002

Cc:  #v4.1+
Signed-off-by: Peter Chen 
---

Changes for v2:
- Clean up dump message by using dmesg output instead of console output.

 drivers/usb/chipidea/core.c | 1 +
 drivers/usb/chipidea/udc.c  | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index a7d2c68..c5e7f3d 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -873,6 +873,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (!ci)
return -ENOMEM;
 
+   spin_lock_init(>lock);
ci->dev = dev;
ci->platdata = dev_get_platdata(dev);
ci->imx28_write_fix = !!(ci->platdata->flags &
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 1e0ffad..e7bd064 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1886,8 +1886,6 @@ static int udc_start(struct ci_hdrc *ci)
struct usb_otg_caps *otg_caps = >platdata->ci_otg_caps;
int retval = 0;
 
-   spin_lock_init(>lock);
-
ci->gadget.ops  = _gadget_ops;
ci->gadget.speed= USB_SPEED_UNKNOWN;
ci->gadget.max_speed= USB_SPEED_HIGH;
-- 
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: [GIT PULL] USB: chipidea fixes for v4.9

[Peter Chen]

On Mon, Nov 14, 2016 at 12:48:45PM +0100, Greg KH wrote:
> On Mon, Nov 14, 2016 at 10:01:53AM +0800, Peter Chen wrote:
> > The following changes since commit 18266403f3fe507f0246faa1d5432333a2f139ca:
> > 
> >   USB: cdc-acm: fix TIOCMIWAIT (2016-11-10 13:12:59 +0100)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ 
> > tags/usb-ci-v4.9-rc6
> > 
> > for you to fetch changes up to 62ee9585fed19e4de9c3941b9c74044205ae13bd:
> > 
> >   usb: chipidea: move the lock initialization to core file (2016-11-14 
> > 09:42:25 +0800)
> > 
> > 
> > One patch to fix kernel dump for non-initialized lock
> 
> Is this a regression, or fixing something new?  Can you just post the
> one patch?  That's easier than a git pull request...
> 

It is a new fix. Patch v2 has sent.

What's criterion for sending patch and pull request? The number of Patch? How
many?

-- 

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 v5 0/4] usb: early: add support for early printk through USB3 debug port

[Lu Baolu]

xHCI debug capability (DbC) is an optional but standalone
functionality provided by an xHCI host controller. With DbC
hardware initialized, the system will present a debug device
through the USB3 debug port (normally the first USB3 port).
The debug device is fully compliant with the USB framework
and provides the equivalent of a very high performance (USB3)
full-duplex serial link between the debug host and target.
The DbC functionality is independent of xHCI host. There
isn't any precondition from xHCI host side for DbC to work.

This patch set adds support for early printk functionality
through a USB3 debug port by 1) initializing and enabling
the DbC hardware during early boot; 2) registering a boot
console to the system so that early printk messages can go
through the USB3 debug port. It also includes some lines
of changes in usb_debug driver so that it can be bound when
a USB3 debug device is enumerated.

This code is designed to be used only for kernel debugging
when machine crashes very early before the console code is
initialized. It makes the life of kernel debugging easier
when people work with a modern machine without any legacy
serial ports.

---
Change log:
v4->v5:
  - add raw_spin_lock to make xdbc_bulk_write() reentrant. 

v3->v4:
  - Rename the document with .dst suffix.
  - Add the list of hardware that has been succesfuly
tested on in the document.

v2->v3:
  - Removed spinlock usage.
  - Removed work queue usage.
  - Refined the user guide document.

v1->v2:
  - Refactor the duplicate code in xdbc_early_start() and
xdbc_handle_external_reset().
  - Free resources when hardware not used any more.
  - Refine the user guide document.

Lu Baolu (4):
  usb: dbc: early driver for xhci debug capability
  x86: add support for earlyprintk via USB3 debug port
  usb: serial: usb_debug: add support for dbc debug device
  usb: doc: add document for USB3 debug port usage

 Documentation/kernel-parameters.txt   |1 +
 Documentation/usb/usb3-debug-port.rst |   95 +++
 arch/x86/Kconfig.debug|   14 +
 arch/x86/kernel/early_printk.c|5 +
 arch/x86/kernel/setup.c   |7 +
 drivers/usb/Kconfig   |3 +
 drivers/usb/Makefile  |2 +-
 drivers/usb/early/Makefile|1 +
 drivers/usb/early/xhci-dbc.c  | 1068 +
 drivers/usb/early/xhci-dbc.h  |  205 +++
 drivers/usb/serial/usb_debug.c|   28 +-
 include/linux/usb/xhci-dbgp.h |   22 +
 12 files changed, 1447 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/usb/usb3-debug-port.rst
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 drivers/usb/early/xhci-dbc.h
 create mode 100644 include/linux/usb/xhci-dbgp.h

-- 
2.1.4

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

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

[Lu Baolu]

xHCI debug capability (DbC) is an optional but standalone
functionality provided by an xHCI host controller. Software
learns this capability by walking through the extended
capability list of the host. xHCI specification describes
DbC in section 7.6.

This patch introduces the code to probe and initialize the
debug capability hardware during early boot. With hardware
initialized, the debug target (system on which this code is
running) will present a debug device through the debug port
(normally the first USB3 port). The debug device is fully
compliant with the USB framework and provides the equivalent
of a very high performance (USB3) full-duplex serial link
between the debug host and target. The DbC functionality is
independent of xHCI host. There isn't any precondition from
xHCI host side for DbC to work.

This patch also includes bulk out and bulk in interfaces.
These interfaces could be used to implement early printk
bootconsole or hook to various system debuggers.

This code is designed to be only used for kernel debugging
when machine crashes very early before the console code is
initialized. For normal operation it is not recommended.

Cc: Mathias Nyman 
Signed-off-by: Lu Baolu 
---
 arch/x86/Kconfig.debug|   14 +
 drivers/usb/Kconfig   |3 +
 drivers/usb/Makefile  |2 +-
 drivers/usb/early/Makefile|1 +
 drivers/usb/early/xhci-dbc.c  | 1068 +
 drivers/usb/early/xhci-dbc.h  |  205 
 include/linux/usb/xhci-dbgp.h |   22 +
 7 files changed, 1314 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 drivers/usb/early/xhci-dbc.h
 create mode 100644 include/linux/usb/xhci-dbgp.h

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 67eec55..13e85b7 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -29,6 +29,7 @@ config EARLY_PRINTK
 config EARLY_PRINTK_DBGP
bool "Early printk via EHCI debug port"
depends on EARLY_PRINTK && PCI
+   select USB_EARLY_PRINTK
---help---
  Write kernel log output directly into the EHCI debug port.
 
@@ -48,6 +49,19 @@ config EARLY_PRINTK_EFI
  This is useful for kernel debugging when your machine crashes very
  early before the console code is initialized.
 
+config EARLY_PRINTK_XDBC
+   bool "Early printk via xHCI debug port"
+   depends on EARLY_PRINTK && PCI
+   select USB_EARLY_PRINTK
+   ---help---
+ Write kernel log output directly into the xHCI debug port.
+
+ This is useful for kernel debugging when your machine crashes very
+ early before the console code is initialized. For normal operation
+ it is not recommended because it looks ugly and doesn't cooperate
+ with klogd/syslogd or the X server. You should normally N here,
+ unless you want to debug such a crash.
+
 config X86_PTDUMP_CORE
def_bool n
 
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index fbe493d..9313fff 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -19,6 +19,9 @@ config USB_EHCI_BIG_ENDIAN_MMIO
 config USB_EHCI_BIG_ENDIAN_DESC
bool
 
+config USB_EARLY_PRINTK
+   bool
+
 menuconfig USB_SUPPORT
bool "USB support"
depends on HAS_IOMEM
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7791af6..0c37838 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -49,7 +49,7 @@ obj-$(CONFIG_USB_MICROTEK)+= image/
 obj-$(CONFIG_USB_SERIAL)   += serial/
 
 obj-$(CONFIG_USB)  += misc/
-obj-$(CONFIG_EARLY_PRINTK_DBGP)+= early/
+obj-$(CONFIG_USB_EARLY_PRINTK) += early/
 
 obj-$(CONFIG_USB_ATM)  += atm/
 obj-$(CONFIG_USB_SPEEDTOUCH)   += atm/
diff --git a/drivers/usb/early/Makefile b/drivers/usb/early/Makefile
index 24bbe51..2db5906 100644
--- a/drivers/usb/early/Makefile
+++ b/drivers/usb/early/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
+obj-$(CONFIG_EARLY_PRINTK_XDBC) += xhci-dbc.o
diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
new file mode 100644
index 000..5ac4223
--- /dev/null
+++ b/drivers/usb/early/xhci-dbc.c
@@ -0,0 +1,1068 @@
+/**
+ * xhci-dbc.c - xHCI debug capability early driver
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ":%s: " fmt, __func__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../host/xhci.h"
+#include "xhci-dbc.h"
+
+static struct xdbc_state xdbc;
+static int early_console_keep;
+
+#ifdef XDBC_TRACE

[PATCH v5 2/4] x86: add support for earlyprintk via USB3 debug port

[Lu Baolu]

Add support for early printk by writing debug messages to the
USB3 debug port.   Users can use this type of early printk by
specifying kernel parameter of "earlyprintk=xdbc". This gives
users a chance of providing debug output.

The hardware for USB3 debug port requires DMA memory blocks.
This requires to delay setting up debugging hardware and
registering boot console until the memblocks are filled.

Cc: Ingo Molnar 
Cc: x...@kernel.org
Signed-off-by: Lu Baolu 
---
 Documentation/kernel-parameters.txt | 1 +
 arch/x86/kernel/early_printk.c  | 5 +
 arch/x86/kernel/setup.c | 7 +++
 3 files changed, 13 insertions(+)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 37babf9..99b64b3 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1178,6 +1178,7 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
earlyprintk=ttySn[,baudrate]
earlyprintk=dbgp[debugController#]
earlyprintk=pciserial,bus:device.function[,baudrate]
+   earlyprintk=xdbc[xhciController#]
 
earlyprintk is useful when the kernel crashes before
the normal console is initialized. It is not enabled by
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 8a12199..c4031b9 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -381,6 +382,10 @@ static int __init setup_early_printk(char *buf)
if (!strncmp(buf, "efi", 3))
early_console_register(_efi_console, keep);
 #endif
+#ifdef CONFIG_EARLY_PRINTK_XDBC
+   if (!strncmp(buf, "xdbc", 4))
+   early_xdbc_parse_parameter(buf + 4);
+#endif
 
buf++;
}
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9c337b0..09d4a56 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -70,6 +70,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 
 #include 
@@ -1096,6 +1098,11 @@ void __init setup_arch(char **cmdline_p)
memblock_set_current_limit(ISA_END_ADDRESS);
memblock_x86_fill();
 
+#ifdef CONFIG_EARLY_PRINTK_XDBC
+   if (!early_xdbc_setup_hardware())
+   early_xdbc_register_console();
+#endif
+
reserve_bios_regions();
 
if (efi_enabled(EFI_MEMMAP)) {
-- 
2.1.4

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

[PATCH v5 3/4] usb: serial: usb_debug: add support for dbc debug device

[Lu Baolu]

This patch add dbc debug device support in usb_debug driver.

Signed-off-by: Lu Baolu 
Acked-by: Johan Hovold 
---
 drivers/usb/serial/usb_debug.c | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
index ca2fa5b..92f7e5c 100644
--- a/drivers/usb/serial/usb_debug.c
+++ b/drivers/usb/serial/usb_debug.c
@@ -32,7 +32,18 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x0525, 0x127a) },
{ },
 };
-MODULE_DEVICE_TABLE(usb, id_table);
+
+static const struct usb_device_id dbc_id_table[] = {
+   { USB_DEVICE(0x1d6b, 0x0004) },
+   { },
+};
+
+static const struct usb_device_id id_table_combined[] = {
+   { USB_DEVICE(0x0525, 0x127a) },
+   { USB_DEVICE(0x1d6b, 0x0004) },
+   { },
+};
+MODULE_DEVICE_TABLE(usb, id_table_combined);
 
 /* This HW really does not support a serial break, so one will be
  * emulated when ever the break state is set to true.
@@ -71,9 +82,20 @@ static struct usb_serial_driver debug_device = {
.process_read_urb = usb_debug_process_read_urb,
 };
 
+static struct usb_serial_driver dbc_device = {
+   .driver = {
+   .owner =THIS_MODULE,
+   .name = "xhci_dbc",
+   },
+   .id_table = dbc_id_table,
+   .num_ports =1,
+   .break_ctl =usb_debug_break_ctl,
+   .process_read_urb = usb_debug_process_read_urb,
+};
+
 static struct usb_serial_driver * const serial_drivers[] = {
-   _device, NULL
+   _device, _device, NULL
 };
 
-module_usb_serial_driver(serial_drivers, id_table);
+module_usb_serial_driver(serial_drivers, id_table_combined);
 MODULE_LICENSE("GPL");
-- 
2.1.4

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

Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Tue, Nov 15, 2016 at 08:35:13AM +1100, NeilBrown wrote:
> On Mon, Nov 14 2016, Mark Brown wrote:
> 
> > On Mon, Nov 14, 2016 at 03:21:13PM +1100, NeilBrown wrote:
> >> On Thu, Nov 10 2016, Baolin Wang wrote:
> >
> >> > Fourth, we need integrate all charger plugin/out
> >> > event in one framework, not from extcon, maybe type-c in future.
> >
> >> Why not extcon?  Given that a charger is connected by an external
> >> connector, extcon seems like exactly the right thing to use.
> >
> >> Obviously extcon doesn't report the current that was negotiated, but
> >> that is best kept separate.  The battery charger can be advised of the
> >> available current either via extcon or separately via the usb
> >> subsystem.  Don't conflate the two.
> >
> > Conflating the two seems like the whole point here.  We're looking for
> > something that sits between the power supply code and the USB code and
> > tells the power supply code what it's allowed to do which is the result
> > of a combination of physical cable detection and USB protocol.  It seems
> > reasonable that extcon drivers ought to be part of this but it doesn't
> > seem like they are the whole story.
> 
> I don't think "between the power supply code and the USB code" is where
> this thing sits. I think it sits inside the power-supply driver.
> We already have extcon which sits between the phy and the power_supply
> code, and the usb_notifier which sits between the USB code and the
> power supply code.  We don't need another go-between.
> 
> If we have extcon able to deliver reliable information about cable type,
> and if with have the usb notifier able to deliver reliable information
> about negotiated current, and if the power supply manager is able to
> register with the correct extcon and the correct usb notifier, then the
> power supply manager *could* handle all the notifications and make the
> correct determinations and set the current limits itself.  All this
> could be done entirely internally, without the help of any new
> subsystem.
> Do you agree?

Through the USB gadget/phy framework (usb_gadget.vbus_draw->usb_phy.set_power)
we can get the USB bus information when the device connects SDP, but the
enum usb_phy_events lacks some events like bus suspend (2mA), and bus
speed (high/super speed, 500mA vs 900mA). Besides many USB PHYs use
generic PHY driver now, it is lack of above event and related notifier.

About getting cable type, the key points are detect vbus and negotiate
the charger type, these two stuffs are much different among platforms.
Extcon has charger type definition, it is good, we can use it.
But it needs the device which has charger detection function as extcon
device too, and at meanwhile, this device needs to have vbus detect
function, most pmic devices are suitable for that, but not for USB PHY.

Asssume wm831x as a power client, according your suggestion, does its
design like below?
At dts, it needs to be described like below:
 {
...
phy-dev = <_phy>;
extcon-dev = <>;
...
}
And at wm831x driver, it gets information through extcon-dev and phy-dev
notifier, and it needs knowledge about current limit for specific
cable type, but these information are from USB (Charger) specification.

Your suggestion is trying use current notifications to get the
information for power client, this patch set is trying to keep
these two notifications at an new framework, and power client
gets refined notification from this new framework.

The biggest problem I concern about your solution is extcon device, it may
not be an universal solution, does current frameworks have a way to
get cable type (usb charger type)? If not, we may need to have a new
framework.

-- 

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 0/2] Two cpp41 pm runtime found when testing with usb

[Tony Lindgren]

* Vinod Koul  [161114 19:20]:
> On Mon, Nov 14, 2016 at 06:49:12AM -0800, Tony Lindgren wrote:
> > * Vinod Koul  [161113 21:19]:
> > > On Wed, Nov 09, 2016 at 09:47:57AM -0700, Tony Lindgren wrote:
> > > > Hi,
> > > > 
> > > > I found two pm runtime issues when testing with usb on beaglebone.
> > > > 
> > > > In the am335x case cppi41 and two instances of musb controller share
> > > > the same interconnect wrapper module, so any pm issues with musb or
> > > > cppi41 will keep the interconnect wrapper module busy.
> > > 
> > > Applied both. And as I have told you previously please use the correct
> > > subsystem tag. I have fixed them again!
> > 
> > Sorry about that. What do you prefer for future reference? We are using
> > both "dma: cppi41" and "dmaengine: cppi41" currently..
> 
> "dmaengine: cppi41: xxx" would be better

OK will use that for future patches then.

Thanks,

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

Re: [PATCH 0/2] Two cpp41 pm runtime found when testing with usb

[Vinod Koul]

On Mon, Nov 14, 2016 at 06:49:12AM -0800, Tony Lindgren wrote:
> * Vinod Koul  [161113 21:19]:
> > On Wed, Nov 09, 2016 at 09:47:57AM -0700, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > I found two pm runtime issues when testing with usb on beaglebone.
> > > 
> > > In the am335x case cppi41 and two instances of musb controller share
> > > the same interconnect wrapper module, so any pm issues with musb or
> > > cppi41 will keep the interconnect wrapper module busy.
> > 
> > Applied both. And as I have told you previously please use the correct
> > subsystem tag. I have fixed them again!
> 
> Sorry about that. What do you prefer for future reference? We are using
> both "dma: cppi41" and "dmaengine: cppi41" currently..

"dmaengine: cppi41: xxx" would be better

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe 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 16/17] usb: dwc2: gadget: Program ep0_mps for LS

[John Youn]

From: Vardan Mikayelyan 

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

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

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

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

[PATCH v3 15/17] usb: dwc2: gadget: Add IOT device IDs, configure core accordingly

[John Youn]

From: Vardan Mikayelyan 

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

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

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

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

[PATCH v3 13/17] usb: dwc2: gadget: Disable enabled HW endpoint in dwc2_hsotg_ep_disable

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH v3 17/17] usb: dwc2: gadget: Add new core parameter for low speed

[John Youn]

From: Vardan Mikayelyan 

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

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

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

[PATCH v3 11/17] usb: dwc2: gadget: For DDMA parse setup only after SetUp interrupt

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

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

[PATCH v3 12/17] usb: dwc2: gadget: Correct dwc2_hsotg_ep_stop_xfr() function

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH v3 02/17] usb: dwc2: gadget: DDMA transfer start and complete

[John Youn]

From: Vahram Aharonyan 

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

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

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

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index caa428a..6c988c2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -760,6 +760,7 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
unsigned length;
unsigned packets;
unsigned maxreq;
+   unsigned int dma_reg;
 
if (index != 0) {
if (hs_ep->req && !continuing) {
@@ -774,6 +775,7 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
}
}
 
+   dma_reg = dir_in ? DIEPDMA(index) : DOEPDMA(index);
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
epsize_reg = dir_in ? DIEPTSIZ(index) : DOEPTSIZ(index);
 
@@ -849,22 +851,51 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
/* store the request as the current one we're doing */
hs_ep->req = hs_req;
 
-   /* write size / packets */
-   dwc2_writel(epsize, hsotg->regs + epsize_reg);
-
-   if (using_dma(hsotg) && !continuing) {
-   unsigned int dma_reg;
+   if (using_desc_dma(hsotg)) {
+   u32 offset = 0;
+   u32 mps = hs_ep->ep.maxpacket;
+
+   /* Adjust length: EP0 - MPS, other OUT EPs - multiple of MPS */
+   if (!dir_in) {
+   if (!index)
+   length = mps;
+   else if (length % mps)
+   length += (mps - (length % mps));
+   }
 
/*
-* write DMA address to control register, buffer already
-* synced by dwc2_hsotg_ep_queue().
+* If more data to send, adjust DMA for EP0 out data stage.
+* ureq->dma stays unchanged, hence increment it by already
+* passed passed data count before starting new transaction.
 */
+   if (!index && hsotg->ep0_state == DWC2_EP0_DATA_OUT &&
+   continuing)
+   offset = ureq->actual;
+
+   /* Fill DDMA chain entries */
+   dwc2_gadget_config_nonisoc_xfer_ddma(hs_ep, ureq->dma + offset,
+length);
+
+   /* write descriptor chain address to control register */
+   dwc2_writel(hs_ep->desc_list_dma, hsotg->regs + dma_reg);
 
-   dma_reg = dir_in ? DIEPDMA(index) : DOEPDMA(index);
-   dwc2_writel(ureq->dma, hsotg->regs + dma_reg);
+   dev_dbg(hsotg->dev, "%s: %08x pad => 0x%08x\n",
+   __func__, (u32)hs_ep->desc_list_dma, dma_reg);
+   } else {
+   /* write size / packets */
+   dwc2_writel(epsize, hsotg->regs + epsize_reg);
+
+   if (using_dma(hsotg) && !continuing) {
+   /*
+* write DMA address to control register, buffer
+* already synced by dwc2_hsotg_ep_queue().
+*/
 
-   dev_dbg(hsotg->dev, "%s: %pad => 0x%08x\n",
-   __func__, >dma, dma_reg);
+   dwc2_writel(ureq->dma, hsotg->regs + dma_reg);
+
+   dev_dbg(hsotg->dev, "%s: %pad => 0x%08x\n",
+   __func__, >dma, dma_reg);
+   }
}
 
if (hs_ep->isochronous && hs_ep->interval == 1) {
@@ -1863,6 +1894,36 @@ static void dwc2_hsotg_change_ep_iso_parity(struct 
dwc2_hsotg *hsotg,
dwc2_writel(ctrl, hsotg->regs + epctl_reg);
 }
 
+/*
+ * dwc2_gadget_get_xfersize_ddma - get transferred bytes amount from desc
+ * @hs_ep - The endpoint on which transfer went
+ *
+ * Iterate over endpoints descriptor chain and get info on bytes remained
+ * in DMA descriptors after transfer has completed. Used for non isoc EPs.
+ */
+static unsigned int dwc2_gadget_get_xfersize_ddma(struct dwc2_hsotg_ep *hs_ep)
+{
+   struct dwc2_hsotg *hsotg = hs_ep->parent;
+   unsigned int bytes_rem = 0;
+   struct dwc2_dma_desc *desc = hs_ep->desc_list;
+   int i;
+   u32 status;
+
+   if (!desc)
+   return -EINVAL;
+
+   for (i = 0; i < hs_ep->desc_count; ++i) {
+   status = desc->status;
+   bytes_rem += status & DEV_DMA_NBYTES_MASK;
+
+  

[PATCH v3 05/17] usb: dwc2: gadget: Enable descriptor DMA mode

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH v3 04/17] usb: dwc2: gadget: Start DDMA IN status phase in StsPhseRcvd handler

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH v3 09/17] usb: dwc2: gadget: Enable the BNA interrupt

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

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

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

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

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

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

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

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

[PATCH v3 10/17] usb: dwc2: gadget: Adjust ISOC OUT request's actual len for DDMA

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH v3 00/17] usb: dwc2: Gadget descriptor DMA and IOT

[John Youn]

This series implements gadget-side descriptor DMA for the DWC_hsotg
controller.

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

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

v3:
* Rebased on Felipe's testing/next
* Fixed compiler warnings by squashing commits with unused static
  functions.

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

Regards,
John


Vahram Aharonyan (14):
  usb: dwc2: gadget: EP 0 specific DDMA programming
  usb: dwc2: gadget: DDMA transfer start and complete
  usb: dwc2: gadget: Fixes for StsPhseRcvd interrupt
  usb: dwc2: gadget: Start DDMA IN status phase in StsPhseRcvd handler
  usb: dwc2: gadget: Enable descriptor DMA mode
  usb: dwc2: gadget: Add DDMA isoc related fields to dwc2_hsotg_ep
  usb: dwc2: gadget: In DDMA keep incompISOOUT and incompISOIN masked
  usb: dwc2: gadget: Start and complete DDMA isoc transfers
  usb: dwc2: gadget: Enable the BNA interrupt
  usb: dwc2: gadget: Adjust ISOC OUT request's actual len for DDMA
  usb: dwc2: gadget: For DDMA parse setup only after SetUp interrupt
  usb: dwc2: gadget: Correct dwc2_hsotg_ep_stop_xfr() function
  usb: dwc2: gadget: Disable enabled HW endpoint in
dwc2_hsotg_ep_disable
  usb: dwc2: Add support of dedicated full-speed PHY interface

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

 drivers/usb/dwc2/core.h   |  24 ++
 drivers/usb/dwc2/gadget.c | 840 +++---
 drivers/usb/dwc2/hcd.c|   8 +-
 drivers/usb/dwc2/hw.h |   2 +
 drivers/usb/dwc2/params.c |  17 +-
 5 files changed, 773 insertions(+), 118 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 v3 08/17] usb: dwc2: gadget: Start and complete DDMA isoc transfers

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

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

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

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

[PATCH v3 03/17] usb: dwc2: gadget: Fixes for StsPhseRcvd interrupt

[John Youn]

From: Vahram Aharonyan 

The StsPhseRcvd interrupt should not be enabled in slave mode.

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

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

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

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

[PATCH v3 07/17] usb: dwc2: gadget: In DDMA keep incompISOOUT and incompISOIN masked

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH v3 01/17] usb: dwc2: gadget: EP 0 specific DDMA programming

[John Youn]

From: Vahram Aharonyan 

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

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

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

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

[PATCH 3/3] usb: dwc2: Remove reading in of invalid property

[John Youn]

This property was mistakenly added, then removed, so don't read it in.

Signed-off-by: John Youn 
---
 drivers/usb/dwc2/params.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 2f18a7b..7152dbf 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1124,16 +1124,11 @@ static void dwc2_set_parameters(struct dwc2_hsotg 
*hsotg,
 
if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
(hsotg->dr_mode == USB_DR_MODE_OTG)) {
-   bool disable;
-
dev_dbg(hsotg->dev, "Setting HOST parameters\n");
 
-   disable = device_property_read_bool(hsotg->dev,
-   "snps,host-dma-disable");
-
dwc2_set_param_bool(hsotg, >host_dma,
false, "host-dma",
-   !disable, false,
+   true, false,
dma_capable);
}
 
-- 
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 0/3] usb: dwc2: params revert and rework

[John Youn]

Hi Felipe,

This reverts and fixes a few commits that are queued on your
testing/next, removing the previously added DT bindings, and the code
that reads them in.

The feedback was that IP validation is not reason enough to add these.
So we'll leave them out for now.

Regards,
John


John Youn (3):
  Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
  Revert "Documentation: devicetree: dwc2: Add host DMA binding"
  usb: dwc2: Remove reading in of invalid property

 Documentation/devicetree/bindings/usb/dwc2.txt |  3 ---
 drivers/usb/dwc2/params.c  | 16 +++-
 2 files changed, 3 insertions(+), 16 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 2/3] Revert "Documentation: devicetree: dwc2: Add host DMA binding"

[John Youn]

This reverts commit 751089ecaab0 ("Documentation: devicetree: dwc2: Add
host DMA binding").

Remove this binding as it is not needed by any hardware.

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

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 389bb13..ad8f7ff 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -25,7 +25,6 @@ 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-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget 
mode.
-- 
2.10.0

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

[PATCH 1/3] Revert "usb: dwc2: Add bindings to disable gadget DMA modes"

[John Youn]

This reverts commit 9acc1ee2b723 ("usb: dwc2: Add bindings to disable
gadget DMA modes").

Don't add bindings and don't read them in. These are not yet needed by
any hardware.

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

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

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

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

[John Youn]

On 11/14/2016 5:11 PM, Rob Herring wrote:
> On Wed, Nov 09, 2016 at 07:27:50PM -0800, John Youn wrote:
>> Now that the gadget driver automatically detects DMA modes, we need to
>> provide a way to disable them. Certain platforms may still have issues
>> with DMA and require it to be disabled. It is also needed for IP
>> validation purposes.
> 
> I don't think IP validation is reason enough for these properties. If 
> there is IP that is broken and lies about it, then yes we should have 
> these.
> 
> Rob
> 

Ok. Mark said basically the same thing in a previous thread that lead
to this patch.

Until they are needed by hardware, I'll remove these bindings.

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 v2 05/30] usb: dwc2: Add bindings to disable gadget DMA modes

[Rob Herring]

On Wed, Nov 09, 2016 at 07:27:50PM -0800, John Youn wrote:
> Now that the gadget driver automatically detects DMA modes, we need to
> provide a way to disable them. Certain platforms may still have issues
> with DMA and require it to be disabled. It is also needed for IP
> validation purposes.

I don't think IP validation is reason enough for these properties. If 
there is IP that is broken and lies about it, then yes we should have 
these.

Rob
--
To unsubscribe from this list: send the line "unsubscribe 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 net 2/2] r8152: rx descriptor check

[Francois Romieu]

Hayes Wang  :
> Francois Romieu [mailto:rom...@fr.zoreil.com]
> > Sent: Friday, November 11, 2016 8:13 PM
> [...]
> > Invalid packet size corrupted receive descriptors in Realtek's device
> > reminds of CVE-2009-4537.
> 
> Do you mean that the driver would get a packet exceed the size
> which is set to RxMaxSize ?

If it was possible to get it wrong once, it should be possible to
get it wrong twice, especially if some part of the hardware design
is recycled. I don't mean anything else.

I won't speculate about some cache consistency issue or some badly
aborted dma transaction to explain the memory corruption.

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe 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: crash by cdc_acm driver in kernels 4.8-rc1/5

[Wim Osterholt]

On Tue, Oct 18, 2016 at 02:18:43PM +0200, Oliver Neukum wrote:

> It definitely does not crash and is probed and your .config is not
> extremely unusual.

Hmmm.

> ... Something odd is going on.

Whell, yes.
The only thing that appears you'll have to do is unset 'CONFIG_SMP'.

My machines didn't have the luxury of multicore processors (until recently),
so there never has been any reason to deliberately switch these options on!

In the process of searching, many options may have changed. The crash/OOPS
has now mitigated into just a WARNING with a call trace.
(Or it could be a totally different bug?)
After the call trace the device is working normally and a shutdown
completes to the end now.
That is with the config given here:
http://webserver.djo.tudelft.nl/.config-4.9-rc4.OK (CONFIG_SMP=y)
http://webserver.djo.tudelft.nl/WARNING-4.9-rc4(call trace for C_S unset)

Tests on other machines with (slightly) different configs all seem to
confirm that the problems are gone when CONFIG_SMP is set.

Regards, Wim.


--
To unsubscribe from this list: send the line "unsubscribe 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 01/30] usb: dwc2: Deprecate g-use-dma binding

[Rob Herring]

On Wed, Nov 09, 2016 at 07:27:40PM -0800, John Youn wrote:
> This is not needed as the gadget now fully supports DMA and it can
> autodetect it. This was initially added because gadget DMA mode was only
> partially implemented so could not be automatically enabled.
> 
> Signed-off-by: John Youn 
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++-
>  arch/arm/boot/dts/rk3036.dtsi  |  1 -
>  arch/arm/boot/dts/rk3288.dtsi  |  1 -
>  arch/arm/boot/dts/rk3xxx.dtsi  |  1 -
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi  |  1 -
>  arch/arm64/boot/dts/rockchip/rk3368.dtsi   |  1 -
>  drivers/usb/dwc2/core.h|  4 +---
>  drivers/usb/dwc2/params.c  | 17 ++---
>  drivers/usb/dwc2/pci.c |  1 -
>  9 files changed, 18 insertions(+), 13 deletions(-)

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

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[John Youn]

On 11/11/2016 3:12 PM, Christian Lamparter wrote:
> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
 On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> This patch adds support for the "amcc,usb-otg" device
> which is found in the PowerPC Canyonlands' dts.
>
> The device definition was added by:
> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands 
> board")'
> but without any driver support as the dwc2 driver wasn't
> available at that time.
>
> Note: The system can't use the generic "snps,dwc2" compatible
> because of the special ahbcfg configuration. The default
> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> when the USB and SATA is used concurrently.

 I don't want to add any more of these param structures to the driver
 unless really necessary. We're trying to remove usage of them in favor
 of using auto-detected defaults and device properties to override
 them.
>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>
 The AHB Burst is actually one of the ones we were going to do next
 because our platform also doesn't work well with INCR4. In fact I'm
 thinking of making the default INCR.
>>> Is that actually possible to change the default still? This would
>>> require to re-evaluate all existing archs/platforms that use 
>>> "snps,dwc2" for INCR16 compatibility. 
>>
>> INCR, not INCR16, but you're right, so we may not change it even
>> though though INCR is usually the right choice over INCR4.
> What about making a device-tree property?

Yes, that's what I meant. I'll send a change for this shortly.

> 
> Recommended properties:
>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>"single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>the safer but inefficient "INCR4" is used. The optimal setting is
>"INCRx".
> 
> Would this work? If so, I can make a patch over the weekend.
>> Anyways, with the binding, can't you just set the compatible string to
>> snps,dwc2?
> 
> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
> a while back about device-tree bindings.
> 
> They made it very clear to me, that they don't want any generic "catch all
> compatible" strings:
> 
> "Bindings should be for hardware (either specific device models, or for
> classes), and not for Linux drivers. The latter is subject to arbitrary
> changes while the former is not, as old hardware continues to exist and
> does not change while drivers get completely reworked." [0]
> 
> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
> and this binding can't be easily changed. Rob Herring explained this in
> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
> to make them work with the changes I made:
> 
> "You can't remove the old drivers as they are needed to work with 
> old dtbs, so there is no gain.
> 
> You would need to match on existing compatibles such as
> moxa,moxart-gpio and provide a match data struct that has all the info
> you are adding here (e.g. data register offset). Then additionally you
> could add "basic-mmio-gpio" (I would drop "basic" part) and the
> additional data associated with it. But it has to be new properties,
> not changing properties. Changing the reg values doesn't work."
> 
> So, for this to work with the existing canyonlands.dts, I need to have
> the "amcc,dwc-otg" compatible string.

Ok, if that's the case. But still a bit confused as to what driver was
working with it before since the binding was not defined for dwc2.

> 
> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
> about this case.
> 
> Regards,
> Christian
> 
> [0] 
> [1] 
> 
> [2] 
> 
>  
>>>
>>> From what I can tell based would be:
>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>
 If that's all you need then a devicetree binding should be enough
 right?
>>> Yes. The device is working fine so far.
>>>
>>> Regards,
>>> Christian
>>>
>>> ---
>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>> From: Christian Lamparter 
>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>
>>> This patch adds support for the "amcc,usb-otg" device
>>> which is found in the PowerPC Canyonlands' dts.
>>>
>>> The device definition was added by:
>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands 
>>> 

Re: [PATCH 2/2] usb: dwc2: fixes host_dma logic

[John Youn]

On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> This patch moves the the host_dma initialization
> before dwc2_set_param_dma_desc_enable and
> dwc2_set_param_dma_desc_fs_enable. The reason being
> that both function need it.
> 
> Fixes: 1205489cee75bf39 ("usb: dwc2: Get host DMA device properties")

This should probably be omitted since it's only in Felipe's
testing/next.

Otherwise looks good.

Acked-by: John Youn 

Regards,
John


> 
> Cc: John Youn 
> Cc: Felipe Balbi 
> Signed-off-by: Christian Lamparter 
> ---
>  drivers/usb/dwc2/params.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 5d822c5..222a83c 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -1157,9 +1157,6 @@ static void dwc2_set_parameters(struct dwc2_hsotg 
> *hsotg,
>   bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
>  
>   dwc2_set_param_otg_cap(hsotg, params->otg_cap);
> - dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
> - dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
> -
>   if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
>   (hsotg->dr_mode == USB_DR_MODE_OTG)) {
>   bool disable;
> @@ -1174,6 +1171,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg 
> *hsotg,
>   !disable, false,
>   dma_capable);
>   }
> + dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
> + dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
>  
>   dwc2_set_param_host_support_fs_ls_low_power(hsotg,
>   params->host_support_fs_ls_low_power);
> 
--
To unsubscribe from this list: send the line "unsubscribe 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 v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[NeilBrown]

On Mon, Nov 14 2016, Mark Brown wrote:

> On Mon, Nov 14, 2016 at 03:21:13PM +1100, NeilBrown wrote:
>> On Thu, Nov 10 2016, Baolin Wang wrote:
>
>> > Fourth, we need integrate all charger plugin/out
>> > event in one framework, not from extcon, maybe type-c in future.
>
>> Why not extcon?  Given that a charger is connected by an external
>> connector, extcon seems like exactly the right thing to use.
>
>> Obviously extcon doesn't report the current that was negotiated, but
>> that is best kept separate.  The battery charger can be advised of the
>> available current either via extcon or separately via the usb
>> subsystem.  Don't conflate the two.
>
> Conflating the two seems like the whole point here.  We're looking for
> something that sits between the power supply code and the USB code and
> tells the power supply code what it's allowed to do which is the result
> of a combination of physical cable detection and USB protocol.  It seems
> reasonable that extcon drivers ought to be part of this but it doesn't
> seem like they are the whole story.

I don't think "between the power supply code and the USB code" is where
this thing sits. I think it sits inside the power-supply driver.
We already have extcon which sits between the phy and the power_supply
code, and the usb_notifier which sits between the USB code and the
power supply code.  We don't need another go-between.

If we have extcon able to deliver reliable information about cable type,
and if with have the usb notifier able to deliver reliable information
about negotiated current, and if the power supply manager is able to
register with the correct extcon and the correct usb notifier, then the
power supply manager *could* handle all the notifications and make the
correct determinations and set the current limits itself.  All this
could be done entirely internally, without the help of any new
subsystem.
Do you agree?

Clearly doing it that way would result in lots of code duplication and
would mean that each driver probably gets its own private set of bugs,
but it would be possible.  Just undesirable.

So yes, it makes perfect to provide common code which handles the
registrations, and captures the events, and translates the different
events into current levels and feeds those back to the driver.  This
isn't some new subsystem, this is just a resource, provided by a
library, that power drivers can allocate and initialize if the want to.

To quote myself:

> 5/ Now that all cable connection notifications are sent over extcon and
>all vbus_draw notifications are sent over the usb_phy notifier, write
>some support code that a power supply client can use to be told what
>power is available.
>e.g. a battery charger driver would call:
>register_power_client(.)
>or similar, providing a phandle (or similar) for the usb phy and a
>function to call back when the available current changes (or maybe a
>work_struct containing the function pointer)
> 
>register_power_client() would then register with extcon and separately
>with the usb_phy notifier.  When the different events arrive it
>calculates what ranges of currents are expected and calls the
>call-back function with those details.

NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 6/6] usb: musb: Drop pointless PM runtime code for dsps glue

[Tony Lindgren]

* Johan Hovold  [161114 07:59]:
> On Fri, Nov 11, 2016 at 10:43:02AM -0800, Tony Lindgren wrote:
> > This already gets done automatically by PM runtime and we have
> > a separate autosuspend timeout in musb_core.c.
> > 
> > Signed-off-by: Tony Lindgren 
> 
> > @@ -816,8 +801,6 @@ static int dsps_remove(struct platform_device *pdev)
> > platform_device_unregister(glue->musb);
> >  
> > /* disable usbss clocks */
> 
> Perhaps also drop this comment which no longer applies.

OK will drop and repost the whole series one more time on Tuesday
in case more comments are still coming.

> > -   pm_runtime_dont_use_autosuspend(>dev);
> > -   pm_runtime_put_sync(>dev);
> > pm_runtime_disable(>dev);
> 
> Reviewed-by: Johan Hovold 

Thanks,

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: [PATHCv10 1/2] usb: USB Type-C connector class

[Guenter Roeck]

On Mon, Nov 14, 2016 at 02:32:35PM +0200, Heikki Krogerus wrote:
> Hi Greg,
> 
> On Mon, Nov 14, 2016 at 10:51:48AM +0100, Greg KH wrote:
> > On Mon, Sep 19, 2016 at 02:16:56PM +0300, Heikki Krogerus wrote:
> > > The purpose of USB Type-C connector class is to provide
> > > unified interface for the user space to get the status and
> > > basic information about USB Type-C connectors on a system,
> > > control over data role swapping, and when the port supports
> > > USB Power Delivery, also control over power role swapping
> > > and Alternate Modes.
> > > 
> > > Reviewed-by: Guenter Roeck 
> > > Tested-by: Guenter Roeck 
> > > Signed-off-by: Heikki Krogerus 
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-typec |  218 ++
> > >  Documentation/usb/typec.txt |  103 +++
> > >  MAINTAINERS |9 +
> > >  drivers/usb/Kconfig |2 +
> > >  drivers/usb/Makefile|2 +
> > >  drivers/usb/typec/Kconfig   |7 +
> > >  drivers/usb/typec/Makefile  |1 +
> > >  drivers/usb/typec/typec.c   | 1075 
> > > +++
> > >  include/linux/usb/typec.h   |  252 +++
> > >  9 files changed, 1669 insertions(+)
> > >  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
> > >  create mode 100644 Documentation/usb/typec.txt
> > >  create mode 100644 drivers/usb/typec/Kconfig
> > >  create mode 100644 drivers/usb/typec/Makefile
> > >  create mode 100644 drivers/usb/typec/typec.c
> > >  create mode 100644 include/linux/usb/typec.h
> > 
[ ... ]

> > > +
> > > +int typec_connect(struct typec_port *port, struct typec_connection *con)
> > > +{
> > > + int ret;
> > > +
> > > + if (!con->partner && !con->cable)
> > > + return -EINVAL;
> > > +
> > > + port->connected = 1;
> > > + port->data_role = con->data_role;
> > > + port->pwr_role = con->pwr_role;
> > > + port->vconn_role = con->vconn_role;
> > > + port->pwr_opmode = con->pwr_opmode;
> > > +
> > > + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> > 
> > This worries me.  Who is listening for it?  What will you do with it?
> > Shouldn't you just poll on an attribute file instead?
> 
> Oliver! Did you need this or can we remove it?
> 
> I remember I removed the "connected" attribute because you did not see
> any use for it at one point. I don't remember the reason exactly why?
> 

The Android team tells me that they are currently using the udev events
to track port role changes, and to detect presence of port partner.

Also, there are plans to track changes on usbc*cable to differentiate
between cable attach vs. device being attached on the remote end. 

What is the problem with using kobject_uevent() and thus presumably
udev events ?

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe 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/3] 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 889dbab..919c252 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1004,6 +1004,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 v4 1/3] usb: dwc3: gadget: Write GEVNTCOUNT in interrupt

[John Youn]

Currently GEVNTCOUNT is written in the threaded interrupt handler while
processing each event. This commit moves the GEVNTCOUNT write to the
hard IRQ. We then copy the events to a separate buffer for the event
handler to read from.

This change is in preparation of working around an issue in core version
3.00a where the interrupt cannot be de-asserted in the normal way.
However, if we enable interrupt moderation, we can also de-assert it by
writing to GEVNTCOUNT.

Signed-off-by: John Youn 
---
 drivers/usb/dwc3/core.c   | 4 
 drivers/usb/dwc3/core.h   | 2 ++
 drivers/usb/dwc3/gadget.c | 7 ---
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index d024d47..87d0cfb 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -224,6 +224,10 @@ static struct dwc3_event_buffer 
*dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
 
evt->dwc= dwc;
evt->length = length;
+   evt->cache  = devm_kzalloc(dwc->dev, length, GFP_KERNEL);
+   if (!evt->cache)
+   return ERR_PTR(-ENOMEM);
+
evt->buf= dma_alloc_coherent(dwc->dev, length,
>dma, GFP_KERNEL);
if (!evt->buf)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 354de24..bf63756 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -472,6 +472,7 @@ struct dwc3_trb;
 /**
  * struct dwc3_event_buffer - Software event buffer representation
  * @buf: _THE_ buffer
+ * @cache: The buffer cache used in the threaded interrupt
  * @length: size of this buffer
  * @lpos: event offset
  * @count: cache of last read event count register
@@ -481,6 +482,7 @@ struct dwc3_trb;
  */
 struct dwc3_event_buffer {
void*buf;
+   void*cache;
unsignedlength;
unsigned intlpos;
unsigned intcount;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 230ffa3..baa2c64 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2821,7 +2821,7 @@ static irqreturn_t dwc3_process_event_buf(struct 
dwc3_event_buffer *evt)
while (left > 0) {
union dwc3_event event;
 
-   event.raw = *(u32 *) (evt->buf + evt->lpos);
+   event.raw = *(u32 *)(evt->cache + evt->lpos);
 
dwc3_process_event_entry(dwc, );
 
@@ -2836,8 +2836,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;
@@ -2892,6 +2890,9 @@ static irqreturn_t dwc3_check_event_buf(struct 
dwc3_event_buffer *evt)
reg |= DWC3_GEVNTSIZ_INTMASK;
dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), reg);
 
+   memcpy(evt->cache, evt->buf, DWC3_EVENT_BUFFERS_SIZE);
+   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
+
return IRQ_WAKE_THREAD;
 }
 
-- 
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 v4 2/3] 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 87d0cfb..889dbab 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -982,12 +982,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 bf63756..ef81fa5 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;
@@ -846,6 +854,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;
@@ -933,6 +943,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;
@@ -991,6 +1002,8 @@ struct dwc3 {
 
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
+
+   u16 imod_interval;
 };
 
 /* -- 
*/
@@ -1162,6 +1175,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 baa2c64..0973167 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1685,6 +1685,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);
 
@@ -2847,6 +2858,11 @@ static irqreturn_t dwc3_process_event_buf(struct 
dwc3_event_buffer *evt)
reg &= ~DWC3_GEVNTSIZ_INTMASK;
dwc3_writel(dwc->regs, 

[PATCH v4 0/3] usb: dwc3: Interrupt moderation

[John Youn]

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

v4:
* Rebased on testing/next
* Always copy the entire cache and retain the accounting code in the
  event handler

v3:
* Cache the events between irq and bh

v2:
* Remove the devicetree binding

Regards,
John



John Youn (3):
  usb: dwc3: gadget: Write GEVNTCOUNT in interrupt
  usb: dwc3: Implement interrupt moderation
  usb: dwc3: Workaround for irq mask issue

 drivers/usb/dwc3/core.c   | 32 
 drivers/usb/dwc3/core.h   | 17 +
 drivers/usb/dwc3/gadget.c | 23 ---
 3 files changed, 69 insertions(+), 3 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

Re: [upstream-release] [PATCH 1/2] drivers: usb: phy: Add qoriq usb 3.0 phy driver support

[Scott Wood]

On 11/13/2016 11:27 PM, Sriram Dash wrote:
> diff --git a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt 
> b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
> new file mode 100644
> index 000..d934c80
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
> @@ -0,0 +1,36 @@
> +Driver for Freescale USB 3.0 PHY
> +
> +Required properties:
> +
> +- compatible :   fsl,qoriq-usb3-phy

This is a very vague compatible.  Are there versioning registers within
this register block?

> +/* Parameter control */
> +#define USB3PRM1CR   0x000
> +#define USB3PRM1CR_VAL   0x27672b2a
> +
> +/*
> + * struct qoriq_usb3_phy - driver data for USB 3.0 PHY
> + * @dev: pointer to device instance of this platform device
> + * @param_ctrl: usb3 phy parameter control register base
> + * @phy_base: usb3 phy register memory base
> + * @has_erratum_flag: keeps track of erratum applicable on device
> + */
> +struct qoriq_usb3_phy {
> + struct device *dev;
> + void __iomem *param_ctrl;
> + void __iomem *phy_base;
> + u32 has_erratum_flag;
> +};
> +
> +static inline u32 qoriq_usb3_phy_readl(void __iomem *addr, u32 offset)
> +{
> + return __raw_readl(addr + offset);
> +}
> +
> +static inline void qoriq_usb3_phy_writel(void __iomem *addr, u32 offset,
> + u32 data)
> +{
> + __raw_writel(data, addr + offset);
> +}

Why raw?  Besides missing barriers, this will cause the accesses to be
native-endian which is not correct.

> +/*
> + * Erratum A008751
> + * SCFG USB3PRM1CR has incorrect default value
> + * SCFG USB3PRM1CR reset value should be 32'h27672B2A instead of 
> 32'h25E72B2A.

When documenting C code, can you stick with C-style numeric constants?

For that matter, just put the constant in the code instead of hiding it
in an overly-generically-named USB3PRM1CR_VAL and then you won't need to
redundantly state the value in a comment.  Normally putting magic
numbers in symbolic constants is a good thing, but in this case it's not
actually describing anything and the number is of no meaning outside of
this one erratum workaround (it might even be a different value if
another chip has a similar erratum).

> + */
> +static void erratum_a008751(struct qoriq_usb3_phy *phy)
> +{
> + qoriq_usb3_phy_writel(phy->param_ctrl, USB3PRM1CR,
> + USB3PRM1CR_VAL);
> +}
> +
> +/*
> + * qoriq_usb3_phy_erratum - List of phy erratum
> + * @qoriq_phy_erratum - erratum application
> + * @compat - comapt string for erratum
> + */
> +
> +struct qoriq_usb3_phy_erratum {
> + void (*qoriq_phy_erratum)(struct qoriq_usb3_phy *phy);
> + char *compat;
> +};
> +
> +/* Erratum list */
> +struct qoriq_usb3_phy_erratum  phy_erratum_tbl[] = {
> + {_a008751, "fsl,usb-erratum-a008751"},
> + /* Add init time erratum here */
> +};

This needs to be static.

Unnecessary & on the function pointer.

> +static int qoriq_usb3_phy_init(struct phy *x)
> +{
> + struct qoriq_usb3_phy *phy = phy_get_drvdata(x);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(phy_erratum_tbl); i++)
> + if (phy->has_erratum_flag & 1 << i)
> + phy_erratum_tbl[i].qoriq_phy_erratum(phy);
> + return 0;
> +}
> +
> +static const struct phy_ops ops = {
> + .init   = qoriq_usb3_phy_init,
> + .owner  = THIS_MODULE,
> +};
> +
> +static int qoriq_usb3_phy_probe(struct platform_device *pdev)
> +{
> + struct qoriq_usb3_phy *phy;
> + struct phy *generic_phy;
> + struct phy_provider *phy_provider;
> + const struct of_device_id *of_id;
> + struct device *dev = >dev;
> + struct resource *res;
> + int i, ret;
> +
> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> + phy->dev = dev;
> +
> + of_id = of_match_device(dev->driver->of_match_table, dev);
> + if (!of_id) {
> + dev_err(dev, "failed to get device match\n");
> + ret = -EINVAL;
> + goto err_out;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "param_ctrl");
> + if (!res) {
> + dev_err(dev, "failed to get param_ctrl memory\n");
> + ret = -ENOENT;
> + goto err_out;
> + }
> +
> + phy->param_ctrl = devm_ioremap_resource(dev, res);
> + if (!phy->param_ctrl) {
> + dev_err(dev, "failed to remap param_ctrl memory\n");
> + ret = -ENOMEM;
> + goto err_out;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_base");
> + if (!res) {
> + dev_err(dev, "failed to get phy_base memory\n");
> + ret = -ENOENT;
> + goto err_out;
> + }
> +
> + phy->phy_base = devm_ioremap_resource(dev, res);
> + if (!phy->phy_base) {
> + dev_err(dev, "failed to remap phy_base memory\n");
> + ret = -ENOMEM;
> + goto err_out;
> + 

Re: [PATCH v5 3/4] usb: musb: Add a new argument to musb_platform_set_mode()

[Alexandre Bailon]

On 11/14/2016 06:36 PM, Bin Liu wrote:
> Hi,
> 
> On Mon, Nov 07, 2016 at 02:05:07PM +0100, Alexandre Bailon wrote:
>> During the init, the driver will use musb_platform_set_mode()
>> to configure the controller mode and the PHY mode.
>> The PHY of DA8xx has some issues when the PHY is forced in host or device,
>> so we want to keep it in OTG mode unless the user request a specific mode
>> by using the sysfs.
>> Add the init argument to musb_platform_set_mode() in order to let the
>> callback change its behavior if it is called during the init.
> 
> Tony's patch set which fixes musb pm regression (but has not been merged
> yet) introduces musb->is_initialized. You might want to use this new
> variable in da8xx_musb_set_mode(), instead of adding this init flag.
OK. I will take a look and update the series.
> 
> Regards,
> -Bin.
> 
>>
>> Signed-off-by: Alexandre Bailon 
>> ---
>>  drivers/usb/musb/am35x.c |  2 +-
>>  drivers/usb/musb/blackfin.c  |  2 +-
>>  drivers/usb/musb/da8xx.c |  2 +-
>>  drivers/usb/musb/davinci.c   |  2 +-
>>  drivers/usb/musb/musb_core.c | 12 ++--
>>  drivers/usb/musb/musb_core.h |  6 +++---
>>  drivers/usb/musb/musb_dsps.c |  2 +-
>>  drivers/usb/musb/sunxi.c |  2 +-
>>  drivers/usb/musb/tusb6010.c  |  2 +-
>>  9 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
>> index 50ca805..7136888 100644
>> --- a/drivers/usb/musb/am35x.c
>> +++ b/drivers/usb/musb/am35x.c
>> @@ -332,7 +332,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void 
>> *hci)
>>  return ret;
>>  }
>>  
>> -static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>>  {
>>  struct device *dev = musb->controller;
>>  struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
>> diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
>> index 310238c..544e98f 100644
>> --- a/drivers/usb/musb/blackfin.c
>> +++ b/drivers/usb/musb/blackfin.c
>> @@ -356,7 +356,7 @@ static int bfin_musb_vbus_status(struct musb *musb)
>>  return 0;
>>  }
>>  
>> -static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>>  {
>>  return -EIO;
>>  }
>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>> index 6749aa1..ac0c2f7 100644
>> --- a/drivers/usb/musb/da8xx.c
>> +++ b/drivers/usb/musb/da8xx.c
>> @@ -335,7 +335,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void 
>> *hci)
>>  return ret;
>>  }
>>  
>> -static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
>> +static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>>  {
>>  struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
>>  enum phy_mode phy_mode;
>> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
>> index cee61a5..d12b902 100644
>> --- a/drivers/usb/musb/davinci.c
>> +++ b/drivers/usb/musb/davinci.c
>> @@ -369,7 +369,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void 
>> *__hci)
>>  return retval;
>>  }
>>  
>> -static int davinci_musb_set_mode(struct musb *musb, u8 mode)
>> +static int davinci_musb_set_mode(struct musb *musb, u8 mode, bool init)
>>  {
>>  /* EVM can't do this (right?) */
>>  return -EIO;
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 27dadc0..4a8d394 100644
>> --- a/drivers/usb/musb/musb_core.c
>> +++ b/drivers/usb/musb/musb_core.c
>> @@ -1730,11 +1730,11 @@ musb_mode_store(struct device *dev, struct 
>> device_attribute *attr,
>>  
>>  spin_lock_irqsave(>lock, flags);
>>  if (sysfs_streq(buf, "host"))
>> -status = musb_platform_set_mode(musb, MUSB_HOST);
>> +status = musb_platform_set_mode(musb, MUSB_HOST, false);
>>  else if (sysfs_streq(buf, "peripheral"))
>> -status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
>> +status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, false);
>>  else if (sysfs_streq(buf, "otg"))
>> -status = musb_platform_set_mode(musb, MUSB_OTG);
>> +status = musb_platform_set_mode(musb, MUSB_OTG, false);
>>  else
>>  status = -EINVAL;
>>  spin_unlock_irqrestore(>lock, flags);
>> @@ -2261,13 +2261,13 @@ musb_init_controller(struct device *dev, int nIrq, 
>> void __iomem *ctrl)
>>  status = musb_host_setup(musb, plat->power);
>>  if (status < 0)
>>  goto fail3;
>> -status = musb_platform_set_mode(musb, MUSB_HOST);
>> +status = musb_platform_set_mode(musb, MUSB_HOST, true);
>>  break;
>>  case MUSB_PORT_MODE_GADGET:
>>  status = musb_gadget_setup(musb);
>>  if (status < 0)
>>  goto fail3;
>> -status = 

Re: [PATCH v5 3/4] usb: musb: Add a new argument to musb_platform_set_mode()

[Bin Liu]

Hi,

On Mon, Nov 07, 2016 at 02:05:07PM +0100, Alexandre Bailon wrote:
> During the init, the driver will use musb_platform_set_mode()
> to configure the controller mode and the PHY mode.
> The PHY of DA8xx has some issues when the PHY is forced in host or device,
> so we want to keep it in OTG mode unless the user request a specific mode
> by using the sysfs.
> Add the init argument to musb_platform_set_mode() in order to let the
> callback change its behavior if it is called during the init.

Tony's patch set which fixes musb pm regression (but has not been merged
yet) introduces musb->is_initialized. You might want to use this new
variable in da8xx_musb_set_mode(), instead of adding this init flag.

Regards,
-Bin.

> 
> Signed-off-by: Alexandre Bailon 
> ---
>  drivers/usb/musb/am35x.c |  2 +-
>  drivers/usb/musb/blackfin.c  |  2 +-
>  drivers/usb/musb/da8xx.c |  2 +-
>  drivers/usb/musb/davinci.c   |  2 +-
>  drivers/usb/musb/musb_core.c | 12 ++--
>  drivers/usb/musb/musb_core.h |  6 +++---
>  drivers/usb/musb/musb_dsps.c |  2 +-
>  drivers/usb/musb/sunxi.c |  2 +-
>  drivers/usb/musb/tusb6010.c  |  2 +-
>  9 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
> index 50ca805..7136888 100644
> --- a/drivers/usb/musb/am35x.c
> +++ b/drivers/usb/musb/am35x.c
> @@ -332,7 +332,7 @@ static irqreturn_t am35x_musb_interrupt(int irq, void 
> *hci)
>   return ret;
>  }
>  
> -static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int am35x_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>  {
>   struct device *dev = musb->controller;
>   struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
> diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
> index 310238c..544e98f 100644
> --- a/drivers/usb/musb/blackfin.c
> +++ b/drivers/usb/musb/blackfin.c
> @@ -356,7 +356,7 @@ static int bfin_musb_vbus_status(struct musb *musb)
>   return 0;
>  }
>  
> -static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int bfin_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>  {
>   return -EIO;
>  }
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index 6749aa1..ac0c2f7 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -335,7 +335,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void 
> *hci)
>   return ret;
>  }
>  
> -static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
> +static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode, bool init)
>  {
>   struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
>   enum phy_mode phy_mode;
> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
> index cee61a5..d12b902 100644
> --- a/drivers/usb/musb/davinci.c
> +++ b/drivers/usb/musb/davinci.c
> @@ -369,7 +369,7 @@ static irqreturn_t davinci_musb_interrupt(int irq, void 
> *__hci)
>   return retval;
>  }
>  
> -static int davinci_musb_set_mode(struct musb *musb, u8 mode)
> +static int davinci_musb_set_mode(struct musb *musb, u8 mode, bool init)
>  {
>   /* EVM can't do this (right?) */
>   return -EIO;
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 27dadc0..4a8d394 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1730,11 +1730,11 @@ musb_mode_store(struct device *dev, struct 
> device_attribute *attr,
>  
>   spin_lock_irqsave(>lock, flags);
>   if (sysfs_streq(buf, "host"))
> - status = musb_platform_set_mode(musb, MUSB_HOST);
> + status = musb_platform_set_mode(musb, MUSB_HOST, false);
>   else if (sysfs_streq(buf, "peripheral"))
> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> + status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, false);
>   else if (sysfs_streq(buf, "otg"))
> - status = musb_platform_set_mode(musb, MUSB_OTG);
> + status = musb_platform_set_mode(musb, MUSB_OTG, false);
>   else
>   status = -EINVAL;
>   spin_unlock_irqrestore(>lock, flags);
> @@ -2261,13 +2261,13 @@ musb_init_controller(struct device *dev, int nIrq, 
> void __iomem *ctrl)
>   status = musb_host_setup(musb, plat->power);
>   if (status < 0)
>   goto fail3;
> - status = musb_platform_set_mode(musb, MUSB_HOST);
> + status = musb_platform_set_mode(musb, MUSB_HOST, true);
>   break;
>   case MUSB_PORT_MODE_GADGET:
>   status = musb_gadget_setup(musb);
>   if (status < 0)
>   goto fail3;
> - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
> + status = musb_platform_set_mode(musb, MUSB_PERIPHERAL, true);
>   break;
>   case MUSB_PORT_MODE_DUAL_ROLE:
>  

Re: [PATCH net 2/2] r8152: rx descriptor check

[David Miller]

From: Hayes Wang 
Date: Mon, 14 Nov 2016 07:23:51 +

> Mark Lord [mailto:ml...@pobox.com]
>> Sent: Monday, November 14, 2016 4:34 AM
> [...]
>> Perhaps the driver
>> is somehow accessing the buffer space again after doing usb_submit_urb()?
>> That would certainly produce this kind of behaviour.
> 
> I don't think so. First, the driver only read the received buffer.
> That is, the driver would not change (or write) the data. Second,
> The driver would lose the point address of the received buffer
> after submitting the urb to the USB host controller, until the
> transfer is completed by the USB host controller. That is, the
> driver doesn't how to access the buffer after calling usb_submit_urb().

This is why it's most likely some DMA implementation issue or similar.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Rob Herring]

On Tue, Nov 08, 2016 at 09:48:03AM -0800, John Youn wrote:
> On 11/8/2016 1:12 AM, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > John Youn  writes:
> >> Add a vendor prefix and make the name more consistent by renaming it to
> >> "snps,gadget-dma-enable".
> >>
> >> Signed-off-by: John Youn 
> >> ---
> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 -
> >>  arch/arm/boot/dts/rk3036.dtsi  | 2 +-
> >>  arch/arm/boot/dts/rk3288.dtsi  | 2 +-
> >>  arch/arm/boot/dts/rk3xxx.dtsi  | 2 +-
> >>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi  | 2 +-
> >>  arch/arm64/boot/dts/rockchip/rk3368.dtsi   | 2 +-
> >>  drivers/usb/dwc2/params.c  | 9 -
> >>  drivers/usb/dwc2/pci.c | 2 +-
> >>  8 files changed, 18 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
> >> b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> index 9472111..389a461 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer 
> >> properties
> >>  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >>Refer to usb/generic.txt
> >>  - snps,host-dma-disable: disable host DMA mode.
> >> -- g-use-dma: enable dma usage in gadget driver.
> >> +- snps,gadget-dma-enable: enable gadget DMA mode.
> > 
> > I don't see why you even have this binding. Looking through the code,
> > you have:
> > 
> > #define GHWCFG2_SLAVE_ONLY_ARCH 0
> > #define GHWCFG2_EXT_DMA_ARCH1
> > #define GHWCFG2_INT_DMA_ARCH2
> > 
> > void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
> > {
> > int valid = 1;
> > 
> > if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
> > valid = 0;
> > if (val < 0)
> > valid = 0;
> > 
> > if (!valid) {
> > if (val >= 0)
> > dev_err(hsotg->dev,
> > "%d invalid for dma_enable parameter. Check HW 
> > configuration.\n",
> > val);
> > val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
> > dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
> > }
> > 
> > hsotg->core_params->dma_enable = val;
> > }
> > 
> > which seems to hint that DMA support is discoverable. If there is DMA,
> > why would disable it?
> > 
> 
> Yes that's the case and I would prefer to make it discoverable and
> enabled by default.
> 
> But the legacy behavior has always been like this because DMA was
> never fully implemented in the gadget driver and it was an opt-in
> feature. Periodic support was only added recently.
> 
> What do you think about enabling it by default now? I think most
> platforms already use DMA.
> 
> We would still need a "disable" binding for IP validation purposes at
> least.

You can hack up your kernel for that. You may need a disable for broken 
h/w perhaps.

Rob
--
To unsubscribe from this list: send the line "unsubscribe 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: add amcc,dwc-otg device tree definition

[Rob Herring]

On Sun, Nov 06, 2016 at 01:56:08AM +0100, Christian Lamparter wrote:
> This patch adds support for the "amcc,usb-otg" device
> which is found in the PowerPC Canyonlands' dts.
> 
> The device definition was added by:
> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands 
> board")'.
> AMCC produced a standalone driver that was sent to the
> linuxppc-dev at the time. However, it was never integrated.
> 
> Signed-off-by: Christian Lamparter 
> ---
> For anyone interested: the driver was sent to the ML multiple times back
> in 2012 [0], [1].
> 
> [0] 
> [1] 
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt |  1 +

I would say this needs to be more specific, but given it has been there 
for 6 years:

Acked-by: Rob Herring 

>  drivers/usb/dwc2/platform.c| 33 
> ++
>  2 files changed, 34 insertions(+)
--
To unsubscribe from this list: send the line "unsubscribe 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 6/6] usb: musb: Drop pointless PM runtime code for dsps glue

[Johan Hovold]

On Fri, Nov 11, 2016 at 10:43:02AM -0800, Tony Lindgren wrote:
> This already gets done automatically by PM runtime and we have
> a separate autosuspend timeout in musb_core.c.
> 
> Signed-off-by: Tony Lindgren 

> @@ -816,8 +801,6 @@ static int dsps_remove(struct platform_device *pdev)
>   platform_device_unregister(glue->musb);
>  
>   /* disable usbss clocks */

Perhaps also drop this comment which no longer applies.

> - pm_runtime_dont_use_autosuspend(>dev);
> - pm_runtime_put_sync(>dev);
>   pm_runtime_disable(>dev);

Reviewed-by: Johan Hovold 

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

Re: [PATCH 5/6] usb: musb: Add missing pm_runtime_disable and drop 2430 PM timeout

[Johan Hovold]

On Fri, Nov 11, 2016 at 10:43:01AM -0800, Tony Lindgren wrote:
> We are missing pm_runtime_disable() in 2430 glue layer. Further,
> we only need to enable PM runtime and disable it on exit. With
> musb_core.c doing PM, the glue layer as a parent will always be
> active when musb_core.c is active.

In fact, we have to do this way to avoid a crash in omap2430_remove().

> This fixes host enumeration issues with some devices as reported
> by Ladislav Michl .
> 
> If changes are needed to the autosuspend timeout, it should be
> done in musb_core.c.
> 
> Reported-by: Ladislav Michl 
> Fixes: 87326e858448 ("usb: musb: Remove extra PM runtime calls from
> 2430 glue layer")
> Tested-by: Ladislav Michl 
> Signed-off-by: Tony Lindgren 

> @@ -535,10 +536,7 @@ static int omap2430_remove(struct platform_device *pdev)
>  {
>   struct omap2430_glue *glue = platform_get_drvdata(pdev);
>  
> - pm_runtime_get_sync(glue->dev);
>   platform_device_unregister(glue->musb);
> - pm_runtime_put_sync(glue->dev);

Holding an RPM reference while deregistering the child, would lead to a
crash in omap2430_runtime_suspend() which dereferences the now freed
child's driver data on put.

Unable to handle kernel paging request at virtual address 6b6b6f17
...
[] (omap2430_runtime_suspend) from [] 
(pm_generic_runtime_suspend+0x3c/0x48)
[] (pm_generic_runtime_suspend) from [] 
(_od_runtime_suspend+0x1c/0x30)
[] (_od_runtime_suspend) from [] (__rpm_callback+0x3c/0x70)
[] (__rpm_callback) from [] (rpm_callback+0x30/0x90)
[] (rpm_callback) from [] (rpm_suspend+0x118/0x6b4)
[] (rpm_suspend) from [] (rpm_idle+0x104/0x440)
[] (rpm_idle) from [] (__pm_runtime_idle+0x7c/0xb0)
[] (__pm_runtime_idle) from [] (omap2430_remove+0x38/0x58)
[] (omap2430_remove) from [] (platform_drv_remove+0x34/0x4c)

> - pm_runtime_dont_use_autosuspend(glue->dev);
>   pm_runtime_disable(glue->dev);
>  
>   return 0;

This might be worth mentioning in the commit message, but feel free to
add

Reviewed-by: Johan Hovold 

either way.

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

Re: [PATHCv10 1/2] usb: USB Type-C connector class

[Greg KH]

On Mon, Nov 14, 2016 at 04:39:10PM +0200, Heikki Krogerus wrote:
> On Mon, Nov 14, 2016 at 03:11:23PM +0100, Greg KH wrote:
> > On Mon, Nov 14, 2016 at 02:32:35PM +0200, Heikki Krogerus wrote:
> > > > > +static void __exit typec_exit(void)
> > > > > +{
> > > > > + class_unregister(_class);
> > > > 
> > > > You forgot to clean up your idr :(
> > > 
> > > Sorry, what idr? The port ids get removed in typec_release().
> > 
> > You have a static idr structure in the driver, right?  You have to clean
> > it up when your code is going away so that it will free any memory it
> > had allocated with a call to idr_destroy() on module exit.
> 
> Ok.
> 
> Regarding the DEVICE_ATTR* macros. So I have attributes with same
> names for different device types. I may be able to identify the device
> types and deal with the correct attribute based on that, but for
> example the attribute "active" with alternate modes is writable, but
> with cables it's not. How do I handle those?

The attribute init callback should let you handle this, right?  You'll
have to add it for your dynamic attributes.

thanks,

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] dma: cpp41: Fix handling of error path

[Tony Lindgren]

* Johan Hovold  [161114 06:59]:
> On Mon, Nov 14, 2016 at 06:47:31AM -0800, Tony Lindgren wrote:
> > Hi,
> > 
> > * Johan Hovold  [161114 06:35]:
> > > On Fri, Nov 11, 2016 at 11:28:52AM -0800, Tony Lindgren wrote:
> > > > If we return early on pm_runtime_get() error, we need to also call
> > > > pm_runtime_put_noidle() as pointed out in a musb related thread
> > > > by Johan Hovold . This is to keep the PM runtime
> > > > use counts happy.
> > > > 
> > > > Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> > > > Cc: Johan Hovold 
> > > > Signed-off-by: Tony Lindgren 
> > > > ---
> > > >  drivers/dma/cppi41.c | 11 +--
> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > >  
> > > > @@ -466,6 +472,7 @@ static void cppi41_dma_issue_pending(struct 
> > > > dma_chan *chan)
> > > >  
> > > > error = pm_runtime_get(cdd->ddev.dev);
> > > > if ((error != -EINPROGRESS) && error < 0) {
> > > > +   pm_runtime_put_noidle(cdd->ddev.dev);
> > > > dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
> > > > error);
> > > 
> > > Will this chunk not introduce rather than fix an imbalance, though? An
> > > error is never returned above, and the corresponding put is done
> > > unconditionally as far as I can tell.
> > 
> > There is already an early return in cppi41_dma_issue_pending() on
> > error.
> 
> Indeed, but before
> 
>   "dma: cppi41: Fix unpaired pm runtime when only a USB hub is
> connected"
> 
> from last week, the corresponding put in cppi41_irq() was done
> unconditionally and would have required an unconditional get here.

Oh yeah that's right as the pm_runtime_mark_last_busy() and
pm_runtime_put_autosuspend() got moved.

Regards,

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

Re: [PATCH] dma: cpp41: Fix handling of error path

[Johan Hovold]

On Mon, Nov 14, 2016 at 06:47:31AM -0800, Tony Lindgren wrote:
> Hi,
> 
> * Johan Hovold  [161114 06:35]:
> > On Fri, Nov 11, 2016 at 11:28:52AM -0800, Tony Lindgren wrote:
> > > If we return early on pm_runtime_get() error, we need to also call
> > > pm_runtime_put_noidle() as pointed out in a musb related thread
> > > by Johan Hovold . This is to keep the PM runtime
> > > use counts happy.
> > > 
> > > Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> > > Cc: Johan Hovold 
> > > Signed-off-by: Tony Lindgren 
> > > ---
> > >  drivers/dma/cppi41.c | 11 +--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> >  
> > > @@ -466,6 +472,7 @@ static void cppi41_dma_issue_pending(struct dma_chan 
> > > *chan)
> > >  
> > >   error = pm_runtime_get(cdd->ddev.dev);
> > >   if ((error != -EINPROGRESS) && error < 0) {
> > > + pm_runtime_put_noidle(cdd->ddev.dev);
> > >   dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
> > >   error);
> > 
> > Will this chunk not introduce rather than fix an imbalance, though? An
> > error is never returned above, and the corresponding put is done
> > unconditionally as far as I can tell.
> 
> There is already an early return in cppi41_dma_issue_pending() on
> error.

Indeed, but before

"dma: cppi41: Fix unpaired pm runtime when only a USB hub is
connected"

from last week, the corresponding put in cppi41_irq() was done
unconditionally and would have required an unconditional get here.

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

[PATCH v5 4/5] USB: ohci: da8xx: Add devicetree bindings

[Axel Haslam]

This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's

Cc: robh...@kernel.org
Cc: mark.rutl...@arm.com
Cc: devicet...@vger.kernel.org
Signed-off-by: Axel Haslam 
---
 .../devicetree/bindings/usb/ohci-da8xx.txt | 23 ++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt

diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt 
b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 000..2dc8f67
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,23 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg:Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys:   Phandle for the PHY device
+ - phy-names:  Should be "usb-phy"
+
+Optional properties:
+ - vbus-supply: phandle of regulator that controls vbus power / over-current
+
+Example:
+
+ohci: usb@0225000 {
+compatible = "ti,da830-ohci";
+reg = <0x225000 0x1000>;
+interrupts = <59>;
+phys = <_phy 1>;
+phy-names = "usb-phy";
+vbus-supply = <_usb_ohci>;
+};
-- 
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: [PATCH 0/2] Two cpp41 pm runtime found when testing with usb

[Tony Lindgren]

* Vinod Koul  [161113 21:19]:
> On Wed, Nov 09, 2016 at 09:47:57AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > I found two pm runtime issues when testing with usb on beaglebone.
> > 
> > In the am335x case cppi41 and two instances of musb controller share
> > the same interconnect wrapper module, so any pm issues with musb or
> > cppi41 will keep the interconnect wrapper module busy.
> 
> Applied both. And as I have told you previously please use the correct
> subsystem tag. I have fixed them again!

Sorry about that. What do you prefer for future reference? We are using
both "dma: cppi41" and "dmaengine: cppi41" currently..

Regards,

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

Re: [PATCH] dma: cpp41: Fix handling of error path

[Tony Lindgren]

Hi,

* Johan Hovold  [161114 06:35]:
> On Fri, Nov 11, 2016 at 11:28:52AM -0800, Tony Lindgren wrote:
> > If we return early on pm_runtime_get() error, we need to also call
> > pm_runtime_put_noidle() as pointed out in a musb related thread
> > by Johan Hovold . This is to keep the PM runtime
> > use counts happy.
> > 
> > Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> > Cc: Johan Hovold 
> > Signed-off-by: Tony Lindgren 
> > ---
> >  drivers/dma/cppi41.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
>  
> > @@ -466,6 +472,7 @@ static void cppi41_dma_issue_pending(struct dma_chan 
> > *chan)
> >  
> > error = pm_runtime_get(cdd->ddev.dev);
> > if ((error != -EINPROGRESS) && error < 0) {
> > +   pm_runtime_put_noidle(cdd->ddev.dev);
> > dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
> > error);
> 
> Will this chunk not introduce rather than fix an imbalance, though? An
> error is never returned above, and the corresponding put is done
> unconditionally as far as I can tell.

There is already an early return in cppi41_dma_issue_pending() on
error.

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: [PATHCv10 1/2] usb: USB Type-C connector class

[Heikki Krogerus]

On Mon, Nov 14, 2016 at 03:11:23PM +0100, Greg KH wrote:
> On Mon, Nov 14, 2016 at 02:32:35PM +0200, Heikki Krogerus wrote:
> > > > +static void __exit typec_exit(void)
> > > > +{
> > > > +   class_unregister(_class);
> > > 
> > > You forgot to clean up your idr :(
> > 
> > Sorry, what idr? The port ids get removed in typec_release().
> 
> You have a static idr structure in the driver, right?  You have to clean
> it up when your code is going away so that it will free any memory it
> had allocated with a call to idr_destroy() on module exit.

Ok.

Regarding the DEVICE_ATTR* macros. So I have attributes with same
names for different device types. I may be able to identify the device
types and deal with the correct attribute based on that, but for
example the attribute "active" with alternate modes is writable, but
with cables it's not. How do I handle those?


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe 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] dma: cpp41: Fix handling of error path

[Johan Hovold]

On Mon, Nov 14, 2016 at 03:34:54PM +0100, Johan Hovold wrote:
> On Fri, Nov 11, 2016 at 11:28:52AM -0800, Tony Lindgren wrote:
> > If we return early on pm_runtime_get() error, we need to also call
> > pm_runtime_put_noidle() as pointed out in a musb related thread
> > by Johan Hovold . This is to keep the PM runtime
> > use counts happy.
> > 
> > Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> > Cc: Johan Hovold 
> > Signed-off-by: Tony Lindgren 
> > ---
> >  drivers/dma/cppi41.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
>  
> > @@ -466,6 +472,7 @@ static void cppi41_dma_issue_pending(struct dma_chan 
> > *chan)
> >  
> > error = pm_runtime_get(cdd->ddev.dev);
> > if ((error != -EINPROGRESS) && error < 0) {
> > +   pm_runtime_put_noidle(cdd->ddev.dev);
> > dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
> > error);
> 
> Will this chunk not introduce rather than fix an imbalance, though? An
> error is never returned above, and the corresponding put is done
> unconditionally as far as I can tell.

Ah, that's no longer an issue after

"dma: cppi41: Fix unpaired pm runtime when only a USB hub is
connected"

from last week.

Sorry for the noise.

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

[PATCH v5 1/5] USB: ohci: da8xx: use ohci priv data instead of globals

[Axel Haslam]

Instead of global variables, use the extra_priv_size of
the ohci driver.

We cannot yet move the ocic mask because this is used on
the interrupt handler which is registerded through platform
data and does not have an hcd pointer. This will be moved
on a later patch.

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 73 +--
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index b3de8bc..438970b 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -35,43 +35,50 @@ static int (*orig_ohci_hub_control)(struct usb_hcd  *hcd, 
u16 typeReq,
u16 wValue, u16 wIndex, char *buf, u16 wLength);
 static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
 
-static struct clk *usb11_clk;
-static struct phy *usb11_phy;
+struct da8xx_ohci_hcd {
+   struct clk *usb11_clk;
+   struct phy *usb11_phy;
+};
+
+#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
 
 /* Over-current indicator change bitmask */
 static volatile u16 ocic_mask;
 
-static int ohci_da8xx_enable(void)
+static int ohci_da8xx_enable(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int ret;
 
-   ret = clk_prepare_enable(usb11_clk);
+   ret = clk_prepare_enable(da8xx_ohci->usb11_clk);
if (ret)
return ret;
 
-   ret = phy_init(usb11_phy);
+   ret = phy_init(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_init;
 
-   ret = phy_power_on(usb11_phy);
+   ret = phy_power_on(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_power_on;
 
return 0;
 
 err_phy_power_on:
-   phy_exit(usb11_phy);
+   phy_exit(da8xx_ohci->usb11_phy);
 err_phy_init:
-   clk_disable_unprepare(usb11_clk);
+   clk_disable_unprepare(da8xx_ohci->usb11_clk);
 
return ret;
 }
 
-static void ohci_da8xx_disable(void)
+static void ohci_da8xx_disable(struct usb_hcd *hcd)
 {
-   phy_power_off(usb11_phy);
-   phy_exit(usb11_phy);
-   clk_disable_unprepare(usb11_clk);
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+   phy_power_off(da8xx_ohci->usb11_phy);
+   phy_exit(da8xx_ohci->usb11_phy);
+   clk_disable_unprepare(da8xx_ohci->usb11_clk);
 }
 
 /*
@@ -97,7 +104,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
 
dev_dbg(dev, "starting USB controller\n");
 
-   result = ohci_da8xx_enable();
+   result = ohci_da8xx_enable(hcd);
if (result < 0)
return result;
 
@@ -109,7 +116,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
 
result = ohci_setup(hcd);
if (result < 0) {
-   ohci_da8xx_disable();
+   ohci_da8xx_disable(hcd);
return result;
}
 
@@ -231,6 +238,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
 static int ohci_da8xx_probe(struct platform_device *pdev)
 {
struct da8xx_ohci_root_hub *hub = dev_get_platdata(>dev);
+   struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd  *hcd;
struct resource *mem;
int error, irq;
@@ -238,25 +246,29 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
if (hub == NULL)
return -ENODEV;
 
-   usb11_clk = devm_clk_get(>dev, "usb11");
-   if (IS_ERR(usb11_clk)) {
-   if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
+   hcd = usb_create_hcd(_da8xx_hc_driver, >dev,
+   dev_name(>dev));
+   if (!hcd)
+   return -ENOMEM;
+
+   da8xx_ohci = to_da8xx_ohci(hcd);
+
+   da8xx_ohci->usb11_clk = devm_clk_get(>dev, "usb11");
+   if (IS_ERR(da8xx_ohci->usb11_clk)) {
+   if (PTR_ERR(da8xx_ohci->usb11_clk) != -EPROBE_DEFER)
dev_err(>dev, "Failed to get clock.\n");
-   return PTR_ERR(usb11_clk);
+   error = PTR_ERR(da8xx_ohci->usb11_clk);
+   goto err;
}
 
-   usb11_phy = devm_phy_get(>dev, "usb-phy");
-   if (IS_ERR(usb11_phy)) {
-   if (PTR_ERR(usb11_phy) != -EPROBE_DEFER)
+   da8xx_ohci->usb11_phy = devm_phy_get(>dev, "usb-phy");
+   if (IS_ERR(da8xx_ohci->usb11_phy)) {
+   if (PTR_ERR(da8xx_ohci->usb11_phy) != -EPROBE_DEFER)
dev_err(>dev, "Failed to get phy.\n");
-   return PTR_ERR(usb11_phy);
+   error = PTR_ERR(da8xx_ohci->usb11_phy);
+   goto err;
}
 
-   hcd = usb_create_hcd(_da8xx_hc_driver, >dev,
-   dev_name(>dev));
-   if (!hcd)
-   return -ENOMEM;
-
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(>dev, mem);
if (IS_ERR(hcd->regs)) {
@@ 

[PATCH v5 2/5] USB: ohci: da8xx: Add wrappers for platform callbacks

[Axel Haslam]

In preparation to use a regulator instead of platform callbacks,
move the platform callbacks into separate functions.

This provides a well defined place to for the regulator API to coexist
with the callbacks until all users are converted, and the callbacks
can be removed.

Signed-off-by: Axel Haslam 
---
 drivers/usb/host/ohci-da8xx.c | 125 ++
 1 file changed, 102 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 438970b..83e3c98 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -81,6 +81,72 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
clk_disable_unprepare(da8xx_ohci->usb11_clk);
 }
 
+static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->set_power)
+   return hub->set_power(1, on);
+
+   return 0;
+}
+
+static int ohci_da8xx_get_power(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->get_power)
+   return hub->get_power(1);
+
+   return 1;
+}
+
+static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->get_oci)
+   return hub->get_oci(1);
+
+   return 0;
+}
+
+static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->set_power)
+   return 1;
+
+   return 0;
+}
+
+static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->get_oci)
+   return 1;
+
+   return 0;
+}
+
+static int ohci_da8xx_has_potpgt(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->potpgt)
+   return 1;
+
+   return 0;
+}
+
 /*
  * Handle the port over-current indicator change.
  */
@@ -94,6 +160,26 @@ static void ohci_da8xx_ocic_handler(struct 
da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
 }
 
+static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->ocic_notify)
+   return hub->ocic_notify(ohci_da8xx_ocic_handler);
+
+   return 0;
+}
+
+static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
+{
+   struct device *dev  = hcd->self.controller;
+   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+   if (hub && hub->ocic_notify)
+   hub->ocic_notify(NULL);
+}
+
 static int ohci_da8xx_reset(struct usb_hcd *hcd)
 {
struct device *dev  = hcd->self.controller;
@@ -127,16 +213,18 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
 * the correct hub descriptor...
 */
rh_a = ohci_readl(ohci, >regs->roothub.a);
-   if (hub->set_power) {
+   if (ohci_da8xx_has_set_power(hcd)) {
rh_a &= ~RH_A_NPS;
rh_a |=  RH_A_PSM;
}
-   if (hub->get_oci) {
+   if (ohci_da8xx_has_oci(hcd)) {
rh_a &= ~RH_A_NOCP;
rh_a |=  RH_A_OCPM;
}
-   rh_a &= ~RH_A_POTPGT;
-   rh_a |= hub->potpgt << 24;
+   if (ohci_da8xx_has_potpgt(hcd)) {
+   rh_a &= ~RH_A_POTPGT;
+   rh_a |= hub->potpgt << 24;
+   }
ohci_writel(ohci, rh_a, >regs->roothub.a);
 
return result;
@@ -169,7 +257,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
  u16 wIndex, char *buf, u16 wLength)
 {
struct device *dev  = hcd->self.controller;
-   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int temp;
 
switch (typeReq) {
@@ -183,11 +270,11 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, 
u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
 
/* The port power status (PPS) bit defaults to 1 */
-   if (hub->get_power && hub->get_power(wIndex) == 0)
+   if (!ohci_da8xx_get_power(hcd))
temp &= ~RH_PS_PPS;
 
/* The port over-current indicator (POCI) bit is always 0 */
-   if (hub->get_oci 

[PATCH v5 3/5] USB: ohci: da8xx: Allow a regulator to handle VBUS

[Axel Haslam]

Using a regulator to handle VBUS will eliminate the need for
platform data and callbacks, and make the driver more generic
allowing different types of regulators to handle VBUS.

The regulator equivalents to the platform callbacks are:
set_power   ->  regulator_enable/regulator_disable
get_power   ->  regulator_is_enabled
get_oci ->  regulator_get_error_flags
ocic_notify ->  regulator event notification

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

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 83e3c98..42eaeb9 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd  *hcd, 
u16 typeReq,
 static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
 
 struct da8xx_ohci_hcd {
+   struct usb_hcd *hcd;
struct clk *usb11_clk;
struct phy *usb11_phy;
+   struct regulator *vbus_reg;
+   struct notifier_block nb;
+   unsigned int is_powered;
 };
 
 #define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
@@ -83,56 +88,103 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
 
 static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+   int ret;
 
if (hub && hub->set_power)
return hub->set_power(1, on);
 
+   if (!da8xx_ohci->vbus_reg)
+   return 0;
+
+   if (on && !da8xx_ohci->is_powered) {
+   ret = regulator_enable(da8xx_ohci->vbus_reg);
+   if (ret) {
+   dev_err(dev, "Fail to enable regulator: %d\n", ret);
+   return ret;
+   }
+   da8xx_ohci->is_powered = 1;
+
+   } else if (!on && da8xx_ohci->is_powered) {
+   ret = regulator_disable(da8xx_ohci->vbus_reg);
+   if (ret) {
+   dev_err(dev, "Fail to disable regulator: %d\n", ret);
+   return ret;
+   }
+   da8xx_ohci->is_powered = 0;
+   }
+
return 0;
 }
 
 static int ohci_da8xx_get_power(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
 
if (hub && hub->get_power)
return hub->get_power(1);
 
+   if (da8xx_ohci->vbus_reg)
+   return regulator_is_enabled(da8xx_ohci->vbus_reg);
+
return 1;
 }
 
 static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+   unsigned int flags;
+   int ret;
 
if (hub && hub->get_oci)
return hub->get_oci(1);
 
+   if (!da8xx_ohci->vbus_reg)
+   return 0;
+
+   ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, );
+   if (ret)
+   return ret;
+
+   if (flags && REGULATOR_ERROR_OVER_CURRENT)
+   return 1;
+
return 0;
 }
 
 static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
 
if (hub && hub->set_power)
return 1;
 
+   if (da8xx_ohci->vbus_reg)
+   return 1;
+
return 0;
 }
 
 static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
 {
+   struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev  = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
 
if (hub && hub->get_oci)
return 1;
 
+   if (da8xx_ohci->vbus_reg)
+   return 1;
+
return 0;
 }
 
@@ -160,15 +212,41 @@ static void ohci_da8xx_ocic_handler(struct 
da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
 }
 
+static int ohci_da8xx_regulator_event(struct notifier_block *nb,
+   unsigned long event, void *data)
+{
+   struct da8xx_ohci_hcd *da8xx_ohci =
+   container_of(nb, struct da8xx_ohci_hcd, nb);
+   struct device *dev = da8xx_ohci->hcd->self.controller;
+
+   if (event & REGULATOR_EVENT_OVER_CURRENT) {
+   dev_warn(dev, "over current event\n");
+

[PATCH v5 5/5] USB: ohci: da8xx: Allow probing from DT

[Axel Haslam]

This adds the compatible string to the ohci driver
to be able to probe from DT

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

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 42eaeb9..b761b2b 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -396,6 +396,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
 }
 
 /*-*/
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_ohci_ids[] = {
+   { .compatible = "ti,da830-ohci" },
+   { }
+};
+MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
+#endif
 
 static int ohci_da8xx_probe(struct platform_device *pdev)
 {
@@ -547,6 +554,7 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
 #endif
.driver = {
.name   = DRV_NAME,
+   .of_match_table = of_match_ptr(da8xx_ohci_ids),
},
 };
 
-- 
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 v5 0/5] USB: ohci-da8xx: Add device tree support

[Axel Haslam]

When booting using device tree, we can not make use of
platform callbacks to handle vbus and over current gpios.

This series allows the ohci-da8xx driver to use a regulator
instead of the platform callbacks to control vbus and adds
the device tree bindings to be able to probe using DT.

Once all users of the platform callbacks will be converted to
use a regulator, we will be able to remove platform data completely.

Changes from v4->v5
* Append the Device tree patches to v4.
* Submit only the first part of the series (no dependencies).
this can be applied and merged through the usb tree.

Changes from v3->v4
* separate the series into smaller series for driver and arch/arm code,
  to ease review and merging to different trees.

Changes form v2->v3
* drop patches that have been integrated to arch/arm
* drop regulator patches which will be integrated through regulator tree
* use of the accepted regulator API to get over current status
* better patch separation with the use of wrappers

Changes from v1->v2
* Rebased and added patch to make ohci a separate driver
* Use a regulator instead of handling Gpios (David Lechner)
* Add an over current mode to regulator framework
* Fixed regulator is able to register for and over current irq
* Added patch by Alexandre to remove build warnings
* Moved global variables into private hcd structure.
Axel Haslam (5):
  USB: ohci: da8xx: use ohci priv data instead of globals
  USB: ohci: da8xx: Add wrappers for platform callbacks
  USB: ohci: da8xx: Allow a regulator to handle VBUS
  USB: ohci: da8xx: Add devicetree bindings
  USB: ohci: da8xx: Allow probing from DT

 .../devicetree/bindings/usb/ohci-da8xx.txt |  23 ++
 drivers/usb/host/ohci-da8xx.c  | 291 +
 2 files changed, 264 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt

-- 
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: [PATHCv10 1/2] usb: USB Type-C connector class

[Guenter Roeck]

On 11/14/2016 04:32 AM, Heikki Krogerus wrote:

Hi Greg,

On Mon, Nov 14, 2016 at 10:51:48AM +0100, Greg KH wrote:

On Mon, Sep 19, 2016 at 02:16:56PM +0300, Heikki Krogerus wrote:

The purpose of USB Type-C connector class is to provide
unified interface for the user space to get the status and
basic information about USB Type-C connectors on a system,
control over data role swapping, and when the port supports
USB Power Delivery, also control over power role swapping
and Alternate Modes.

Reviewed-by: Guenter Roeck 
Tested-by: Guenter Roeck 
Signed-off-by: Heikki Krogerus 


[ ... ]


+
+int typec_connect(struct typec_port *port, struct typec_connection *con)
+{
+   int ret;
+
+   if (!con->partner && !con->cable)
+   return -EINVAL;
+
+   port->connected = 1;
+   port->data_role = con->data_role;
+   port->pwr_role = con->pwr_role;
+   port->vconn_role = con->vconn_role;
+   port->pwr_opmode = con->pwr_opmode;
+
+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);


This worries me.  Who is listening for it?  What will you do with it?
Shouldn't you just poll on an attribute file instead?


Oliver! Did you need this or can we remove it?



I'll also have to make sure that the Android folks don't use it.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe 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] dma: cpp41: Fix handling of error path

[Johan Hovold]

On Fri, Nov 11, 2016 at 11:28:52AM -0800, Tony Lindgren wrote:
> If we return early on pm_runtime_get() error, we need to also call
> pm_runtime_put_noidle() as pointed out in a musb related thread
> by Johan Hovold . This is to keep the PM runtime
> use counts happy.
> 
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: Johan Hovold 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/dma/cppi41.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
 
> @@ -466,6 +472,7 @@ static void cppi41_dma_issue_pending(struct dma_chan 
> *chan)
>  
>   error = pm_runtime_get(cdd->ddev.dev);
>   if ((error != -EINPROGRESS) && error < 0) {
> + pm_runtime_put_noidle(cdd->ddev.dev);
>   dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
>   error);

Will this chunk not introduce rather than fix an imbalance, though? An
error is never returned above, and the corresponding put is done
unconditionally as far as I can tell.

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

Re: [PATCH 1/1] usb: chipidea: move the lock initialization to core file

[Greg KH]

On Mon, Oct 31, 2016 at 09:55:40AM +0800, Peter Chen wrote:
> This can fix below dump when the lock is accessed at host
> mode due to it is not initialized.
> 
> root@imx6qdlsolo:~# cat /sys/kernel/debug/ci_hdrc.1/port_test
> [  929.904518] INFO: trying to register non-static key.
> [  929.909536] the code is fine but needs lockdep annotation.
> [  929.915043] turning off the locking correctness validator.
> [  929.920567] CPU: 0 PID: 687 Comm: cat Not tainted 4.9.0-rc1-00064-g903de10 
> #1155
> [  929.927987] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
> [  929.934189] Backtrace:
> [  929.936719] [] (dump_backtrace) from [] 
> (show_stack+0x18/0x1c)
> [  929.944312]  r7:de47a000[  929.946690]  r6:6193
>  r5:[  929.950322]  r4:c0e2346c
>  [  929.952883]
>  [  929.954413] [] (show_stack) from [] 
> (dump_stack+0xb4/0xe8)
> [  929.961675] [] (dump_stack) from [] 
> (register_lock_class+0x4fc/0x56c)
> [  929.969871]  r10:c0e23564[  929.972335]  r9:de47be70
>  r8:c163f444[  929.975967]  r7:ddcd4024
>   r6:[  929.979598]  r5:
>   [  929.982158]  r4:[  929.984534]  r3:0001
>   [  929.987092]
>   [  929.988622] [] (register_lock_class) from [] 
> (__lock_acquire+0x80/0x10f0)
> [  929.997166]  r10:c0e23564[  929.999632]  r9:de47be70
>  r8:ddcd4024[  930.003265]  r7:c163f444
>   r6:ddec3c00[  930.006898]  r5:6193
>   [  930.009458]  r4:[  930.011832]
>   [  930.013368] [] (__lock_acquire) from [] 
> (lock_acquire+0x74/0x94)
> [  930.021129]  r10:0001[  930.023594]  r9:de47be70
>  r8:de47bf80[  930.027225]  r7:0001
>   r6:0001[  930.030857]  r5:6193
>   [  930.033417]  r4:[  930.035791]
>   [  930.037329] [] (lock_acquire) from [] 
> (_raw_spin_lock_irqsave+0x40/0x54)
> [  930.045787]  r7:dddb3c80[  930.048163]  r6:c062a468
>  r5:2113[  930.051795]  r4:ddcd4014
>  [  930.054354]
>  [  930.055885] [] (_raw_spin_lock_irqsave) from [] 
> (ci_port_test_show+0x2c/0x70)
> [  930.064776]  r6:ddd930c0[  930.067153]  r5:ddcd4010
>  r4:ddcd4014[  930.070783]
>  [  930.072329] [] (ci_port_test_show) from [] 
> (seq_read+0x1ac/0x4f8)
> [  930.080179]  r9:de47be70[  930.082555]  r8:de47bf80
>  r7:dddb3c80[  930.086188]  r6:0001
>   r5:[  930.089820]  r4:ddd930c0
>   [  930.092404] [] (seq_read) from [] 
> (full_proxy_read+0x54/0x6c)
> [  930.099907]  r10:[  930.102372]  r9:c0a6ad70
>  r8:de47bf80[  930.106004]  r7:0002
>   r6:b6dd1000[  930.109636]  r5:dddb3c80
>   [  930.112197]  r4:c0248ed8[  930.114572]
>   [  930.116110] [] (full_proxy_read) from [] 
> (__vfs_read+0x34/0x118)
> [  930.123872]  r9:de47a000[  930.126250]  r8:c0107fc4
>  r7:0002[  930.129883]  r6:de47bf80
>   r5:c039eb88[  930.133515]  r4:dddb3c80
>   [  930.136091] [] (__vfs_read) from [] 
> (vfs_read+0x8c/0x11c)
> [  930.143247]  r9:de47a000[  930.145625]  r8:c0107fc4
>  r7:de47bf80[  930.149259]  r6:b6dd1000
>   r5:dddb3c80[  930.152891]  r4:0002
>   [  930.155469] [] (vfs_read) from [] 
> (SyS_read+0x4c/0xa8)
> [  930.162363]  r8:c0107fc4[  930.164741]  r7:0002
>  r6:b6dd1000[  930.168373]  r5:dddb3c80
>   r4:dddb3c80[  930.172001]
>   [  930.173540] [] (SyS_read) from [] 
> (ret_fast_syscall+0x0/0x1c)
> [  930.181041]  r7:0003[  930.183419]  r6:b6dd1000
>  r5:0002[  930.187051]  r4:0002
>  [  930.189611]
>  mode = 0

Clean up the dump please :(
--
To unsubscribe from this list: send the line "unsubscribe 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: [PATHCv10 1/2] usb: USB Type-C connector class

[Greg KH]

On Mon, Nov 14, 2016 at 02:32:35PM +0200, Heikki Krogerus wrote:
> > > +static void __exit typec_exit(void)
> > > +{
> > > + class_unregister(_class);
> > 
> > You forgot to clean up your idr :(
> 
> Sorry, what idr? The port ids get removed in typec_release().

You have a static idr structure in the driver, right?  You have to clean
it up when your code is going away so that it will free any memory it
had allocated with a call to idr_destroy() on module exit.

thanks,

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

Issue with Telit LE922 and cdc_mbim

[Daniele Palmas]

Hi,

I'm struggling with Telit LE922 modem that presents an MBIM device.
The modem works fine in Windows, while in Linux (tested with 4.9 rc1)
data connection is not functional: using ifconfig I can see

wwp0s20u8i2 Link encap:Ethernet  HWaddr e6:c0:3b:97:80:de
  inet addr:176.246.94.9  Bcast:176.246.94.11  Mask:255.255.255.252
  UP BROADCAST RUNNING NOARP MULTICAST  MTU:1500  Metric:1
  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
  TX packets:1 errors:15 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000
  RX bytes:0 (0.0 B)  TX bytes:40 (40.0 B)

so the issue seems to be in tx.

I'm using an USB hw sniffer for trying to understand the problem; this
is an example of a NCM packet with Windows 10 driver:

4E 43 4D 48 0C 00 0B 02 48 00 38 00 45 00 00 28
78 E8 40 00 80 06 63 68 05 5C 7A E5 D8 3A C6 03
E9 08 01 BB FB 36 F0 EA C6 8C D8 CB 50 10 80 00
9B 16 00 00 00 00 00 00 49 50 53 00 10 00 00 00
0C 00 28 00 00 00 00 00

16 bit NTB, with NDP at the end. So I enabled CDC_NCM_FLAG_NDP_TO_END,
but still the device is not properly working.

This is the first properly transmitted acked packet in Linux:

4E 43 4D 48 0C 00 00 00 80 00 34 00 46 C0 00 28
00 00 40 00 01 02 F4 F9 B0 F6 5E 09 E0 00 00 16
94 04 00 00 22 00 F9 02 00 00 00 01 04 00 00 00
E0 00 00 FB 49 50 53 00 10 00 00 00 0C 00 28 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

that looks like quite similar than the Windows one (besides the final padding).

Then all the following out packets are nacked, e.g:

4E 43 4D 48 0C 00 01 00 D8 00 8C 00 45 00 00 3E
05 71 40 00 40 11 0C E8 B0 F6 5E 09 0A 85 0E D2
B3 30 00 35 00 2A D3 57 58 6A 01 00 00 01 00 00
00 00 00 00 05 64 61 69 73 79 06 75 62 75 6E 74
75 03 63 6F 6D 00 00 01 00 01 00 00 45 00 00 3E
2A EC 40 00 40 11 91 8A B0 F6 5E 09 0A 84 64 B5
B3 30 00 35 00 2A 7D 75 58 6A 01 00 00 01 00 00
00 00 00 00 05 64 61 69 73 79 06 75 62 75 6E 74
75 03 63 6F 6D 00 00 01 00 01 00 00 49 50 53 00
14 00 00 00 0C 00 3E 00 4C 00 3E 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00

Those packets look good to me: the main difference with Windows is the
presence of more than one datagram, but, as far as I understand, this
should not be a problem since the modem reports:

dwNtbInMaxSize=16384 dwNtbOutMaxSize=16384 wNdpOutPayloadRemainder=0
wNdpOutDivisor=4 wNdpOutAlignment=4 wNtbOutMaxDatagrams=16 flags=0x20

while in Windows it seems that always only one datagram is sent.

My thought is that the problem is in the modem firmware: in some way,
the first acked packet is breaking things inside the modem that is not
able to receive packets anymore, but maybe there is some other
problems in the tx packets created by the driver and I'm not able to
catch it.

Does someone have an hint about this?

Is there a way to configure the cdc_mbim driver in order to have
exactly the same packet format sent in Windows?

USB traces taken with TotalPhase Datacenter are available at

https://drive.google.com/open?id=0B1kPnH2g8ISIZWJiV05qeWN5dVE

Thanks,
Daniele
--
To unsubscribe from this list: send the line "unsubscribe 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 v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 14 November 2016 at 12:21, NeilBrown  wrote:
> On Thu, Nov 10 2016, Baolin Wang wrote:
>
>> Hi
>>
>> On 8 November 2016 at 04:36, NeilBrown  wrote:
>>> On Mon, Nov 07 2016, Baolin Wang wrote:
>>>
 On 3 November 2016 at 09:25, NeilBrown  wrote:
> On Tue, Nov 01 2016, Baolin Wang wrote:

 I agree with your most opinions, but these are optimization.
>>>
>>> I see them as bug fixes, not optimizations.
>>>
  Firstly I
 think we should upstream the USB charger driver.
>>>
>>> I think you missed the point.  The point is that we don't *need* your
>>> "USB charger driver" because all the infrastructure we need is *already*
>>> present in the kernel.  It is buggy and not used uniformly, and could
>>> usefully be polished and improved.  But the structure is already
>>> present.
>>>
>>> If everyone just added new infrastructure when they didn't like, or
>>> didn't understand, what was already present, the kernel would become
>>> like the Mad Hatter's tea party, full of dirty dishes.
>>>
  What I want to ask is
 how can we notify power driver if we don't set the
 usb_register_notifier(), then I think you give the answer is: power
 driver can register by 'struct usb_phy->notifier'. But why usb phy
 should notify the power driver how much current should be drawn, and I
 still think we should notify the current in usb charger driver which
 is better, and do not need to notify current for power driver in usb
 phy driver.
>>>
>>> I accept that it isn't clear that the phy *should* be involved in
>>> communicating the negotiated power availability, but nor is it clear
>>> that it shouldn't.  The power does travel through the physical
>>> interface, so physically it plays a role.
>>>
>>> But more importantly, it already *does* get told (in some cases).
>>> There is an interface "usb_phy_set_power()" which exists explicitly to
>>> tell the phy what power has been negotiated.  Given that infrastructure
>>> exists and works, it make sense to use it.
>>>
>>> If you think it is a broken design and should be removed, then fine:
>>> make a case for that.  Examine the history.  Make sure you know why it
>>> is there (or make sure that information cannot be found), and then
>>> present a case as to why it should be removed and replaced with
>>> something else.  But don't just leave it there and pretend it doesn't
>>> exist and create something similar-but-different and hope people will
>>> know why yours is better.  That way lies madness.
>>
>> Like Peter said, it is not only PHY can detect the USB charger type,
>> which means there are other places can detect the charger type.
>
> If I understand Peter's example correctly, it shows a configuration
> where the USB PHysical interface is partly implemented in the SOC and
> partly in the PMIC.  I appreciate that would make it more challenging to
> implement a PHY driver, but there is no reason it should impact anything
> outside of the PHY.

Like Peter's example, it need to use controller register to pull up dp
to begin the secondary detection, which is not belonged to phy driver
and I don't think it is suitable we implemented these in phy driver.

>
>> Second, some controller need to detect the charger type manually which
>> USB phy did not support.
>
> "manually" is an odd term to use in this context.

Sorry for the confusing.

> I think you mean that to detect the charger type you need to issue some
> command to the hardware and wait for it to respond, then assess the
> response.

Yes.

> That isn't at all surprising.  The charger type is detected by measuring
> resistance between ID and GND, which may require setting up a potential
> and activating ADCs to measure the voltage.  This can all be done
> internally to the phy driver.
> Sometimes it is easy (I did https://lkml.org/lkml/2015/2/23/746 for
> twl4030, though it never got upstream).
> The code for the imx7d does look more complex, but not intrinsically
> different.

But you should implement these in every phy driver, why not one
standard framework?

>
>> Third, it did not handle what current should
>> be drawn in USB phy.
>
> The standards define that.  The extcon just reports the cable type.
> Certainly it would be sensible to provide a library function to
> translate from cable type to current range.  You don't need a new
> subsystem to do that, just a library function.

I don't think the extcon should handle current things. For example,
the extcon can not know the gadget speed, which is used to change the
default current values for super speed gadget.

>
>> Fourth, we need integrate all charger plugin/out
>> event in one framework, not from extcon, maybe type-c in future.
>
> Why not extcon?  Given that a charger is connected by an external
> connector, extcon seems like exactly the right thing to use.

My 

Re: [PATHCv10 1/2] usb: USB Type-C connector class

[Heikki Krogerus]

Hi Greg,

On Mon, Nov 14, 2016 at 10:51:48AM +0100, Greg KH wrote:
> On Mon, Sep 19, 2016 at 02:16:56PM +0300, Heikki Krogerus wrote:
> > The purpose of USB Type-C connector class is to provide
> > unified interface for the user space to get the status and
> > basic information about USB Type-C connectors on a system,
> > control over data role swapping, and when the port supports
> > USB Power Delivery, also control over power role swapping
> > and Alternate Modes.
> > 
> > Reviewed-by: Guenter Roeck 
> > Tested-by: Guenter Roeck 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec |  218 ++
> >  Documentation/usb/typec.txt |  103 +++
> >  MAINTAINERS |9 +
> >  drivers/usb/Kconfig |2 +
> >  drivers/usb/Makefile|2 +
> >  drivers/usb/typec/Kconfig   |7 +
> >  drivers/usb/typec/Makefile  |1 +
> >  drivers/usb/typec/typec.c   | 1075 
> > +++
> >  include/linux/usb/typec.h   |  252 +++
> >  9 files changed, 1669 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
> >  create mode 100644 Documentation/usb/typec.txt
> >  create mode 100644 drivers/usb/typec/Kconfig
> >  create mode 100644 drivers/usb/typec/Makefile
> >  create mode 100644 drivers/usb/typec/typec.c
> >  create mode 100644 include/linux/usb/typec.h
> 
> Overall, this looks good, just a few minor comments that will require
> you to respin this...
> 
> 
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec 
> > b/Documentation/ABI/testing/sysfs-class-typec
> > new file mode 100644
> > index 000..dcca6bd
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -0,0 +1,218 @@
> > +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> > +
> > +What:  /sys/class/typec//current_data_role
> > +Date:  June 2016
> 
> It's no longer June.  I don't know why we have these dates, sorry, just
> make it be December and you should be fine.

OK

> > --- /dev/null
> > +++ b/Documentation/usb/typec.txt
> 
> We want to use .rst formats now, but this should be fine as-is for now.
> 
> > +static int sysfs_strmatch(const char * const *array, size_t n, const char 
> > *str)
> > +{
> > +   const char *item;
> > +   int index;
> > +
> > +   for (index = 0; index < n; index++) {
> > +   item = array[index];
> > +   if (!item)
> > +   break;
> > +   if (sysfs_streq(item, str))
> > +   return index;
> > +   }
> > +
> > +   return -EINVAL;
> > +}
> 
> should we make this a core sysfs function?

I already have the patch. I was planning on proposing that separately
after this series. This turned out to be something that can be used in
many drivers, so I was thinking about proposing it for at least few
other drivers.

But if you prefer, I can also introduce the function alone as new
sysfs core function in this series, and just use it in this driver.

> > +
> > +/* 
> > - */
> > +/* Type-C Partners */
> > +
> > +static void typec_dev_release(struct device *dev)
> > +{
> > +}
> 
> Yeah, thanks to the in-kernel documentation, I now get to make fun of
> you
> 
> Please, NEVER DO THIS EVER!!!  You are trying to tell the kernel "hey,
> shut up for your stupid warning about an empty release function!"  Did
> you ever think about _why_ I made the kernel warn about that?  Don't
> think you are smarter than the kernel, you will always loose that bet...

Point taken.

> > +
> > +static ssize_t partner_usb_pd_show(struct device *dev,
> > +  struct device_attribute *attr, char *buf)
> > +{
> > +   struct typec_partner *p = container_of(dev, struct typec_partner, dev);
> > +
> > +   return sprintf(buf, "%d\n", p->usb_pd);
> > +}
> > +
> > +static struct device_attribute dev_attr_partner_usb_pd = {
> > +   .attr = {
> > +   .name = "supports_usb_power_delivery",
> > +   .mode = S_IRUGO,
> > +   },
> > +   .show = partner_usb_pd_show,
> > +};
> 
> DEVICE_ATTR_RO()?

I'm using the same attribute names with different types of devises. It
felt more wrong to use shared functions for some of the attributes
(but not all), and try to identify the device type in them, then to
simply ignore the macros and name the functions how ever I wanted. At
least it made the driver look less messy for sure.

But I'll try change these.

> > +
> > +static ssize_t
> > +partner_accessory_mode_show(struct device *dev, struct device_attribute 
> > *attr,
> > +   char *buf)
> > +{
> > +   struct typec_partner *p = container_of(dev, struct typec_partner, dev);
> > +
> > +   return sprintf(buf, "%s\n", 

Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

On Mon, Nov 14, 2016 at 03:21:13PM +1100, NeilBrown wrote:
> On Thu, Nov 10 2016, Baolin Wang wrote:

> > Fourth, we need integrate all charger plugin/out
> > event in one framework, not from extcon, maybe type-c in future.

> Why not extcon?  Given that a charger is connected by an external
> connector, extcon seems like exactly the right thing to use.

> Obviously extcon doesn't report the current that was negotiated, but
> that is best kept separate.  The battery charger can be advised of the
> available current either via extcon or separately via the usb
> subsystem.  Don't conflate the two.

Conflating the two seems like the whole point here.  We're looking for
something that sits between the power supply code and the USB code and
tells the power supply code what it's allowed to do which is the result
of a combination of physical cable detection and USB protocol.  It seems
reasonable that extcon drivers ought to be part of this but it doesn't
seem like they are the whole story.

> > word, we need one standard integration of this feature we need, though
> > like you said we should do some clean up or fix to make it better.

> But really, I'm not the person you need to convince.  I'm just a vaguely
> interested bystander who is rapidly losing interest.  You need to
> convince a maintainer, but they have so far shown remarkably little
> interest.  I don't know why, but I'd guess that reviewing a complex new
> subsystem isn't much fun.  Reviewing and applying clear bugfixes and
> incremental improvements is much easier and more enjoyable.  But that is
> just a guess.

OTOH having someone else having reviewed might help them apply something
they don't care about.


signature.asc
Description: PGP signature

Re: [GIT PULL] USB: chipidea fixes for v4.9

[Greg KH]

On Mon, Nov 14, 2016 at 10:01:53AM +0800, Peter Chen wrote:
> The following changes since commit 18266403f3fe507f0246faa1d5432333a2f139ca:
> 
>   USB: cdc-acm: fix TIOCMIWAIT (2016-11-10 13:12:59 +0100)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ 
> tags/usb-ci-v4.9-rc6
> 
> for you to fetch changes up to 62ee9585fed19e4de9c3941b9c74044205ae13bd:
> 
>   usb: chipidea: move the lock initialization to core file (2016-11-14 
> 09:42:25 +0800)
> 
> 
> One patch to fix kernel dump for non-initialized lock

Is this a regression, or fixing something new?  Can you just post the
one patch?  That's easier than a git pull request...

thanks,

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 v3 5/6] usb: dwc3: use bus->sysdev for DMA configuration

[Sriram Dash]

>From: Peter Chen [mailto:hzpeterc...@gmail.com]
>On Fri, Nov 11, 2016 at 09:31:09PM +0100, Arnd Bergmann wrote:
>> On Thursday, November 10, 2016 1:02:11 PM CET Felipe Balbi wrote:
>> > > @@ -123,8 +119,8 @@ int dwc3_host_init(struct dwc3 *dwc)  void
>> > > dwc3_host_exit(struct dwc3 *dwc)  {
>> > >   phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy",
>> > > -   dev_name(>xhci->dev));
>> > > +   dev_name(dwc->dev));
>> > >   phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
>> > > -   dev_name(>xhci->dev));
>> > > +   dev_name(dwc->dev));
>> >
>> > this looks unrelated to $subject. Care to explain?
>>
>> bus->sysdev is used for retrieving any information that comes from
>> the platform (DT or otherwise), and the phy lookup in has to be done
>> through sysdev as well because the platform cannot add it to the child
>> device it has no knowledge of.
>>
>> When we set the sysdev to the parent, the phy lookup has to be added
>> to that device as well, rather than the child device.
>>
>
>Then, the device should change to dwc->sysdev, and phy_create_lookup in this 
>file
>needs to change too.
>

Yes Peter. I agree to that. 

>--
>
>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: dwc3: core: remove dwc3_soft_reset()

[Felipe Balbi]

dwc3_soft_reset() is doing the same thing as
dwc3_core_soft_reset(). Let's remove
dwc3_soft_reset() since that's not needed anymore.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.c | 39 ---
 1 file changed, 39 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index af423468f89e..d024d47c86d1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -169,33 +169,6 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
return -ETIMEDOUT;
 }
 
-/**
- * dwc3_soft_reset - Issue soft reset
- * @dwc: Pointer to our controller context structure
- */
-static int dwc3_soft_reset(struct dwc3 *dwc)
-{
-   unsigned long timeout;
-   u32 reg;
-
-   timeout = jiffies + msecs_to_jiffies(500);
-   dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
-   do {
-   reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-   if (!(reg & DWC3_DCTL_CSFTRST))
-   break;
-
-   if (time_after(jiffies, timeout)) {
-   dev_err(dwc->dev, "Reset Timed Out\n");
-   return -ETIMEDOUT;
-   }
-
-   cpu_relax();
-   } while (true);
-
-   return 0;
-}
-
 /*
  * dwc3_frame_length_adjustment - Adjusts frame length if required
  * @dwc3: Pointer to our controller context structure
@@ -515,13 +488,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
}
/* FALLTHROUGH */
case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI:
-   /* Making sure the interface and PHY are operational */
-   ret = dwc3_soft_reset(dwc);
-   if (ret)
-   return ret;
-
-   udelay(1);
-
ret = dwc3_ulpi_init(dwc);
if (ret)
return ret;
@@ -710,11 +676,6 @@ static int dwc3_core_init(struct dwc3 *dwc)
dwc->maximum_speed = USB_SPEED_HIGH;
}
 
-   /* issue device SoftReset too */
-   ret = dwc3_soft_reset(dwc);
-   if (ret)
-   goto err0;
-
ret = dwc3_core_soft_reset(dwc);
if (ret)
goto err0;
-- 
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: [PATCH 00/23] xhci features for usb-next

[Mathias Nyman]

On 14.11.2016 11:10, Greg KH wrote:

On Mon, Nov 14, 2016 at 10:58:18AM +0200, Mathias Nyman wrote:

On 14.11.2016 09:49, Greg KH wrote:

On Fri, Nov 11, 2016 at 03:13:09PM +0200, Mathias Nyman wrote:

Hi Greg

In addition to all the xhci cleanups, refactoring, and features for
xhci, there's a patch for usb core hub driver that changes how usb3
devices are disabled.

It fixes a automatic re-mount issue seen when users try to "safely remove"
or eject a usb mass storage device.


I'd like to get Alan's ack on that before applying it.

Also, shouldn't that go to the stable kernel trees as well?



I'll remove that patch from the series and send it separately.


It's the last one, it's trivial for me to just not apply it :)

I'll go do that now...



Thanks,
-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: cdc_acm - fragmented notifications

[Oliver Neukum]

On Thu, 2016-11-10 at 22:30 +0100, Tobias Herzog wrote:
> Hi,
> 
> I'm trying to build an usb device conforming to the CDC ACM device
> class. The device uses an interrupt IN endpoint with a max packet size
> of 8 bytes.
> I tried to send a SERIAL_STATE notification to the host (to report
> parity errors, etc.) and noticed, that it is not handled as I would
> expect:
> 
> Because I have to transmit the notification in two consecutive packets
> due to the limited packe size (SERIAL_STATE notification consists of an
> 8 byte header + 2 byte data), acm_ctrl_irq is called twice, too. So the
> notification is not reassembled before processing in acm_ctrl_irq. Of
> course acm_ctrl_irq only handles "garbage" in this cases because both
> recieved packages are to short.

That case was not considered in the spec or in the driver.

> Obviously the max packet size of the interrup IN endpoint needs to be
> equal (or greater) to the size of the largest notification the CDC
> device may transmit, to work with the cdc_acm module.

Yes

> Question:
> Is this just a (probably quite realistic) assumption made in the
> current kernel code, or is this constraint defined in the usb
> specification?

The assumption is made in the driver. Transmitting the package in two
steps like you do, is an obvious method to deal with the constraint,
but the spec does not mention it.

> I'm new to the usb stuff and am not sure if i missed something.
> Additionally I am confused, because Atmel provides some documentation
> how to build a CDC ACM device [1], where the fragmented transmission of
> notifications is explicitely stated as one possible implementation.

If they mention it, it may be more widely used.
I'd welcome a patch for the ACM driver.
Would you provide one?

Regards
Oliver


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

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

[Roger Quadros]

On 14/11/16 10:56, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  writes:
>>> 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?
>>>
>> If I understood right am335x doesn't use nop-phy so this patch shouldn't
>> impact AM335x. Bin, do you agree?
> 
> phy-am335x.c uses phy-generic.c as a library. Look for
> usb_phy_gen_create_phy().
> 
OK, but it doesn't seem to use the PHY clocks at all.

cheers,
-roger



signature.asc
Description: OpenPGP digital signature

Re: [PATHCv10 1/2] usb: USB Type-C connector class

[Greg KH]

On Mon, Sep 19, 2016 at 02:16:56PM +0300, Heikki Krogerus wrote:
> The purpose of USB Type-C connector class is to provide
> unified interface for the user space to get the status and
> basic information about USB Type-C connectors on a system,
> control over data role swapping, and when the port supports
> USB Power Delivery, also control over power role swapping
> and Alternate Modes.
> 
> Reviewed-by: Guenter Roeck 
> Tested-by: Guenter Roeck 
> Signed-off-by: Heikki Krogerus 
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  218 ++
>  Documentation/usb/typec.txt |  103 +++
>  MAINTAINERS |9 +
>  drivers/usb/Kconfig |2 +
>  drivers/usb/Makefile|2 +
>  drivers/usb/typec/Kconfig   |7 +
>  drivers/usb/typec/Makefile  |1 +
>  drivers/usb/typec/typec.c   | 1075 
> +++
>  include/linux/usb/typec.h   |  252 +++
>  9 files changed, 1669 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
>  create mode 100644 Documentation/usb/typec.txt
>  create mode 100644 drivers/usb/typec/Kconfig
>  create mode 100644 drivers/usb/typec/Makefile
>  create mode 100644 drivers/usb/typec/typec.c
>  create mode 100644 include/linux/usb/typec.h

Overall, this looks good, just a few minor comments that will require
you to respin this...


> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec 
> b/Documentation/ABI/testing/sysfs-class-typec
> new file mode 100644
> index 000..dcca6bd
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,218 @@
> +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> +
> +What:/sys/class/typec//current_data_role
> +Date:June 2016

It's no longer June.  I don't know why we have these dates, sorry, just
make it be December and you should be fine.

> --- /dev/null
> +++ b/Documentation/usb/typec.txt

We want to use .rst formats now, but this should be fine as-is for now.

> +static int sysfs_strmatch(const char * const *array, size_t n, const char 
> *str)
> +{
> + const char *item;
> + int index;
> +
> + for (index = 0; index < n; index++) {
> + item = array[index];
> + if (!item)
> + break;
> + if (sysfs_streq(item, str))
> + return index;
> + }
> +
> + return -EINVAL;
> +}

should we make this a core sysfs function?

> +
> +/* - 
> */
> +/* Type-C Partners */
> +
> +static void typec_dev_release(struct device *dev)
> +{
> +}

Yeah, thanks to the in-kernel documentation, I now get to make fun of
you

Please, NEVER DO THIS EVER!!!  You are trying to tell the kernel "hey,
shut up for your stupid warning about an empty release function!"  Did
you ever think about _why_ I made the kernel warn about that?  Don't
think you are smarter than the kernel, you will always loose that bet...

> +
> +static ssize_t partner_usb_pd_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct typec_partner *p = container_of(dev, struct typec_partner, dev);
> +
> + return sprintf(buf, "%d\n", p->usb_pd);
> +}
> +
> +static struct device_attribute dev_attr_partner_usb_pd = {
> + .attr = {
> + .name = "supports_usb_power_delivery",
> + .mode = S_IRUGO,
> + },
> + .show = partner_usb_pd_show,
> +};

DEVICE_ATTR_RO()?

> +
> +static ssize_t
> +partner_accessory_mode_show(struct device *dev, struct device_attribute 
> *attr,
> + char *buf)
> +{
> + struct typec_partner *p = container_of(dev, struct typec_partner, dev);
> +
> + return sprintf(buf, "%s\n", typec_accessory_modes[p->accessory]);
> +}
> +
> +static struct device_attribute dev_attr_partner_accessory = {
> + .attr = {
> + .name = "accessory_mode",
> + .mode = S_IRUGO,
> + },
> + .show = partner_accessory_mode_show,
> +};

DEVICE_ATTR_RO()?

> +
> +static struct attribute *typec_partner_attrs[] = {
> + _attr_partner_accessory.attr,
> + _attr_partner_usb_pd.attr,
> + NULL
> +};
> +
> +static struct attribute_group typec_partner_group = {
> + .attrs = typec_partner_attrs,
> +};
> +
> +static const struct attribute_group *typec_partner_groups[] = {
> + _partner_group,
> + NULL
> +};

ATTRIBUTE_GROUPS()?


> +
> +static struct device_type typec_partner_dev_type = {
> + .name = "typec_partner_device",
> + .groups = typec_partner_groups,
> + .release = typec_dev_release,
> +};
> +
> +static int
> +typec_add_partner(struct typec_port *port, struct typec_partner *partner)
> +{
> + struct device *dev = >dev;
> +   

RE: [PATCH v3 3/5] net: asix: Fix AX88772x resume failures

[ASIX_Allan [Office]]

Hi Jon,

Please help to double check if the USB host controller of your Terga
platform had been powered OFF while running the ax88772_suspend() routine or
not? 

---
Best regards,
Allan Chou


-Original Message-
From: Jon Hunter [mailto:jonath...@nvidia.com] 
Sent: Monday, November 14, 2016 5:34 PM
To: al...@asix.com.tw; robert.f...@collabora.com; fre...@asix.com.tw;
dean_jenk...@mentor.com; mark_cra...@mentor.com; da...@davemloft.net;
ivec...@redhat.com; john.stu...@linaro.org; vpala...@chromium.org;
step...@networkplumber.org; grund...@chromium.org; changch...@gmail.com;
and...@lunn.ch; trem...@gmail.com; colin.k...@canonical.com;
linux-usb@vger.kernel.org; net...@vger.kernel.org;
linux-ker...@vger.kernel.org; vpala...@google.com
Subject: Re: [PATCH v3 3/5] net: asix: Fix AX88772x resume failures

Hi Allan,

On 14/11/16 08:50, ASIX_Allan [Home] wrote:
> It seems the AX88772x dongle had been unexpectedly removed while 
> running the
> ax88772_suspend() routine. If yes, you might see these error messages
> because the hardware had been absent.

In my case the hardware was never removed. The boards are in a test fixture
that are not touched. This is seen on more than one board. By reverting this
change I no longer see the error messages and appears to be 100%
reproducible.

Jon

--
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe 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] net: asix: Fix AX88772x resume failures

[Jon Hunter]

Hi Allan,

On 14/11/16 08:50, ASIX_Allan [Home] wrote:
> It seems the AX88772x dongle had been unexpectedly removed while running the
> ax88772_suspend() routine. If yes, you might see these error messages
> because the hardware had been absent.

In my case the hardware was never removed. The boards are in a test
fixture that are not touched. This is seen on more than one board. By
reverting this change I no longer see the error messages and appears to
be 100% reproducible.

Jon

-- 
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe 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: core: urb make use of usb_endpoint_maxp_mult

[Felipe Balbi]

Hi,

Greg KH  writes:
> On Sun, Nov 13, 2016 at 01:31:16PM +0300, Mike Krinkin wrote:
>> Since usb_endpoint_maxp now returns only lower 11 bits mult
>> calculation here isn't correct anymore and that breaks webcam
>> for me. Patch make use of usb_endpoint_maxp_mult instead of
>> direct calculation.
>> 
>> Fixes: abb621844f6a ("usb: ch9: make usb_endpoint_maxp() return
>>only packet size")
>> 
>> Signed-off-by: Mike Krinkin 
>> ---
>>  drivers/usb/core/urb.c | 7 ++-
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
>> index 0be49a1..d75cb8c 100644
>> --- a/drivers/usb/core/urb.c
>> +++ b/drivers/usb/core/urb.c
>> @@ -412,11 +412,8 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
>>  }
>>  
>>  /* "high bandwidth" mode, 1-3 packets/uframe? */
>> -if (dev->speed == USB_SPEED_HIGH) {
>> -int mult = 1 + ((max >> 11) & 0x03);
>> -max &= 0x07ff;
>> -max *= mult;
>> -}
>> +if (dev->speed == USB_SPEED_HIGH)
>> +max *= usb_endpoint_maxp_mult(>desc);
>>  
>>  if (urb->number_of_packets <= 0)
>>  return -EINVAL;
>
> Felipe, this looks like it belongs in your tree...

Right, I've queued it up :-)

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v3 3/5] net: asix: Fix AX88772x resume failures

[ASIX_Allan [Home]]

Hi Jon,

It seems the AX88772x dongle had been unexpectedly removed while running the
ax88772_suspend() routine. If yes, you might see these error messages
because the hardware had been absent.

---
Best regards,
Allan Chou


-Original Message-
From: Jon Hunter [mailto:jonath...@nvidia.com] 
Sent: Thursday, November 10, 2016 8:01 PM
To: robert.f...@collabora.com; fre...@asix.com.tw; dean_jenk...@mentor.com;
mark_cra...@mentor.com; da...@davemloft.net; ivec...@redhat.com;
john.stu...@linaro.org; vpala...@chromium.org; step...@networkplumber.org;
grund...@chromium.org; changch...@gmail.com; al...@asix.com.tw;
and...@lunn.ch; trem...@gmail.com; colin.k...@canonical.com;
linux-usb@vger.kernel.org; net...@vger.kernel.org;
linux-ker...@vger.kernel.org; vpala...@google.com
Subject: Re: [PATCH v3 3/5] net: asix: Fix AX88772x resume failures

Hi Robert,

On 29/08/16 14:32, robert.f...@collabora.com wrote:
> From: Robert Foss 
> 
> From: Allan Chou 
> 
> The change fixes AX88772x resume failure by
> - Restore incorrect AX88772A PHY registers when resetting
> - Need to stop MAC operation when suspending
> - Need to restart MII when restoring PHY
> 
> Signed-off-by: Allan Chou 
> Signed-off-by: Robert Foss 
> Tested-by: Robert Foss 

After this commit, I have started seeing the following messages during
system suspend on various tegra boards using asix ethernet dongles ...

[  288.667010] PM: Syncing filesystems ... done.
[  288.672223] Freezing user space processes ... (elapsed 0.001 seconds)
done.
[  288.680505] Double checking all user space processes after OOM killer
disable... (elapsed 0.000 seconds) [  288.690193] Freezing remaining
freezable tasks ... (elapsed 0.001 seconds) done.
[  288.698987] Suspending console(s) (use no_console_suspend to debug) [
288.706605] asix 1-1:1.0 eth0: Failed to read reg index 0x: -19 [
288.706613] asix 1-1:1.0 eth0: Error reading Medium Status register:
ffed [  288.706621] asix 1-1:1.0 eth0: Failed to write reg index 0x:
-19 [  288.706629] asix 1-1:1.0 eth0: Failed to write Medium Mode mode to
0xfeed: ffed [  288.759167] PM: suspend of devices complete after 52.772
msecs

Interestingly, it only seems to happen if the ethernet is in a disconnected
state when entering suspend. I have not had chance to look at this any
further, but wanted to see if you had any thoughts.

Cheers
Jon

--
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe 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: core: urb make use of usb_endpoint_maxp_mult

[Greg KH]

On Sun, Nov 13, 2016 at 01:31:16PM +0300, Mike Krinkin wrote:
> Since usb_endpoint_maxp now returns only lower 11 bits mult
> calculation here isn't correct anymore and that breaks webcam
> for me. Patch make use of usb_endpoint_maxp_mult instead of
> direct calculation.
> 
> Fixes: abb621844f6a ("usb: ch9: make usb_endpoint_maxp() return
>only packet size")
> 
> Signed-off-by: Mike Krinkin 
> ---
>  drivers/usb/core/urb.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> index 0be49a1..d75cb8c 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -412,11 +412,8 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
>   }
>  
>   /* "high bandwidth" mode, 1-3 packets/uframe? */
> - if (dev->speed == USB_SPEED_HIGH) {
> - int mult = 1 + ((max >> 11) & 0x03);
> - max &= 0x07ff;
> - max *= mult;
> - }
> + if (dev->speed == USB_SPEED_HIGH)
> + max *= usb_endpoint_maxp_mult(>desc);
>  
>   if (urb->number_of_packets <= 0)
>   return -EINVAL;

Felipe, this looks like it belongs in your tree...
--
To unsubscribe from this list: send the line "unsubscribe 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 00/23] xhci features for usb-next

[Greg KH]

On Mon, Nov 14, 2016 at 10:58:18AM +0200, Mathias Nyman wrote:
> On 14.11.2016 09:49, Greg KH wrote:
> > On Fri, Nov 11, 2016 at 03:13:09PM +0200, Mathias Nyman wrote:
> > > Hi Greg
> > > 
> > > In addition to all the xhci cleanups, refactoring, and features for
> > > xhci, there's a patch for usb core hub driver that changes how usb3
> > > devices are disabled.
> > > 
> > > It fixes a automatic re-mount issue seen when users try to "safely remove"
> > > or eject a usb mass storage device.
> > 
> > I'd like to get Alan's ack on that before applying it.
> > 
> > Also, shouldn't that go to the stable kernel trees as well?
> > 
> 
> I'll remove that patch from the series and send it separately.

It's the last one, it's trivial for me to just not apply it :)

I'll go do that now...

> In addition to Alan's ack and the stable tag I just
> noticed that some parts of it depend on CONFIG_PM.

Ok, I'll ignore that one for now.

thanks,

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 00/23] xhci features for usb-next

[Mathias Nyman]

On 14.11.2016 09:49, Greg KH wrote:

On Fri, Nov 11, 2016 at 03:13:09PM +0200, Mathias Nyman wrote:

Hi Greg

In addition to all the xhci cleanups, refactoring, and features for
xhci, there's a patch for usb core hub driver that changes how usb3
devices are disabled.

It fixes a automatic re-mount issue seen when users try to "safely remove"
or eject a usb mass storage device.


I'd like to get Alan's ack on that before applying it.

Also, shouldn't that go to the stable kernel trees as well?



I'll remove that patch from the series and send it separately.

In addition to Alan's ack and the stable tag I just
noticed that some parts of it depend on CONFIG_PM.
 
-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: phy: phy-generic: add the implementation of .set_suspend

[Felipe Balbi]

Hi,

Roger Quadros  writes:
>> 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?
>> 
> If I understood right am335x doesn't use nop-phy so this patch shouldn't
> impact AM335x. Bin, do you agree?

phy-am335x.c uses phy-generic.c as a library. Look for
usb_phy_gen_create_phy().

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: twl6030-usb: make driver DT only

[Felipe Balbi]

Hi,

Nicolae Rosia  writes:
> All users are DT-only and it makes no sense to keep
> unused code
>
> Signed-off-by: Nicolae Rosia 

I need an Acked-by from either Tony, Roger or Bin

> ---
>  drivers/usb/phy/Kconfig   |  1 +
>  drivers/usb/phy/phy-twl6030-usb.c | 23 ++-
>  2 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index b9c409a..61cef75 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -84,6 +84,7 @@ config SAMSUNG_USBPHY
>  config TWL6030_USB
>   tristate "TWL6030 USB Transceiver Driver"
>   depends on TWL4030_CORE && OMAP_USB2 && USB_MUSB_OMAP2PLUS
> + depends on OF
>   help
> Enable this to support the USB OTG transceiver on TWL6030
> family chips. This TWL6030 transceiver has the VBUS and ID GND
> diff --git a/drivers/usb/phy/phy-twl6030-usb.c 
> b/drivers/usb/phy/phy-twl6030-usb.c
> index a72e8d6..628b600 100644
> --- a/drivers/usb/phy/phy-twl6030-usb.c
> +++ b/drivers/usb/phy/phy-twl6030-usb.c
> @@ -108,7 +108,6 @@ struct twl6030_usb {
>   enum musb_vbus_id_status linkstat;
>   u8  asleep;
>   boolvbus_enable;
> - const char  *regulator;
>  };
>  
>  #define  comparator_to_twl(x) container_of((x), struct twl6030_usb, 
> comparator)
> @@ -166,7 +165,7 @@ static int twl6030_usb_ldo_init(struct twl6030_usb *twl)
>   /* Program MISC2 register and set bit VUSB_IN_VBAT */
>   twl6030_writeb(twl, TWL6030_MODULE_ID0, 0x10, TWL6030_MISC2);
>  
> - twl->usb3v3 = regulator_get(twl->dev, twl->regulator);
> + twl->usb3v3 = regulator_get(twl->dev, "usb");
>   if (IS_ERR(twl->usb3v3))
>   return -ENODEV;
>  
> @@ -341,7 +340,11 @@ static int twl6030_usb_probe(struct platform_device 
> *pdev)
>   int status, err;
>   struct device_node  *np = pdev->dev.of_node;
>   struct device   *dev = >dev;
> - struct twl4030_usb_data *pdata = dev_get_platdata(dev);
> +
> + if (!np) {
> + dev_err(dev, "no DT info\n");
> + return -EINVAL;
> + }
>  
>   twl = devm_kzalloc(dev, sizeof(*twl), GFP_KERNEL);
>   if (!twl)
> @@ -361,18 +364,6 @@ static int twl6030_usb_probe(struct platform_device 
> *pdev)
>   return -EPROBE_DEFER;
>   }
>  
> - if (np) {
> - twl->regulator = "usb";
> - } else if (pdata) {
> - if (pdata->features & TWL6032_SUBCLASS)
> - twl->regulator = "ldousb";
> - else
> - twl->regulator = "vusb";
> - } else {
> - dev_err(>dev, "twl6030 initialized without pdata\n");
> - return -EINVAL;
> - }
> -
>   /* init spinlock for workqueue */
>   spin_lock_init(>lock);
>  
> @@ -436,13 +427,11 @@ static int twl6030_usb_remove(struct platform_device 
> *pdev)
>   return 0;
>  }
>  
> -#ifdef CONFIG_OF
>  static const struct of_device_id twl6030_usb_id_table[] = {
>   { .compatible = "ti,twl6030-usb" },
>   {}
>  };
>  MODULE_DEVICE_TABLE(of, twl6030_usb_id_table);
> -#endif
>  
>  static struct platform_driver twl6030_usb_driver = {
>   .probe  = twl6030_usb_probe,
> -- 
> 2.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe 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

RE: [PATCH v12 8/9] usbip: exporting devices: change to usbip_list.c

[fx IWATA NOBUO]

> there is no cover letter when the commits are merged, so please put it here
> as well.

OK. I will put in next version.

Thank you for your help,

nobuo.iwata
//
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, November 10, 2016 9:11 PM
> To: fx IWATA NOBUO
> Cc: valentina.mane...@gmail.com; shuah...@samsung.com;
> linux-usb@vger.kernel.org; fx MICHIMURA TADAO
> Subject: Re: [PATCH v12 8/9] usbip: exporting devices: change to
> usbip_list.c
> 
> On Thu, Oct 13, 2016 at 12:52:12PM +0900, Nobuo Iwata wrote:
> > Correction to wording inconsistency around import and export in
> > usbip_list.c.
> >
> > Please, see also cover letter about wording.
> 
> there is no cover letter when the commits are merged, so please put it here
> as well.
> 
> thanks,
> 
> 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