On Wed, Oct 19, 2016 at 09:26:39AM +0200, Grigori Goronzy wrote: > On 2016-10-18 15:45, Johan Hovold wrote: > > On Tue, Oct 18, 2016 at 12:39:23PM -0000, Karl Palsson wrote: > >> Perhaps it needs to be repeated more, but the current driver is > >> so badly broken for so many devices that people simply don't use > >> them. This insistence that any patch trying to fix this driver > >> somehow preserves all the existing undocumented, unexplained and > >> _non-functioning_ bizarre lines of code is.... exhausting. > > > > No one has insisted on that as far as I'm aware. Please re-read my > > reply above where I explicitly say we should be moving towards how > > the vendor driver does things and try to get rid of this legacy, > > reverse-engineered cruft. This still needs to be done using the > > normal kernel development process, which is to do things > > incrementally in self-contained steps of logical changes. > > Well, the out-of-tree CH340 driver I found floating on the net that I > tried didn't work correctly with any of my devices, in fact. So maybe > reverse engineering is still the way to go. The modified > reinitialization on reconfiguration I actually took from that > non-working driver, though. Ok. Have you tried the driver that what recently posted by Winchiphead? https://lkml.kernel.org/r/1466732874-12649-1-git-send-email-tech@xxxxxxxxxxxxxxx That's the one I refer to as the vendor driver, and was hoping would give us an idea of what needs to go in the init sequence for example. The fact that your (and Aidan's) modified baud-rate handling is in accordance with how that driver does things makes me less worried about breaking some legacy device even if that could still happen. [ And if so we need to try to detect such devices and fall back to the current way of setting the line speed, but again, let's worry about that later. ] > >> There's been probably ~6 people now submit patches to this > >> driver, all of which make marked useful improvements to a driver > >> that _is_broken_ and they're all in limbo because "compatibility > >> with unknown magic number XYZ" that cant' be explained anyway. > > > > We still cannot completely disregard current user of this driver, and > > the normal process of incremental changes still applies. > > > > If we have reasonable test coverage for a change, we'll go ahead. > > And in the unlikely event that this breaks some legacy device, we'll > > deal with it then. > > Maybe we can establish a small group of people for testing > compatibility? We're already 4 people discussing here. If the driver > works well for all of the devices the test group has, it should be fine > to push some change. That would be great. Which device types do you have access to? > >> Is there any chance of any improvements to this driver ever > >> landing? The requirements have been so high, when the existing > >> was soooo poor. > > > > Certainly. This series is one line away from getting merged for > > example. > > So let's just undo the change to the initialization sequence and get > this merged. Then we can tune or simplify the init sequence in the next > series. That is the process you propose, right? Yes. That LCR write could either stay or be removed completely from this series, and then the rest can be cleaned up through follow-ons using the vendor driver as inspiration. 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
