On 3/11/21 11:08 AM, Johan Hovold wrote:
> I think we should just a new flag for the alternate divisor encoding
> named "alt_divisors" (cf. no_divisors).
>
> Chances are this encoding is used for other types as well.
>
>> },
>> };
>>
>> static int pl2303_startup(struct usb_serial *serial)
>> {
>> ....
>> if ( serial->dev->descriptor.bcdDevice == 0x0300 && serial->dev->descriptor.bcdUSB == 0x0200 )
>> type = TYPE_TA;
> This needs to go after the bDeviceClass == 0x02 check, and the
> descriptor fields need to be accessed using le16_to_cpu().
>
> I've prepared a patch series that clean up and tighten the type
> detection which I suggest you built upon instead.
>
> I'll post the series after replying here.
>
>> else if (serial->dev->descriptor.bDeviceClass == 0x02)
>> ....
>> }
>>
>> static speed_t pl2303_encode_baud_rate_divisor( struct usb_serial_port *port,
>> unsigned char buf[4],
>> speed_t baud)
>> {
>> unsigned int baseline, mantissa, exponent;
>> struct usb_serial *serial = port->serial;
>> struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
>>
>> /*
>> * Apparently the formula is:
>> * baudrate = 12M * 32 / (mantissa * 4^exponent)
>> * where
>> * mantissa = buf[8:0]
>> * exponent = buf[11:9]
>> *
>> * TA version has more precision
>> * uses mantissa = buf[bits 10:0 ]
> So you discovered that there were even more bits here? Your first
> version used ten bits, I believe.
>
> I got an offline mail from a third person having problems with the TA
> and who had also verified eleven bits here.
I was basing this on Joe's discovery of the value used for 110 bd by the windows driver (confirmed by Charles). The sequence 80 01 a6 a8 implies that the mantissa is 0x6a8 (i.e. 11 bits). The tests that I did seemed to confirm this.
>> * exponent = buf[bits 15:13]
>> * and x2 prescaler enable by buf[bit 16]
>> */
>> baseline = 12000000 * 32;
>> mantissa = baseline / baud;
>> if (mantissa == 0)
>> mantissa = 1; /* Avoid dividing by zero if baud > 32*12M. */
>> exponent = 0;
>>
>> if (spriv->quirks & PL2303_QUIRK_DIVISOR_TA) {
>> while (mantissa >= 2048) {
>> // exponent is three bits (after shifting right)
>> if (exponent < 15) { // we are going to divide this by 2 later
>> mantissa >>= 1; // divide by 2
>> exponent++; // currently log2 ... will become log4
>> } else {
>> /* Exponent is maxed. Trim mantissa and leave. */
>> mantissa = 2047 ;
>> break;
>> }
>> }
>> buf[2] = exponent & 0x01; // activate x2 prescaler if needed
>> exponent >>= 1; // now log base 4 (losing LSB)
>> buf[1] = (exponent << 5) | (mantissa >> 8);
> Again, this is really nice work.
>
> But it seems to me that we should simply think about the encoding as
> using base-2 with the LSB of the exponent in bit 16. That should make it
> easier to follow what's going on here.
I quite agree. It would be cleaner.
>
> I've been thinking about ways of merging the two schemes (both using
> base 2), and I even checked if my HXD happened to have a 2-prescaler bit
> somewhere as well but I couldn't find one.
>
> It's probably not worth it at this point (and may not end up being more
> readable anyway) so I therefore suggest adding a separate function for
> the alternate scheme for now. You can just call it at the start of the
> "default" function:
>
> if (spriv->type->alt_divisors)
> return pl2303_encode_baud_rate_divisor_alt(buf, baud);
Pardon my ignorance of the process but where is the git repo for this development branch?
Michael
