On Thu, May 13, 2021 at 09:24:03AM +0200, Jiri Slaby wrote:
> On 10. 05. 21, 11:47, Johan Hovold wrote:
> >> --- a/drivers/tty/tty_ioctl.c
> >> +++ b/drivers/tty/tty_ioctl.c
> >> @@ -300,6 +300,48 @@ int tty_termios_hw_change(const struct ktermios *a, const struct ktermios *b)
> ...
> >> +unsigned char tty_get_byte_size(unsigned int cflag, bool account_flags)
> >> +{
> >> + unsigned char bits;
> >> +
> >> + switch (cflag & CSIZE) {
> >> + case CS5:
> >> + bits = 5;
> >> + break;
> >> + case CS6:
> >> + bits = 6;
> >> + break;
> >> + case CS7:
> >> + bits = 7;
> >> + break;
> >> + case CS8:
> >> + default:
> >> + bits = 8;
> >> + break;
> >> + }
> >> +
> >> + if (!account_flags)
> >> + return bits;
> >> +
> >> + if (cflag & CSTOPB)
> >> + bits++;
> >> + if (cflag & PARENB)
> >> + bits++;
> >> +
> >> + return bits + 2;
> >> +}
> >> +EXPORT_SYMBOL_GPL(tty_get_byte_size);
> >
> > This should really be two functions rather than passing a bool argument.
> >
> > I think naming them
> >
> > tty_get_word_size()
> >
> > and
> >
> > tty_get_frame_size()
> >
> > would be much more clear than than "byte size" + flag.
>
> Maybe I am screwed, but word means exactly 2B here.
No, not in this context.
Some UART datasheets use "word" and "word length", while others use
"character length" or "data bits" (and variations thereof).
> So instead, I would
> go for:
> s/word/char/ -- might be confused with C's char, 1B, or maybe not -- or
> s/word/data/ -- more generic and generally used in serial terminology.
But "data size" it not very precise.
I'd go for either
tty_get_word_size() or tty_get_char_size(), and
tty_get_frame_size()
or possibly
tty_get_data_bits(), and
tty_get_frame_bits()
Since posix and the termios interface use "CSIZE" (even if that "C" is
ambiguous) and our man pages use "character size" perhaps we should
stick with that and use:
tty_get_char_size(), and
tty_get_frame_size()
That should be clear enough for everyone.
> > I realise that the serial-driver interface only uses a cflag argument,
> > but I think we should consider passing a pointer to the termios
> > structure instead.
>
> That's impossible as termios is not always at hand. Examples are:
> pch_uart_startup -> uart_update_timeout
> sunsab_console_setup -> sunsab_convert_to_sab -> uart_update_timeout
> sunsu_kbd_ms_init -> sunsu_change_speed -> uart_update_timeout
>
> Let me document that in the commit.
Sounds good.
Johan
