Am 03.07.2013 12:45, schrieb Johan Hovold: > On Tue, Jul 02, 2013 at 05:44:01PM +0200, Frank Schäfer wrote: >>> - /* NOTE: Only the values defined in baud_sup are supported ! >>> + /* >>> + * NOTE: Only the values defined in baud_sup are supported! >>> * => if unsupported values are set, the PL2303 seems to use >>> * 9600 baud (at least my PL2303X always does) >>> */ >>> baud = tty_get_baud_rate(tty); >>> dev_dbg(&port->dev, "baud requested = %d\n", baud); >>> - if (baud) { >>> - /* Set baudrate to nearest supported value */ >>> - for (k=0; k<ARRAY_SIZE(baud_sup); k++) { >>> - if (baud_sup[k] / baud) { >>> - baud_ceil = baud_sup[k]; >>> - if (k==0) { >>> - baud = baud_ceil; >>> - } else { >>> - baud_floor = baud_sup[k-1]; >>> - if ((baud_ceil % baud) >>> - > (baud % baud_floor)) >>> - baud = baud_floor; >>> - else >>> - baud = baud_ceil; >>> - } >>> - break; >>> - } >>> - } >> [...] >>> - if (baud > 1228800) { >>> - /* type_0, type_1 only support up to 1228800 baud */ >>> - if (spriv->type != HX) >>> - baud = 1228800; >>> - else if (baud > 6000000) >>> - baud = 6000000; >>> - } >>> - dev_dbg(&port->dev, "baud set = %d\n", baud); >>> - if (baud <= 115200) { >>> - buf[0] = baud & 0xff; >>> - buf[1] = (baud >> 8) & 0xff; >>> - buf[2] = (baud >> 16) & 0xff; >>> - buf[3] = (baud >> 24) & 0xff; >>> - } else { >>> - /* apparently the formula for higher speeds is: >>> - * baudrate = 12M * 32 / (2^buf[1]) / buf[0] >>> - */ >>> - unsigned tmp = 12*1000*1000*32 / baud; >>> - buf[3] = 0x80; >>> - buf[2] = 0; >>> - buf[1] = (tmp >= 256); >>> - while (tmp >= 256) { >>> - tmp >>= 2; >>> - buf[1] <<= 1; >>> - } >>> - buf[0] = tmp; >>> + if (!baud) >>> + return; >>> + >>> + /* Set baudrate to nearest supported value */ >>> + for (i = 0; i < ARRAY_SIZE(baud_sup); ++i) { >>> + if (baud_sup[i] > baud) >>> + break; >>> + } >> This isn't just a clean-up. >> It actually changes the baud rate determination from "next nearest" to >> "round downwards". >> I really think the current behavior is preferred/better, so please keep >> the old code. >> For example: a selection of 14300 baud results in 9600 baud with this >> patch applied, while the next nearest supported baud rate is 14400 baud. >> >> The rest of this patch (and AFAICS also the other patches of this >> series) is fine. > Thanks for looking at the patch. However, you missed the next few > lines which are part of the clean up of the old implementation: > >>> + >>> + if (i == ARRAY_SIZE(baud_sup)) >>> + baud = baud_sup[i - 1]; >>> + else if (i > 0 && (baud_sup[i] - baud) > (baud - baud_sup[i - 1])) >>> + baud = baud_sup[i - 1]; >>> + else >>> + baud = baud_sup[i]; > You see? Here the closest baud rate is selected just like before, but > without the need for those crazy division and modulo operations. > > Johan Argh... uhm... yes, sorry for the noise. :D I've read these lines, but apparently I suffered from a total blackout in the middle of this statement... Great patch ! :D Frank > >>> + >>> + /* type_0, type_1 only support up to 1228800 baud */ >>> + if (spriv->type != HX) >>> + baud = max_t(int, baud, 1228800); >>> + >>> + if (baud <= 115200) { >>> + put_unaligned_le32(baud, buf); >>> + } else { >>> + /* >>> + * Apparently the formula for higher speeds is: >>> + * baudrate = 12M * 32 / (2^buf[1]) / buf[0] >>> + */ >>> + unsigned tmp = 12000000 * 32 / baud; >>> + buf[3] = 0x80; >>> + buf[2] = 0; >>> + buf[1] = (tmp >= 256); >>> + while (tmp >= 256) { >>> + tmp >>= 2; >>> + buf[1] <<= 1; >>> } >>> + buf[0] = tmp; >>> } >>> >>> /* Save resulting baud rate */ >>> - if (baud) >>> - tty_encode_baud_rate(tty, baud, baud); >>> + tty_encode_baud_rate(tty, baud, baud); >>> + dev_dbg(&port->dev, "baud set = %d\n", baud); >>> } >>> >>> static void pl2303_set_termios(struct tty_struct *tty, -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html