On Tue, Oct 18, 2016 at 12:39:23PM -0000, Karl Palsson wrote: > > Johan Hovold <jo...@kernel.org> wrote: > > On Sun, Oct 16, 2016 at 05:09:24PM +0100, Aidan Thornton wrote: > > > On Mon, Oct 10, 2016 at 8:56 PM, Grigori Goronzy <g...@chown.ath.cx> > > > wrote: > > > > On 2016-10-08 23:07, Aidan Thornton wrote: > > > >> > > > >> On Fri, Oct 7, 2016 at 12:30 PM, Johan Hovold <jo...@kernel.org> wrote: > > > >>> > > > >>> Why is this change needed? I see no write to this register in the > > > >>> vendor driver. > > > >> > > > >> > > > >> In principle, it might not be because the value written to register > > > >> 0x18 is probably overwritten by ch341_init_set_baudrate anyway. It's > > > >> there because it's in Grigori's original patch, it looks superficially > > > >> reasonable (corresponds to ENABLE_RX|ENABLE_TX|CS8, as opposed to the > > > >> rather implausible ENABLE_TX|PAR_EVEN|CS5), and given that some > > > >> hardware revisions are apparently a little temperamental I was > > > >> reluctant to remove it without fully understanding why it was added in > > > >> the first place. As the vendor driver does without it might make sense > > > >> to delete the write altogether, but it does do quite a few things > > > >> differently from this driver. Depends what Grigori says and the > > > >> results of actual testing. > > > > Ok, thanks for the explanation. I think we should try to get > > rid of anything apparently redundant eventually, moving towards > > how the vendor driver does things. It would also be good to > > replace more of the magic constants in there to make the code > > more self-describing. > > > > As long this is done in steps that can be (more easily) > > rolled-back in case it turns out we do actually break support > > for some device in the process. > > 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. > 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. > 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. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
