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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
