Re: [patch v2 3/3] staging: speakup: make ttyio synths use device name

From: Andy Shevchenko
Date: Sun Jun 18 2017 - 09:39:48 EST


On Sun, Jun 18, 2017 at 11:58 AM, Okash Khawaja <okash.khawaja@xxxxxxxxx> wrote:
> 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.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

...with an assumption Greg is okay with new module parameter here.

> Signed-off-by: Okash Khawaja <okash.khawaja@xxxxxxxxx>
> Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
>
> ---
> 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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_name = 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_name, 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
> @@ -151,11 +151,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) {
> @@ -163,13 +164,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);
>
> @@ -281,7 +280,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;
>



--
With Best Regards,
Andy Shevchenko