[v2,0/9] xhci and hub features for usb-next

Hi Greg

Second try.
This series mostly adds support for running xhci DbC on more than one
xHC controller in a setup at the same time.

There are also some link power management changes, of which one touches
usb core hub code, removing an extra LPM disable before device reset.

Thanks
-Mathias

---
v2: use correct dbc branch with cleaned up "fixme" code comments

Mathias Nyman (7):
  xhci: dbc: refactor xhci_dbc_init()
  xhci: dbc: create and remove dbc structure in dbgtty driver.
  xhci: dbc: Rename xhci_dbc_init and xhci_dbc_exit
  xhci: dbc: Don't call dbc_tty_init() on every dbc tty probe
  xhci: dbgtty: use IDR to support several dbc instances.
  xhci: Allocate separate command structures for each LPM command
  usb: remove Link Powermanagement (LPM) disable before port reset.

Sergey Shtylyov (1):
  usb: host: xhci: drop redundant checks

kernel test robot (1):
  usb: xhci: fix minmax.cocci warnings

 drivers/usb/core/hub.c         |  13 +--
 drivers/usb/host/xhci-dbgcap.c | 145 ++++++++++++++++-----------------
 drivers/usb/host/xhci-dbgcap.h |  26 ++++--
 drivers/usb/host/xhci-dbgtty.c |  86 ++++++++++++-------
 drivers/usb/host/xhci-mem.c    |  10 +--
 drivers/usb/host/xhci.c        |  31 +++----
 drivers/usb/host/xhci.h        |   2 -
 7 files changed, 163 insertions(+), 150 deletions(-)

On Wed, Feb 16, 2022 at 11:51:49AM +0200, Mathias Nyman wrote:
> To support systems with several xhci controllers with active
> dbc on each xhci we need to use IDR to identify and give
> an index to each port.
> 
> Avoid using global struct tty_driver.driver_state for storing
> dbc port pointer as it won't work with several dbc ports
> 
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci-dbgcap.h |  1 +
>  drivers/usb/host/xhci-dbgtty.c | 46 ++++++++++++++++++++++++++++------
>  2 files changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
> index 5f3304a06591..ca04192fdab1 100644
> --- a/drivers/usb/host/xhci-dbgcap.h
> +++ b/drivers/usb/host/xhci-dbgcap.h
> @@ -100,6 +100,7 @@ struct dbc_ep {
>  struct dbc_port {
>  	struct tty_port			port;
>  	spinlock_t			port_lock;	/* port access */
> +	int				minor;
>  
>  	struct list_head		read_pool;
>  	struct list_head		read_queue;
> diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
> index 059b58f48e3a..d3acc0829ee5 100644
> --- a/drivers/usb/host/xhci-dbgtty.c
> +++ b/drivers/usb/host/xhci-dbgtty.c
> @@ -10,11 +10,14 @@
>  #include <linux/slab.h>
>  #include <linux/tty.h>
>  #include <linux/tty_flip.h>
> +#include <linux/idr.h>
>  
>  #include "xhci.h"
>  #include "xhci-dbgcap.h"
>  
>  static struct tty_driver *dbc_tty_driver;
> +static struct idr dbc_tty_minors;
> +static DEFINE_MUTEX(dbc_tty_minors_lock);

Why not just use an ida instead?  That way you do not need another lock
and it becomes a tiny bit simpler overall.

I'll take this now, but in the future it might be worth to change this.

thanks,

greg k-h

On 17.2.2022 17.16, Greg KH wrote:
> On Wed, Feb 16, 2022 at 11:51:49AM +0200, Mathias Nyman wrote:
>> To support systems with several xhci controllers with active
>> dbc on each xhci we need to use IDR to identify and give
>> an index to each port.
>>
>> Avoid using global struct tty_driver.driver_state for storing
>> dbc port pointer as it won't work with several dbc ports
>>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>  drivers/usb/host/xhci-dbgcap.h |  1 +
>>  drivers/usb/host/xhci-dbgtty.c | 46 ++++++++++++++++++++++++++++------
>>  2 files changed, 40 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
>> index 5f3304a06591..ca04192fdab1 100644
>> --- a/drivers/usb/host/xhci-dbgcap.h
>> +++ b/drivers/usb/host/xhci-dbgcap.h
>> @@ -100,6 +100,7 @@ struct dbc_ep {
>>  struct dbc_port {
>>  	struct tty_port			port;
>>  	spinlock_t			port_lock;	/* port access */
>> +	int				minor;
>>  
>>  	struct list_head		read_pool;
>>  	struct list_head		read_queue;
>> diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
>> index 059b58f48e3a..d3acc0829ee5 100644
>> --- a/drivers/usb/host/xhci-dbgtty.c
>> +++ b/drivers/usb/host/xhci-dbgtty.c
>> @@ -10,11 +10,14 @@
>>  #include <linux/slab.h>
>>  #include <linux/tty.h>
>>  #include <linux/tty_flip.h>
>> +#include <linux/idr.h>
>>  
>>  #include "xhci.h"
>>  #include "xhci-dbgcap.h"
>>  
>>  static struct tty_driver *dbc_tty_driver;
>> +static struct idr dbc_tty_minors;
>> +static DEFINE_MUTEX(dbc_tty_minors_lock);
> 
> Why not just use an ida instead?  That way you do not need another lock
> and it becomes a tiny bit simpler overall.

idr seemed like a good way to tie together the minor number (in tty_struct index)
to the right dbc_port structure, which contains a pointer to that specific
xhci device 

This way I could find the right dbc_port and store it in tty_struct driver_data
in the  tty_operations install callback, allowing later tty read and and write calls
to find the correct xhci controller from tty_struct driver_data.

Looking at it now it might be possible to use just ida instead, and set the
tty_struct driver_data right after calling tty_port_register_device().

We should know everything we need there, id (minor), dbc_port, and can probably
get the tty_struct from the newly created tty port device somehow.

Should dig into tty a bit more for this.

> 
> I'll take this now, but in the future it might be worth to change this.

Thanks

-Mathias