[PATCH] drivers/staging: refactor dgnc tty registration.

[PATCH] drivers/staging: refactor dgnc tty registration.

[Haim Daniel]

-remove duplicate tty allocation code for serial and printer drivers.
-fix sparse warning: too long initializer-string for array of char.

Signed-off-by: Haim Daniel <haimdaniel@gmail.com>
---
 drivers/staging/dgnc/dgnc_driver.h |  13 ----
 drivers/staging/dgnc/dgnc_tty.c    | 137 ++++++++++++++-----------------------
 2 files changed, 50 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
index 980410f..764d6fe 100644
--- a/drivers/staging/dgnc/dgnc_driver.h
+++ b/drivers/staging/dgnc/dgnc_driver.h
@@ -52,19 +52,6 @@
 
 #define dgnc_jiffies_from_ms(a) (((a) * HZ) / 1000)
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.  This is the same structure that is defined
- * as the default in tty_io.c with the same settings overridden as in serial.c
- *
- * In short, this should match the internal serial ports' defaults.
- */
-#define	DEFAULT_IFLAGS	(ICRNL | IXON)
-#define	DEFAULT_OFLAGS	(OPOST | ONLCR)
-#define	DEFAULT_CFLAGS	(B9600 | CS8 | CREAD | HUPCL | CLOCAL)
-#define	DEFAULT_LFLAGS	(ISIG | ICANON | ECHO | ECHOE | ECHOK | \
-			ECHOCTL | ECHOKE | IEXTEN)
-
 #ifndef _POSIX_VDISABLE
 #define   _POSIX_VDISABLE '\0'
 #endif
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 9e98781..87af304 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -51,22 +51,6 @@
 	.digi_term =	"ansi"		/* default terminal type */
 };
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.
- *
- * This defines a raw port at 9600 baud, 8 data bits, no parity,
- * 1 stop bit.
- */
-static const struct ktermios default_termios = {
-	.c_iflag =	(DEFAULT_IFLAGS),
-	.c_oflag =	(DEFAULT_OFLAGS),
-	.c_cflag =	(DEFAULT_CFLAGS),
-	.c_lflag =	(DEFAULT_LFLAGS),
-	.c_cc =		INIT_C_CC,
-	.c_line =	0,
-};
-
 static int dgnc_tty_open(struct tty_struct *tty, struct file *file);
 static void dgnc_tty_close(struct tty_struct *tty, struct file *file);
 static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file,
@@ -129,6 +113,41 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
 
 /* TTY Initialization/Cleanup Functions */
 
+static struct tty_driver *__dgnc_tty_register(char *serial_name, uint maxports,
+					      int major, int minor)
+{
+	int rc;
+	struct tty_driver *drv = tty_alloc_driver(maxports,
+						  TTY_DRIVER_REAL_RAW |
+						  TTY_DRIVER_DYNAMIC_DEV |
+						  TTY_DRIVER_HARDWARE_BREAK);
+	if (IS_ERR(drv))
+		return drv;
+
+	drv->name = serial_name;
+	drv->name_base = 0;
+	drv->major = major;
+	drv->minor_start = minor;
+	drv->type = TTY_DRIVER_TYPE_SERIAL;
+	drv->subtype = SERIAL_TYPE_NORMAL;
+	drv->init_termios = tty_std_termios;
+	drv->init_termios.c_cflag = (B9600 | CS8 | CREAD | HUPCL | CLOCAL);
+	drv->init_termios.c_ispeed = 9600;
+	drv->init_termios.c_ospeed = 9600;
+	drv->driver_name = DRVSTR;
+	/*
+	 * Entry points for driver.  Called by the kernel from
+	 * tty_io.c and n_tty.c.
+	 */
+	tty_set_operations(drv, &dgnc_tty_ops);
+	rc = tty_register_driver(drv);
+	if (rc < 0) {
+		put_tty_driver(drv);
+		return ERR_PTR(rc);
+	}
+	return drv;
+}
+
 /**
  * dgnc_tty_register() - Init the tty subsystem for this board.
  */
@@ -136,87 +155,31 @@ int dgnc_tty_register(struct dgnc_board *brd)
 {
 	int rc;
 
-	brd->serial_driver = tty_alloc_driver(brd->maxports,
-					      TTY_DRIVER_REAL_RAW |
-					      TTY_DRIVER_DYNAMIC_DEV |
-					      TTY_DRIVER_HARDWARE_BREAK);
-	if (IS_ERR(brd->serial_driver))
-		return PTR_ERR(brd->serial_driver);
-
 	snprintf(brd->serial_name, MAXTTYNAMELEN, "tty_dgnc_%d_",
 		 brd->boardnum);
 
-	brd->serial_driver->name = brd->serial_name;
-	brd->serial_driver->name_base = 0;
-	brd->serial_driver->major = 0;
-	brd->serial_driver->minor_start = 0;
-	brd->serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->serial_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->serial_driver->init_termios = default_termios;
-	brd->serial_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->serial_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->serial_driver);
-	if (rc < 0) {
-		dev_dbg(&brd->pdev->dev,
-			"Can't register tty device (%d)\n", rc);
-		goto free_serial_driver;
+	brd->serial_driver = __dgnc_tty_register(brd->serial_name,
+						 brd->maxports, 0, 0);
+	if (IS_ERR(brd->serial_driver)) {
+		rc = PTR_ERR(brd->serial_driver);
+		dev_dbg(&brd->pdev->dev, "Can't register tty device (%d)\n",
+			rc);
+		return rc;
 	}
 
-	/*
-	 * If we're doing transparent print, we have to do all of the above
-	 * again, separately so we don't get the LD confused about what major
-	 * we are when we get into the dgnc_tty_open() routine.
-	 */
-	brd->print_driver = tty_alloc_driver(brd->maxports,
-					     TTY_DRIVER_REAL_RAW |
-					     TTY_DRIVER_DYNAMIC_DEV |
-					     TTY_DRIVER_HARDWARE_BREAK);
+	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
+	brd->print_driver = __dgnc_tty_register(brd->print_name, brd->maxports,
+						0x80,
+						brd->serial_driver->major);
 	if (IS_ERR(brd->print_driver)) {
 		rc = PTR_ERR(brd->print_driver);
-		goto unregister_serial_driver;
-	}
-
-	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
-
-	brd->print_driver->name = brd->print_name;
-	brd->print_driver->name_base = 0;
-	brd->print_driver->major = brd->serial_driver->major;
-	brd->print_driver->minor_start = 0x80;
-	brd->print_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->print_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->print_driver->init_termios = default_termios;
-	brd->print_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->print_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->print_driver);
-	if (rc < 0) {
 		dev_dbg(&brd->pdev->dev,
-			"Can't register Transparent Print device(%d)\n",
-			rc);
-		goto free_print_driver;
+			"Can't register Transparent Print device(%d)\n", rc);
+		tty_unregister_driver(brd->serial_driver);
+		put_tty_driver(brd->serial_driver);
+		return rc;
 	}
-
 	return 0;
-
-free_print_driver:
-	put_tty_driver(brd->print_driver);
-unregister_serial_driver:
-	tty_unregister_driver(brd->serial_driver);
-free_serial_driver:
-	put_tty_driver(brd->serial_driver);
-
-	return rc;
 }
 
 void dgnc_tty_unregister(struct dgnc_board *brd)
-- 
1.9.1

Re: [PATCH] drivers/staging: refactor dgnc tty registration.

[Dan Carpenter]

On Sun, May 14, 2017 at 06:50:24PM +0300, Haim Daniel wrote:
> @@ -129,6 +113,41 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
>  
>  /* TTY Initialization/Cleanup Functions */
>  
> +static struct tty_driver *__dgnc_tty_register(char *serial_name, uint maxports,

No need for double underscores.

> +					      int major, int minor)
> +{
> +	int rc;
> +	struct tty_driver *drv = tty_alloc_driver(maxports,
> +						  TTY_DRIVER_REAL_RAW |
> +						  TTY_DRIVER_DYNAMIC_DEV |
> +						  TTY_DRIVER_HARDWARE_BREAK);
> +	if (IS_ERR(drv))
> +		return drv;

Blank line between declaration and code.  Do it like this:

	struct tty_driver *drv;
	int rc;

	drv = tty_alloc_driver(maxports, TTY_DRIVER_REAL_RAW |
				         TTY_DRIVER_DYNAMIC_DEV |
					 TTY_DRIVER_HARDWARE_BREAK);
	if (IS_ERR(drv))
		return drv;


Also create a matching dgnc_tty_unregister function that does:

static void dgnc_tty_unregister(struct tty_driver *drv)
{
	tty_unregister_driver(drv);
	put_tty_driver(drv);
}

regards,
dan carpenter

[PATCH v2] drivers/staging: refactor dgnc tty registration.

[Haim Daniel]

-remove duplicate tty allocation code for serial and printer drivers.
-fix sparse warning: too long initializer-string for array of char.

Signed-off-by: Haim Daniel <haimdaniel@gmail.com>
---
 drivers/staging/dgnc/dgnc_driver.h |  13 ----
 drivers/staging/dgnc/dgnc_tty.c    | 150 +++++++++++++++----------------------
 2 files changed, 59 insertions(+), 104 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
index 980410f..764d6fe 100644
--- a/drivers/staging/dgnc/dgnc_driver.h
+++ b/drivers/staging/dgnc/dgnc_driver.h
@@ -52,19 +52,6 @@
 
 #define dgnc_jiffies_from_ms(a) (((a) * HZ) / 1000)
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.  This is the same structure that is defined
- * as the default in tty_io.c with the same settings overridden as in serial.c
- *
- * In short, this should match the internal serial ports' defaults.
- */
-#define	DEFAULT_IFLAGS	(ICRNL | IXON)
-#define	DEFAULT_OFLAGS	(OPOST | ONLCR)
-#define	DEFAULT_CFLAGS	(B9600 | CS8 | CREAD | HUPCL | CLOCAL)
-#define	DEFAULT_LFLAGS	(ISIG | ICANON | ECHO | ECHOE | ECHOK | \
-			ECHOCTL | ECHOKE | IEXTEN)
-
 #ifndef _POSIX_VDISABLE
 #define   _POSIX_VDISABLE '\0'
 #endif
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 9e98781..189a731 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -51,22 +51,6 @@
 	.digi_term =	"ansi"		/* default terminal type */
 };
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.
- *
- * This defines a raw port at 9600 baud, 8 data bits, no parity,
- * 1 stop bit.
- */
-static const struct ktermios default_termios = {
-	.c_iflag =	(DEFAULT_IFLAGS),
-	.c_oflag =	(DEFAULT_OFLAGS),
-	.c_cflag =	(DEFAULT_CFLAGS),
-	.c_lflag =	(DEFAULT_LFLAGS),
-	.c_cc =		INIT_C_CC,
-	.c_line =	0,
-};
-
 static int dgnc_tty_open(struct tty_struct *tty, struct file *file);
 static void dgnc_tty_close(struct tty_struct *tty, struct file *file);
 static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file,
@@ -129,6 +113,49 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
 
 /* TTY Initialization/Cleanup Functions */
 
+static struct tty_driver *_dgnc_tty_register(char *serial_name, uint maxports,
+					     int major, int minor)
+{
+	int rc;
+	struct tty_driver *drv;
+
+	drv = tty_alloc_driver(maxports,
+			       TTY_DRIVER_REAL_RAW |
+			       TTY_DRIVER_DYNAMIC_DEV |
+			       TTY_DRIVER_HARDWARE_BREAK);
+	if (IS_ERR(drv))
+		return drv;
+
+	drv->name = serial_name;
+	drv->name_base = 0;
+	drv->major = major;
+	drv->minor_start = minor;
+	drv->type = TTY_DRIVER_TYPE_SERIAL;
+	drv->subtype = SERIAL_TYPE_NORMAL;
+	drv->init_termios = tty_std_termios;
+	drv->init_termios.c_cflag = (B9600 | CS8 | CREAD | HUPCL | CLOCAL);
+	drv->init_termios.c_ispeed = 9600;
+	drv->init_termios.c_ospeed = 9600;
+	drv->driver_name = DRVSTR;
+	/*
+	 * Entry points for driver.  Called by the kernel from
+	 * tty_io.c and n_tty.c.
+	 */
+	tty_set_operations(drv, &dgnc_tty_ops);
+	rc = tty_register_driver(drv);
+	if (rc < 0) {
+		put_tty_driver(drv);
+		return ERR_PTR(rc);
+	}
+	return drv;
+}
+
+static void _dgnc_tty_unregister(struct tty_driver *drv)
+{
+	tty_unregister_driver(drv);
+	put_tty_driver(drv);
+}
+
 /**
  * dgnc_tty_register() - Init the tty subsystem for this board.
  */
@@ -136,95 +163,36 @@ int dgnc_tty_register(struct dgnc_board *brd)
 {
 	int rc;
 
-	brd->serial_driver = tty_alloc_driver(brd->maxports,
-					      TTY_DRIVER_REAL_RAW |
-					      TTY_DRIVER_DYNAMIC_DEV |
-					      TTY_DRIVER_HARDWARE_BREAK);
-	if (IS_ERR(brd->serial_driver))
-		return PTR_ERR(brd->serial_driver);
-
 	snprintf(brd->serial_name, MAXTTYNAMELEN, "tty_dgnc_%d_",
 		 brd->boardnum);
 
-	brd->serial_driver->name = brd->serial_name;
-	brd->serial_driver->name_base = 0;
-	brd->serial_driver->major = 0;
-	brd->serial_driver->minor_start = 0;
-	brd->serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->serial_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->serial_driver->init_termios = default_termios;
-	brd->serial_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->serial_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->serial_driver);
-	if (rc < 0) {
-		dev_dbg(&brd->pdev->dev,
-			"Can't register tty device (%d)\n", rc);
-		goto free_serial_driver;
+	brd->serial_driver = _dgnc_tty_register(brd->serial_name,
+						brd->maxports, 0, 0);
+	if (IS_ERR(brd->serial_driver)) {
+		rc = PTR_ERR(brd->serial_driver);
+		dev_dbg(&brd->pdev->dev, "Can't register tty device (%d)\n",
+			rc);
+		return rc;
 	}
 
-	/*
-	 * If we're doing transparent print, we have to do all of the above
-	 * again, separately so we don't get the LD confused about what major
-	 * we are when we get into the dgnc_tty_open() routine.
-	 */
-	brd->print_driver = tty_alloc_driver(brd->maxports,
-					     TTY_DRIVER_REAL_RAW |
-					     TTY_DRIVER_DYNAMIC_DEV |
-					     TTY_DRIVER_HARDWARE_BREAK);
+	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
+	brd->print_driver = _dgnc_tty_register(brd->print_name, brd->maxports,
+					       0x80,
+					       brd->serial_driver->major);
 	if (IS_ERR(brd->print_driver)) {
 		rc = PTR_ERR(brd->print_driver);
-		goto unregister_serial_driver;
-	}
-
-	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
-
-	brd->print_driver->name = brd->print_name;
-	brd->print_driver->name_base = 0;
-	brd->print_driver->major = brd->serial_driver->major;
-	brd->print_driver->minor_start = 0x80;
-	brd->print_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->print_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->print_driver->init_termios = default_termios;
-	brd->print_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->print_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->print_driver);
-	if (rc < 0) {
 		dev_dbg(&brd->pdev->dev,
-			"Can't register Transparent Print device(%d)\n",
-			rc);
-		goto free_print_driver;
+			"Can't register Transparent Print device(%d)\n", rc);
+		_dgnc_tty_unregister(brd->serial_driver);
+		return rc;
 	}
-
 	return 0;
-
-free_print_driver:
-	put_tty_driver(brd->print_driver);
-unregister_serial_driver:
-	tty_unregister_driver(brd->serial_driver);
-free_serial_driver:
-	put_tty_driver(brd->serial_driver);
-
-	return rc;
 }
 
 void dgnc_tty_unregister(struct dgnc_board *brd)
 {
-	tty_unregister_driver(brd->print_driver);
-	tty_unregister_driver(brd->serial_driver);
-	put_tty_driver(brd->print_driver);
-	put_tty_driver(brd->serial_driver);
+	_dgnc_tty_unregister(brd->print_driver);
+	_dgnc_tty_unregister(brd->serial_driver);
 }
 
 /**
-- 
1.9.1

Re: [PATCH v2] drivers/staging: refactor dgnc tty registration.

[Dan Carpenter]

On Mon, May 15, 2017 at 02:48:52PM +0300, Haim Daniel wrote:
> +static struct tty_driver *_dgnc_tty_register(char *serial_name, uint maxports,


Ahh...  I see why you are adding underscores.  Please, just get a better
name instead of taking an existing name and adding underscores.

regards,
dan carpenter

[PATCH v3] drivers/staging: refactor dgnc tty registration.

[Haim Daniel]

-remove duplicate tty allocation code for serial and printer drivers.
-fix sparse warning: too long initializer-string for array of char.

Signed-off-by: Haim Daniel <haimdaniel@gmail.com>
---
 drivers/staging/dgnc/dgnc_driver.h |  13 ----
 drivers/staging/dgnc/dgnc_tty.c    | 150 +++++++++++++++----------------------
 2 files changed, 59 insertions(+), 104 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
index 980410f..764d6fe 100644
--- a/drivers/staging/dgnc/dgnc_driver.h
+++ b/drivers/staging/dgnc/dgnc_driver.h
@@ -52,19 +52,6 @@
 
 #define dgnc_jiffies_from_ms(a) (((a) * HZ) / 1000)
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.  This is the same structure that is defined
- * as the default in tty_io.c with the same settings overridden as in serial.c
- *
- * In short, this should match the internal serial ports' defaults.
- */
-#define	DEFAULT_IFLAGS	(ICRNL | IXON)
-#define	DEFAULT_OFLAGS	(OPOST | ONLCR)
-#define	DEFAULT_CFLAGS	(B9600 | CS8 | CREAD | HUPCL | CLOCAL)
-#define	DEFAULT_LFLAGS	(ISIG | ICANON | ECHO | ECHOE | ECHOK | \
-			ECHOCTL | ECHOKE | IEXTEN)
-
 #ifndef _POSIX_VDISABLE
 #define   _POSIX_VDISABLE '\0'
 #endif
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 9e98781..d3736da 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -51,22 +51,6 @@
 	.digi_term =	"ansi"		/* default terminal type */
 };
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.
- *
- * This defines a raw port at 9600 baud, 8 data bits, no parity,
- * 1 stop bit.
- */
-static const struct ktermios default_termios = {
-	.c_iflag =	(DEFAULT_IFLAGS),
-	.c_oflag =	(DEFAULT_OFLAGS),
-	.c_cflag =	(DEFAULT_CFLAGS),
-	.c_lflag =	(DEFAULT_LFLAGS),
-	.c_cc =		INIT_C_CC,
-	.c_line =	0,
-};
-
 static int dgnc_tty_open(struct tty_struct *tty, struct file *file);
 static void dgnc_tty_close(struct tty_struct *tty, struct file *file);
 static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file,
@@ -129,6 +113,49 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
 
 /* TTY Initialization/Cleanup Functions */
 
+static struct tty_driver *dgnc_tty_create(char *serial_name, uint maxports,
+					  int major, int minor)
+{
+	int rc;
+	struct tty_driver *drv;
+
+	drv = tty_alloc_driver(maxports,
+			       TTY_DRIVER_REAL_RAW |
+			       TTY_DRIVER_DYNAMIC_DEV |
+			       TTY_DRIVER_HARDWARE_BREAK);
+	if (IS_ERR(drv))
+		return drv;
+
+	drv->name = serial_name;
+	drv->name_base = 0;
+	drv->major = major;
+	drv->minor_start = minor;
+	drv->type = TTY_DRIVER_TYPE_SERIAL;
+	drv->subtype = SERIAL_TYPE_NORMAL;
+	drv->init_termios = tty_std_termios;
+	drv->init_termios.c_cflag = (B9600 | CS8 | CREAD | HUPCL | CLOCAL);
+	drv->init_termios.c_ispeed = 9600;
+	drv->init_termios.c_ospeed = 9600;
+	drv->driver_name = DRVSTR;
+	/*
+	 * Entry points for driver.  Called by the kernel from
+	 * tty_io.c and n_tty.c.
+	 */
+	tty_set_operations(drv, &dgnc_tty_ops);
+	rc = tty_register_driver(drv);
+	if (rc < 0) {
+		put_tty_driver(drv);
+		return ERR_PTR(rc);
+	}
+	return drv;
+}
+
+static void dgnc_tty_free(struct tty_driver *drv)
+{
+	tty_unregister_driver(drv);
+	put_tty_driver(drv);
+}
+
 /**
  * dgnc_tty_register() - Init the tty subsystem for this board.
  */
@@ -136,95 +163,36 @@ int dgnc_tty_register(struct dgnc_board *brd)
 {
 	int rc;
 
-	brd->serial_driver = tty_alloc_driver(brd->maxports,
-					      TTY_DRIVER_REAL_RAW |
-					      TTY_DRIVER_DYNAMIC_DEV |
-					      TTY_DRIVER_HARDWARE_BREAK);
-	if (IS_ERR(brd->serial_driver))
-		return PTR_ERR(brd->serial_driver);
-
 	snprintf(brd->serial_name, MAXTTYNAMELEN, "tty_dgnc_%d_",
 		 brd->boardnum);
 
-	brd->serial_driver->name = brd->serial_name;
-	brd->serial_driver->name_base = 0;
-	brd->serial_driver->major = 0;
-	brd->serial_driver->minor_start = 0;
-	brd->serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->serial_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->serial_driver->init_termios = default_termios;
-	brd->serial_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->serial_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->serial_driver);
-	if (rc < 0) {
-		dev_dbg(&brd->pdev->dev,
-			"Can't register tty device (%d)\n", rc);
-		goto free_serial_driver;
+	brd->serial_driver = dgnc_tty_create(brd->serial_name,
+					     brd->maxports, 0, 0);
+	if (IS_ERR(brd->serial_driver)) {
+		rc = PTR_ERR(brd->serial_driver);
+		dev_dbg(&brd->pdev->dev, "Can't register tty device (%d)\n",
+			rc);
+		return rc;
 	}
 
-	/*
-	 * If we're doing transparent print, we have to do all of the above
-	 * again, separately so we don't get the LD confused about what major
-	 * we are when we get into the dgnc_tty_open() routine.
-	 */
-	brd->print_driver = tty_alloc_driver(brd->maxports,
-					     TTY_DRIVER_REAL_RAW |
-					     TTY_DRIVER_DYNAMIC_DEV |
-					     TTY_DRIVER_HARDWARE_BREAK);
+	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
+	brd->print_driver = dgnc_tty_create(brd->print_name, brd->maxports,
+					    0x80,
+					    brd->serial_driver->major);
 	if (IS_ERR(brd->print_driver)) {
 		rc = PTR_ERR(brd->print_driver);
-		goto unregister_serial_driver;
-	}
-
-	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
-
-	brd->print_driver->name = brd->print_name;
-	brd->print_driver->name_base = 0;
-	brd->print_driver->major = brd->serial_driver->major;
-	brd->print_driver->minor_start = 0x80;
-	brd->print_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->print_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->print_driver->init_termios = default_termios;
-	brd->print_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->print_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->print_driver);
-	if (rc < 0) {
 		dev_dbg(&brd->pdev->dev,
-			"Can't register Transparent Print device(%d)\n",
-			rc);
-		goto free_print_driver;
+			"Can't register Transparent Print device(%d)\n", rc);
+		dgnc_tty_free(brd->serial_driver);
+		return rc;
 	}
-
 	return 0;
-
-free_print_driver:
-	put_tty_driver(brd->print_driver);
-unregister_serial_driver:
-	tty_unregister_driver(brd->serial_driver);
-free_serial_driver:
-	put_tty_driver(brd->serial_driver);
-
-	return rc;
 }
 
 void dgnc_tty_unregister(struct dgnc_board *brd)
 {
-	tty_unregister_driver(brd->print_driver);
-	tty_unregister_driver(brd->serial_driver);
-	put_tty_driver(brd->print_driver);
-	put_tty_driver(brd->serial_driver);
+	dgnc_tty_free(brd->print_driver);
+	dgnc_tty_free(brd->serial_driver);
 }
 
 /**
-- 
1.9.1

Re: [PATCH v3] drivers/staging: refactor dgnc tty registration.

[Dan Carpenter]

On Mon, May 15, 2017 at 03:30:50PM +0300, Haim Daniel wrote:
> -remove duplicate tty allocation code for serial and printer drivers.
> -fix sparse warning: too long initializer-string for array of char.

I think my version of Sparse is too old.  I don't get a warning.  Please
cut and paste the exact warning.

> 
> Signed-off-by: Haim Daniel <haimdaniel@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_driver.h |  13 ----
>  drivers/staging/dgnc/dgnc_tty.c    | 150 +++++++++++++++----------------------
>  2 files changed, 59 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
> index 980410f..764d6fe 100644
> --- a/drivers/staging/dgnc/dgnc_driver.h
> +++ b/drivers/staging/dgnc/dgnc_driver.h
> @@ -52,19 +52,6 @@
>  
>  #define dgnc_jiffies_from_ms(a) (((a) * HZ) / 1000)
>  
> -/*
> - * Define a local default termios struct. All ports will be created
> - * with this termios initially.  This is the same structure that is defined
> - * as the default in tty_io.c with the same settings overridden as in serial.c
> - *
> - * In short, this should match the internal serial ports' defaults.
> - */
> -#define	DEFAULT_IFLAGS	(ICRNL | IXON)
> -#define	DEFAULT_OFLAGS	(OPOST | ONLCR)
> -#define	DEFAULT_CFLAGS	(B9600 | CS8 | CREAD | HUPCL | CLOCAL)
> -#define	DEFAULT_LFLAGS	(ISIG | ICANON | ECHO | ECHOE | ECHOK | \
> -			ECHOCTL | ECHOKE | IEXTEN)
> -
>  #ifndef _POSIX_VDISABLE
>  #define   _POSIX_VDISABLE '\0'
>  #endif
> diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
> index 9e98781..d3736da 100644
> --- a/drivers/staging/dgnc/dgnc_tty.c
> +++ b/drivers/staging/dgnc/dgnc_tty.c
> @@ -51,22 +51,6 @@
>  	.digi_term =	"ansi"		/* default terminal type */
>  };
>  
> -/*
> - * Define a local default termios struct. All ports will be created
> - * with this termios initially.
> - *
> - * This defines a raw port at 9600 baud, 8 data bits, no parity,
> - * 1 stop bit.
> - */
> -static const struct ktermios default_termios = {
> -	.c_iflag =	(DEFAULT_IFLAGS),
> -	.c_oflag =	(DEFAULT_OFLAGS),
> -	.c_cflag =	(DEFAULT_CFLAGS),
> -	.c_lflag =	(DEFAULT_LFLAGS),
> -	.c_cc =		INIT_C_CC,

We don't need INIT_C_CC?

> -	.c_line =	0,
> -};
> -
>  static int dgnc_tty_open(struct tty_struct *tty, struct file *file);
>  static void dgnc_tty_close(struct tty_struct *tty, struct file *file);
>  static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file,
> @@ -129,6 +113,49 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
>  
>  /* TTY Initialization/Cleanup Functions */
>  
> +static struct tty_driver *dgnc_tty_create(char *serial_name, uint maxports,
> +					  int major, int minor)
> +{
> +	int rc;
> +	struct tty_driver *drv;
> +
> +	drv = tty_alloc_driver(maxports,
> +			       TTY_DRIVER_REAL_RAW |
> +			       TTY_DRIVER_DYNAMIC_DEV |
> +			       TTY_DRIVER_HARDWARE_BREAK);
> +	if (IS_ERR(drv))
> +		return drv;
> +
> +	drv->name = serial_name;
> +	drv->name_base = 0;
> +	drv->major = major;
> +	drv->minor_start = minor;
> +	drv->type = TTY_DRIVER_TYPE_SERIAL;
> +	drv->subtype = SERIAL_TYPE_NORMAL;
> +	drv->init_termios = tty_std_termios;
> +	drv->init_termios.c_cflag = (B9600 | CS8 | CREAD | HUPCL | CLOCAL);
> +	drv->init_termios.c_ispeed = 9600;
> +	drv->init_termios.c_ospeed = 9600;

Setting c_ispeed and c_ospeed is almost certainly correct, but they're
not mentioned in the changelog.

Btw, the overwhelming vast majority of staging drivers are sent without
testing.  But it looks like maybe you are testing yours?  Please mention
that in the changelog because it makes us feel warm inside.

Otherwise it looks fine to me.  Sorry for making you redo it so many
times instead of reviewing thouroughly all at once.  I shouldn't have
been so lazy.

I still feel a bit bad about my review that I didn't spot bug Sparse
saw...

regards,
dan carpenter

Re: [PATCH v3] drivers/staging: refactor dgnc tty registration.

[Haim Daniel]

me@haim-toshiba1 ~ $ dpkg -l sparse
Desired=Unknown/Install/Remove/Purge/Hold
| 
Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name              Version       Architecture  Description
+++-=================-=============-=============-========================================
ii  sparse            0.4.5~rc1-1   amd64         semantic parser of 
source files

me@haim-toshiba1 linux (next-20170512) $ make clean M=drivers/staging 
/dgnc/; make C=1 M=drivers/staging/dgnc/

drivers/staging/dgnc/dgnc_tty.c:66:25: warning: too long 
initializer-string for array of char

On 05/15/2017 03:52 PM, Dan Carpenter wrote:
> On Mon, May 15, 2017 at 03:30:50PM +0300, Haim Daniel wrote:
>> -remove duplicate tty allocation code for serial and printer drivers.
>> -fix sparse warning: too long initializer-string for array of char.
>
> I think my version of Sparse is too old.  I don't get a warning.  Please
> cut and paste the exact warning.
>
>>
>> Signed-off-by: Haim Daniel <haimdaniel@gmail.com>
>> ---
>>  drivers/staging/dgnc/dgnc_driver.h |  13 ----
>>  drivers/staging/dgnc/dgnc_tty.c    | 150 +++++++++++++++----------------------
>>  2 files changed, 59 insertions(+), 104 deletions(-)
>>
>> diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
>> index 980410f..764d6fe 100644
>> --- a/drivers/staging/dgnc/dgnc_driver.h
>> +++ b/drivers/staging/dgnc/dgnc_driver.h
>> @@ -52,19 +52,6 @@
>>
>>  #define dgnc_jiffies_from_ms(a) (((a) * HZ) / 1000)
>>
>> -/*
>> - * Define a local default termios struct. All ports will be created
>> - * with this termios initially.  This is the same structure that is defined
>> - * as the default in tty_io.c with the same settings overridden as in serial.c
>> - *
>> - * In short, this should match the internal serial ports' defaults.
>> - */
>> -#define	DEFAULT_IFLAGS	(ICRNL | IXON)
>> -#define	DEFAULT_OFLAGS	(OPOST | ONLCR)
>> -#define	DEFAULT_CFLAGS	(B9600 | CS8 | CREAD | HUPCL | CLOCAL)
>> -#define	DEFAULT_LFLAGS	(ISIG | ICANON | ECHO | ECHOE | ECHOK | \
>> -			ECHOCTL | ECHOKE | IEXTEN)
>> -
>>  #ifndef _POSIX_VDISABLE
>>  #define   _POSIX_VDISABLE '\0'
>>  #endif
>> diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
>> index 9e98781..d3736da 100644
>> --- a/drivers/staging/dgnc/dgnc_tty.c
>> +++ b/drivers/staging/dgnc/dgnc_tty.c
>> @@ -51,22 +51,6 @@
>>  	.digi_term =	"ansi"		/* default terminal type */
>>  };
>>
>> -/*
>> - * Define a local default termios struct. All ports will be created
>> - * with this termios initially.
>> - *
>> - * This defines a raw port at 9600 baud, 8 data bits, no parity,
>> - * 1 stop bit.
>> - */
>> -static const struct ktermios default_termios = {
>> -	.c_iflag =	(DEFAULT_IFLAGS),
>> -	.c_oflag =	(DEFAULT_OFLAGS),
>> -	.c_cflag =	(DEFAULT_CFLAGS),
>> -	.c_lflag =	(DEFAULT_LFLAGS),
>> -	.c_cc =		INIT_C_CC,
>
> We don't need INIT_C_CC?
No, the idea was reusing the tty_std_termios, and setting only the 
relevant params.

>
>> -	.c_line =	0,
>> -};
>> -
>>  static int dgnc_tty_open(struct tty_struct *tty, struct file *file);
>>  static void dgnc_tty_close(struct tty_struct *tty, struct file *file);
>>  static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file,
>> @@ -129,6 +113,49 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
>>
>>  /* TTY Initialization/Cleanup Functions */
>>
>> +static struct tty_driver *dgnc_tty_create(char *serial_name, uint maxports,
>> +					  int major, int minor)
>> +{
>> +	int rc;
>> +	struct tty_driver *drv;
>> +
>> +	drv = tty_alloc_driver(maxports,
>> +			       TTY_DRIVER_REAL_RAW |
>> +			       TTY_DRIVER_DYNAMIC_DEV |
>> +			       TTY_DRIVER_HARDWARE_BREAK);
>> +	if (IS_ERR(drv))
>> +		return drv;
>> +
>> +	drv->name = serial_name;
>> +	drv->name_base = 0;
>> +	drv->major = major;
>> +	drv->minor_start = minor;
>> +	drv->type = TTY_DRIVER_TYPE_SERIAL;
>> +	drv->subtype = SERIAL_TYPE_NORMAL;
>> +	drv->init_termios = tty_std_termios;
>> +	drv->init_termios.c_cflag = (B9600 | CS8 | CREAD | HUPCL | CLOCAL);
>> +	drv->init_termios.c_ispeed = 9600;
>> +	drv->init_termios.c_ospeed = 9600;
>
> Setting c_ispeed and c_ospeed is almost certainly correct, but they're
> not mentioned in the changelog.
Ack, will do.
> Btw, the overwhelming vast majority of staging drivers are sent without
> testing.  But it looks like maybe you are testing yours?  Please mention
> that in the changelog because it makes us feel warm inside.
I only unit tested a fraction of the code, will mention it in the 
changelog, for the sake of that fuzzy feeling :)

> Otherwise it looks fine to me.  Sorry for making you redo it so many
> times instead of reviewing thouroughly all at once.  I shouldn't have
> been so lazy.
np.

> I still feel a bit bad about my review that I didn't spot bug Sparse
> saw...
>
> regards,
> dan carpenter
>
>

[PATCH v4] drivers/staging: refactor dgnc tty registration.

[Haim Daniel]

-remove duplicate tty allocation code for serial and printer drivers.
-add missing tty c_ispeed and c_ospeed initialization to 9600.
-fix sparse warning: too long initializer-string for array of char.

This patch was only unit tested due to lack of the actual hardware.

Signed-off-by: Haim Daniel <haimdaniel@gmail.com>
---
 drivers/staging/dgnc/dgnc_driver.h |  13 ----
 drivers/staging/dgnc/dgnc_tty.c    | 150 +++++++++++++++----------------------
 2 files changed, 59 insertions(+), 104 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
index 980410f..764d6fe 100644
--- a/drivers/staging/dgnc/dgnc_driver.h
+++ b/drivers/staging/dgnc/dgnc_driver.h
@@ -52,19 +52,6 @@
 
 #define dgnc_jiffies_from_ms(a) (((a) * HZ) / 1000)
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.  This is the same structure that is defined
- * as the default in tty_io.c with the same settings overridden as in serial.c
- *
- * In short, this should match the internal serial ports' defaults.
- */
-#define	DEFAULT_IFLAGS	(ICRNL | IXON)
-#define	DEFAULT_OFLAGS	(OPOST | ONLCR)
-#define	DEFAULT_CFLAGS	(B9600 | CS8 | CREAD | HUPCL | CLOCAL)
-#define	DEFAULT_LFLAGS	(ISIG | ICANON | ECHO | ECHOE | ECHOK | \
-			ECHOCTL | ECHOKE | IEXTEN)
-
 #ifndef _POSIX_VDISABLE
 #define   _POSIX_VDISABLE '\0'
 #endif
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 9e98781..d3736da 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -51,22 +51,6 @@
 	.digi_term =	"ansi"		/* default terminal type */
 };
 
-/*
- * Define a local default termios struct. All ports will be created
- * with this termios initially.
- *
- * This defines a raw port at 9600 baud, 8 data bits, no parity,
- * 1 stop bit.
- */
-static const struct ktermios default_termios = {
-	.c_iflag =	(DEFAULT_IFLAGS),
-	.c_oflag =	(DEFAULT_OFLAGS),
-	.c_cflag =	(DEFAULT_CFLAGS),
-	.c_lflag =	(DEFAULT_LFLAGS),
-	.c_cc =		INIT_C_CC,
-	.c_line =	0,
-};
-
 static int dgnc_tty_open(struct tty_struct *tty, struct file *file);
 static void dgnc_tty_close(struct tty_struct *tty, struct file *file);
 static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file,
@@ -129,6 +113,49 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
 
 /* TTY Initialization/Cleanup Functions */
 
+static struct tty_driver *dgnc_tty_create(char *serial_name, uint maxports,
+					  int major, int minor)
+{
+	int rc;
+	struct tty_driver *drv;
+
+	drv = tty_alloc_driver(maxports,
+			       TTY_DRIVER_REAL_RAW |
+			       TTY_DRIVER_DYNAMIC_DEV |
+			       TTY_DRIVER_HARDWARE_BREAK);
+	if (IS_ERR(drv))
+		return drv;
+
+	drv->name = serial_name;
+	drv->name_base = 0;
+	drv->major = major;
+	drv->minor_start = minor;
+	drv->type = TTY_DRIVER_TYPE_SERIAL;
+	drv->subtype = SERIAL_TYPE_NORMAL;
+	drv->init_termios = tty_std_termios;
+	drv->init_termios.c_cflag = (B9600 | CS8 | CREAD | HUPCL | CLOCAL);
+	drv->init_termios.c_ispeed = 9600;
+	drv->init_termios.c_ospeed = 9600;
+	drv->driver_name = DRVSTR;
+	/*
+	 * Entry points for driver.  Called by the kernel from
+	 * tty_io.c and n_tty.c.
+	 */
+	tty_set_operations(drv, &dgnc_tty_ops);
+	rc = tty_register_driver(drv);
+	if (rc < 0) {
+		put_tty_driver(drv);
+		return ERR_PTR(rc);
+	}
+	return drv;
+}
+
+static void dgnc_tty_free(struct tty_driver *drv)
+{
+	tty_unregister_driver(drv);
+	put_tty_driver(drv);
+}
+
 /**
  * dgnc_tty_register() - Init the tty subsystem for this board.
  */
@@ -136,95 +163,36 @@ int dgnc_tty_register(struct dgnc_board *brd)
 {
 	int rc;
 
-	brd->serial_driver = tty_alloc_driver(brd->maxports,
-					      TTY_DRIVER_REAL_RAW |
-					      TTY_DRIVER_DYNAMIC_DEV |
-					      TTY_DRIVER_HARDWARE_BREAK);
-	if (IS_ERR(brd->serial_driver))
-		return PTR_ERR(brd->serial_driver);
-
 	snprintf(brd->serial_name, MAXTTYNAMELEN, "tty_dgnc_%d_",
 		 brd->boardnum);
 
-	brd->serial_driver->name = brd->serial_name;
-	brd->serial_driver->name_base = 0;
-	brd->serial_driver->major = 0;
-	brd->serial_driver->minor_start = 0;
-	brd->serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->serial_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->serial_driver->init_termios = default_termios;
-	brd->serial_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->serial_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->serial_driver);
-	if (rc < 0) {
-		dev_dbg(&brd->pdev->dev,
-			"Can't register tty device (%d)\n", rc);
-		goto free_serial_driver;
+	brd->serial_driver = dgnc_tty_create(brd->serial_name,
+					     brd->maxports, 0, 0);
+	if (IS_ERR(brd->serial_driver)) {
+		rc = PTR_ERR(brd->serial_driver);
+		dev_dbg(&brd->pdev->dev, "Can't register tty device (%d)\n",
+			rc);
+		return rc;
 	}
 
-	/*
-	 * If we're doing transparent print, we have to do all of the above
-	 * again, separately so we don't get the LD confused about what major
-	 * we are when we get into the dgnc_tty_open() routine.
-	 */
-	brd->print_driver = tty_alloc_driver(brd->maxports,
-					     TTY_DRIVER_REAL_RAW |
-					     TTY_DRIVER_DYNAMIC_DEV |
-					     TTY_DRIVER_HARDWARE_BREAK);
+	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
+	brd->print_driver = dgnc_tty_create(brd->print_name, brd->maxports,
+					    0x80,
+					    brd->serial_driver->major);
 	if (IS_ERR(brd->print_driver)) {
 		rc = PTR_ERR(brd->print_driver);
-		goto unregister_serial_driver;
-	}
-
-	snprintf(brd->print_name, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);
-
-	brd->print_driver->name = brd->print_name;
-	brd->print_driver->name_base = 0;
-	brd->print_driver->major = brd->serial_driver->major;
-	brd->print_driver->minor_start = 0x80;
-	brd->print_driver->type = TTY_DRIVER_TYPE_SERIAL;
-	brd->print_driver->subtype = SERIAL_TYPE_NORMAL;
-	brd->print_driver->init_termios = default_termios;
-	brd->print_driver->driver_name = DRVSTR;
-
-	/*
-	 * Entry points for driver.  Called by the kernel from
-	 * tty_io.c and n_tty.c.
-	 */
-	tty_set_operations(brd->print_driver, &dgnc_tty_ops);
-
-	rc = tty_register_driver(brd->print_driver);
-	if (rc < 0) {
 		dev_dbg(&brd->pdev->dev,
-			"Can't register Transparent Print device(%d)\n",
-			rc);
-		goto free_print_driver;
+			"Can't register Transparent Print device(%d)\n", rc);
+		dgnc_tty_free(brd->serial_driver);
+		return rc;
 	}
-
 	return 0;
-
-free_print_driver:
-	put_tty_driver(brd->print_driver);
-unregister_serial_driver:
-	tty_unregister_driver(brd->serial_driver);
-free_serial_driver:
-	put_tty_driver(brd->serial_driver);
-
-	return rc;
 }
 
 void dgnc_tty_unregister(struct dgnc_board *brd)
 {
-	tty_unregister_driver(brd->print_driver);
-	tty_unregister_driver(brd->serial_driver);
-	put_tty_driver(brd->print_driver);
-	put_tty_driver(brd->serial_driver);
+	dgnc_tty_free(brd->print_driver);
+	dgnc_tty_free(brd->serial_driver);
 }
 
 /**
-- 
1.9.1

Re: [PATCH v3] drivers/staging: refactor dgnc tty registration.

[Dan Carpenter]

On Mon, May 15, 2017 at 04:49:16PM +0300, Haim Daniel wrote:
> me@haim-toshiba1 ~ $ dpkg -l sparse
> Desired=Unknown/Install/Remove/Purge/Hold
> |
> Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
> |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
> ||/ Name              Version       Architecture  Description
> +++-=================-=============-=============-========================================
> ii  sparse            0.4.5~rc1-1   amd64         semantic parser of source
> files
> 
> me@haim-toshiba1 linux (next-20170512) $ make clean M=drivers/staging
> /dgnc/; make C=1 M=drivers/staging/dgnc/
> 
> drivers/staging/dgnc/dgnc_tty.c:66:25: warning: too long initializer-string
> for array of char
> 

I downloaded the latest Sparse and don't see that warning.  But GCC has
a warning like that so I think this is really a GCC warning.  It doesn't
trigger for me, and with my config it's in fact, not a bug.  The buffer
is 19 characters and INIT_C_CC is 17 chars plus 1 NUL terminator.

All these things are generic termios defines so I'm not even sure how
you're triggering the warning.

Anyway, your patch is harmless enough.

regards,
dan carpenter

Re: [PATCH v4] drivers/staging: refactor dgnc tty registration.

[Dan Carpenter]

On Mon, May 15, 2017 at 05:10:18PM +0300, Haim Daniel wrote:
> -remove duplicate tty allocation code for serial and printer drivers.
> -add missing tty c_ispeed and c_ospeed initialization to 9600.
> -fix sparse warning: too long initializer-string for array of char.

This is a GCC warning and not a Sparse warning.  I still have no idea
how it's getting triggered.  But otherwise I have reviewed the patch and
it's a nice clean up.

regards,
dan carpenter