Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753081AbdFRNir (ORCPT ); Sun, 18 Jun 2017 09:38:47 -0400 Received: from mail-qt0-f194.google.com ([209.85.216.194]:33886 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752102AbdFRNiq (ORCPT ); Sun, 18 Jun 2017 09:38:46 -0400 MIME-Version: 1.0 In-Reply-To: <20170618093538.752436783@gmail.com> References: <20170618085825.601359240@gmail.com> <20170618093538.752436783@gmail.com> From: Andy Shevchenko Date: Sun, 18 Jun 2017 16:38:44 +0300 Message-ID: Subject: Re: [patch v2 3/3] staging: speakup: make ttyio synths use device name To: Okash Khawaja Cc: Greg Kroah-Hartman , Jiri Slaby , Samuel Thibault , "linux-kernel@vger.kernel.org" , William Hubbs , Chris Brannon , Kirk Reiser , speakup@linux-speakup.org, devel@driverdev.osuosl.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 12106 Lines: 305 On Sun, Jun 18, 2017 at 11:58 AM, Okash Khawaja 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 ...with an assumption Greg is okay with new module parameter here. > Signed-off-by: Okash Khawaja > Reviewed-by: Samuel Thibault > > --- > 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