[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170614082330.nshvjidl3zq37bvz@mwanda>
Date: Wed, 14 Jun 2017 11:23:31 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: okash.khawaja@...il.com
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>,
Samuel Thibault <samuel.thibault@...-lyon.org>,
linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
Kirk Reiser <kirk@...sers.ca>, speakup@...ux-speakup.org,
Chris Brannon <chris@...-brannons.com>
Subject: Re: [patch 1/2] staging: speakup: add function to convert dev name
to number
On Tue, Jun 13, 2017 at 11:37:03PM +0100, okash.khawaja@...il.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@...il.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@...-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
Powered by blists - more mailing lists