Raise the maximum number of usb-serial devices to 256, which is the
actual limit supported by the codebase.
Signed-off-by: Jakob-Tobias Winter <[email protected]>
Tested-by: Jakob-Tobias Winter <[email protected]>
---
include/linux/usb/serial.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 302ddf5..c0ce5ed 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -20,7 +20,7 @@
#include <linux/kfifo.h>
#define SERIAL_TTY_MAJOR 188 /* Nice legal number now */
-#define SERIAL_TTY_MINORS 254 /* loads of devices :) */
+#define SERIAL_TTY_MINORS 256 /* loads of devices :) */
#define SERIAL_TTY_NO_MINOR 255 /* No minor was assigned */
/* The maximum number of ports one device can grab at once */
--
1.7.10.4
On 05/27/2013 04:30:12 AM, Tobias Winter wrote:
> Raise the maximum number of usb-serial devices to 256, which is the
> actual limit supported by the codebase.
>
> Signed-off-by: Jakob-Tobias Winter <[email protected]>
> Tested-by: Jakob-Tobias Winter <[email protected]>
>
> ---
> include/linux/usb/serial.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
> index 302ddf5..c0ce5ed 100644
> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -20,7 +20,7 @@
> #include <linux/kfifo.h>
>
> #define SERIAL_TTY_MAJOR 188 /* Nice legal number now */
> -#define SERIAL_TTY_MINORS 254 /* loads of devices :) */
> +#define SERIAL_TTY_MINORS 256 /* loads of devices :) */
> #define SERIAL_TTY_NO_MINOR 255 /* No minor was assigned */
So SERIAL_TTY_NO_MINOR is now a valid minor?
Rob-
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 <[email protected]>
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 <linux/usb.h>
#include <linux/usb/serial.h>
#include <linux/kfifo.h>
+#include <linux/idr.h>
#include "pl2303.h"
#define DRIVER_AUTHOR "Greg Kroah-Hartman <[email protected]>"
@@ -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 = 0xffffffff;
+ for (i = 0; i < num_ports; ++i) {
+ x = get_free_port(serial->port[i]);
+ if (x < 0)
+ goto error;
+ if (*minor == 0xffffffff)
+ *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,7 +1035,7 @@ static int usb_serial_probe(struct usb_interface *interface,
*/
serial->disconnected = 1;
- if (get_free_serial(serial, num_ports, &minor) == NULL) {
+ if (get_free_serial(serial, num_ports, &minor)) {
dev_err(ddev, "No more free serial devices\n");
goto probe_error;
}
@@ -1225,7 +1219,6 @@ static struct usb_driver usb_serial_driver = {
static int __init usb_serial_init(void)
{
- int i;
int result;
usb_serial_tty_driver = alloc_tty_driver(SERIAL_TTY_MINORS);
@@ -1233,9 +1226,6 @@ static int __init usb_serial_init(void)
return -ENOMEM;
/* Initialize our global data */
- for (i = 0; i < SERIAL_TTY_MINORS; ++i)
- serial_table[i] = NULL;
-
result = bus_register(&usb_serial_bus_type);
if (result) {
pr_err("%s - registering bus driver failed\n", __func__);
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
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 <[email protected]>
>
> 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 <linux/usb.h>
> #include <linux/usb/serial.h>
> #include <linux/kfifo.h>
> +#include <linux/idr.h>
> #include "pl2303.h"
>
> #define DRIVER_AUTHOR "Greg Kroah-Hartman <[email protected]>"
> @@ -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 = 0xffffffff;
> + for (i = 0; i < num_ports; ++i) {
> + x = get_free_port(serial->port[i]);
> + if (x < 0)
> + goto error;
> + if (*minor == 0xffffffff)
> + *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.
>
> 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,7 +1035,7 @@ static int usb_serial_probe(struct usb_interface *interface,
> */
> serial->disconnected = 1;
>
> - if (get_free_serial(serial, num_ports, &minor) == NULL) {
> + if (get_free_serial(serial, num_ports, &minor)) {
> dev_err(ddev, "No more free serial devices\n");
> goto probe_error;
> }
> @@ -1225,7 +1219,6 @@ static struct usb_driver usb_serial_driver = {
>
> static int __init usb_serial_init(void)
> {
> - int i;
> int result;
>
> usb_serial_tty_driver = alloc_tty_driver(SERIAL_TTY_MINORS);
> @@ -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);
> if (result) {
> pr_err("%s - registering bus driver failed\n", __func__);
Thanks,
Johan
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
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
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
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 = 0xffffffff;
> > + for (i = 0; i < num_ports; ++i) {
> > + x = get_free_port(serial->port[i]);
> > + if (x < 0)
> > + goto error;
> > + if (*minor == 0xffffffff)
> > + *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
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
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