[PATCH 1/1] serial: dz: Replace DZ_XMIT_SIZE with UART_XMIT_SIZE

[PATCH 1/1] serial: dz: Replace DZ_XMIT_SIZE with UART_XMIT_SIZE

[Ilpo Järvinen]

Use the normal UART_XMIT_SIZE directly.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/dz.c | 2 +-
 drivers/tty/serial/dz.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 2e21acf39720..5d2588f3e6a9 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -279,7 +279,7 @@ static inline void dz_transmit_chars(struct dz_mux *mux)
 	 * so we go one char at a time) :-<
 	 */
 	tmp = xmit->buf[xmit->tail];
-	xmit->tail = (xmit->tail + 1) & (DZ_XMIT_SIZE - 1);
+	xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 	dz_out(dport, DZ_TDR, tmp);
 	dport->port.icount.tx++;
 
diff --git a/drivers/tty/serial/dz.h b/drivers/tty/serial/dz.h
index 3b3e31954f24..59120ad2bda0 100644
--- a/drivers/tty/serial/dz.h
+++ b/drivers/tty/serial/dz.h
@@ -12,6 +12,8 @@
 #ifndef DZ_SERIAL_H
 #define DZ_SERIAL_H
 
+#include <linux/serial_core.h>
+
 /*
  * Definitions for the Control and Status Register.
  */
@@ -124,7 +126,6 @@
 
 #define DZ_NB_PORT 4
 
-#define DZ_XMIT_SIZE   4096                 /* buffer size */
-#define DZ_WAKEUP_CHARS   DZ_XMIT_SIZE/4
+#define DZ_WAKEUP_CHARS   (UART_XMIT_SIZE / 4)
 
 #endif /* DZ_SERIAL_H */
-- 
2.30.2

Re: [PATCH 1/1] serial: dz: Replace DZ_XMIT_SIZE with UART_XMIT_SIZE

[Maciej W. Rozycki]

On Thu, 25 Aug 2022, Ilpo Järvinen wrote:

> Use the normal UART_XMIT_SIZE directly.

 I gather this is to fix a potential inconsistency with the size of the 
buffer allocated by the serial core (though in reality this driver will 
only be used with 4KiB-page systems), right?  If so, then please state it 
in the change description.

 Also I'd rather:

#define DZ_WAKEUP_CHARS      UART_XMIT_SIZE

and there's no need to include <linux/serial_core.h> in dz.h as the driver 
itself already does that (and dz.h is an auxiliary private header).

 Thanks for your submission.

  Maciej

Re: [PATCH 1/1] serial: dz: Replace DZ_XMIT_SIZE with UART_XMIT_SIZE

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

On Thu, 25 Aug 2022, Maciej W. Rozycki wrote:

> On Thu, 25 Aug 2022, Ilpo Järvinen wrote:
> 
> > Use the normal UART_XMIT_SIZE directly.
> 
>  I gather this is to fix a potential inconsistency with the size of the 
> buffer allocated by the serial core (though in reality this driver will 
> only be used with 4KiB-page systems), right?  If so, then please state it 
> in the change description.

No idea, but I guess it has to be true because nobody has complained about 
the missing characters such an inconsistency would cause. It could 
seemingly also cause infinite bogus tx as tail cannot never reach head 
when head is about 4k (uart_circ_empty & uart_circ_chars_pending would 
return bogus values).

>  Also I'd rather:
> 
> #define DZ_WAKEUP_CHARS      UART_XMIT_SIZE
> 
> and there's no need to include <linux/serial_core.h> in dz.h as the driver 
> itself already does that (and dz.h is an auxiliary private header).
> 
>  Thanks for your submission.

I have started to becomes more inclined into the direction of dropping 
DZ_WAKEUP_CHARS entirely and use WAKEUP_CHARS like most of the drivers do
after staring now at WAKEUP_CHARS & uart_write_wakeup() lines just now.

There is just a handful of exceptions, rest of the drivers all use 256 as 
WAKEUP_CHARS. dz uses 1024 (4k/4) and rest of the exceptions use 
uart_circ_empty() but I suspect they should also be just converted to 
use WAKEUP_CHARS.

-- 
 i.

Re: [PATCH 1/1] serial: dz: Replace DZ_XMIT_SIZE with UART_XMIT_SIZE

[Maciej W. Rozycki]

On Thu, 25 Aug 2022, Ilpo Järvinen wrote:

> >  Also I'd rather:
> > 
> > #define DZ_WAKEUP_CHARS      UART_XMIT_SIZE
> > 
> > and there's no need to include <linux/serial_core.h> in dz.h as the driver 
> > itself already does that (and dz.h is an auxiliary private header).
> > 
> >  Thanks for your submission.
> 
> I have started to becomes more inclined into the direction of dropping 
> DZ_WAKEUP_CHARS entirely and use WAKEUP_CHARS like most of the drivers do
> after staring now at WAKEUP_CHARS & uart_write_wakeup() lines just now.
> 
> There is just a handful of exceptions, rest of the drivers all use 256 as 
> WAKEUP_CHARS. dz uses 1024 (4k/4) and rest of the exceptions use 
> uart_circ_empty() but I suspect they should also be just converted to 
> use WAKEUP_CHARS.

 It may have to do with the particularly low speed of the machines the 
driver/hardware is used with, one of the slowest Linux has ever supported 
(I think only the m68k port may serve slower machines) and certainly the 
slowest and earliest MIPS processors, down to R2000 clocked at 12MHz.

 Also bear in mind that the DZ11 interface is a serial line multiplexer 
rather than a classic single or multiple UART, handling up to 8 lines via 
shared Tx/Rx buffers (the original implementation was in the form of a 
rather large discrete board built of SSI chips).  In this particular 
integrated ASIC implementation 4 lines are handled and a character to be 
transmitted may have to wait for the other 3 lines to be handled first.  

 This may have contributed to the choice made by the original implementer 
here and any change will have to be thoroughly understood and evaluated.

 See <https://gunkies.org/wiki/DZ11_asynchronous_serial_line_interface> 
for an overview of the device, technical documentation, and a photo of a 
specimen dating back to mid 1970s.

  Maciej