Date:   Thu, 17 Feb 2022 16:16:02 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Mathias Nyman <mathias.nyman@linux.intel.com>
Cc:     linux-usb@vger.kernel.org
Subject: Re: [PATCH v2 5/9] xhci: dbgtty: use IDR to support several dbc
 instances.
Message-ID: <Yg5msgveDOAaYh5u@kroah.com>
References: <20220216095153.1303105-1-mathias.nyman@linux.intel.com>
 <20220216095153.1303105-6-mathias.nyman@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220216095153.1303105-6-mathias.nyman@linux.intel.com>
Precedence: bulk

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