linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
@ 2015-12-13  1:41 Paul Gortmaker
  2015-12-13  1:41 ` [PATCH 04/10] drivers/pci: make host/pci-dra7xx.c explicitly non-modular Paul Gortmaker
       [not found] ` <1449970917-12633-1-git-send-email-paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
  0 siblings, 2 replies; 10+ messages in thread
From: Paul Gortmaker @ 2015-12-13  1:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-sh, linux-pci, Paul Gortmaker, Thierry Reding,
	Alexandre Courbot, Pratyush Anand, Michal Simek,
	Kishon Vijay Abraham I, Murali Karicheri, Sören Brinkmann,
	Jason Cooper, Stephen Warren, Simon Horman, linux-tegra,
	linux-omap, linux-arm-kernel, Thomas Petazzoni, Richard Zhu, rfi,
	Ley Foon Tan, Bjorn Helgaas, Lucas Stach

This series of commits is a slice of a larger project to ensure
people don't have dead code for module removal in non-modular
drivers.  Overall there was roughly 5k lines of dead code in the
kernel due to this.  So far we've fixed several areas, like tty,
x86, net, etc. and we continue to work on other areas.

There are several reasons to not use module_init for code that can
never be built as a module, but the big ones are:

 (1) it is easy to accidentally code up unused module_exit and remove code
 (2) it can be misleading when reading the source, thinking it can be
      modular when the Makefile and/or Kconfig prohibit it
 (3) it requires the include of the module.h header file which in turn
     includes nearly everything else.

Here we convert some module_init() calls into device_initcall() and delete
any module_exit and remove code that gets orphaned in the process, for
an overall net code reduction, which is always welcome.

The use of device_initcall ensures that the init function ordering
remains unchanged, but one could argue that PCI host code might be more
appropriate to be handled under subsys_initcall.  Fortunately we can
revisit making this extra change at a later date if desired; it does
not need to happen now, and we reduce the risk of introducing
regressions at this point in time by separating the two changes.

Over half of the drivers changed here already explicitly disallowed any
unbind operations.  For the rest we make them the same, since there is
not really any sensible use case to unbind any built-in bus support that
I can think of.

I have more "avoid module usage in non-modular code" cleanups for the
PCI subsystem, but these all have a common theme and it makes for a
more maintainer friendly series size to just ask to digest these 1st.

Testing was done on linux-next, using an ARCH=arm allmodconfig and then
explicitly building the files changed in this series.  If desired, I
can provide a v4.4-rc4 based branch for merging vs e-mail processing,
since I don't think the underlying baseline is overly important for
this (largely trivial) series of patches.

Paul.
---

Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Ley Foon Tan <lftan@altera.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Murali Karicheri <m-karicheri2@ti.com>
Cc: Pratyush Anand <pratyush.anand@gmail.com>
Cc: Richard Zhu <Richard.Zhu@freescale.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-omap@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: linux-tegra@vger.kernel.org
Cc: rfi@lists.rocketboards.org

Paul Gortmaker (10):
  drivers/pci: make host/pci-imx6.c driver explicitly non-modular
  drivers/pci: make host/pcie-spear13xx.c driver explicitly non-modular
  drivers/pci: make host/pci-mvebu.c explicitly non-modular
  drivers/pci: make host/pci-dra7xx.c explicitly non-modular
  drivers/pci: make host/pci-rcar-gen2.c explicitly non-modular
  drivers/pci: make host/pci-tegra.c explicitly non-modular
  drivers/pci: make host/pcie-rcar.c explicitly non-modular
  drivers/pci: make host/pcie-xilinx.c explicitly non-modular
  drivers/pci: make host/pci-keystone.c explicitly non-modular
  drivers/pci: make host/pcie-altera.c explicitly non-modular

 drivers/pci/host/pci-dra7xx.c     | 31 +++--------------------
 drivers/pci/host/pci-imx6.c       | 12 +++------
 drivers/pci/host/pci-keystone.c   | 21 +++-------------
 drivers/pci/host/pci-mvebu.c      | 11 +++-----
 drivers/pci/host/pci-rcar-gen2.c  | 12 +++------
 drivers/pci/host/pci-tegra.c      | 11 +++-----
 drivers/pci/host/pcie-altera.c    | 12 ++++-----
 drivers/pci/host/pcie-rcar.c      | 11 +++-----
 drivers/pci/host/pcie-spear13xx.c | 10 +++-----
 drivers/pci/host/pcie-xilinx.c    | 53 ++-------------------------------------
 10 files changed, 35 insertions(+), 149 deletions(-)

-- 
2.6.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 04/10] drivers/pci: make host/pci-dra7xx.c explicitly non-modular
  2015-12-13  1:41 [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci* Paul Gortmaker
@ 2015-12-13  1:41 ` Paul Gortmaker
       [not found] ` <1449970917-12633-1-git-send-email-paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
  1 sibling, 0 replies; 10+ messages in thread
From: Paul Gortmaker @ 2015-12-13  1:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Paul Gortmaker, Kishon Vijay Abraham I, Bjorn Helgaas, linux-omap,
	linux-pci

The Kconfig currently controlling compilation of this code is:

drivers/pci/host/Kconfig:config PCI_DRA7XX
drivers/pci/host/Kconfig:       bool "TI DRA7xx PCIe controller"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, and it allows us to drop the ".remove"
code for non-modular drivers.

Since module_platform_driver() uses the same init level priority as
builtin_platform_driver() the init ordering remains unchanged with
this commit.

We don't replace module.h with init.h since the file already has that.

Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-omap@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/pci/host/pci-dra7xx.c | 31 +++----------------------------
 1 file changed, 3 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 8c3688046c02..3769b7654c5d 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -16,7 +16,7 @@
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/of_gpio.h>
 #include <linux/pci.h>
 #include <linux/phy/phy.h>
@@ -451,25 +451,6 @@ err_phy:
 	return ret;
 }
 
-static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
-{
-	struct dra7xx_pcie *dra7xx = platform_get_drvdata(pdev);
-	struct pcie_port *pp = &dra7xx->pp;
-	struct device *dev = &pdev->dev;
-	int count = dra7xx->phy_count;
-
-	if (pp->irq_domain)
-		irq_domain_remove(pp->irq_domain);
-	pm_runtime_put(dev);
-	pm_runtime_disable(dev);
-	while (count--) {
-		phy_power_off(dra7xx->phy[count]);
-		phy_exit(dra7xx->phy[count]);
-	}
-
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int dra7xx_pcie_suspend(struct device *dev)
 {
@@ -553,19 +534,13 @@ static const struct of_device_id of_dra7xx_pcie_match[] = {
 	{ .compatible = "ti,dra7-pcie", },
 	{},
 };
-MODULE_DEVICE_TABLE(of, of_dra7xx_pcie_match);
 
 static struct platform_driver dra7xx_pcie_driver = {
-	.remove		= __exit_p(dra7xx_pcie_remove),
 	.driver = {
 		.name	= "dra7-pcie",
 		.of_match_table = of_dra7xx_pcie_match,
+		.suppress_bind_attrs = true,
 		.pm	= &dra7xx_pcie_pm_ops,
 	},
 };
-
-module_platform_driver_probe(dra7xx_pcie_driver, dra7xx_pcie_probe);
-
-MODULE_AUTHOR("Kishon Vijay Abraham I <kishon@ti.com>");
-MODULE_DESCRIPTION("TI PCIe controller driver");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver_probe(dra7xx_pcie_driver, dra7xx_pcie_probe);
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
       [not found] ` <1449970917-12633-1-git-send-email-paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
@ 2015-12-14  8:19   ` Geert Uytterhoeven
  2015-12-14  8:24     ` Thierry Reding
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-12-14  8:19 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux-sh list, linux-pci, Thierry Reding, Alexandre Courbot,
	Pratyush Anand, Michal Simek, Kishon Vijay Abraham I,
	Murali Karicheri, Sören Brinkmann, Jason Cooper,
	Stephen Warren, Simon Horman, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Thomas Petazzoni, Richard Zhu,
	rfi-g9ZBwUv/Ih/yUk5EbOjzuce+I+R0W71w, Ley Foon Tan

Hi Paul,

On Sun, Dec 13, 2015 at 2:41 AM, Paul Gortmaker
<paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org> wrote:
> This series of commits is a slice of a larger project to ensure
> people don't have dead code for module removal in non-modular
> drivers.  Overall there was roughly 5k lines of dead code in the
> kernel due to this.  So far we've fixed several areas, like tty,
> x86, net, etc. and we continue to work on other areas.
>
> There are several reasons to not use module_init for code that can
> never be built as a module, but the big ones are:
>
>  (1) it is easy to accidentally code up unused module_exit and remove code
>  (2) it can be misleading when reading the source, thinking it can be
>       modular when the Makefile and/or Kconfig prohibit it
>  (3) it requires the include of the module.h header file which in turn
>      includes nearly everything else.
>
> Here we convert some module_init() calls into device_initcall() and delete
> any module_exit and remove code that gets orphaned in the process, for
> an overall net code reduction, which is always welcome.
>
> The use of device_initcall ensures that the init function ordering
> remains unchanged, but one could argue that PCI host code might be more
> appropriate to be handled under subsys_initcall.  Fortunately we can
> revisit making this extra change at a later date if desired; it does
> not need to happen now, and we reduce the risk of introducing
> regressions at this point in time by separating the two changes.
>
> Over half of the drivers changed here already explicitly disallowed any
> unbind operations.  For the rest we make them the same, since there is
> not really any sensible use case to unbind any built-in bus support that
> I can think of.

Personally, I think all of these should become tristate, so distro kernels
don't have to build in PCI(e) support for all SoCs. multi_v7_defconfig kernels
are becoming too big.

That does not preclude making these modules un-unloadable, though.

> Paul Gortmaker (10):
>   drivers/pci: make host/pci-imx6.c driver explicitly non-modular
>   drivers/pci: make host/pcie-spear13xx.c driver explicitly non-modular
>   drivers/pci: make host/pci-mvebu.c explicitly non-modular
>   drivers/pci: make host/pci-dra7xx.c explicitly non-modular
>   drivers/pci: make host/pci-rcar-gen2.c explicitly non-modular
>   drivers/pci: make host/pci-tegra.c explicitly non-modular
>   drivers/pci: make host/pcie-rcar.c explicitly non-modular
>   drivers/pci: make host/pcie-xilinx.c explicitly non-modular
>   drivers/pci: make host/pci-keystone.c explicitly non-modular
>   drivers/pci: make host/pcie-altera.c explicitly non-modular

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
  2015-12-14  8:19   ` [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci* Geert Uytterhoeven
@ 2015-12-14  8:24     ` Thierry Reding
  2015-12-14  8:26       ` Michal Simek
  2015-12-14  8:33       ` Ley Foon Tan
  0 siblings, 2 replies; 10+ messages in thread
From: Thierry Reding @ 2015-12-14  8:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Paul Gortmaker, linux-kernel@vger.kernel.org, Linux-sh list,
	linux-pci, Alexandre Courbot, Pratyush Anand, Michal Simek,
	Kishon Vijay Abraham I, Murali Karicheri, Sören Brinkmann,
	Jason Cooper, Stephen Warren, Simon Horman, linux-tegra,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Thomas Petazzoni, Richard Zhu, rfi

[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]

On Mon, Dec 14, 2015 at 09:19:30AM +0100, Geert Uytterhoeven wrote:
> Hi Paul,
> 
> On Sun, Dec 13, 2015 at 2:41 AM, Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
> > This series of commits is a slice of a larger project to ensure
> > people don't have dead code for module removal in non-modular
> > drivers.  Overall there was roughly 5k lines of dead code in the
> > kernel due to this.  So far we've fixed several areas, like tty,
> > x86, net, etc. and we continue to work on other areas.
> >
> > There are several reasons to not use module_init for code that can
> > never be built as a module, but the big ones are:
> >
> >  (1) it is easy to accidentally code up unused module_exit and remove code
> >  (2) it can be misleading when reading the source, thinking it can be
> >       modular when the Makefile and/or Kconfig prohibit it
> >  (3) it requires the include of the module.h header file which in turn
> >      includes nearly everything else.
> >
> > Here we convert some module_init() calls into device_initcall() and delete
> > any module_exit and remove code that gets orphaned in the process, for
> > an overall net code reduction, which is always welcome.
> >
> > The use of device_initcall ensures that the init function ordering
> > remains unchanged, but one could argue that PCI host code might be more
> > appropriate to be handled under subsys_initcall.  Fortunately we can
> > revisit making this extra change at a later date if desired; it does
> > not need to happen now, and we reduce the risk of introducing
> > regressions at this point in time by separating the two changes.
> >
> > Over half of the drivers changed here already explicitly disallowed any
> > unbind operations.  For the rest we make them the same, since there is
> > not really any sensible use case to unbind any built-in bus support that
> > I can think of.
> 
> Personally, I think all of these should become tristate, so distro kernels
> don't have to build in PCI(e) support for all SoCs. multi_v7_defconfig kernels
> are becoming too big.
> 
> That does not preclude making these modules un-unloadable, though.

Most of these can't be made tristate as-is, because they use symbols
that aren't exported. Many of those symbols can easily be exported, so
its just a matter of getting the respective patches merged. I disagree
with making the modules non-unloadable, though. I have a local branch
with changes necessary to unload the host controller driver and it
works just fine.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
  2015-12-14  8:24     ` Thierry Reding
@ 2015-12-14  8:26       ` Michal Simek
  2015-12-14  8:33       ` Ley Foon Tan
  1 sibling, 0 replies; 10+ messages in thread
From: Michal Simek @ 2015-12-14  8:26 UTC (permalink / raw)
  To: Thierry Reding, Geert Uytterhoeven
  Cc: Paul Gortmaker, linux-kernel@vger.kernel.org, Linux-sh list,
	linux-pci, Alexandre Courbot, Pratyush Anand, Michal Simek,
	Kishon Vijay Abraham I, Murali Karicheri, Sören Brinkmann,
	Jason Cooper, Stephen Warren, Simon Horman, linux-tegra,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Thomas Petazzoni, Richard Zhu, rfi

On 14.12.2015 09:24, Thierry Reding wrote:
> On Mon, Dec 14, 2015 at 09:19:30AM +0100, Geert Uytterhoeven wrote:
>> Hi Paul,
>>
>> On Sun, Dec 13, 2015 at 2:41 AM, Paul Gortmaker
>> <paul.gortmaker@windriver.com> wrote:
>>> This series of commits is a slice of a larger project to ensure
>>> people don't have dead code for module removal in non-modular
>>> drivers.  Overall there was roughly 5k lines of dead code in the
>>> kernel due to this.  So far we've fixed several areas, like tty,
>>> x86, net, etc. and we continue to work on other areas.
>>>
>>> There are several reasons to not use module_init for code that can
>>> never be built as a module, but the big ones are:
>>>
>>>  (1) it is easy to accidentally code up unused module_exit and remove code
>>>  (2) it can be misleading when reading the source, thinking it can be
>>>       modular when the Makefile and/or Kconfig prohibit it
>>>  (3) it requires the include of the module.h header file which in turn
>>>      includes nearly everything else.
>>>
>>> Here we convert some module_init() calls into device_initcall() and delete
>>> any module_exit and remove code that gets orphaned in the process, for
>>> an overall net code reduction, which is always welcome.
>>>
>>> The use of device_initcall ensures that the init function ordering
>>> remains unchanged, but one could argue that PCI host code might be more
>>> appropriate to be handled under subsys_initcall.  Fortunately we can
>>> revisit making this extra change at a later date if desired; it does
>>> not need to happen now, and we reduce the risk of introducing
>>> regressions at this point in time by separating the two changes.
>>>
>>> Over half of the drivers changed here already explicitly disallowed any
>>> unbind operations.  For the rest we make them the same, since there is
>>> not really any sensible use case to unbind any built-in bus support that
>>> I can think of.
>>
>> Personally, I think all of these should become tristate, so distro kernels
>> don't have to build in PCI(e) support for all SoCs. multi_v7_defconfig kernels
>> are becoming too big.
>>
>> That does not preclude making these modules un-unloadable, though.
> 
> Most of these can't be made tristate as-is, because they use symbols
> that aren't exported. Many of those symbols can easily be exported, so
> its just a matter of getting the respective patches merged. I disagree
> with making the modules non-unloadable, though. I have a local branch
> with changes necessary to unload the host controller driver and it
> works just fine.

Great.

Send them out.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
  2015-12-14  8:24     ` Thierry Reding
  2015-12-14  8:26       ` Michal Simek
@ 2015-12-14  8:33       ` Ley Foon Tan
  2015-12-14  9:19         ` Thierry Reding
  1 sibling, 1 reply; 10+ messages in thread
From: Ley Foon Tan @ 2015-12-14  8:33 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Geert Uytterhoeven, Paul Gortmaker, linux-kernel@vger.kernel.org,
	Linux-sh list, linux-pci, Alexandre Courbot, Pratyush Anand,
	Michal Simek, Kishon Vijay Abraham I, Murali Karicheri,
	Sören Brinkmann, Jason Cooper, Stephen Warren, Simon Horman,
	linux-tegra, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Thomas Petazzoni,
	Richard Zhu

On Mon, Dec 14, 2015 at 4:24 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 09:19:30AM +0100, Geert Uytterhoeven wrote:
>> Hi Paul,
>>
>> On Sun, Dec 13, 2015 at 2:41 AM, Paul Gortmaker
>> <paul.gortmaker@windriver.com> wrote:
>> > This series of commits is a slice of a larger project to ensure
>> > people don't have dead code for module removal in non-modular
>> > drivers.  Overall there was roughly 5k lines of dead code in the
>> > kernel due to this.  So far we've fixed several areas, like tty,
>> > x86, net, etc. and we continue to work on other areas.
>> >
>> > There are several reasons to not use module_init for code that can
>> > never be built as a module, but the big ones are:
>> >
>> >  (1) it is easy to accidentally code up unused module_exit and remove code
>> >  (2) it can be misleading when reading the source, thinking it can be
>> >       modular when the Makefile and/or Kconfig prohibit it
>> >  (3) it requires the include of the module.h header file which in turn
>> >      includes nearly everything else.
>> >
>> > Here we convert some module_init() calls into device_initcall() and delete
>> > any module_exit and remove code that gets orphaned in the process, for
>> > an overall net code reduction, which is always welcome.
>> >
>> > The use of device_initcall ensures that the init function ordering
>> > remains unchanged, but one could argue that PCI host code might be more
>> > appropriate to be handled under subsys_initcall.  Fortunately we can
>> > revisit making this extra change at a later date if desired; it does
>> > not need to happen now, and we reduce the risk of introducing
>> > regressions at this point in time by separating the two changes.
>> >
>> > Over half of the drivers changed here already explicitly disallowed any
>> > unbind operations.  For the rest we make them the same, since there is
>> > not really any sensible use case to unbind any built-in bus support that
>> > I can think of.
>>
>> Personally, I think all of these should become tristate, so distro kernels
>> don't have to build in PCI(e) support for all SoCs. multi_v7_defconfig kernels
>> are becoming too big.
>>
>> That does not preclude making these modules un-unloadable, though.
>
> Most of these can't be made tristate as-is, because they use symbols
> that aren't exported. Many of those symbols can easily be exported, so
> its just a matter of getting the respective patches merged. I disagree
> with making the modules non-unloadable, though. I have a local branch
> with changes necessary to unload the host controller driver and it
> works just fine.
>
PCIe host driver that use fixup (DECLARE_PCI_FIXUP_*) can't use tristate.
Fixup region is in kernel region and this region if not updated when
loading a module.

Regards
Ley Foon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
  2015-12-14  8:33       ` Ley Foon Tan
@ 2015-12-14  9:19         ` Thierry Reding
  2015-12-14 10:27           ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2015-12-14  9:19 UTC (permalink / raw)
  To: Ley Foon Tan
  Cc: Geert Uytterhoeven, Paul Gortmaker, linux-kernel@vger.kernel.org,
	Linux-sh list, linux-pci, Alexandre Courbot, Pratyush Anand,
	Michal Simek, Kishon Vijay Abraham I, Murali Karicheri,
	Sören Brinkmann, Jason Cooper, Stephen Warren, Simon Horman,
	linux-tegra, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Thomas Petazzoni,
	Richard Zhu

[-- Attachment #1: Type: text/plain, Size: 3428 bytes --]

On Mon, Dec 14, 2015 at 04:33:51PM +0800, Ley Foon Tan wrote:
> On Mon, Dec 14, 2015 at 4:24 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Mon, Dec 14, 2015 at 09:19:30AM +0100, Geert Uytterhoeven wrote:
> >> Hi Paul,
> >>
> >> On Sun, Dec 13, 2015 at 2:41 AM, Paul Gortmaker
> >> <paul.gortmaker@windriver.com> wrote:
> >> > This series of commits is a slice of a larger project to ensure
> >> > people don't have dead code for module removal in non-modular
> >> > drivers.  Overall there was roughly 5k lines of dead code in the
> >> > kernel due to this.  So far we've fixed several areas, like tty,
> >> > x86, net, etc. and we continue to work on other areas.
> >> >
> >> > There are several reasons to not use module_init for code that can
> >> > never be built as a module, but the big ones are:
> >> >
> >> >  (1) it is easy to accidentally code up unused module_exit and remove code
> >> >  (2) it can be misleading when reading the source, thinking it can be
> >> >       modular when the Makefile and/or Kconfig prohibit it
> >> >  (3) it requires the include of the module.h header file which in turn
> >> >      includes nearly everything else.
> >> >
> >> > Here we convert some module_init() calls into device_initcall() and delete
> >> > any module_exit and remove code that gets orphaned in the process, for
> >> > an overall net code reduction, which is always welcome.
> >> >
> >> > The use of device_initcall ensures that the init function ordering
> >> > remains unchanged, but one could argue that PCI host code might be more
> >> > appropriate to be handled under subsys_initcall.  Fortunately we can
> >> > revisit making this extra change at a later date if desired; it does
> >> > not need to happen now, and we reduce the risk of introducing
> >> > regressions at this point in time by separating the two changes.
> >> >
> >> > Over half of the drivers changed here already explicitly disallowed any
> >> > unbind operations.  For the rest we make them the same, since there is
> >> > not really any sensible use case to unbind any built-in bus support that
> >> > I can think of.
> >>
> >> Personally, I think all of these should become tristate, so distro kernels
> >> don't have to build in PCI(e) support for all SoCs. multi_v7_defconfig kernels
> >> are becoming too big.
> >>
> >> That does not preclude making these modules un-unloadable, though.
> >
> > Most of these can't be made tristate as-is, because they use symbols
> > that aren't exported. Many of those symbols can easily be exported, so
> > its just a matter of getting the respective patches merged. I disagree
> > with making the modules non-unloadable, though. I have a local branch
> > with changes necessary to unload the host controller driver and it
> > works just fine.
> >
> PCIe host driver that use fixup (DECLARE_PCI_FIXUP_*) can't use tristate.
> Fixup region is in kernel region and this region if not updated when
> loading a module.

Interesting, I hadn't thought about that. I suppose this means that the
module will end up containing an unused section with the fixup code. It
might be useful to add a way for that to trigger a warning at build
time.

Perhaps to fix this a mechanism could be introduced to add a table of
fixups to a host controller driver and that will get applied to all
children of the bridge. It could be problematic to cover all of the
different fixup stages, though.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
  2015-12-14  9:19         ` Thierry Reding
@ 2015-12-14 10:27           ` Arnd Bergmann
  2015-12-15 15:16             ` Paul Gortmaker
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2015-12-14 10:27 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Ley Foon Tan, Geert Uytterhoeven, Paul Gortmaker,
	linux-kernel@vger.kernel.org, Linux-sh list, linux-pci,
	Alexandre Courbot, Pratyush Anand, Michal Simek,
	Kishon Vijay Abraham I, Murali Karicheri, Sören Brinkmann,
	Jason Cooper, Stephen Warren, Simon Horman, linux-tegra,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Thomas Petazzoni

On Monday 14 December 2015 10:19:40 Thierry Reding wrote:
> > PCIe host driver that use fixup (DECLARE_PCI_FIXUP_*) can't use tristate.
> > Fixup region is in kernel region and this region if not updated when
> > loading a module.
> 
> Interesting, I hadn't thought about that. I suppose this means that the
> module will end up containing an unused section with the fixup code. It
> might be useful to add a way for that to trigger a warning at build
> time.
> 
> Perhaps to fix this a mechanism could be introduced to add a table of
> fixups to a host controller driver and that will get applied to all
> children of the bridge. It could be problematic to cover all of the
> different fixup stages, though.
> 


I think a lot of the fixups shouldn't really be there in the first place,
they are about stuff that we can fix up in the probe function, or that should
be fixed up in the probe function with some appropriate core support added.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
  2015-12-14 10:27           ` Arnd Bergmann
@ 2015-12-15 15:16             ` Paul Gortmaker
       [not found]               ` <20151215151624.GB2772-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Gortmaker @ 2015-12-15 15:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux-sh list, linux-pci, Rocketboard Maillist, linux-tegra,
	Thierry Reding, Alexandre Courbot, Pratyush Anand, Michal Simek,
	Kishon Vijay Abraham I, Murali Karicheri, Geert Uytterhoeven,
	linux-arm-kernel@lists.infradead.org, Jason Cooper,
	Stephen Warren, Simon Horman, Bjorn Helgaas,
	linux-omap@vger.kernel.org, Sören Brinkmann,
	Thomas Petazzoni

[Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*] On 14/12/2015 (Mon 11:27) Arnd Bergmann wrote:

> On Monday 14 December 2015 10:19:40 Thierry Reding wrote:
> > > PCIe host driver that use fixup (DECLARE_PCI_FIXUP_*) can't use tristate.
> > > Fixup region is in kernel region and this region if not updated when
> > > loading a module.
> > 
> > Interesting, I hadn't thought about that. I suppose this means that the
> > module will end up containing an unused section with the fixup code. It
> > might be useful to add a way for that to trigger a warning at build
> > time.
> > 
> > Perhaps to fix this a mechanism could be introduced to add a table of
> > fixups to a host controller driver and that will get applied to all
> > children of the bridge. It could be problematic to cover all of the
> > different fixup stages, though.
> > 
> 
> 
> I think a lot of the fixups shouldn't really be there in the first place,
> they are about stuff that we can fix up in the probe function, or that should
> be fixed up in the probe function with some appropriate core support added.

So, the feedback on this is a bit all over the map, leaving me unsure
what to do next.  And is the choice we make on a per board/bsp basis or
ideally across all platforms?  I see the choices as:

1) do nothing; which IMHO is least desirable as it leaves the code
misrepresenting itself as modular; one of the key issues I wanted to fix

2) use the patches I've sent ; then as they are genuinely made modular,
the person doing so essentially "patch -R" or reverts the change as
step one.  This has the advantage of solving the "we'll get to it
someday" issue if someday never comes.

3) make them all tristate;  beat it with a stick until it compiles [M]
and modposts -- leaving the fixups and functional testing to people with
the boards and low level knowledge to make it _work_ as a module.  The
downside here is the code is still kind of misrepresenting itself as
modularly functional -- a ban of unloading might mitigate that some.

Paul.
--

> 
> 	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*
       [not found]               ` <20151215151624.GB2772-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
@ 2016-01-08 20:31                 ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-01-08 20:31 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: Arnd Bergmann, Thierry Reding, Ley Foon Tan, Geert Uytterhoeven,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux-sh list, linux-pci, Alexandre Courbot, Pratyush Anand,
	Michal Simek, Kishon Vijay Abraham I, Murali Karicheri,
	Sören Brinkmann, Jason Cooper, Stephen Warren, Simon Horman,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Thomas Petazzoni

On Tue, Dec 15, 2015 at 10:16:24AM -0500, Paul Gortmaker wrote:
> [Re: [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci*] On 14/12/2015 (Mon 11:27) Arnd Bergmann wrote:
> 
> > On Monday 14 December 2015 10:19:40 Thierry Reding wrote:
> > > > PCIe host driver that use fixup (DECLARE_PCI_FIXUP_*) can't use tristate.
> > > > Fixup region is in kernel region and this region if not updated when
> > > > loading a module.
> > > 
> > > Interesting, I hadn't thought about that. I suppose this means that the
> > > module will end up containing an unused section with the fixup code. It
> > > might be useful to add a way for that to trigger a warning at build
> > > time.
> > > 
> > > Perhaps to fix this a mechanism could be introduced to add a table of
> > > fixups to a host controller driver and that will get applied to all
> > > children of the bridge. It could be problematic to cover all of the
> > > different fixup stages, though.
> > 
> > I think a lot of the fixups shouldn't really be there in the first place,
> > they are about stuff that we can fix up in the probe function, or that should
> > be fixed up in the probe function with some appropriate core support added.
> 
> So, the feedback on this is a bit all over the map, leaving me unsure
> what to do next.  And is the choice we make on a per board/bsp basis or
> ideally across all platforms?  I see the choices as:
> 
> 1) do nothing; which IMHO is least desirable as it leaves the code
> misrepresenting itself as modular; one of the key issues I wanted to fix
> 
> 2) use the patches I've sent ; then as they are genuinely made modular,
> the person doing so essentially "patch -R" or reverts the change as
> step one.  This has the advantage of solving the "we'll get to it
> someday" issue if someday never comes.
> 
> 3) make them all tristate;  beat it with a stick until it compiles [M]
> and modposts -- leaving the fixups and functional testing to people with
> the boards and low level knowledge to make it _work_ as a module.  The
> downside here is the code is still kind of misrepresenting itself as
> modularly functional -- a ban of unloading might mitigate that some.

I'd like to preserve the mind-set that host controller drivers are
*expected* to be modular, even if we aren't there yet.  I guess that
means I'm in favor of option 3, at least for drivers that don't use
fixups.

Bjorn

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-01-08 20:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-13  1:41 [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci* Paul Gortmaker
2015-12-13  1:41 ` [PATCH 04/10] drivers/pci: make host/pci-dra7xx.c explicitly non-modular Paul Gortmaker
     [not found] ` <1449970917-12633-1-git-send-email-paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
2015-12-14  8:19   ` [PATCH 00/10] drivers/pci: avoid module_init in non-modular host/pci* Geert Uytterhoeven
2015-12-14  8:24     ` Thierry Reding
2015-12-14  8:26       ` Michal Simek
2015-12-14  8:33       ` Ley Foon Tan
2015-12-14  9:19         ` Thierry Reding
2015-12-14 10:27           ` Arnd Bergmann
2015-12-15 15:16             ` Paul Gortmaker
     [not found]               ` <20151215151624.GB2772-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
2016-01-08 20:31                 ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).