linux-kernel - Re: [PATCH 0/3] 8250: Split Fintek PCIE to UART to independent file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 19 Jan 2016 16:45:55 +0800
From:	Peter Hung <hpeter@...il.com>
To:	Paul Gortmaker <paul.gortmaker@...driver.com>
Cc:	gregkh@...uxfoundation.org, jslaby@...e.com,
	andriy.shevchenko@...ux.intel.com, heikki.krogerus@...ux.intel.com,
	peter@...leysoftware.com, soeren.grunewald@...y.de,
	udknight@...il.com, adam.lee@...onical.com, arnd@...db.de,
	yamada.masahiro@...ionext.com, mans@...sr.com,
	scottwood@...escale.com, paul.burton@...tec.com,
	matthias.bgg@...il.com, manabian@...il.com, peter.ujfalusi@...com,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	peter_hong@...tek.com.tw,
	Peter Hung <hpeter+linux_kernel@...il.com>
Subject: Re: [PATCH 0/3] 8250: Split Fintek PCIE to UART to independent file

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?

> 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.

>
> 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.

>>    8250_fintek_pci: Add Fintek PCIE UART driver
>
> This creates a new Kconfig var. which is default=m.  How does that work
> if people were using these for built-in early console support in the
> past?  Are these cards universal, or should it be default=m if (...)
> based on a Kconfig where this hardware exists?

Thanks for point this out, for the early console I should make the
default mode to SERIAL_8250 if it need to split as a new file.

>>    8250_fintek_pci: Add GPIOLIB support
>
> What does this add?  The commit log is not at all clear.  Leaving me to
> ask if it does belong in the core PCI support code at all?  I honestly
> don't know, since I don't know the hardware details here.  The commit
> long logs could go a long way to closing this knowledge gap if the 0/N
> listed the shortcomings and the 3/3 here indicated what the GPIO magic
> had managed to add.

Sorry for the ambiguous logs. We'll implement GPIOLIB due to the
following circumstance.

Some H/W manufacturer use this IC and transform some port into GPIO
mode. The current 8250_pci.c not handle this so it maybe confuse
end-user.

> Again, this may be obvious to others, but the long logs should try and
> give a hint to people on the fringe who maybe don't have all the
> specific Fintek hardware details when reading the logs.
>

I'll try to make more sense with long long.
Thanks for your advices,
-- 
With Best Regards,
Peter Hung