linux-kernel - Re: [PATCH v3] serial: Add Milbeaut serial control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 8 May 2019 21:22:05 +0900
From:   "Sugaya, Taichi" <sugaya.taichi@...ionext.com>
To:     Alan Cox <gnomes@...rguk.ukuu.org.uk>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, Arnd Bergmann <arnd@...db.de>,
        Takao Orito <orito.takao@...ionext.com>,
        Kazuhiro Kasai <kasai.kazuhiro@...ionext.com>,
        Shinji Kanematsu <kanematsu.shinji@...ionext.com>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Masami Hiramatsu <masami.hiramatsu@...aro.org>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v3] serial: Add Milbeaut serial control

Hi,

Thank you for pointing out.

On 2019/04/27 3:15, Alan Cox wrote:
> O
>> +static void mlb_usio_set_termios(struct uart_port *port,
>> +			struct ktermios *termios, struct ktermios *old)
>> +{
>> +	unsigned int escr, smr = MLB_USIO_SMR_SOE;
>> +	unsigned long flags, baud, quot;
>> +
>> +	switch (termios->c_cflag & CSIZE) {
>> +	case CS5:
>> +		escr = MLB_USIO_ESCR_L_5BIT;
>> +		break;
>> +	case CS6:
>> +		escr = MLB_USIO_ESCR_L_6BIT;
>> +		break;
>> +	case CS7:
>> +		escr = MLB_USIO_ESCR_L_7BIT;
>> +		break;
>> +	case CS8:
>> +	default:
>> +		escr = MLB_USIO_ESCR_L_8BIT;
>> +		break;
>> +	}
>> +
>> +	if (termios->c_cflag & CSTOPB)
>> +		smr |= MLB_USIO_SMR_SBL;
>> +
>> +	if (termios->c_cflag & PARENB) {
>> +		escr |= MLB_USIO_ESCR_PEN;
>> +		if (termios->c_cflag & PARODD)
>> +			escr |= MLB_USIO_ESCR_P;
>> +	}
> 
> If you don't suport CMSPAR then clear that bit in termios as well
> 

OK, clear the bit because of not supported.

>> +	/* Set hard flow control */
>> +	if (of_property_read_bool(port->dev->of_node, "auto-flow-control") ||
>> +			(termios->c_cflag & CRTSCTS))
>> +		escr |= MLB_USIO_ESCR_FLWEN;
> 
> That's just broken. The termios bits are the definitive things for the
> port, and in addition even if they are forced you need to correct the
> termios data.
> 
> You might want to control flow control *at boot* with an OF property but
> doing it post boot is just busted.
> 

Ah, Yes.
I think OF property should not be here, and it may only be used to determine
the characteristics of the port.
I try to make a fixes patch.

Thanks,
Sugaya Taichi


> Alan
>