Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752189AbdFNJwr (ORCPT ); Wed, 14 Jun 2017 05:52:47 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:28394 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932073AbdFNIZL (ORCPT ); Wed, 14 Jun 2017 04:25:11 -0400 Date: Wed, 14 Jun 2017 11:23:31 +0300 From: Dan Carpenter To: okash.khawaja@gmail.com Cc: Greg Kroah-Hartman , Jiri Slaby , Samuel Thibault , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, Kirk Reiser , speakup@linux-speakup.org, Chris Brannon Subject: Re: [patch 1/2] staging: speakup: add function to convert dev name to number Message-ID: <20170614082330.nshvjidl3zq37bvz@mwanda> References: <20170613223702.534373529@gmail.com> <20170613224050.646277124@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170613224050.646277124@gmail.com> User-Agent: NeoMutt/20170113 (1.7.2) X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5735 Lines: 192 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 > Reviewed-by: Samuel Thibault > > --- > 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