On Thu, Oct 31, 2013 at 02:03:22PM -0700, Greg KH wrote:
> On Thu, Oct 31, 2013 at 09:56:25PM +0100, Johan Hovold wrote:
> > 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.
> >
> > > > 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.
> >
> > 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:
>
> Ick :(
>
> Would fixing the "return the baudrate" issue solve this in an easier way
> than to revert all of these? That might be an easier, and smaller,
> patch that can get added to 3.12-stable, right?
It would possibly be sufficient, but I don't have much time the next
couple of days to do much further digging and verification of this.
There could be other ways to trigger the problem and I'd like to get a
chance to look into that as well.
> > 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
>
> Or I can just revert all of them for 3.12-stable as well, let me know
> which one you want me to do.
Yes, I think we should revert (if possible before 3.12 is out), clean up
the above series and do some more verification before reapplying as a
single, clean series (there appears to already be a regression with fix
within the patches listed above).
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
