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@xxxxxxxxxxxxxxx>
>> ---
>> 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
