2022-08-25 09:22:19

by Ilpo Järvinen

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

Use the normal UART_XMIT_SIZE directly.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
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


2022-08-25 12:17:21

by Maciej W. Rozycki

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

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

2022-08-25 12:27:35

by Ilpo Järvinen

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

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.

2022-08-25 15:01:59

by Maciej W. Rozycki

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

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