usb: dwc2: Use seq_buf instead of hand writing it

Message ID 4c8b71efe4fe05ed0cc37f33ef774746d4d55299.1698489641.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series usb: dwc2: Use seq_buf instead of hand writing it |

Commit Message

Christophe JAILLET Oct. 28, 2023, 10:41 a.m. UTC
  cat_printf() re-implements what the seq_buf API does.
So, switch to the seq_buf API to save some line of code.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
seq_buf_printf(&buf, ", "); could be seq_buf_puts(), but the result could
be slightly different. So I kept a conservative approach.

If only some seq_buf_printf() are used, the final seq_buf_terminate() can
be avoided, but I think it is cleaner with it.
---
 drivers/usb/dwc2/hcd_queue.c | 53 ++++++++----------------------------
 1 file changed, 11 insertions(+), 42 deletions(-)
  

Comments

Christophe JAILLET Oct. 28, 2023, 3:27 p.m. UTC | #1
Le 28/10/2023 à 12:41, Christophe JAILLET a écrit :
> cat_printf() re-implements what the seq_buf API does.
> So, switch to the seq_buf API to save some line of code.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

NACK.

This was built tested, but i think that DWC2_PRINT_SCHEDULE was not defined.

	seq_buf_init((&s, tmp, sizeof(tmp));
can't compile, 's' is not defined.

Let me give another look at it.

CJ

> ---
> seq_buf_printf(&buf, ", "); could be seq_buf_puts(), but the result could
> be slightly different. So I kept a conservative approach.
> 
> If only some seq_buf_printf() are used, the final seq_buf_terminate() can
> be avoided, but I think it is cleaner with it.
> ---
>   drivers/usb/dwc2/hcd_queue.c | 53 ++++++++----------------------------
>   1 file changed, 11 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
> index 0d4495c6b9f7..66fb74a70bdd 100644
> --- a/drivers/usb/dwc2/hcd_queue.c
> +++ b/drivers/usb/dwc2/hcd_queue.c
> @@ -18,6 +18,7 @@
>   #include <linux/io.h>
>   #include <linux/slab.h>
>   #include <linux/usb.h>
> +#include <linux/seq_buf.h>
>   
>   #include <linux/usb/hcd.h>
>   #include <linux/usb/ch11.h>
> @@ -359,41 +360,6 @@ static unsigned long *dwc2_get_ls_map(struct dwc2_hsotg *hsotg,
>   }
>   
>   #ifdef DWC2_PRINT_SCHEDULE
> -/*
> - * cat_printf() - A printf() + strcat() helper
> - *
> - * This is useful for concatenating a bunch of strings where each string is
> - * constructed using printf.
> - *
> - * @buf:   The destination buffer; will be updated to point after the printed
> - *         data.
> - * @size:  The number of bytes in the buffer (includes space for '\0').
> - * @fmt:   The format for printf.
> - * @...:   The args for printf.
> - */
> -static __printf(3, 4)
> -void cat_printf(char **buf, size_t *size, const char *fmt, ...)
> -{
> -	va_list args;
> -	int i;
> -
> -	if (*size == 0)
> -		return;
> -
> -	va_start(args, fmt);
> -	i = vsnprintf(*buf, *size, fmt, args);
> -	va_end(args);
> -
> -	if (i >= *size) {
> -		(*buf)[*size - 1] = '\0';
> -		*buf += *size;
> -		*size = 0;
> -	} else {
> -		*buf += i;
> -		*size -= i;
> -	}
> -}
> -
>   /*
>    * pmap_print() - Print the given periodic map
>    *
> @@ -417,8 +383,7 @@ static void pmap_print(unsigned long *map, int bits_per_period,
>   
>   	for (period = 0; period < periods_in_map; period++) {
>   		char tmp[64];
> -		char *buf = tmp;
> -		size_t buf_size = sizeof(tmp);
> +		struct seq_buf buf;
>   		int period_start = period * bits_per_period;
>   		int period_end = period_start + bits_per_period;
>   		int start = 0;
> @@ -426,6 +391,8 @@ static void pmap_print(unsigned long *map, int bits_per_period,
>   		bool printed = false;
>   		int i;
>   
> +		seq_buf_init((&s, tmp, sizeof(tmp));
> +
>   		for (i = period_start; i < period_end + 1; i++) {
>   			/* Handle case when ith bit is set */
>   			if (i < period_end &&
> @@ -442,17 +409,19 @@ static void pmap_print(unsigned long *map, int bits_per_period,
>   				continue;
>   
>   			if (!printed)
> -				cat_printf(&buf, &buf_size, "%s %d: ",
> -					   period_name, period);
> +				seq_buf_printf(&buf, "%s %d: ", period_name,
> +					       period);
>   			else
> -				cat_printf(&buf, &buf_size, ", ");
> +				seq_buf_printf(&buf, ", ");
>   			printed = true;
>   
> -			cat_printf(&buf, &buf_size, "%d %s -%3d %s", start,
> -				   units, start + count - 1, units);
> +			seq_buf_printf(&buf, "%d %s -%3d %s", start, units,
> +				       start + count - 1, units);
>   			count = 0;
>   		}
>   
> +		seq_buf_terminate(&s);
> +
>   		if (printed)
>   			print_fn(tmp, print_data);
>   	}
  

Patch

diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
index 0d4495c6b9f7..66fb74a70bdd 100644
--- a/drivers/usb/dwc2/hcd_queue.c
+++ b/drivers/usb/dwc2/hcd_queue.c
@@ -18,6 +18,7 @@ 
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
+#include <linux/seq_buf.h>
 
 #include <linux/usb/hcd.h>
 #include <linux/usb/ch11.h>
@@ -359,41 +360,6 @@  static unsigned long *dwc2_get_ls_map(struct dwc2_hsotg *hsotg,
 }
 
 #ifdef DWC2_PRINT_SCHEDULE
-/*
- * cat_printf() - A printf() + strcat() helper
- *
- * This is useful for concatenating a bunch of strings where each string is
- * constructed using printf.
- *
- * @buf:   The destination buffer; will be updated to point after the printed
- *         data.
- * @size:  The number of bytes in the buffer (includes space for '\0').
- * @fmt:   The format for printf.
- * @...:   The args for printf.
- */
-static __printf(3, 4)
-void cat_printf(char **buf, size_t *size, const char *fmt, ...)
-{
-	va_list args;
-	int i;
-
-	if (*size == 0)
-		return;
-
-	va_start(args, fmt);
-	i = vsnprintf(*buf, *size, fmt, args);
-	va_end(args);
-
-	if (i >= *size) {
-		(*buf)[*size - 1] = '\0';
-		*buf += *size;
-		*size = 0;
-	} else {
-		*buf += i;
-		*size -= i;
-	}
-}
-
 /*
  * pmap_print() - Print the given periodic map
  *
@@ -417,8 +383,7 @@  static void pmap_print(unsigned long *map, int bits_per_period,
 
 	for (period = 0; period < periods_in_map; period++) {
 		char tmp[64];
-		char *buf = tmp;
-		size_t buf_size = sizeof(tmp);
+		struct seq_buf buf;
 		int period_start = period * bits_per_period;
 		int period_end = period_start + bits_per_period;
 		int start = 0;
@@ -426,6 +391,8 @@  static void pmap_print(unsigned long *map, int bits_per_period,
 		bool printed = false;
 		int i;
 
+		seq_buf_init((&s, tmp, sizeof(tmp));
+
 		for (i = period_start; i < period_end + 1; i++) {
 			/* Handle case when ith bit is set */
 			if (i < period_end &&
@@ -442,17 +409,19 @@  static void pmap_print(unsigned long *map, int bits_per_period,
 				continue;
 
 			if (!printed)
-				cat_printf(&buf, &buf_size, "%s %d: ",
-					   period_name, period);
+				seq_buf_printf(&buf, "%s %d: ", period_name,
+					       period);
 			else
-				cat_printf(&buf, &buf_size, ", ");
+				seq_buf_printf(&buf, ", ");
 			printed = true;
 
-			cat_printf(&buf, &buf_size, "%d %s -%3d %s", start,
-				   units, start + count - 1, units);
+			seq_buf_printf(&buf, "%d %s -%3d %s", start, units,
+				       start + count - 1, units);
 			count = 0;
 		}
 
+		seq_buf_terminate(&s);
+
 		if (printed)
 			print_fn(tmp, print_data);
 	}