* [PATCH v2 0/2] serial: dz: UART_XMIT_SIZE/WAKEUP_CHARS cleanups
@ 2022-08-25 13:17 Ilpo Järvinen
2022-08-25 13:17 ` [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd Ilpo Järvinen
2022-08-25 13:17 ` [PATCH v2 2/2] serial: dz: Remove custom DZ_WAKEUP_CHARS Ilpo Järvinen
0 siblings, 2 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2022-08-25 13:17 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Maciej W . Rozycki
Cc: linux-kernel, Ilpo Järvinen
v2:
- Make separate patch out of xmit mask change (which, in theory, is
a fix but due to lack of issue reports, likely doesn't occur for real).
- Replace also DZ_WAKEUP_CHARS with the normal WAKEUP_CHARS (matching
to n_tty_poll)
Ilpo Järvinen (2):
serial: dz: xmit buffer is UART_XMIT_SIZE'd
serial: dz: Remove custom DZ_WAKEUP_CHARS
drivers/tty/serial/dz.c | 4 ++--
drivers/tty/serial/dz.h | 3 ---
2 files changed, 2 insertions(+), 5 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd
2022-08-25 13:17 [PATCH v2 0/2] serial: dz: UART_XMIT_SIZE/WAKEUP_CHARS cleanups Ilpo Järvinen
@ 2022-08-25 13:17 ` Ilpo Järvinen
2022-08-26 13:34 ` Maciej W. Rozycki
2022-08-25 13:17 ` [PATCH v2 2/2] serial: dz: Remove custom DZ_WAKEUP_CHARS Ilpo Järvinen
1 sibling, 1 reply; 5+ messages in thread
From: Ilpo Järvinen @ 2022-08-25 13:17 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Maciej W . Rozycki,
linux-kernel
Cc: Ilpo Järvinen
Instead of DZ_XMIT_SIZE, use the normal UART_XMIT_SIZE directly as it's
the correct size of the xmit circular buffer.
In theory, the Tx code would be buggy if UART_XMIT_SIZE differs from
4096 (occurs when PAGE_SIZE > 4k), however, given the lack of issue
reports such configuration likely doesn't occur with any real platform
with dz HW. The inconsisted sizes would cause missing characters and
never-ending bogus Tx when ->head reaches the region above 4k. The
issue, if it would be real, would predate git days.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/serial/dz.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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++;
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] serial: dz: Remove custom DZ_WAKEUP_CHARS
2022-08-25 13:17 [PATCH v2 0/2] serial: dz: UART_XMIT_SIZE/WAKEUP_CHARS cleanups Ilpo Järvinen
2022-08-25 13:17 ` [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd Ilpo Järvinen
@ 2022-08-25 13:17 ` Ilpo Järvinen
1 sibling, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2022-08-25 13:17 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Maciej W . Rozycki,
linux-kernel
Cc: Ilpo Järvinen
Almost all serial drivers use WAKEUP_CHARS (256) from serial_core.h which
also matches what n_tty_poll() uses for asserting EPOLLOUT.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/serial/dz.c | 2 +-
drivers/tty/serial/dz.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 5d2588f3e6a9..5df46f22abaa 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -283,7 +283,7 @@ static inline void dz_transmit_chars(struct dz_mux *mux)
dz_out(dport, DZ_TDR, tmp);
dport->port.icount.tx++;
- if (uart_circ_chars_pending(xmit) < DZ_WAKEUP_CHARS)
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(&dport->port);
/* Are we are done. */
diff --git a/drivers/tty/serial/dz.h b/drivers/tty/serial/dz.h
index 3b3e31954f24..4b502bfad560 100644
--- a/drivers/tty/serial/dz.h
+++ b/drivers/tty/serial/dz.h
@@ -124,7 +124,4 @@
#define DZ_NB_PORT 4
-#define DZ_XMIT_SIZE 4096 /* buffer size */
-#define DZ_WAKEUP_CHARS DZ_XMIT_SIZE/4
-
#endif /* DZ_SERIAL_H */
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd
2022-08-25 13:17 ` [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd Ilpo Järvinen
@ 2022-08-26 13:34 ` Maciej W. Rozycki
2022-08-26 14:16 ` Ilpo Järvinen
0 siblings, 1 reply; 5+ messages in thread
From: Maciej W. Rozycki @ 2022-08-26 13:34 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
On Thu, 25 Aug 2022, Ilpo Järvinen wrote:
> In theory, the Tx code would be buggy if UART_XMIT_SIZE differs from
> 4096 (occurs when PAGE_SIZE > 4k), however, given the lack of issue
> reports such configuration likely doesn't occur with any real platform
> with dz HW. The inconsisted sizes would cause missing characters and
> never-ending bogus Tx when ->head reaches the region above 4k. The
> issue, if it would be real, would predate git days.
This is misleading. There are exactly 3 machine models (2 major ones and
1 extra submodel) that we currently support which make use of this serial
port hardware and driver, and they all have their R2000/R3000 MIPS CPU
soldered onto their respective mainboards. And the CPUs they use all have
their page size hardwired to 4KiB, so it's not the lack of reports, but a
firm assertion that this driver as it stands shall never be used with a
different page size.
There exists an option card using a DZ11-compatible chipset that can be
used with systems we currently support with page sizes of up to 64KiB, but
to the best of my knowledge only a number of prototype cards has been made
and I have heard of exactly one person having such a card. Therefore we
do not support it and may never do, so it is not a concern for the driver
as it stands and shall not be mentioned.
Please just state then that the change is for design consistency with the
serial core and redefine DZ_XMIT_SIZE in terms of UART_XMIT_SIZE as I
suggested for v1. I'll ack such a change. Please drop 2/2 at this stage
as it does not fix any bug and does not appear to add any value to this
driver.
Thanks,
Maciej
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd
2022-08-26 13:34 ` Maciej W. Rozycki
@ 2022-08-26 14:16 ` Ilpo Järvinen
0 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2022-08-26 14:16 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML
[-- Attachment #1: Type: text/plain, Size: 3039 bytes --]
On Fri, 26 Aug 2022, Maciej W. Rozycki wrote:
> On Thu, 25 Aug 2022, Ilpo Järvinen wrote:
>
> > In theory, the Tx code would be buggy if UART_XMIT_SIZE differs from
> > 4096 (occurs when PAGE_SIZE > 4k), however, given the lack of issue
> > reports such configuration likely doesn't occur with any real platform
> > with dz HW. The inconsisted sizes would cause missing characters and
> > never-ending bogus Tx when ->head reaches the region above 4k. The
> > issue, if it would be real, would predate git days.
>
> This is misleading. There are exactly 3 machine models (2 major ones and
> 1 extra submodel) that we currently support which make use of this serial
> port hardware and driver, and they all have their R2000/R3000 MIPS CPU
> soldered onto their respective mainboards. And the CPUs they use all have
> their page size hardwired to 4KiB, so it's not the lack of reports, but a
> firm assertion that this driver as it stands shall never be used with a
> different page size.
Ah, sorry. I misread your original statement to contain a question to me
rather than just you stating a fact.
> There exists an option card using a DZ11-compatible chipset that can be
> used with systems we currently support with page sizes of up to 64KiB, but
> to the best of my knowledge only a number of prototype cards has been made
> and I have heard of exactly one person having such a card. Therefore we
> do not support it and may never do, so it is not a concern for the driver
> as it stands and shall not be mentioned.
>
> Please just state then that the change is for design consistency with the
> serial core and redefine DZ_XMIT_SIZE in terms of UART_XMIT_SIZE as I
> suggested for v1.
You had a small error in your suggestion for v1 though (which confused me
somewhat as there obviously was an error in it and I guessed wrong what
you meant):
>> Also I'd rather:
>>
>>#define DZ_WAKEUP_CHARS UART_XMIT_SIZE
...I guess with that you actually meant doing simply (and nothing else):
#define DW_XMIT_SIZE UART_XMIT_SIZE
?
But whatever. That line 1/2 is touching is anyway going to die pretty soon
if the 2nd part (yet to be submitted) of the uart_xmit_advance() series
(1st part here [1]) gets applied so I don't care too much what the
xmit->tail line will be in between.
I just thought it would have been nice to also get rid of what clearly
appears to be just a duplicated define of something core already has.
> I'll ack such a change. Please drop 2/2 at this stage
> as it does not fix any bug and does not appear to add any value to this
> driver.
Ok.
I personally don't see the connection between *WAKEUP_CHARS and circular
buffer size would be strong enough to warrant defining former using the
latter. ...If it would be there, the other drivers would have a similar
construct. But I can leave it as is, no significant harm done.
Thanks a lot for your feedback and insight!
[1] https://lore.kernel.org/linux-serial/20220825091707.8112-1-ilpo.jarvinen@linux.intel.com/T/#t
--
i.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-26 14:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25 13:17 [PATCH v2 0/2] serial: dz: UART_XMIT_SIZE/WAKEUP_CHARS cleanups Ilpo Järvinen
2022-08-25 13:17 ` [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd Ilpo Järvinen
2022-08-26 13:34 ` Maciej W. Rozycki
2022-08-26 14:16 ` Ilpo Järvinen
2022-08-25 13:17 ` [PATCH v2 2/2] serial: dz: Remove custom DZ_WAKEUP_CHARS Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).