On Tue, 2016-01-19 at 16:45 +0800, Peter Hung wrote:

Hi Paul,

Paul Gortmaker 於 2016/1/19 上午 11:56 寫道:

quoted

quoted

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?

quoted

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.

quoted

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 [off-list ref]
Intel Finland Oy