linux-kernel - Re: Re: [PATCH V5.1] serial/uart/8250: Add tunable RX interrupt trigger I/F of FIFO buffers

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <534DD786.7090702@hitachi.com>
Date:	Wed, 16 Apr 2014 10:06:14 +0900
From:	Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@...achi.com>
To:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Stephen Warren <swarren@...dia.com>,
	Jingoo Han <jg1.han@...sung.com>, linux-kernel@...r.kernel.org,
	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>,
	linux-serial@...r.kernel.org, yrl.pp-manager.tt@...achi.com,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Aaron Sierra <asierra@...-inc.com>, Jiri Slaby <jslaby@...e.cz>
Subject: Re: Re: [PATCH V5.1] serial/uart/8250: Add tunable RX interrupt
 trigger I/F of FIFO buffers

Hi Alan,

Thank you for your review.

(2014/04/15 19:10), One Thousand Gnomes wrote:
>
>> +static int convert_val2rxtrig(struct uart_8250_port *up, unsigned char val)
>> +{
>> +	const struct serial8250_config *conf_type = &uart_config[up->port.type];
>> +	int i;
>> +
>> +	if (!conf_type->rx_trig_byte[FCR_RX_TRIG_BITS(UART_FCR_R_TRIG_00)]) {
>> +		pr_info("Not support RX-trigger setting for this serial %s\n",
>> +			conf_type->name);
>> +		return -EOPNOTSUPP;
>
> We don't want the pr_info here - otherwise users can flood the logs. The
> err code is sufficient.

OK, I'll delete this pr_info().

>> +	for (i = 1; i < FCR_RX_TRIG_MAX_STATE; i++) {
>> +		if (val < conf_type->rx_trig_byte[i])
>> +			/* Use the nearest lower value */
>> +			return (--i) << FCR_RX_TRIG_OFFSET;
>
> Odd case here - your 16654 entry has a minimum of 8 so if I pass 1 then
> it'll return -1 ? I guess for that case the best we can do is to set 8.

No, in that case, this function returns 0.

Note that the "i" starts from 1, so the minimal value is 0.
Actually convert_val2rxtrig() returns the index of rx_trig_byte, and
when val == 1, it returns 0 and caller gets rx_trig_byte[0] == 8.

> Other trivial thing to tweak - you have "uaser" in spot instead of "user".

Oops, I'll fix it.

Thanks,
Yoshihiro YUNOMAE

> With those two fixed it looks good to me.
>
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

-- 
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@...achi.com


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/