I managed to reproduce this old issue, both on vanilla v4.1.1 and with
my patch, IF and ONLY if I reverted commit 623c82633 by changing:
- if (!old_termios || memcmp(buf, priv->line_settings, 7)) {
ret = pl2303_set_line_request(port, buf);
if (!ret)
memcpy(priv->line_settings, buf, 7);
- }
I also added 'printk("Hello")' to pl2303_set_line_request.
After that, spawning agetty on the pl2303, logging in through FTDI
and listing a directory with two dozen files indeed outputs mangled
filenames. Hello is printed twice for each ls.
No problems happen with getty on FTDI and minicom on pl2303.
The culprit seems to be bash, calling TCGETS/TCSETSW on its tty before
and after every command to toggle some flags (echo, icanon, ...).
Nothing bad happens with standard baud rates, i.e. bash still calls
these ioctls, but there are no Hellos and no corruption.
I also tried forcing divisor encoding on all baud rates with 623c82633
reverted, like it was done back in the days of 3.12-rc. In this
configuration, Hellos and corruptions happen for stardard baud rates
if and only if the driver reports baud rate different than requested.
In the end, my conlusions are as follows:
1. The corruption is caused by repeated attempts to set baud rate which
can't be exactly realized. It has nothing to do with divisor encoding.
2. 623c82633 protects us no matter what bytes we send to the chip
and what we return to the tty stack (until the userspace really sets a
different rate).
3. Without 623c82633, we can only protect ourselves by lying about the
actual rate. It seems that tty_termios_hw_change saves us in this case.
That's why encode_direct and pre-57ce61aad748 encode_divisor work, but
57ce61aad748 failed. It came before 623c82633.
Bottom line: my patch seems safe and fixes custom baud rates below 94k,
which are completely screwed without it.
The only thing I could imagine going wrong is chips which actually
interpret baud rate settings the way described in the old comment.
This definitely isn't my HX (rev A) nor my other HX knockoff.
This also isn't whatever chips Frank Schäfer used during 57ce61aad748
development.
Finally, this probably isn't this comment author's chip either. I bet
he wrote the comment and then randomly tweaked the code until it started
working with actual hardware without realizing that the comment is wrong
and doesn't describe the code anymore.
> On Mon, Jul 13, 2015 at 06:08:50PM +0200, Michał Pecio wrote:
> > > Commit 57ce61aad748 might be helpful... ;)
> > >
> > > Good luck,
> > > Frank
> > >
> > >
> >
> > Pretty much the same thing I have done, except that I didn't notice
> > that 0 = 512 :)
> >
> > Apparently, 57ce61aad748 fell victim of a mass-revert caused by some
> > underdebugged issues. Is it known what they were? Is there any
> > chance of getting this driver fixed again?
>
> Yes, that series caused some regressions late in the release cycle and
> was reverted so that it could get some more review and testing.
>
> There are some details in this thread:
>
> https://marc.info/?l=linux-usb&m=138325299205954
>
> If you want to pick this up and improve the divisor calculations
> that'd be great.
>
> 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
