All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve
@ 2016-08-19  1:05 ` Daeseok Youn
  0 siblings, 0 replies; 6+ messages in thread
From: Daeseok Youn @ 2016-08-19  1:05 UTC (permalink / raw)
  To: lidza.louina
  Cc: markh, gregkh, driverdev-devel, devel, linux-kernel, kernel-janitors

The whole code in dgnc_maxcps_room() function surrounds with
one if-statement for checking channel's maxcps and buffer size.
I tried to separate the logic for this function from if-condition.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
 drivers/staging/dgnc/dgnc_tty.c | 46 +++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 31b18e6..cb31b83 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1494,30 +1494,32 @@ static int dgnc_tty_chars_in_buffer(struct tty_struct *tty)
  */
 static int dgnc_maxcps_room(struct channel_t *ch, int bytes_available)
 {
-	if (ch->ch_digi.digi_maxcps > 0 && ch->ch_digi.digi_bufsize > 0) {
-		int cps_limit = 0;
-		unsigned long current_time = jiffies;
-		unsigned long buffer_time = current_time +
-			(HZ * ch->ch_digi.digi_bufsize) /
-			ch->ch_digi.digi_maxcps;
-
-		if (ch->ch_cpstime < current_time) {
-			/* buffer is empty */
-			ch->ch_cpstime = current_time;	/* reset ch_cpstime */
-			cps_limit = ch->ch_digi.digi_bufsize;
-		} else if (ch->ch_cpstime < buffer_time) {
-			/* still room in the buffer */
-			cps_limit = ((buffer_time - ch->ch_cpstime) *
-					ch->ch_digi.digi_maxcps) / HZ;
-		} else {
-			/* no room in the buffer */
-			cps_limit = 0;
-		}
-
-		bytes_available = min(cps_limit, bytes_available);
+	int cps_limit;
+	unsigned long current_time;
+	unsigned long buffer_time;
+
+	if (ch->ch_digi.digi_maxcps <= 0 ||
+	    ch->ch_digi.digi_bufsize <= 0)
+		return bytes_available;
+
+	current_time = jiffies;
+	buffer_time = current_time + (HZ * ch->ch_digi.digi_bufsize) /
+		      ch->ch_digi.digi_maxcps;
+
+	if (ch->ch_cpstime < current_time) {
+		/* buffer is empty */
+		ch->ch_cpstime = current_time;	/* reset ch_cpstime */
+		cps_limit = ch->ch_digi.digi_bufsize;
+	} else if (ch->ch_cpstime < buffer_time) {
+		/* still room in the buffer */
+		cps_limit = ((buffer_time - ch->ch_cpstime) *
+				ch->ch_digi.digi_maxcps) / HZ;
+	} else {
+		/* no room in the buffer */
+		cps_limit = 0;
 	}
 
-	return bytes_available;
+	return min(cps_limit, bytes_available);
 }
 
 /*
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve
@ 2016-08-19  1:05 ` Daeseok Youn
  0 siblings, 0 replies; 6+ messages in thread
From: Daeseok Youn @ 2016-08-19  1:05 UTC (permalink / raw)
  To: lidza.louina
  Cc: devel, gregkh, driverdev-devel, kernel-janitors, linux-kernel

The whole code in dgnc_maxcps_room() function surrounds with
one if-statement for checking channel's maxcps and buffer size.
I tried to separate the logic for this function from if-condition.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
 drivers/staging/dgnc/dgnc_tty.c | 46 +++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 31b18e6..cb31b83 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1494,30 +1494,32 @@ static int dgnc_tty_chars_in_buffer(struct tty_struct *tty)
  */
 static int dgnc_maxcps_room(struct channel_t *ch, int bytes_available)
 {
-	if (ch->ch_digi.digi_maxcps > 0 && ch->ch_digi.digi_bufsize > 0) {
-		int cps_limit = 0;
-		unsigned long current_time = jiffies;
-		unsigned long buffer_time = current_time +
-			(HZ * ch->ch_digi.digi_bufsize) /
-			ch->ch_digi.digi_maxcps;
-
-		if (ch->ch_cpstime < current_time) {
-			/* buffer is empty */
-			ch->ch_cpstime = current_time;	/* reset ch_cpstime */
-			cps_limit = ch->ch_digi.digi_bufsize;
-		} else if (ch->ch_cpstime < buffer_time) {
-			/* still room in the buffer */
-			cps_limit = ((buffer_time - ch->ch_cpstime) *
-					ch->ch_digi.digi_maxcps) / HZ;
-		} else {
-			/* no room in the buffer */
-			cps_limit = 0;
-		}
-
-		bytes_available = min(cps_limit, bytes_available);
+	int cps_limit;
+	unsigned long current_time;
+	unsigned long buffer_time;
+
+	if (ch->ch_digi.digi_maxcps <= 0 ||
+	    ch->ch_digi.digi_bufsize <= 0)
+		return bytes_available;
+
+	current_time = jiffies;
+	buffer_time = current_time + (HZ * ch->ch_digi.digi_bufsize) /
+		      ch->ch_digi.digi_maxcps;
+
+	if (ch->ch_cpstime < current_time) {
+		/* buffer is empty */
+		ch->ch_cpstime = current_time;	/* reset ch_cpstime */
+		cps_limit = ch->ch_digi.digi_bufsize;
+	} else if (ch->ch_cpstime < buffer_time) {
+		/* still room in the buffer */
+		cps_limit = ((buffer_time - ch->ch_cpstime) *
+				ch->ch_digi.digi_maxcps) / HZ;
+	} else {
+		/* no room in the buffer */
+		cps_limit = 0;
 	}
 
-	return bytes_available;
+	return min(cps_limit, bytes_available);
 }
 
 /*
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve
@ 2016-08-19  1:05 ` Daeseok Youn
  0 siblings, 0 replies; 6+ messages in thread
From: Daeseok Youn @ 2016-08-19  1:05 UTC (permalink / raw)
  To: lidza.louina
  Cc: devel, gregkh, driverdev-devel, kernel-janitors, linux-kernel

The whole code in dgnc_maxcps_room() function surrounds with
one if-statement for checking channel's maxcps and buffer size.
I tried to separate the logic for this function from if-condition.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
 drivers/staging/dgnc/dgnc_tty.c | 46 +++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 31b18e6..cb31b83 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1494,30 +1494,32 @@ static int dgnc_tty_chars_in_buffer(struct tty_struct *tty)
  */
 static int dgnc_maxcps_room(struct channel_t *ch, int bytes_available)
 {
-	if (ch->ch_digi.digi_maxcps > 0 && ch->ch_digi.digi_bufsize > 0) {
-		int cps_limit = 0;
-		unsigned long current_time = jiffies;
-		unsigned long buffer_time = current_time +
-			(HZ * ch->ch_digi.digi_bufsize) /
-			ch->ch_digi.digi_maxcps;
-
-		if (ch->ch_cpstime < current_time) {
-			/* buffer is empty */
-			ch->ch_cpstime = current_time;	/* reset ch_cpstime */
-			cps_limit = ch->ch_digi.digi_bufsize;
-		} else if (ch->ch_cpstime < buffer_time) {
-			/* still room in the buffer */
-			cps_limit = ((buffer_time - ch->ch_cpstime) *
-					ch->ch_digi.digi_maxcps) / HZ;
-		} else {
-			/* no room in the buffer */
-			cps_limit = 0;
-		}
-
-		bytes_available = min(cps_limit, bytes_available);
+	int cps_limit;
+	unsigned long current_time;
+	unsigned long buffer_time;
+
+	if (ch->ch_digi.digi_maxcps <= 0 ||
+	    ch->ch_digi.digi_bufsize <= 0)
+		return bytes_available;
+
+	current_time = jiffies;
+	buffer_time = current_time + (HZ * ch->ch_digi.digi_bufsize) /
+		      ch->ch_digi.digi_maxcps;
+
+	if (ch->ch_cpstime < current_time) {
+		/* buffer is empty */
+		ch->ch_cpstime = current_time;	/* reset ch_cpstime */
+		cps_limit = ch->ch_digi.digi_bufsize;
+	} else if (ch->ch_cpstime < buffer_time) {
+		/* still room in the buffer */
+		cps_limit = ((buffer_time - ch->ch_cpstime) *
+				ch->ch_digi.digi_maxcps) / HZ;
+	} else {
+		/* no room in the buffer */
+		cps_limit = 0;
 	}
 
-	return bytes_available;
+	return min(cps_limit, bytes_available);
 }
 
 /*
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve
  2016-08-19  1:05 ` Daeseok Youn
  (?)
@ 2016-08-21 15:56   ` Greg KH
  -1 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-08-21 15:56 UTC (permalink / raw)
  To: Daeseok Youn
  Cc: lidza.louina, devel, driverdev-devel, kernel-janitors, linux-kernel

On Fri, Aug 19, 2016 at 10:05:19AM +0900, Daeseok Youn wrote:
> The whole code in dgnc_maxcps_room() function surrounds with
> one if-statement for checking channel's maxcps and buffer size.
> I tried to separate the logic for this function from if-condition.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_tty.c | 46 +++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)

Have you tested this on the hardware?

Until you do, I'm loath to take changes like this, especially as you are
adding more code than you remove :(

sorry,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve
@ 2016-08-21 15:56   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-08-21 15:56 UTC (permalink / raw)
  To: Daeseok Youn
  Cc: devel, lidza.louina, driverdev-devel, kernel-janitors, linux-kernel

On Fri, Aug 19, 2016 at 10:05:19AM +0900, Daeseok Youn wrote:
> The whole code in dgnc_maxcps_room() function surrounds with
> one if-statement for checking channel's maxcps and buffer size.
> I tried to separate the logic for this function from if-condition.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_tty.c | 46 +++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)

Have you tested this on the hardware?

Until you do, I'm loath to take changes like this, especially as you are
adding more code than you remove :(

sorry,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve
@ 2016-08-21 15:56   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-08-21 15:56 UTC (permalink / raw)
  To: Daeseok Youn
  Cc: devel, lidza.louina, driverdev-devel, kernel-janitors, linux-kernel

On Fri, Aug 19, 2016 at 10:05:19AM +0900, Daeseok Youn wrote:
> The whole code in dgnc_maxcps_room() function surrounds with
> one if-statement for checking channel's maxcps and buffer size.
> I tried to separate the logic for this function from if-condition.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_tty.c | 46 +++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)

Have you tested this on the hardware?

Until you do, I'm loath to take changes like this, especially as you are
adding more code than you remove :(

sorry,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-21 21:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-19  1:05 [PATCH 2/2] staging: dgnc: refactor the dgnc_maxcps_room() to improve Daeseok Youn
2016-08-19  1:05 ` Daeseok Youn
2016-08-19  1:05 ` Daeseok Youn
2016-08-21 15:56 ` Greg KH
2016-08-21 15:56   ` Greg KH
2016-08-21 15:56   ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.