Messages in this thread | | | Subject | Re: [PATCH 0/3] 8250: Split Fintek PCIE to UART to independent file | From | Andy Shevchenko <> | Date | Tue, 19 Jan 2016 11:33:44 +0200 |
| |
On Tue, 2016-01-19 at 16:45 +0800, Peter Hung wrote: > Hi Paul, > > Paul Gortmaker 於 2016/1/19 上午 11:56 寫道: > > > The serial ports support from 50bps to 1.5Mbps with Linux > > > baudrate > > > define excluding 1.0Mbps due to not support 16MHz clock source. > > > > How does this differ from what was achieved or possible with the > > old way > > of things? What was the limitation in the existing 8250 code > > sharing > > that required Fintek code to fork and become independent? > > The architecture of 8250_pci.c is good for PCIE device with 8250 > compatible serial ports. We want to implement all functions of > F81504/508/512, but it'll make 8250_pci.c bloated and complex if we > implement GPIOLIB in 8250_pci.c > > Could I implement GPIOLIB within 8250_pci.c instead of a newer file?
Hm… So, can we stick with separate driver, or you're gonna shake for each reviewer's comment?
> > > How much code was just copied 8250 boilerplate vs. being a new > > implementation? The diffstat shows approx 500 lines of new > > code. What > > does that add vs. just copying? > > Due to this IC contains 8250-compatible ports, the most functions is > copy from fintek section of 8250_pci.c. The differences are highbaud > rate & GPIOLIB implementations.
I agree with Paul, I think what you have done is to:
1) split out existing code to separate driver (no your changes, but minimum necessary to this split) — one patch! 2) clean up it (at least I see the old PM code which should be refactored) 3) enhance functionality accordingly to what you need.
> > > > > If someone had 8250 (PCI) builtin before, and Fintek stops working, > > they will most guaranteed bisect to this commit above where you > > remove > > support. That is less than ideal. We try to avoid code deletions > > or > > Kconfig addtions that will be obvious bisect magnets. > > It can be prevented if implements GPIOLIB in 8250_pci.c.
Yeah, see item 1) above.
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |