Re: [PATCH v2 34/35] tty: make tty_get_byte_size available

[Jiri Slaby]

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. 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.

> 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.

thanks,
--
js
suse labs