linux-kernel - Re: [PATCH v7 2/2] PCI: Rockchip: Add Rockchip PCIe controller support

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] [day] [month] [year] [list]

Message-ID: <20160727215739.GV11410@windriver.com>
Date:	Wed, 27 Jul 2016 17:57:39 -0400
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	Brian Norris <briannorris@...omium.org>
CC:	Bjorn Helgaas <helgaas@...nel.org>,
	Shawn Lin <shawn.lin@...k-chips.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Marc Zyngier <marc.zyngier@....com>,
	<linux-pci@...r.kernel.org>, Arnd Bergmann <arnd@...db.de>,
	LKML <linux-kernel@...r.kernel.org>,
	<linux-rockchip@...ts.infradead.org>,
	Heiko Stuebner <heiko@...ech.de>,
	Doug Anderson <dianders@...omium.org>,
	Wenrui Li <wenrui.li@...k-chips.com>,
	Rob Herring <robh+dt@...nel.org>, <devicetree@...r.kernel.org>,
	Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH v7 2/2] PCI: Rockchip: Add Rockchip PCIe controller
 support

[Re: [PATCH v7 2/2] PCI: Rockchip: Add Rockchip PCIe controller support] On 27/07/2016 (Wed 13:04) Brian Norris wrote:

> + Guenter
> 
> Hi,
> 
> On Wed, Jul 27, 2016 at 03:56:36PM -0400, Paul Gortmaker wrote:
> > On Wed, Jul 27, 2016 at 2:22 PM, Bjorn Helgaas <helgaas@...nel.org> wrote:
> > > On Mon, Jul 18, 2016 at 08:42:13AM +0800, Shawn Lin wrote:
> > >> This patch adds Rockchip PCIe controller support found
> > >> on RK3399 Soc platform.
> > >>
> > >> Signed-off-by: Shawn Lin <shawn.lin@...k-chips.com>
> > >>
> > >> ---
> > >>
> > >> Changes in v7:
> > >> - make it as a build-in driver
> > 
> > [...]
> > 
> 
> ...
> 
> > [...]
> > 
> > >> +static struct platform_driver rockchip_pcie_driver = {
> > >> +     .driver = {
> > >> +             .name = "rockchip-pcie",
> > >> +             .of_match_table = rockchip_pcie_of_match,
> > >> +     },
> > >> +     .probe = rockchip_pcie_probe,
> > >> +
> > >> +};
> > >> +module_platform_driver(rockchip_pcie_driver);
> > >> +
> > >> +MODULE_AUTHOR("Rockchip Inc");
> > >> +MODULE_DESCRIPTION("Rockchip AXI PCIe driver");
> > >> +MODULE_LICENSE("GPL v2");
> > >
> > > Per your Kconfig, this driver cannot be a module, so remove these MODULE_*
> > > annotations.  This is to follow Paul Gortmaker's recent "Make explicitly
> > > non-modular" work.  Also change the include of <linux/module.h> to
> > > <linux/init.h>.
> > 
> > Thanks!  You beat me to it.    I didn't see any EXPORT_SYMBOL so it doesn't
> > look like we need export.h  added here.   Also, at the risk of stating
> > the obvious,
> > the module_platform_driver becomes builtin_platform_driver.
> 
> Just for reference (not to necessarily disagree here): we've kinda
> ping-ponged on this one. I suggested it could be built as a module, so
> Shawn added it, but apparently we're missing an export:
> 
> https://lkml.org/lkml/2016/7/8/704
> 
> so he reverted back. It's probably best to just leave this built-in and
> resolve the comments Paul and Bjorn made, to avoid too much more
> flip-flopping. If we still want to patch up the __weak
> pci_remap_iospace(), then we can make this a module at a later time.

Agreed, and this was the general consensus we came to for other PCI code
as well.   It might be different if we were deleting hundreds of lines
of code, but we are just making a few simple changes to make it clear
that things aren't modular, and that ensures others don't continue to
copy the same inconsistency into their new driver(s).  It doesn't in any
way prevent a future conversion to tristate if that use case makes sense.

THanks,
Paul.
--

> 
> Regards,
> Brian