Re: [PATCH 09/24] USB: pl2303: clean up baud-rate handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux