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

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

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.