[PATCH v1 1/1] hvc_console: Allow backends to set I/O buffer size

[PATCH v1 1/1] hvc_console: Allow backends to set I/O buffer size

[Geoff Levand]

To allow HVC backends to set the I/O buffer sizes to values that are most
efficient for the backend, change the macro definitions where the buffer sizes
are set to be conditional on whether or not the macros are already defined.
Also, rename the macros from N_OUTBUF to HVC_N_OUBUF and from N_INBUF to
HVC_N_INBUF.

Typical usage in the backend source file would be:

  #define HVC_N_OUTBUF 32
  #define HVC_N_INBUF 32
  #include "hvc_console.h"

Signed-off-by: Geoff Levand <geoff@infradead.org>
---

Hi,

With this patch the buffer sizes are set by defining preprocessor macros before
including the hvc_console.h header file.  Another way would be to have Kconfig
options for the buffer sizes.  Since the optimal buffer size is so closely tied
to the backend implementation I thought that using these preprocessor macros
would be the better way.

-Geoff

The following changes since commit 5dc4c995db9eb45f6373a956eb1f69460e69e6d4:

  Linux 6.2-rc4 (2023-01-15 09:22:43 -0600)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/geoff/ps3-linux.git for-merge-hvc-v1

for you to fetch changes up to 8f3cd1e0589f134380f80a1f551c8232ed0bc1f2:

  hvc_console: Allow backends to set I/O buffer size (2023-01-15 09:36:22 -0800)


 drivers/tty/hvc/hvc_console.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index a683e21df19c..f7809d19e2cd 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -42,12 +42,15 @@
 #define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
 
 /*
- * These sizes are most efficient for vio, because they are the
- * native transfer size. We could make them selectable in the
- * future to better deal with backends that want other buffer sizes.
+ * These default sizes are most efficient for vio, because they are
+ * the native transfer size.
  */
-#define N_OUTBUF	16
-#define N_INBUF		16
+#if !defined(HVC_N_OUTBUF)
+# define HVC_N_OUTBUF	16
+#endif
+#if !defined(HVC_N_INBUF)
+# define HVC_N_INBUF	16
+#endif
 
 #define __ALIGNED__ __attribute__((__aligned__(L1_CACHE_BYTES)))
 
@@ -151,7 +154,7 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
 static void hvc_console_print(struct console *co, const char *b,
 			      unsigned count)
 {
-	char c[N_OUTBUF] __ALIGNED__;
+	char c[HVC_N_OUTBUF] __ALIGNED__;
 	unsigned i = 0, n = 0;
 	int r, donecr = 0, index = co->index;
 
@@ -633,7 +636,7 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
 {
 	struct tty_struct *tty;
 	int i, n, count, poll_mask = 0;
-	char buf[N_INBUF] __ALIGNED__;
+	char buf[HVC_N_INBUF] __ALIGNED__;
 	unsigned long flags;
 	int read_total = 0;
 	int written_total = 0;
@@ -674,7 +677,7 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
 
  read_again:
 	/* Read data if any */
-	count = tty_buffer_request_room(&hp->port, N_INBUF);
+	count = tty_buffer_request_room(&hp->port, HVC_N_INBUF);
 
 	/* If flip is full, just reschedule a later read */
 	if (count == 0) {
-- 
2.34.1

Re: [PATCH v1 1/1] hvc_console: Allow backends to set I/O buffer size

[Christophe Leroy]

Hi,

Le 15/01/2023 à 20:56, Geoff Levand a écrit :
> To allow HVC backends to set the I/O buffer sizes to values that are most
> efficient for the backend, change the macro definitions where the buffer sizes
> are set to be conditional on whether or not the macros are already defined.
> Also, rename the macros from N_OUTBUF to HVC_N_OUBUF and from N_INBUF to
> HVC_N_INBUF.
> 
> Typical usage in the backend source file would be:
> 
>    #define HVC_N_OUTBUF 32
>    #define HVC_N_INBUF 32
>    #include "hvc_console.h"
> 
> Signed-off-by: Geoff Levand <geoff@infradead.org>

Most patches in drivers/tty/hvc/ are merged by greg through the serial 
tree, you should send to him.

And I think it is not correct to send that as pull request.

Christophe

> ---
> 
> Hi,
> 
> With this patch the buffer sizes are set by defining preprocessor macros before
> including the hvc_console.h header file.  Another way would be to have Kconfig
> options for the buffer sizes.  Since the optimal buffer size is so closely tied
> to the backend implementation I thought that using these preprocessor macros
> would be the better way.
> 
> -Geoff
> 
> The following changes since commit 5dc4c995db9eb45f6373a956eb1f69460e69e6d4:
> 
>    Linux 6.2-rc4 (2023-01-15 09:22:43 -0600)
> 
> are available in the Git repository at:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/geoff/ps3-linux.git for-merge-hvc-v1
> 
> for you to fetch changes up to 8f3cd1e0589f134380f80a1f551c8232ed0bc1f2:
> 
>    hvc_console: Allow backends to set I/O buffer size (2023-01-15 09:36:22 -0800)
> 
> 
>   drivers/tty/hvc/hvc_console.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index a683e21df19c..f7809d19e2cd 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -42,12 +42,15 @@
>   #define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
>   
>   /*
> - * These sizes are most efficient for vio, because they are the
> - * native transfer size. We could make them selectable in the
> - * future to better deal with backends that want other buffer sizes.
> + * These default sizes are most efficient for vio, because they are
> + * the native transfer size.
>    */
> -#define N_OUTBUF	16
> -#define N_INBUF		16
> +#if !defined(HVC_N_OUTBUF)
> +# define HVC_N_OUTBUF	16
> +#endif
> +#if !defined(HVC_N_INBUF)
> +# define HVC_N_INBUF	16
> +#endif
>   
>   #define __ALIGNED__ __attribute__((__aligned__(L1_CACHE_BYTES)))
>   
> @@ -151,7 +154,7 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
>   static void hvc_console_print(struct console *co, const char *b,
>   			      unsigned count)
>   {
> -	char c[N_OUTBUF] __ALIGNED__;
> +	char c[HVC_N_OUTBUF] __ALIGNED__;
>   	unsigned i = 0, n = 0;
>   	int r, donecr = 0, index = co->index;
>   
> @@ -633,7 +636,7 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
>   {
>   	struct tty_struct *tty;
>   	int i, n, count, poll_mask = 0;
> -	char buf[N_INBUF] __ALIGNED__;
> +	char buf[HVC_N_INBUF] __ALIGNED__;
>   	unsigned long flags;
>   	int read_total = 0;
>   	int written_total = 0;
> @@ -674,7 +677,7 @@ static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
>   
>    read_again:
>   	/* Read data if any */
> -	count = tty_buffer_request_room(&hp->port, N_INBUF);
> +	count = tty_buffer_request_room(&hp->port, HVC_N_INBUF);
>   
>   	/* If flip is full, just reschedule a later read */
>   	if (count == 0) {