lkml.org 
[lkml]   [2022]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/2] serial: dz: xmit buffer is UART_XMIT_SIZE'd
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.
\
 
 \ /
  Last update: 2022-08-26 16:18    [W:3.164 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site