Am 01.11.2013 12:15, schrieb Johan Hovold: > On Fri, Nov 01, 2013 at 11:44:33AM +0100, Frank Schäfer wrote: >> Am 01.11.2013 11:22, schrieb Johan Hovold: >>>>> 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. >> Well, unless you don't find another regression, I see no reason to >> revert. ;) >> >>> However, we have a clear regression and 3.12 is coming out very, very >>> soon. >> Indeed, but fortunately we are also close to a solution/decision. >> >>>> 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. >> Well, Mika and you have have such a device, so you can test it. > I'm afraid I can't just drop everything else I'm working with to do that > testing at the moment. No problem. :) In the meantime, Mika has tested it and the improved divisor method works fine. > >> Alternatively, we can apply the patch I've sent that restores the old >> behavior (of course without the calculation of the resulting baud rate). > But that's another change. Late change. [superseeded] > >>> 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. >> Ok, between the lines I can read that you just don't trust this patch >> series. ;) >> I'm looking forward to your suggestions for a cleaner way to implement >> this. :) > You have done a great job here in reverse-engineering the chip and > documenting the quirks. It's not an easy job at all with all the > different chips out there, including pirate ones. > > I just think we should take the safe way here and hold off the changes > another cycle. Get some more people to do more testing (which chip types > do we have among us at the moment?). This would also allow us to come up > with a series which abstracts the differences in a cleaner way rather > than spreading this information all over the driver. I'm aware that it > has grown to that state incrementally, but at this point I think it's > clear that some abstraction is needed. You're the maintainer, the decission is of course up to you. I have generally no problem with reverting patches. If something isn't ready or plain crap, it should of course be reverted. But AFAICS, the problem here isn't the patch series itself. It's just that this series unveils another long standing bug. Please also consider that the patch I've sent 5 minutes ago should also needs to be applied to stable. We have the same issue there and it's not restricted to the divisor based baud rate encoding method. The same can happen with the direct encoding method... So IMHO, going a step forward and fixing this bug is the better solution. Regards, Frank > I also believe using the direct encoding method also for (known, > supported) baudrates > 115200, as you suggested somewhere, should be > considered (and only fall back to divisor method for other rates if the > chip supports it). > > 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
