[patch 0/2] staging: speakup: support more than ttyS*

[patch 0/2] staging: speakup: support more than ttyS*

[okash.khawaja]

Hi,

These patches extend speakup support to ttyUSB* and lp*. They introduce
a new module param dev whose function is similar to ser but instead of
taking serial port number as argument, it takes strings like ttyS0 or
ttyUSB0. First patch just adds functionality to convert such strings
into dev_t. Second patch makes use of that functionlity.

Thanks,
Okash

[patch 1/2] staging: speakup: add function to convert dev name to number

[okash.khawaja]

[-- Attachment #1: 11_convert_devname_to_devno --]
[-- Type: text/plain, Size: 5122 bytes --]

The function converts strings like ttyS0 and ttyUSB0 to dev_t like
(4, 64) and (188, 0). Subsequent patch in this set will call it to
convert user-supplied device into device number. The function does
some basic sanity checks on the string passed in. It currently supports
ttyS*, ttyUSB* and, for selected synths, lp*.

In order to do this, the patch also introduces a string member variable
named 'dev' to struct spk_synth. 'dev' represents the device name -
ttyUSB0 etc - which needs conversion to dev_t.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

---
 drivers/staging/speakup/spk_priv.h  |    2 
 drivers/staging/speakup/spk_ttyio.c |  105 ++++++++++++++++++++++++++++++++++++
 drivers/staging/speakup/spk_types.h |    1 
 3 files changed, 108 insertions(+)

--- a/drivers/staging/speakup/spk_priv.h
+++ b/drivers/staging/speakup/spk_priv.h
@@ -40,6 +40,8 @@
 
 #define KT_SPKUP 15
 #define SPK_SYNTH_TIMEOUT 100000 /* in micro-seconds */
+#define SYNTH_DEFAULT_DEV "ttyS0"
+#define SYNTH_DEFAULT_SER 0
 
 const struct old_serial_port *spk_serial_init(int index);
 void spk_stop_serial_interrupt(void);
--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -7,6 +7,13 @@
 #include "spk_types.h"
 #include "spk_priv.h"
 
+/* Supported device types */
+#define DEV_PREFIX_TTYS "ttyS"
+#define DEV_PREFIX_TTYUSB "ttyUSB"
+#define DEV_PREFIX_LP "lp"
+
+const char *lp_supported[] = { "acntsa", "bns", "dummy", "txprt" };
+
 struct spk_ldisc_data {
 	char buf;
 	struct semaphore sem;
@@ -16,6 +23,104 @@ struct spk_ldisc_data {
 static struct spk_synth *spk_ttyio_synth;
 static struct tty_struct *speakup_tty;
 
+static int name_to_dev(const char *name, dev_t *dev_no)
+{
+        int maj = -1, min = -1;
+
+        if (strncmp(name, DEV_PREFIX_TTYS, strlen(DEV_PREFIX_TTYS)) == 0) {
+                if (kstrtoint(name + strlen(DEV_PREFIX_TTYS), 10, &min)) {
+			pr_err("speakup: Invalid ser param. Must be \
+					between 0 and 191 inclusive.\n");
+                        return -EINVAL;
+                }
+                maj = 4;
+
+                if (min < 0 || min > 191) {
+			pr_err("speakup: Invalid ser param. Must be \
+					between 0 and 191 inclusive.\n");
+                        return -EINVAL;
+                }
+                min = min + 64;
+        } else if (strncmp(name, DEV_PREFIX_TTYUSB, strlen(DEV_PREFIX_TTYUSB))
+			== 0) {
+                if (kstrtoint(name + strlen(DEV_PREFIX_TTYUSB), 10, &min)) {
+                        pr_err("speakup: Invalid ttyUSB number. \
+					Must be a number from 0 onwards\n");
+                        return -EINVAL;
+                }
+                maj = 188;
+
+                if (min < 0) {
+                        pr_err("speakup: Invalid ttyUSB number. \
+					Must be a number from 0 onwards\n");
+                        return -EINVAL;
+                }
+        } else if (strncmp(name, DEV_PREFIX_LP, strlen(DEV_PREFIX_LP)) == 0) {
+                if (kstrtoint(name + strlen(DEV_PREFIX_LP), 10, &min)) {
+                        pr_warn("speakup: Invalid lp number. \
+					Must be a number from 0 onwards\n");
+                        return -EINVAL;
+                }
+                maj = 6;
+
+                if (min < 0) {
+                        pr_warn("speakup: Invalid lp number. \
+					Must be a number from 0 onwards\n");
+                        return -EINVAL;
+                }
+        }
+
+        if (maj == -1 || min == -1)
+                return -EINVAL;
+
+        /* if here, maj and min must be valid */
+        *dev_no = MKDEV(maj, min);
+
+        return 0;
+}
+
+int ser_to_dev(int ser, dev_t *dev_no)
+{
+	if (ser < 0 || ser > (255 - 64)) {
+                pr_err("speakup: Invalid ser param. \
+				Must be between 0 and 191 inclusive.\n");
+
+		return -EINVAL;
+        }
+
+	*dev_no = MKDEV(4, (64 + ser));
+	return 0;
+}
+
+static int get_dev_to_use(struct spk_synth *synth, dev_t *dev_no)
+{
+	/* use ser only when dev is not specified */
+	if (strcmp(synth->dev, SYNTH_DEFAULT_DEV) || synth->ser == SYNTH_DEFAULT_SER) {
+		/* for /dev/lp* check if synth is supported */
+		if (strncmp(synth->dev, DEV_PREFIX_LP, strlen(DEV_PREFIX_LP)) == 0) {
+			int i;
+
+			for (i = 0; i < ARRAY_SIZE(lp_supported); i++) {
+				if (strcmp(synth->name, lp_supported[i]) == 0)
+					break;
+			}
+
+			if (i >= ARRAY_SIZE(lp_supported)) {
+				pr_err("speakup: lp* is only supported on:");
+				for (i = 0; i < ARRAY_SIZE(lp_supported); i++)
+					pr_cont(" %s", lp_supported[i]);
+				pr_cont("\n");
+
+				return -ENOTSUPP;
+			}
+		}
+
+		return name_to_dev(synth->dev, dev_no);
+	}
+
+	return ser_to_dev(synth->ser, dev_no);
+}
+
 static int spk_ttyio_ldisc_open(struct tty_struct *tty)
 {
 	struct spk_ldisc_data *ldisc_data;
--- a/drivers/staging/speakup/spk_types.h
+++ b/drivers/staging/speakup/spk_types.h
@@ -169,6 +169,7 @@ struct spk_synth {
 	int jiffies;
 	int full;
 	int ser;
+	char *dev;
 	short flags;
 	short startup;
 	const int checkval; /* for validating a proper synth module */

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Dan Carpenter]

On Tue, Jun 13, 2017 at 11:37:03PM +0100, okash.khawaja@gmail.com wrote:
> The function converts strings like ttyS0 and ttyUSB0 to dev_t like
> (4, 64) and (188, 0). Subsequent patch in this set will call it to
> convert user-supplied device into device number. The function does
> some basic sanity checks on the string passed in. It currently supports
> ttyS*, ttyUSB* and, for selected synths, lp*.
> 
> In order to do this, the patch also introduces a string member variable
> named 'dev' to struct spk_synth. 'dev' represents the device name -
> ttyUSB0 etc - which needs conversion to dev_t.
> 
> Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> ---
>  drivers/staging/speakup/spk_priv.h  |    2 
>  drivers/staging/speakup/spk_ttyio.c |  105 ++++++++++++++++++++++++++++++++++++
>  drivers/staging/speakup/spk_types.h |    1 
>  3 files changed, 108 insertions(+)
> 
> --- a/drivers/staging/speakup/spk_priv.h
> +++ b/drivers/staging/speakup/spk_priv.h
> @@ -40,6 +40,8 @@
>  
>  #define KT_SPKUP 15
>  #define SPK_SYNTH_TIMEOUT 100000 /* in micro-seconds */
> +#define SYNTH_DEFAULT_DEV "ttyS0"
> +#define SYNTH_DEFAULT_SER 0
>  
>  const struct old_serial_port *spk_serial_init(int index);
>  void spk_stop_serial_interrupt(void);
> --- a/drivers/staging/speakup/spk_ttyio.c
> +++ b/drivers/staging/speakup/spk_ttyio.c
> @@ -7,6 +7,13 @@
>  #include "spk_types.h"
>  #include "spk_priv.h"
>  
> +/* Supported device types */
> +#define DEV_PREFIX_TTYS "ttyS"
> +#define DEV_PREFIX_TTYUSB "ttyUSB"
> +#define DEV_PREFIX_LP "lp"
> +
> +const char *lp_supported[] = { "acntsa", "bns", "dummy", "txprt" };
> +
>  struct spk_ldisc_data {
>  	char buf;
>  	struct semaphore sem;
> @@ -16,6 +23,104 @@ struct spk_ldisc_data {
>  static struct spk_synth *spk_ttyio_synth;
>  static struct tty_struct *speakup_tty;
>  
> +static int name_to_dev(const char *name, dev_t *dev_no)
> +{
> +        int maj = -1, min = -1;
> +
> +        if (strncmp(name, DEV_PREFIX_TTYS, strlen(DEV_PREFIX_TTYS)) == 0) {
> +                if (kstrtoint(name + strlen(DEV_PREFIX_TTYS), 10, &min)) {
> +			pr_err("speakup: Invalid ser param. Must be \
> +					between 0 and 191 inclusive.\n");

String needs fixed.

> +                        return -EINVAL;

Preserve the error code from kstrtoint().

> +                }
> +                maj = 4;
> +
> +                if (min < 0 || min > 191) {
> +			pr_err("speakup: Invalid ser param. Must be \
> +					between 0 and 191 inclusive.\n");


This too.

> +                        return -EINVAL;
> +                }
> +                min = min + 64;
> +        } else if (strncmp(name, DEV_PREFIX_TTYUSB, strlen(DEV_PREFIX_TTYUSB))
> +			== 0) {
> +                if (kstrtoint(name + strlen(DEV_PREFIX_TTYUSB), 10, &min)) {
> +                        pr_err("speakup: Invalid ttyUSB number. \
> +					Must be a number from 0 onwards\n");
> +                        return -EINVAL;

Same.

> +                }
> +                maj = 188;
> +
> +                if (min < 0) {
> +                        pr_err("speakup: Invalid ttyUSB number. \
> +					Must be a number from 0 onwards\n");

Same.

> +                        return -EINVAL;
> +                }
> +        } else if (strncmp(name, DEV_PREFIX_LP, strlen(DEV_PREFIX_LP)) == 0) {
> +                if (kstrtoint(name + strlen(DEV_PREFIX_LP), 10, &min)) {
> +                        pr_warn("speakup: Invalid lp number. \
> +					Must be a number from 0 onwards\n");
> +                        return -EINVAL;

Same.  Preserve.

> +                }
> +                maj = 6;
> +
> +                if (min < 0) {
> +                        pr_warn("speakup: Invalid lp number. \
> +					Must be a number from 0 onwards\n");

Again.

> +                        return -EINVAL;
> +                }
> +        }
> +
> +        if (maj == -1 || min == -1)
> +                return -EINVAL;
> +
> +        /* if here, maj and min must be valid */
> +        *dev_no = MKDEV(maj, min);
> +
> +        return 0;
> +}
> +
> +int ser_to_dev(int ser, dev_t *dev_no)
> +{
> +	if (ser < 0 || ser > (255 - 64)) {
> +                pr_err("speakup: Invalid ser param. \
> +				Must be between 0 and 191 inclusive.\n");

String.  I feel like 191 and 255 - 64 are the same.  Let's just use 191
everywhere.

> +
> +		return -EINVAL;
> +        }
> +
> +	*dev_no = MKDEV(4, (64 + ser));
> +	return 0;
> +}
> +
> +static int get_dev_to_use(struct spk_synth *synth, dev_t *dev_no)
> +{
> +	/* use ser only when dev is not specified */
> +	if (strcmp(synth->dev, SYNTH_DEFAULT_DEV) || synth->ser == SYNTH_DEFAULT_SER) {
> +		/* for /dev/lp* check if synth is supported */
> +		if (strncmp(synth->dev, DEV_PREFIX_LP, strlen(DEV_PREFIX_LP)) == 0) {
> +			int i;
> +
> +			for (i = 0; i < ARRAY_SIZE(lp_supported); i++) {
> +				if (strcmp(synth->name, lp_supported[i]) == 0)
> +					break;
> +			}
> +
> +			if (i >= ARRAY_SIZE(lp_supported)) {
> +				pr_err("speakup: lp* is only supported on:");
> +				for (i = 0; i < ARRAY_SIZE(lp_supported); i++)
> +					pr_cont(" %s", lp_supported[i]);
> +				pr_cont("\n");
> +
> +				return -ENOTSUPP;
> +			}
> +		}
> +
> +		return name_to_dev(synth->dev, dev_no);
> +	}
> +
> +	return ser_to_dev(synth->ser, dev_no);
> +}
> +
>  static int spk_ttyio_ldisc_open(struct tty_struct *tty)
>  {
>  	struct spk_ldisc_data *ldisc_data;
> --- a/drivers/staging/speakup/spk_types.h
> +++ b/drivers/staging/speakup/spk_types.h
> @@ -169,6 +169,7 @@ struct spk_synth {
>  	int jiffies;
>  	int full;
>  	int ser;
> +	char *dev;

Could you call it "dev_name" instead?  I normally expect "dev" to be a
device struct.

regards,
dan carpenter

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Okash Khawaja]

Hi,

On Wed, Jun 14, 2017 at 9:23 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
[...]
>
> Could you call it "dev_name" instead?  I normally expect "dev" to be a
> device struct.

Thanks for the feedback. Will keep these in mind for next version of the patch.

Okash

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Greg Kroah-Hartman]

On Tue, Jun 13, 2017 at 11:37:03PM +0100, okash.khawaja@gmail.com wrote:
> The function converts strings like ttyS0 and ttyUSB0 to dev_t like
> (4, 64) and (188, 0). Subsequent patch in this set will call it to
> convert user-supplied device into device number. The function does
> some basic sanity checks on the string passed in. It currently supports
> ttyS*, ttyUSB* and, for selected synths, lp*.
> 
> In order to do this, the patch also introduces a string member variable
> named 'dev' to struct spk_synth. 'dev' represents the device name -
> ttyUSB0 etc - which needs conversion to dev_t.
> 
> Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> ---
>  drivers/staging/speakup/spk_priv.h  |    2 
>  drivers/staging/speakup/spk_ttyio.c |  105 ++++++++++++++++++++++++++++++++++++
>  drivers/staging/speakup/spk_types.h |    1 
>  3 files changed, 108 insertions(+)
> 
> --- a/drivers/staging/speakup/spk_priv.h
> +++ b/drivers/staging/speakup/spk_priv.h
> @@ -40,6 +40,8 @@
>  
>  #define KT_SPKUP 15
>  #define SPK_SYNTH_TIMEOUT 100000 /* in micro-seconds */
> +#define SYNTH_DEFAULT_DEV "ttyS0"
> +#define SYNTH_DEFAULT_SER 0
>  
>  const struct old_serial_port *spk_serial_init(int index);
>  void spk_stop_serial_interrupt(void);
> --- a/drivers/staging/speakup/spk_ttyio.c
> +++ b/drivers/staging/speakup/spk_ttyio.c
> @@ -7,6 +7,13 @@
>  #include "spk_types.h"
>  #include "spk_priv.h"
>  
> +/* Supported device types */
> +#define DEV_PREFIX_TTYS "ttyS"
> +#define DEV_PREFIX_TTYUSB "ttyUSB"
> +#define DEV_PREFIX_LP "lp"
> +
> +const char *lp_supported[] = { "acntsa", "bns", "dummy", "txprt" };
> +
>  struct spk_ldisc_data {
>  	char buf;
>  	struct semaphore sem;
> @@ -16,6 +23,104 @@ struct spk_ldisc_data {
>  static struct spk_synth *spk_ttyio_synth;
>  static struct tty_struct *speakup_tty;
>  
> +static int name_to_dev(const char *name, dev_t *dev_no)
> +{
> +        int maj = -1, min = -1;
> +
> +        if (strncmp(name, DEV_PREFIX_TTYS, strlen(DEV_PREFIX_TTYS)) == 0) {
> +                if (kstrtoint(name + strlen(DEV_PREFIX_TTYS), 10, &min)) {
> +			pr_err("speakup: Invalid ser param. Must be \
> +					between 0 and 191 inclusive.\n");
> +                        return -EINVAL;
> +                }
> +                maj = 4;
> +
> +                if (min < 0 || min > 191) {
> +			pr_err("speakup: Invalid ser param. Must be \
> +					between 0 and 191 inclusive.\n");
> +                        return -EINVAL;
> +                }
> +                min = min + 64;
> +        } else if (strncmp(name, DEV_PREFIX_TTYUSB, strlen(DEV_PREFIX_TTYUSB))
> +			== 0) {
> +                if (kstrtoint(name + strlen(DEV_PREFIX_TTYUSB), 10, &min)) {
> +                        pr_err("speakup: Invalid ttyUSB number. \
> +					Must be a number from 0 onwards\n");
> +                        return -EINVAL;
> +                }
> +                maj = 188;
> +
> +                if (min < 0) {
> +                        pr_err("speakup: Invalid ttyUSB number. \
> +					Must be a number from 0 onwards\n");
> +                        return -EINVAL;
> +                }
> +        } else if (strncmp(name, DEV_PREFIX_LP, strlen(DEV_PREFIX_LP)) == 0) {
> +                if (kstrtoint(name + strlen(DEV_PREFIX_LP), 10, &min)) {
> +                        pr_warn("speakup: Invalid lp number. \
> +					Must be a number from 0 onwards\n");
> +                        return -EINVAL;
> +                }
> +                maj = 6;
> +
> +                if (min < 0) {
> +                        pr_warn("speakup: Invalid lp number. \
> +					Must be a number from 0 onwards\n");
> +                        return -EINVAL;
> +                }
> +        }
> +
> +        if (maj == -1 || min == -1)
> +                return -EINVAL;
> +
> +        /* if here, maj and min must be valid */
> +        *dev_no = MKDEV(maj, min);
> +
> +        return 0;
> +}

Eeek, no, let's never try to parse strings like this and "figure out"
what major/minor number it is.  That's madness and will break if we ever
make all char majors dynamic (there's a thread on lkml about that.)

Why would the kernel need to know major/minor?  Is it going to open a
device node?  If so, again, crazy stuff, that's not good...

thanks,

greg k-h

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Greg Kroah-Hartman]

On Wed, Jun 14, 2017 at 12:13:26PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 13, 2017 at 11:37:03PM +0100, okash.khawaja@gmail.com wrote:
> > The function converts strings like ttyS0 and ttyUSB0 to dev_t like
> > (4, 64) and (188, 0). Subsequent patch in this set will call it to
> > convert user-supplied device into device number. The function does
> > some basic sanity checks on the string passed in. It currently supports
> > ttyS*, ttyUSB* and, for selected synths, lp*.
> > 
> > In order to do this, the patch also introduces a string member variable
> > named 'dev' to struct spk_synth. 'dev' represents the device name -
> > ttyUSB0 etc - which needs conversion to dev_t.
> > 
> > Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
> > Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> > 
> > ---
> >  drivers/staging/speakup/spk_priv.h  |    2 
> >  drivers/staging/speakup/spk_ttyio.c |  105 ++++++++++++++++++++++++++++++++++++
> >  drivers/staging/speakup/spk_types.h |    1 
> >  3 files changed, 108 insertions(+)
> > 
> > --- a/drivers/staging/speakup/spk_priv.h
> > +++ b/drivers/staging/speakup/spk_priv.h
> > @@ -40,6 +40,8 @@
> >  
> >  #define KT_SPKUP 15
> >  #define SPK_SYNTH_TIMEOUT 100000 /* in micro-seconds */
> > +#define SYNTH_DEFAULT_DEV "ttyS0"
> > +#define SYNTH_DEFAULT_SER 0
> >  
> >  const struct old_serial_port *spk_serial_init(int index);
> >  void spk_stop_serial_interrupt(void);
> > --- a/drivers/staging/speakup/spk_ttyio.c
> > +++ b/drivers/staging/speakup/spk_ttyio.c
> > @@ -7,6 +7,13 @@
> >  #include "spk_types.h"
> >  #include "spk_priv.h"
> >  
> > +/* Supported device types */
> > +#define DEV_PREFIX_TTYS "ttyS"
> > +#define DEV_PREFIX_TTYUSB "ttyUSB"
> > +#define DEV_PREFIX_LP "lp"
> > +
> > +const char *lp_supported[] = { "acntsa", "bns", "dummy", "txprt" };
> > +
> >  struct spk_ldisc_data {
> >  	char buf;
> >  	struct semaphore sem;
> > @@ -16,6 +23,104 @@ struct spk_ldisc_data {
> >  static struct spk_synth *spk_ttyio_synth;
> >  static struct tty_struct *speakup_tty;
> >  
> > +static int name_to_dev(const char *name, dev_t *dev_no)
> > +{
> > +        int maj = -1, min = -1;
> > +
> > +        if (strncmp(name, DEV_PREFIX_TTYS, strlen(DEV_PREFIX_TTYS)) == 0) {
> > +                if (kstrtoint(name + strlen(DEV_PREFIX_TTYS), 10, &min)) {
> > +			pr_err("speakup: Invalid ser param. Must be \
> > +					between 0 and 191 inclusive.\n");
> > +                        return -EINVAL;
> > +                }
> > +                maj = 4;
> > +
> > +                if (min < 0 || min > 191) {
> > +			pr_err("speakup: Invalid ser param. Must be \
> > +					between 0 and 191 inclusive.\n");
> > +                        return -EINVAL;
> > +                }
> > +                min = min + 64;
> > +        } else if (strncmp(name, DEV_PREFIX_TTYUSB, strlen(DEV_PREFIX_TTYUSB))
> > +			== 0) {
> > +                if (kstrtoint(name + strlen(DEV_PREFIX_TTYUSB), 10, &min)) {
> > +                        pr_err("speakup: Invalid ttyUSB number. \
> > +					Must be a number from 0 onwards\n");
> > +                        return -EINVAL;
> > +                }
> > +                maj = 188;
> > +
> > +                if (min < 0) {
> > +                        pr_err("speakup: Invalid ttyUSB number. \
> > +					Must be a number from 0 onwards\n");
> > +                        return -EINVAL;
> > +                }
> > +        } else if (strncmp(name, DEV_PREFIX_LP, strlen(DEV_PREFIX_LP)) == 0) {
> > +                if (kstrtoint(name + strlen(DEV_PREFIX_LP), 10, &min)) {
> > +                        pr_warn("speakup: Invalid lp number. \
> > +					Must be a number from 0 onwards\n");
> > +                        return -EINVAL;
> > +                }
> > +                maj = 6;
> > +
> > +                if (min < 0) {
> > +                        pr_warn("speakup: Invalid lp number. \
> > +					Must be a number from 0 onwards\n");
> > +                        return -EINVAL;
> > +                }
> > +        }
> > +
> > +        if (maj == -1 || min == -1)
> > +                return -EINVAL;
> > +
> > +        /* if here, maj and min must be valid */
> > +        *dev_no = MKDEV(maj, min);
> > +
> > +        return 0;
> > +}
> 
> Eeek, no, let's never try to parse strings like this and "figure out"
> what major/minor number it is.  That's madness and will break if we ever
> make all char majors dynamic (there's a thread on lkml about that.)
> 
> Why would the kernel need to know major/minor?  Is it going to open a
> device node?  If so, again, crazy stuff, that's not good...

Ah, no, nevermind, you just need the major/minor.  So why not just take
that as the input here, and not a string?

thanks,

greg k-h

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Samuel Thibault]

Greg KH, on mer. 14 juin 2017 12:15:41 +0200, wrote:
> Ah, no, nevermind, you just need the major/minor.  So why not just take
> that as the input here, and not a string?

Because real users don't know about major/minor numbers.

Samuel

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Greg Kroah-Hartman]

On Wed, Jun 14, 2017 at 01:26:09PM +0200, Samuel Thibault wrote:
> Greg KH, on mer. 14 juin 2017 12:15:41 +0200, wrote:
> > Ah, no, nevermind, you just need the major/minor.  So why not just take
> > that as the input here, and not a string?
> 
> Because real users don't know about major/minor numbers.

I'm not disagreeing, it's just that the kernel doesn't know about
"device names" either :)

And trying to have the kernel do the mapping based on strings like this
is not ok, sorry.

greg k-h

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Samuel Thibault]

Greg KH, on mer. 14 juin 2017 13:48:02 +0200, wrote:
> On Wed, Jun 14, 2017 at 01:26:09PM +0200, Samuel Thibault wrote:
> > Greg KH, on mer. 14 juin 2017 12:15:41 +0200, wrote:
> > > Ah, no, nevermind, you just need the major/minor.  So why not just take
> > > that as the input here, and not a string?
> > 
> > Because real users don't know about major/minor numbers.
> 
> I'm not disagreeing, it's just that the kernel doesn't know about
> "device names" either :)

Well, it does for the console= parameter, for instance.  I know it's not
actually related with major/minor numbering, but it's still meant to
designate the same thing.

> And trying to have the kernel do the mapping based on strings like this
> is not ok, sorry.

So what is the solution?  Users would find it completely crazy to have
to provide major/minor numbers.

Samuel

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Andy Shevchenko]

On Wed, Jun 14, 2017 at 3:18 PM, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> Greg KH, on mer. 14 juin 2017 13:48:02 +0200, wrote:
>> On Wed, Jun 14, 2017 at 01:26:09PM +0200, Samuel Thibault wrote:
>> > Greg KH, on mer. 14 juin 2017 12:15:41 +0200, wrote:

>> And trying to have the kernel do the mapping based on strings like this
>> is not ok, sorry.
>
> So what is the solution?  Users would find it completely crazy to have
> to provide major/minor numbers.

Shouldn't udev take care of major/minor and alike stuff in user space?

-- 
With Best Regards,
Andy Shevchenko

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Samuel Thibault]

Andy Shevchenko, on mer. 14 juin 2017 15:36:30 +0300, wrote:
> On Wed, Jun 14, 2017 at 3:18 PM, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> > Greg KH, on mer. 14 juin 2017 13:48:02 +0200, wrote:
> >> On Wed, Jun 14, 2017 at 01:26:09PM +0200, Samuel Thibault wrote:
> >> > Greg KH, on mer. 14 juin 2017 12:15:41 +0200, wrote:
> 
> >> And trying to have the kernel do the mapping based on strings like this
> >> is not ok, sorry.
> >
> > So what is the solution?  Users would find it completely crazy to have
> > to provide major/minor numbers.
> 
> Shouldn't udev take care of major/minor and alike stuff in user space?

We want speakup to get initialized before udev etc., just like the early
console.

Samuel

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Greg Kroah-Hartman]

On Wed, Jun 14, 2017 at 03:36:30PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 14, 2017 at 3:18 PM, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> > Greg KH, on mer. 14 juin 2017 13:48:02 +0200, wrote:
> >> On Wed, Jun 14, 2017 at 01:26:09PM +0200, Samuel Thibault wrote:
> >> > Greg KH, on mer. 14 juin 2017 12:15:41 +0200, wrote:
> 
> >> And trying to have the kernel do the mapping based on strings like this
> >> is not ok, sorry.
> >
> > So what is the solution?  Users would find it completely crazy to have
> > to provide major/minor numbers.
> 
> Shouldn't udev take care of major/minor and alike stuff in user space?

No, that's all handled by the kernel now, in devtmpfs.

The console stuff is odd though, but that is each driver defining the
name for itself, major/minor does not apply.  Also, a separate driver is
not having to figure this all out.  I wonder if we could just add a tty
core function for this as it does know the name of everything that has
been registered with it, right?

thanks,

greg k-h

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Okash Khawaja]

Hi,

On Wed, Jun 14, 2017 at 03:04:18PM +0200, Greg Kroah-Hartman wrote: 
> The console stuff is odd though, but that is each driver defining the
> name for itself, major/minor does not apply.  Also, a separate driver is
> not having to figure this all out.  I wonder if we could just add a tty
> core function for this as it does know the name of everything that has
> been registered with it, right?

I can start working on a patch for this. I'm still new to the tty code
so will follow up with any questions I might have.

Thanks,
Okash

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Okash Khawaja]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

Hi,

On Thu, Jun 15, 2017 at 07:52:51AM +0100, Okash Khawaja wrote:
> On Wed, Jun 14, 2017 at 03:04:18PM +0200, Greg Kroah-Hartman wrote: 
> > The console stuff is odd though, but that is each driver defining the
> > name for itself, major/minor does not apply.  Also, a separate driver is
> > not having to figure this all out.  I wonder if we could just add a tty
> > core function for this as it does know the name of everything that has
> > been registered with it, right?
> 
> I can start working on a patch for this. I'm still new to the tty code
> so will follow up with any questions I might have.

I've put together this function for converting device name to number.
It traverses tty_drivers list looking for corresponding dev name and
index. Is this the right approach?

Thanks,
Okash

[-- Attachment #2: X_tty_dev_name_to_number --]
[-- Type: text/plain, Size: 2282 bytes --]

---
 drivers/tty/tty_io.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/tty.h  |    4 ++++
 2 files changed, 49 insertions(+)

--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -325,6 +325,51 @@ static struct tty_driver *get_tty_driver
 	return NULL;
 }
 
+/**
+ *	tty_dev_name_to_number	-	return dev_t for device name
+ *	@device_name: user space name of device under /dev
+ *	@dev_no: pointer to dev_t that this function will populate
+ *
+ *	This function converts device names like ttyS0 or ttyUSB1 into dev_t
+ *	like (4, 64) or (188, 1). If no corresponding driver is registered then
+ *	the function returns -ENODEV.
+ *
+ *	Locking: this acquires tty_mutex
+ */
+int tty_dev_name_to_number(char *dev_name, dev_t *dev_no)
+{
+	struct tty_driver *p;
+	int rv, index, prefix_length = 0;
+
+	while (!isdigit(*(dev_name + prefix_length)) && prefix_length <
+			strlen(dev_name) )
+		prefix_length++;
+
+	if (prefix_length == strlen(dev_name))
+		return -EINVAL;
+
+	mutex_lock(&tty_mutex);
+
+	list_for_each_entry(p, &tty_drivers, tty_drivers)
+		if (prefix_length == strlen(p->name) && strncmp(dev_name,
+					p->name, prefix_length) == 0) {
+			rv = kstrtoint(dev_name + prefix_length, 10, &index);
+			if (rv) {
+				mutex_unlock(&tty_mutex);
+				return rv;
+			}
+			if (index < p->num) {
+				*dev_no = MKDEV(p->major, p->minor_start + index);
+				mutex_unlock(&tty_mutex);
+				return 0;
+			}
+		}
+
+	mutex_unlock(&tty_mutex);
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(tty_dev_name_to_number);
+
 #ifdef CONFIG_CONSOLE_POLL
 
 /**
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -402,6 +402,7 @@ extern int __init tty_init(void);
 extern const char *tty_name(const struct tty_struct *tty);
 extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
 		struct file *filp);
+extern int tty_dev_name_to_number(char *dev_name, dev_t *dev_no);
 #else
 static inline void tty_kref_put(struct tty_struct *tty)
 { }
@@ -422,6 +423,9 @@ static inline int __init tty_init(void)
 { return 0; }
 static inline const char *tty_name(const struct tty_struct *tty)
 { return "(none)"; }
+extern int tty_dev_name_to_number(char *dev_name, dev_t *dev_no)
+{ return -ENOTSUPP; }
+
 #endif
 
 extern struct ktermios tty_std_termios;

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Greg Kroah-Hartman]

On Sat, Jun 17, 2017 at 11:16:44AM +0100, Okash Khawaja wrote:
> Hi,
> 
> On Thu, Jun 15, 2017 at 07:52:51AM +0100, Okash Khawaja wrote:
> > On Wed, Jun 14, 2017 at 03:04:18PM +0200, Greg Kroah-Hartman wrote: 
> > > The console stuff is odd though, but that is each driver defining the
> > > name for itself, major/minor does not apply.  Also, a separate driver is
> > > not having to figure this all out.  I wonder if we could just add a tty
> > > core function for this as it does know the name of everything that has
> > > been registered with it, right?
> > 
> > I can start working on a patch for this. I'm still new to the tty code
> > so will follow up with any questions I might have.
> 
> I've put together this function for converting device name to number.
> It traverses tty_drivers list looking for corresponding dev name and
> index. Is this the right approach?

Yes, this looks great, nice job!  It is a lot simpler than your previous
function, and now you are not limited to just a subset off the different
serial ports in the system.

Keep up the good work, it's really appreciated.

greg k-h

Re: [patch 1/2] staging: speakup: add function to convert dev name to number

[Okash Khawaja]

On Sat, Jun 17, 2017 at 12:25:34PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Jun 17, 2017 at 11:16:44AM +0100, Okash Khawaja wrote:
> > Hi,
> > 
> > On Thu, Jun 15, 2017 at 07:52:51AM +0100, Okash Khawaja wrote:
> > > On Wed, Jun 14, 2017 at 03:04:18PM +0200, Greg Kroah-Hartman wrote: 
> > > > The console stuff is odd though, but that is each driver defining the
> > > > name for itself, major/minor does not apply.  Also, a separate driver is
> > > > not having to figure this all out.  I wonder if we could just add a tty
> > > > core function for this as it does know the name of everything that has
> > > > been registered with it, right?
> > > 
> > > I can start working on a patch for this. I'm still new to the tty code
> > > so will follow up with any questions I might have.
> > 
> > I've put together this function for converting device name to number.
> > It traverses tty_drivers list looking for corresponding dev name and
> > index. Is this the right approach?
> 
> Yes, this looks great, nice job!  It is a lot simpler than your previous
> function, and now you are not limited to just a subset off the different
> serial ports in the system.
> 
> Keep up the good work, it's really appreciated.
Great, thanks. I'll re-send the full patch set as v2.

Okash

[patch 2/2] staging: speakup: make ttyio synths use device name

[okash.khawaja]

[-- Attachment #1: 12_make_ttyio_use_device_name --]
[-- Type: text/plain, Size: 10636 bytes --]

This patch introduces new module parameter, dev, which takes a string
representing the device that the external synth is connected to, e.g.
ttyS0, ttyUSB0 etc. This is then used to communicate with the synth.
That way, speakup can support more than ttyS*. As of this patch, it
only supports ttyS*, ttyUSB* and selected synths for lp*. dev parameter
is only available for tty-migrated synths.

Users will either use dev or ser as both serve same purpose. This patch
maintains backward compatility by allowing ser to be specified. When
both are specified, whichever is non-default, i.e. not ttyS0, is used.
If both are non-default then dev is used.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

---
 drivers/staging/speakup/speakup_acntsa.c |    3 +++
 drivers/staging/speakup/speakup_apollo.c |    3 +++
 drivers/staging/speakup/speakup_audptr.c |    3 +++
 drivers/staging/speakup/speakup_bns.c    |    3 +++
 drivers/staging/speakup/speakup_decext.c |    3 +++
 drivers/staging/speakup/speakup_dectlk.c |    3 +++
 drivers/staging/speakup/speakup_dummy.c  |    3 +++
 drivers/staging/speakup/speakup_ltlk.c   |    3 +++
 drivers/staging/speakup/speakup_spkout.c |    3 +++
 drivers/staging/speakup/speakup_txprt.c  |    3 +++
 drivers/staging/speakup/spk_ttyio.c      |   15 +++++++--------
 11 files changed, 37 insertions(+), 8 deletions(-)

--- a/drivers/staging/speakup/speakup_acntsa.c
+++ b/drivers/staging/speakup/speakup_acntsa.c
@@ -96,6 +96,7 @@ static struct spk_synth synth_acntsa = {
 	.trigger = 50,
 	.jiffies = 30,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -135,9 +136,11 @@ static int synth_probe(struct spk_synth
 }
 
 module_param_named(ser, synth_acntsa.ser, int, 0444);
+module_param_named(dev, synth_acntsa.dev, charp, S_IRUGO);
 module_param_named(start, synth_acntsa.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_acntsa);
--- a/drivers/staging/speakup/speakup_apollo.c
+++ b/drivers/staging/speakup/speakup_apollo.c
@@ -105,6 +105,7 @@ static struct spk_synth synth_apollo = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -199,9 +200,11 @@ static void do_catch_up(struct spk_synth
 }
 
 module_param_named(ser, synth_apollo.ser, int, 0444);
+module_param_named(dev, synth_apollo.dev, charp, S_IRUGO);
 module_param_named(start, synth_apollo.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_apollo);
--- a/drivers/staging/speakup/speakup_audptr.c
+++ b/drivers/staging/speakup/speakup_audptr.c
@@ -100,6 +100,7 @@ static struct spk_synth synth_audptr = {
 	.trigger = 50,
 	.jiffies = 30,
 	.full = 18000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -162,9 +163,11 @@ static int synth_probe(struct spk_synth
 }
 
 module_param_named(ser, synth_audptr.ser, int, 0444);
+module_param_named(dev, synth_audptr.dev, charp, S_IRUGO);
 module_param_named(start, synth_audptr.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_audptr);
--- a/drivers/staging/speakup/speakup_bns.c
+++ b/drivers/staging/speakup/speakup_bns.c
@@ -93,6 +93,7 @@ static struct spk_synth synth_bns = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -119,9 +120,11 @@ static struct spk_synth synth_bns = {
 };
 
 module_param_named(ser, synth_bns.ser, int, 0444);
+module_param_named(dev, synth_bns.dev, charp, S_IRUGO);
 module_param_named(start, synth_bns.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_bns);
--- a/drivers/staging/speakup/speakup_decext.c
+++ b/drivers/staging/speakup/speakup_decext.c
@@ -120,6 +120,7 @@ static struct spk_synth synth_decext = {
 	.jiffies = 50,
 	.full = 40000,
 	.flags = SF_DEC,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -226,9 +227,11 @@ static void synth_flush(struct spk_synth
 }
 
 module_param_named(ser, synth_decext.ser, int, 0444);
+module_param_named(dev, synth_decext.dev, charp, S_IRUGO);
 module_param_named(start, synth_decext.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_decext);
--- a/drivers/staging/speakup/speakup_dectlk.c
+++ b/drivers/staging/speakup/speakup_dectlk.c
@@ -124,6 +124,7 @@ static struct spk_synth synth_dectlk = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -298,9 +299,11 @@ static void synth_flush(struct spk_synth
 }
 
 module_param_named(ser, synth_dectlk.ser, int, 0444);
+module_param_named(dev, synth_dectlk.dev, charp, S_IRUGO);
 module_param_named(start, synth_dectlk.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_dectlk);
--- a/drivers/staging/speakup/speakup_dummy.c
+++ b/drivers/staging/speakup/speakup_dummy.c
@@ -95,6 +95,7 @@ static struct spk_synth synth_dummy = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -121,9 +122,11 @@ static struct spk_synth synth_dummy = {
 };
 
 module_param_named(ser, synth_dummy.ser, int, 0444);
+module_param_named(dev, synth_dummy.dev, charp, S_IRUGO);
 module_param_named(start, synth_dummy.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_dummy);
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -107,6 +107,7 @@ static struct spk_synth synth_ltlk = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -166,9 +167,11 @@ static int synth_probe(struct spk_synth
 }
 
 module_param_named(ser, synth_ltlk.ser, int, 0444);
+module_param_named(dev, synth_ltlk.dev, charp, S_IRUGO);
 module_param_named(start, synth_ltlk.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_ltlk);
--- a/drivers/staging/speakup/speakup_spkout.c
+++ b/drivers/staging/speakup/speakup_spkout.c
@@ -98,6 +98,7 @@ static struct spk_synth synth_spkout = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -130,9 +131,11 @@ static void synth_flush(struct spk_synth
 }
 
 module_param_named(ser, synth_spkout.ser, int, 0444);
+module_param_named(dev, synth_spkout.dev, charp, S_IRUGO);
 module_param_named(start, synth_spkout.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_spkout);
--- a/drivers/staging/speakup/speakup_txprt.c
+++ b/drivers/staging/speakup/speakup_txprt.c
@@ -92,6 +92,7 @@ static struct spk_synth synth_txprt = {
 	.trigger = 50,
 	.jiffies = 50,
 	.full = 40000,
+	.dev = SYNTH_DEFAULT_DEV,
 	.startup = SYNTH_START,
 	.checkval = SYNTH_CHECK,
 	.vars = vars,
@@ -118,9 +119,11 @@ static struct spk_synth synth_txprt = {
 };
 
 module_param_named(ser, synth_txprt.ser, int, 0444);
+module_param_named(dev, synth_txprt.dev, charp, S_IRUGO);
 module_param_named(start, synth_txprt.startup, short, 0444);
 
 MODULE_PARM_DESC(ser, "Set the serial port for the synthesizer (0-based).");
+MODULE_PARM_DESC(dev, "Set the device e.g. ttyUSB0, for the synthesizer.");
 MODULE_PARM_DESC(start, "Start the synthesizer once it is loaded.");
 
 module_spk_synth(synth_txprt);
--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -209,11 +209,12 @@ static inline void get_termios(struct tt
 	up_read(&tty->termios_rwsem);
 }
 
-static int spk_ttyio_initialise_ldisc(int ser)
+static int spk_ttyio_initialise_ldisc(struct spk_synth *synth)
 {
 	int ret = 0;
 	struct tty_struct *tty;
 	struct ktermios tmp_termios;
+	dev_t dev;
 
 	ret = tty_register_ldisc(N_SPEAKUP, &spk_ttyio_ldisc_ops);
 	if (ret) {
@@ -221,13 +222,11 @@ static int spk_ttyio_initialise_ldisc(in
 		return ret;
 	}
 
-	if (ser < 0 || ser > (255 - 64)) {
-		pr_err("speakup: Invalid ser param. Must be between 0 and 191 inclusive.\n");
-		return -EINVAL;
-	}
+	ret = get_dev_to_use(synth, &dev);
+	if (ret)
+		return ret;
 
-	/* TODO: support more than ttyS* */
-	tty = tty_open_by_driver(MKDEV(4, (ser +  64)), NULL, NULL);
+	tty = tty_open_by_driver(dev, NULL, NULL);
 	if (IS_ERR(tty))
 		return PTR_ERR(tty);
 
@@ -338,7 +337,7 @@ static void spk_ttyio_flush_buffer(void)
 
 int spk_ttyio_synth_probe(struct spk_synth *synth)
 {
-	int rv = spk_ttyio_initialise_ldisc(synth->ser);
+	int rv = spk_ttyio_initialise_ldisc(synth);
 
 	if (rv)
 		return rv;