On Fri, Nov 01, 2013 at 11:06:40AM +0100, Frank Schäfer wrote:
> Am 31.10.2013 21:56, schrieb Johan Hovold:
> > On Thu, Oct 31, 2013 at 09:26:06PM +0100, Johan Hovold wrote:
> >> On Thu, Oct 31, 2013 at 7:45 PM, Frank Schäfer wrote:
> >>> Am 31.10.2013 13:30, schrieb Mika Westerberg:
> >>>> On Thu, Oct 31, 2013 at 01:02:56PM +0100, Frank Schäfer wrote:
> >>>>> 2) comment out the following line at the end of
> >>>>> pl2303_baudrate_encode_divisor_HXD():
> >>>>>
> >>>>> baud = 12000000 * 32 / ((1 << buf[1]) * buf[0]);
> >>>> This seems to fix the problem :)
> >>>>
> >>>> Once the line is commented out, the serial console works fine with the
> >>>> speeds I've been testing (115200, 230400 and 460800).
> >>> Urgh... so it seems getty gets confused if the actually set baud rate is
> >>> reported back.
> >>> The kernel still reports the standard baud rate (e.g. B230400), it only
> >>> sets the exact value in the c_ispeed and c_ospeed fields of the termios
> >>> struct.
> >>> So even if getty doesn't support non-standard baud rates, everything
> >>> should be fine.
> >>> I'll have to take a closer look at this issue later.
> > I think I know what's going on. The pl2303 is known to loose bytes when
> > changing the termios settings (see commit bf5e5834b ("pl2303: Fix mode
> > switching regression")).
> >
> > Your commit 57ce61aad ("usb: pl2303: fix+improve the divisor based baud
> > rate encoding method") introduced a regression when it started reporting
> > back the baudrates determined by the divisor algorithm.
> >
> > Since that commit, tty_termios_hw_change(&tty->termios, old_termios)
> > will always return true -- even when userspace isn't requesting a
> > different baudrate.
> Hmm... so there is a bug in tty_termios_hw_change() ?
Not necessarily.
> >>> Ok, so now let's see if the fixed/improved divisor based method also
> >>> works for the HXD chip if we don't report the actually set baud rate.
> >>> Try commenting out the line
> >>>
> >>> baud = 12000000 * 32 / ((1 << A) * B);
> >>>
> >>> at the end of pl2303_baudrate_encode_divisor() of a clean 3.12-rc kernel.
> >>> It's unlikely that this makes baud rates < 115200 working, but testing
> >>> both cases doesn't harm. ;)
> > I can trigger the problem here with my HXD/EA/RA/SA-device as well, and
> > I've verified that not returning the calculated baudrate makes this
> > particular issue *seem* to go away.
> >
> > This obviously has to be fixed or reverted quickly.
> Indeed.
>
> > Greg, what do you say about this? Is reverting for 3.12 the correct way
> > to deal with this, and make sure these fairly invasive patches get some
> > more testing before being reapplied?
> >
> > Reverting would mean reverting a whole bunch of commits though, as they
> > appear to depend quite heavily on each other:
> >
> > 7d26a78 USB: pl2303: distinguish between original and cloned HX chips
> > 034d152 pl2303: improve the chip type detection/distinction
> > a77a8c2 pl2303: improve the chip type information output on startup
> > 73b583a pl2303: simplify the else-if contruct for type_1 chips in pl2303_startup()
> > c23bda3 usb: pl2303: add two comments concerning the supported baud rates with HX chips
> > 61fa8d6 usb: pl2303: also use the divisor based baud rate encoding method for baud rates < 115200 with HX chips
> > b5c16c6 usb: pl2303: increase the allowed baud rate range for the divisor based encoding method
> > e917ba0 usb: pl2303: move the two baud rate encoding methods to separate functions
> > b9208c7 usb: pl2303: remove 500000 baud from the list of standard baud rates
> > 75417d9 usb: pl2303: do not round to the next nearest standard baud rate for the divisor based baud rate encoding method
> > 57ce61a usb: pl2303: fix+improve the divsor based baud rate encoding method
> > b8bdad6 USB: pl2303: restrict the divisor based baud rate encoding method to the "HX" chip type
> No need to revert all these patches, removing the calculation of the
> resulting baud rateat the end of fcn pl2303_baudrate_encode_divisor()
> should fix this issue.
It could, if the problem I identified above is the only issue here.
However, we have a clear regression and 3.12 is coming out very, very
soon.
> The remaining question is, does the HXD work with the improved divisor
> method or do we have to reintroduce the old method ?
This is exactly the kind of issues we can discuss and investigate after
reverting the changes. Let's not break any systems meanwhile.
I think that adding support for odd baud rates (and the improved device
detection) can be implemented in a cleaner way and want to have a chance
to discuss that with you.
Thanks,
Johan
--
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
