Re: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Greg KH
On Tue, Jun 04, 2013 at 07:27:20PM +0200, Tobias Winter wrote:
> On 04.06.2013 04:49, Greg KH wrote:
> > Tobias, can you test this patch out?  Note, I only compiled it, did not
> > get the chance to actually run it, so it might not work at all.
> 
> Sure, I'll gladly give it a try. Seeing the comments on the code, I'm
> just wondering if there might be a more recent version to run?

Give me a day, it's going to take some more reworks to get this all to
work properly, I'll send a new patch series then.

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: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Tobias Winter
On 04.06.2013 04:49, Greg KH wrote:
> Tobias, can you test this patch out?  Note, I only compiled it, did not
> get the chance to actually run it, so it might not work at all.

Sure, I'll gladly give it a try. Seeing the comments on the code, I'm
just wondering if there might be a more recent version to run?

> But I'm not so sure anymore, so here's a patch to change to use the
> idr code, and should remove all minor number limitations (well 65k is
> the limit the tty core should be setting I think.)

That's a tough one to break :)

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


Re: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Greg KH
On Tue, Jun 04, 2013 at 01:04:01PM +0200, Johan Hovold wrote:
> On Mon, Jun 03, 2013 at 07:49:59PM -0700, Greg KH wrote:
> > On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
> > > But, IMHO, a nicer approach would be to make the allocation completely
> > > dynamic, using e.g. the idr subsystem. Static tables are always feel
> > > like straight jackets to me, no matter how big they are :)
> > 
> > You are right, I didn't change the code to use idr (it predates idr by
> > about a decade or so), because I thought we needed the "rage" logic that
> > the usb-serial minor reservation does.
> > 
> > But I'm not so sure anymore, so here's a patch to change to use the idr
> > code, and should remove all minor number limitations (well 65k is the
> > limit the tty core should be setting I think.)
> > 
> > Tobias, can you test this patch out?  Note, I only compiled it, did not
> > get the chance to actually run it, so it might not work at all.
> 
> I'm afraid this won't work in it's current form. Several drivers and
> parts of usb-serial core still depend on the minor numbers being
> consecutive for multi-port devices.

You are right, let me go fix up the "port->number" assumption first,
which will let this become easier.  That should have been fixed a long
time ago.

> There are also still references to SERIAL_TTY_NO_MINOR (255) as well
> as SERIAL_TTY_MINORS that need to be addressed.

Yeah, I knew it was too good to be true that a simple 45 line patch
would solve this properly :)

> > +   port = idr_find(&serial_minors, index);
> > mutex_unlock(&table_lock);
> > +   if (!port)
> > +   return NULL;
> > +
> > +   serial = port->serial;
> > +   kref_get(&serial->kref);
> > return serial;
> >  }
> 
> We would still need to handle disconnect, and make sure to return with
> the disc_mutex held unless disconnected.

Alan noticed this as well, I've now fixed that, thanks.

> > +   dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> > +
> > +   *minor = 0x;
> > +   for (i = 0; i < num_ports; ++i) {
> > +   x = get_free_port(serial->port[i]);
> > +   if (x < 0)
> > +   goto error;
> > +   if (*minor == 0x)
> > +   *minor = x;
> > }
> > -   mutex_unlock(&table_lock);
> > -   return NULL;
> > +   return 0;
> > +error:
> > +   // FIXME unwind the already allocated minors
> > +   return -ENODEV;
> >  }
> 
> As mentioned above, usb-serial core and several drivers currently depend
> on us returning the first minor in a consecutive range. It's mostly used
> to determine the per device port index, so storing that index in the port
> structure could possibly be sufficient.

Yes, I'll go fix that up first, before making these changes, as it's
independant.

> > @@ -1233,9 +1226,6 @@ static int __init usb_serial_init(void)
> > return -ENOMEM;
> >  
> > /* Initialize our global data */
> 
> You could remove the above comment as well.
> 
> > -   for (i = 0; i < SERIAL_TTY_MINORS; ++i)
> > -   serial_table[i] = NULL;
> > -
> > result = bus_register(&usb_serial_bus_type);

I thought about it, but we also register the bus variables, and some
other things here as well, so I left it in.

thanks for the review.

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: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Greg KH
On Tue, Jun 04, 2013 at 10:13:47AM -0400, Alan Stern wrote:
> On Mon, 3 Jun 2013, Greg KH wrote:
> 
> > On Mon, May 27, 2013 at 02:28:51PM +0200, Bj?rn Mork wrote:
> > > But, IMHO, a nicer approach would be to make the allocation completely
> > > dynamic, using e.g. the idr subsystem. Static tables are always feel
> > > like straight jackets to me, no matter how big they are :)
> > 
> > You are right, I didn't change the code to use idr (it predates idr by
> > about a decade or so), because I thought we needed the "rage" logic that
> > the usb-serial minor reservation does.
> > 
> > But I'm not so sure anymore, so here's a patch to change to use the idr
> > code, and should remove all minor number limitations (well 65k is the
> > limit the tty core should be setting I think.)
> > 
> > Tobias, can you test this patch out?  Note, I only compiled it, did not
> > get the chance to actually run it, so it might not work at all.
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> > @@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
> >  struct usb_serial *usb_serial_get_by_index(unsigned index)
> >  {
> > struct usb_serial *serial;
> > +   struct usb_serial_port *port;
> >  
> > mutex_lock(&table_lock);
> > -   serial = serial_table[index];
> > -
> > -   if (serial) {
> > -   mutex_lock(&serial->disc_mutex);
> > -   if (serial->disconnected) {
> > -   mutex_unlock(&serial->disc_mutex);
> > -   serial = NULL;
> > -   } else {
> > -   kref_get(&serial->kref);
> > -   }
> > -   }
> > +   port = idr_find(&serial_minors, index);
> > mutex_unlock(&table_lock);
> > +   if (!port)
> > +   return NULL;
> > +
> > +   serial = port->serial;
> > +   kref_get(&serial->kref);
> > return serial;
> >  }
> 
> The test for serial->disconnected got lost.  And the locking isn't 
> right; the routine is documented to return with serial->disc_mutex held 
> (in the case where the device hasn't been disconnected).
> 
> Also, the kref_get() needs to occur within the scope of the table_lock.

Thanks, for some reason I ignored this when converting the code, that's
what I get for not even testing...

> I didn't check the rest of the patch for similar errors.  Finding three 
> in the first function seemed like enough.  :-)

Fair enough, I've now fixed this up, and will see if it runs properly.

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: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Greg KH
On Mon, Jun 03, 2013 at 10:59:08PM -0400, Dave Jones wrote:
> On Mon, Jun 03, 2013 at 07:49:59PM -0700, Greg Kroah-Hartman wrote:
>  > On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
>  > > But, IMHO, a nicer approach would be to make the allocation completely
>  > > dynamic, using e.g. the idr subsystem. Static tables are always feel
>  > > like straight jackets to me, no matter how big they are :)
>  > 
>  > You are right, I didn't change the code to use idr (it predates idr by
>  > about a decade or so), because I thought we needed the "rage" logic that
>  > the usb-serial minor reservation does.
> 
> Rage logic sounds like my kinda code.

Late nite typo :)

>  > +static int get_free_port(struct usb_serial_port *port)
>  >  {
>  > -  unsigned int i, j;
>  > -  int good_spot;
>  > -
>  > -  dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
>  > +  int i;
>  >  
>  > -  *minor = 0;
>  >mutex_lock(&table_lock);
>  > -  for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
>  > -  if (serial_table[i])
>  > -  continue;
>  > +  i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
>  > +  if (i < 0)
>  > +  return -EEXIST;
>  > +  port->number = i;
>  > +  mutex_unlock(&table_lock);
>  > +  return i;
>  > +}
> 
> -EEXIST case misses the mutex unlock.

Thanks, now fixed.

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: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Alan Stern
On Mon, 3 Jun 2013, Greg KH wrote:

> On Mon, May 27, 2013 at 02:28:51PM +0200, Bj�rn Mork wrote:
> > But, IMHO, a nicer approach would be to make the allocation completely
> > dynamic, using e.g. the idr subsystem. Static tables are always feel
> > like straight jackets to me, no matter how big they are :)
> 
> You are right, I didn't change the code to use idr (it predates idr by
> about a decade or so), because I thought we needed the "rage" logic that
> the usb-serial minor reservation does.
> 
> But I'm not so sure anymore, so here's a patch to change to use the idr
> code, and should remove all minor number limitations (well 65k is the
> limit the tty core should be setting I think.)
> 
> Tobias, can you test this patch out?  Note, I only compiled it, did not
> get the chance to actually run it, so it might not work at all.
> 
> thanks,
> 
> greg k-h


> @@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
>  struct usb_serial *usb_serial_get_by_index(unsigned index)
>  {
>   struct usb_serial *serial;
> + struct usb_serial_port *port;
>  
>   mutex_lock(&table_lock);
> - serial = serial_table[index];
> -
> - if (serial) {
> - mutex_lock(&serial->disc_mutex);
> - if (serial->disconnected) {
> - mutex_unlock(&serial->disc_mutex);
> - serial = NULL;
> - } else {
> - kref_get(&serial->kref);
> - }
> - }
> + port = idr_find(&serial_minors, index);
>   mutex_unlock(&table_lock);
> + if (!port)
> + return NULL;
> +
> + serial = port->serial;
> + kref_get(&serial->kref);
>   return serial;
>  }

The test for serial->disconnected got lost.  And the locking isn't 
right; the routine is documented to return with serial->disc_mutex held 
(in the case where the device hasn't been disconnected).

Also, the kref_get() needs to occur within the scope of the table_lock.

I didn't check the rest of the patch for similar errors.  Finding three 
in the first function seemed like enough.  :-)

Alan Stern

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


Re: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-04 Thread Johan Hovold
On Mon, Jun 03, 2013 at 07:49:59PM -0700, Greg KH wrote:
> On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
> > But, IMHO, a nicer approach would be to make the allocation completely
> > dynamic, using e.g. the idr subsystem. Static tables are always feel
> > like straight jackets to me, no matter how big they are :)
> 
> You are right, I didn't change the code to use idr (it predates idr by
> about a decade or so), because I thought we needed the "rage" logic that
> the usb-serial minor reservation does.
> 
> But I'm not so sure anymore, so here's a patch to change to use the idr
> code, and should remove all minor number limitations (well 65k is the
> limit the tty core should be setting I think.)
> 
> Tobias, can you test this patch out?  Note, I only compiled it, did not
> get the chance to actually run it, so it might not work at all.

I'm afraid this won't work in it's current form. Several drivers and
parts of usb-serial core still depend on the minor numbers being
consecutive for multi-port devices. There are also still references to
SERIAL_TTY_NO_MINOR (255) as well as SERIAL_TTY_MINORS that need to be
addressed.

> 
> thanks,
> 
> greg k-h
> 
> Subject: [PATCH] usb: serial: remove minor number limitation of 255
> 
> 
> Signed-off-by: Greg Kroah-Hartman 
> 
>  drivers/usb/serial/usb-serial.c |   86 
> +---
>  1 file changed, 38 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index 4753c00..74b6f08 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "pl2303.h"
>  
>  #define DRIVER_AUTHOR "Greg Kroah-Hartman "
> @@ -49,7 +50,7 @@
> drivers depend on it.
>  */
>  
> -static struct usb_serial *serial_table[SERIAL_TTY_MINORS];
> +static DEFINE_IDR(serial_minors);
>  static DEFINE_MUTEX(table_lock);
>  static LIST_HEAD(usb_serial_driver_list);
>  
> @@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
>  struct usb_serial *usb_serial_get_by_index(unsigned index)
>  {
>   struct usb_serial *serial;
> + struct usb_serial_port *port;
>  
>   mutex_lock(&table_lock);
> - serial = serial_table[index];
> -
> - if (serial) {
> - mutex_lock(&serial->disc_mutex);
> - if (serial->disconnected) {
> - mutex_unlock(&serial->disc_mutex);
> - serial = NULL;
> - } else {
> - kref_get(&serial->kref);
> - }
> - }
> + port = idr_find(&serial_minors, index);
>   mutex_unlock(&table_lock);
> + if (!port)
> + return NULL;
> +
> + serial = port->serial;
> + kref_get(&serial->kref);
>   return serial;
>  }

We would still need to handle disconnect, and make sure to return with
the disc_mutex held unless disconnected.

>  
> -static struct usb_serial *get_free_serial(struct usb_serial *serial,
> - int num_ports, unsigned int *minor)
> +static int get_free_port(struct usb_serial_port *port)
>  {
> - unsigned int i, j;
> - int good_spot;
> -
> - dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> + int i;
>  
> - *minor = 0;
>   mutex_lock(&table_lock);
> - for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
> - if (serial_table[i])
> - continue;
> + i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
> + if (i < 0)

Missing mutex unlock (as already Dave noted).

> + return -EEXIST;
> + port->number = i;
> + mutex_unlock(&table_lock);
> + return i;
> +}
>  
> - good_spot = 1;
> - for (j = 1; j <= num_ports-1; ++j)
> - if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
> - good_spot = 0;
> - i += j;
> - break;
> - }
> - if (good_spot == 0)
> - continue;
> +static int get_free_serial(struct usb_serial *serial, int num_ports,
> +unsigned int *minor)
> +{
> + unsigned int i;
> + unsigned int x;
>  
> - *minor = i;
> - j = 0;
> - dev_dbg(&serial->interface->dev, "%s - minor base = %d\n", 
> __func__, *minor);
> - for (i = *minor; (i < (*minor + num_ports)) && (i < 
> SERIAL_TTY_MINORS); ++i) {
> - serial_table[i] = serial;
> - serial->port[j++]->number = i;
> - }
> - mutex_unlock(&table_lock);
> - return serial;
> + dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
> +
> + *minor = 0x;
> + for (i = 0; i < num_ports; ++i) {
> + x = get_free_port(serial->port[i]);
> + if (x < 0)
> + 

Re: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-03 Thread Dave Jones
On Mon, Jun 03, 2013 at 07:49:59PM -0700, Greg Kroah-Hartman wrote:
 > On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
 > > But, IMHO, a nicer approach would be to make the allocation completely
 > > dynamic, using e.g. the idr subsystem. Static tables are always feel
 > > like straight jackets to me, no matter how big they are :)
 > 
 > You are right, I didn't change the code to use idr (it predates idr by
 > about a decade or so), because I thought we needed the "rage" logic that
 > the usb-serial minor reservation does.

Rage logic sounds like my kinda code.


 > +static int get_free_port(struct usb_serial_port *port)
 >  {
 > -unsigned int i, j;
 > -int good_spot;
 > -
 > -dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
 > +int i;
 >  
 > -*minor = 0;
 >  mutex_lock(&table_lock);
 > -for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
 > -if (serial_table[i])
 > -continue;
 > +i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
 > +if (i < 0)
 > +return -EEXIST;
 > +port->number = i;
 > +mutex_unlock(&table_lock);
 > +return i;
 > +}

-EEXIST case misses the mutex unlock.

Dave

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


Re: [RFC] raise the maximum number of usb-serial devices to 512

2013-06-03 Thread Greg KH
On Mon, May 27, 2013 at 02:28:51PM +0200, Bjørn Mork wrote:
> But, IMHO, a nicer approach would be to make the allocation completely
> dynamic, using e.g. the idr subsystem. Static tables are always feel
> like straight jackets to me, no matter how big they are :)

You are right, I didn't change the code to use idr (it predates idr by
about a decade or so), because I thought we needed the "rage" logic that
the usb-serial minor reservation does.

But I'm not so sure anymore, so here's a patch to change to use the idr
code, and should remove all minor number limitations (well 65k is the
limit the tty core should be setting I think.)

Tobias, can you test this patch out?  Note, I only compiled it, did not
get the chance to actually run it, so it might not work at all.

thanks,

greg k-h

Subject: [PATCH] usb: serial: remove minor number limitation of 255


Signed-off-by: Greg Kroah-Hartman 

 drivers/usb/serial/usb-serial.c |   86 +---
 1 file changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 4753c00..74b6f08 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "pl2303.h"
 
 #define DRIVER_AUTHOR "Greg Kroah-Hartman "
@@ -49,7 +50,7 @@
drivers depend on it.
 */
 
-static struct usb_serial *serial_table[SERIAL_TTY_MINORS];
+static DEFINE_IDR(serial_minors);
 static DEFINE_MUTEX(table_lock);
 static LIST_HEAD(usb_serial_driver_list);
 
@@ -61,59 +62,52 @@ static LIST_HEAD(usb_serial_driver_list);
 struct usb_serial *usb_serial_get_by_index(unsigned index)
 {
struct usb_serial *serial;
+   struct usb_serial_port *port;
 
mutex_lock(&table_lock);
-   serial = serial_table[index];
-
-   if (serial) {
-   mutex_lock(&serial->disc_mutex);
-   if (serial->disconnected) {
-   mutex_unlock(&serial->disc_mutex);
-   serial = NULL;
-   } else {
-   kref_get(&serial->kref);
-   }
-   }
+   port = idr_find(&serial_minors, index);
mutex_unlock(&table_lock);
+   if (!port)
+   return NULL;
+
+   serial = port->serial;
+   kref_get(&serial->kref);
return serial;
 }
 
-static struct usb_serial *get_free_serial(struct usb_serial *serial,
-   int num_ports, unsigned int *minor)
+static int get_free_port(struct usb_serial_port *port)
 {
-   unsigned int i, j;
-   int good_spot;
-
-   dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
+   int i;
 
-   *minor = 0;
mutex_lock(&table_lock);
-   for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
-   if (serial_table[i])
-   continue;
+   i = idr_alloc(&serial_minors, port, 0, 0, GFP_KERNEL);
+   if (i < 0)
+   return -EEXIST;
+   port->number = i;
+   mutex_unlock(&table_lock);
+   return i;
+}
 
-   good_spot = 1;
-   for (j = 1; j <= num_ports-1; ++j)
-   if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
-   good_spot = 0;
-   i += j;
-   break;
-   }
-   if (good_spot == 0)
-   continue;
+static int get_free_serial(struct usb_serial *serial, int num_ports,
+  unsigned int *minor)
+{
+   unsigned int i;
+   unsigned int x;
 
-   *minor = i;
-   j = 0;
-   dev_dbg(&serial->interface->dev, "%s - minor base = %d\n", 
__func__, *minor);
-   for (i = *minor; (i < (*minor + num_ports)) && (i < 
SERIAL_TTY_MINORS); ++i) {
-   serial_table[i] = serial;
-   serial->port[j++]->number = i;
-   }
-   mutex_unlock(&table_lock);
-   return serial;
+   dev_dbg(&serial->interface->dev, "%s %d\n", __func__, num_ports);
+
+   *minor = 0x;
+   for (i = 0; i < num_ports; ++i) {
+   x = get_free_port(serial->port[i]);
+   if (x < 0)
+   goto error;
+   if (*minor == 0x)
+   *minor = x;
}
-   mutex_unlock(&table_lock);
-   return NULL;
+   return 0;
+error:
+   // FIXME unwind the already allocated minors
+   return -ENODEV;
 }
 
 static void return_serial(struct usb_serial *serial)
@@ -122,7 +116,7 @@ static void return_serial(struct usb_serial *serial)
 
mutex_lock(&table_lock);
for (i = 0; i < serial->num_ports; ++i)
-   serial_table[serial->minor + i] = NULL;
+   idr_remove(&serial_minors, serial->port[i]->number);
mutex_unlock(&table_lock);
 }
 
@@ -1041

Re: [RFC] raise the maximum number of usb-serial devices to 512

2013-05-27 Thread Bjørn Mork
Tobias Winter  writes:

> Hi,
>
> I did a bit more fiddling with the usb-serial stack and got it to
> support more than 256 devices. I tested it with up to 281 FTDI
> singleport adapters. (After that i ran out of usb cables.. )
>
> Signed-off-by: Jakob-Tobias Winter 
> ---
>  include/linux/usb/serial.h |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
> index 302ddf5..ab5e01b 100644
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -20,7 +20,7 @@
>  #include 
>
>  #define SERIAL_TTY_MAJOR   188 /* Nice legal number now */
> -#define SERIAL_TTY_MINORS  254 /* loads of devices :) */
> +#define SERIAL_TTY_MINORS  512 /* loads of devices :) */
>  #define SERIAL_TTY_NO_MINOR255 /* No minor was assigned */
>
>  /* The maximum number of ports one device can grab at once */

Note the special meaning assigned to 255, which is the reason for the
original limit being 254.  I believe you need to deal with this in the
allocation code if you are going to increase the number like this?  Did
the ttyUSB255 device work?  Might be as simple as changing the
SERIAL_TTY_NO_MINOR macro.  I don't know..

But, IMHO, a nicer approach would be to make the allocation completely
dynamic, using e.g. the idr subsystem. Static tables are always feel
like straight jackets to me, no matter how big they are :)


Bjørn
--
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


[RFC] raise the maximum number of usb-serial devices to 512

2013-05-27 Thread Tobias Winter
Hi,

I did a bit more fiddling with the usb-serial stack and got it to
support more than 256 devices. I tested it with up to 281 FTDI
singleport adapters. (After that i ran out of usb cables.. )

Signed-off-by: Jakob-Tobias Winter 
---
 include/linux/usb/serial.h |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 302ddf5..ab5e01b 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -20,7 +20,7 @@
 #include 

 #define SERIAL_TTY_MAJOR   188 /* Nice legal number now */
-#define SERIAL_TTY_MINORS  254 /* loads of devices :) */
+#define SERIAL_TTY_MINORS  512 /* loads of devices :) */
 #define SERIAL_TTY_NO_MINOR255 /* No minor was assigned */

 /* The maximum number of ports one device can grab at once */
@@ -80,7 +80,7 @@ struct usb_serial_port {
struct usb_serial   *serial;
struct tty_port port;
spinlock_t  lock;
-   unsigned char   number;
+   unsigned short  number;

unsigned char   *interrupt_in_buffer;
struct urb  *interrupt_in_urb;
@@ -159,7 +159,7 @@ struct usb_serial {
unsigned char   disconnected:1;
unsigned char   suspending:1;
unsigned char   attached:1;
-   unsigned char   minor;
+   unsigned short  minor;
unsigned char   num_ports;
unsigned char   num_port_pointers;
charnum_interrupt_in;
-- 
1.7.10.4


To avoid possible regressions, I also modified a few drivers:

Signed-off-by: Jakob-Tobias Winter 
---
 drivers/usb/serial/io_edgeport.c |2 +-
 drivers/usb/serial/mos7720.c |2 +-
 drivers/usb/serial/mos7840.c |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/io_edgeport.c
b/drivers/usb/serial/io_edgeport.c
index 1477e85..eac8641 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -2266,7 +2266,7 @@ static int send_cmd_write_baud_rate(struct
edgeport_port *edge_port,
int cmdLen = 0;
int divisor;
int status;
-   unsigned char number =
+   unsigned short number =
edge_port->port->number - edge_port->port->serial->minor;

if (edge_serial->is_epic &&
diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index cc0e543..81cbc84 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1468,7 +1468,7 @@ static int send_cmd_write_baud_rate(struct
moschip_port *mos7720_port,
struct usb_serial *serial;
int divisor;
int status;
-   unsigned char number;
+   unsigned short number;

if (mos7720_port == NULL)
return -1;
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index a0d5ea5..2d62efb 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -1718,7 +1718,7 @@ static int mos7840_send_cmd_write_baud_rate(struct
moschip_port *mos7840_port,
int divisor = 0;
int status;
__u16 Data;
-   unsigned char number;
+   unsigned short number;
__u16 clk_sel_val;
struct usb_serial_port *port;

-- 
1.7.10.4

But since I don't have those types of hardware, I can't test it. Also
I'm not sure if I took everything that can break into account as I don't
have any kernel coding experience.

To whom it may concern, here is the kern.log
http://linuxdingsda.de/~wintix/kern.log

and again the output of `lsusb | sort`, now with up to 127 devices on a
single USB root hub:

http://de.pastebin.ca/2383084

Thanks,

Tobias

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