From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com --- Documentation/driver-model/platform.txt | 11 ++++++ drivers/base/platform.c | 60 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 5 +++ 3 files changed, 76 insertions(+)
diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt index 07795ec51cde..e80468738ba9 100644 --- a/Documentation/driver-model/platform.txt +++ b/Documentation/driver-model/platform.txt @@ -63,6 +63,17 @@ runtime memory footprint: int platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *))
+Kernel modules can be composed of several platform drivers. The platform core +provides helpers to register and unregister an array of drivers: + + int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count); + void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count); + +If one of the drivers fails to register, all drivers registered up to that +point will be unregistered in reverse order. +
Device Enumeration ~~~~~~~~~~~~~~~~~~ diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..b7d7987fda97 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,66 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle);
+/** + * platform_register_drivers - register an array of platform drivers + * @drivers: an array of drivers to register + * @count: the number of drivers to register + * + * Registers platform drivers specified by an array. On failure to register a + * driver, all previously registered drivers will be unregistered. Callers of + * this API should use platform_unregister_drivers() to unregister drivers in + * the reverse order. + * + * Returns: 0 on success or a negative error code on failure. + */ +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + unsigned int i; + int err; + + for (i = 0; i < count; i++) { + pr_debug("registering platform driver %ps\n", drivers[i]); + + err = platform_driver_register(drivers[i]); + if (err < 0) { + pr_err("failed to register platform driver %ps: %d\n", + drivers[i], err); + goto error; + } + } + + return 0; + +error: + while (i--) { + pr_debug("unregistering platform driver %ps\n", drivers[i]); + platform_driver_unregister(drivers[i]); + } + + return err; +} +EXPORT_SYMBOL_GPL(platform_register_drivers); + +/** + * platform_unregister_drivers - unregister an array of platform drivers + * @drivers: an array of drivers to unregister + * @count: the number of drivers to unregister + * + * Unegisters platform drivers specified by an array. This is typically used + * to complement an earlier call to platform_register_drivers(). Drivers are + * unregistered in the reverse order in which they were registered. + */ +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + while (count--) { + pr_debug("unregistering platform driver %ps\n", drivers[count]); + platform_driver_unregister(drivers[count]); + } +} +EXPORT_SYMBOL_GPL(platform_unregister_drivers); + /* modalias support enables more hands-off userspace setup: * (a) environment variable lets new-style hotplug events work once system is * fully running: "modprobe $MODALIAS" diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index bba08f44cc97..0c9f16bfdd99 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -270,6 +270,11 @@ extern struct platform_device *__platform_create_bundle( struct resource *res, unsigned int n_res, const void *data, size_t size, struct module *module);
+int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count); +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count); + /* early platform driver interface */ struct early_platform_driver { const char *class_str;
From: Thierry Reding treding@nvidia.com
Use the new multi-driver module helpers to get rid of some boilerplate in the module initialization and cleanup functions.
Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/tegra/drm.c | 56 ++++++++++----------------------------------- drivers/gpu/drm/tegra/drm.h | 4 ++-- 2 files changed, 14 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 52ba57aa65f2..52df741b9f64 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -1088,6 +1088,16 @@ static struct host1x_driver host1x_drm_driver = { .subdevs = host1x_drm_subdevs, };
+static struct platform_driver * const drivers[] = { + &tegra_dc_driver, + &tegra_hdmi_driver, + &tegra_dsi_driver, + &tegra_dpaux_driver, + &tegra_sor_driver, + &tegra_gr2d_driver, + &tegra_gr3d_driver, +}; + static int __init host1x_drm_init(void) { int err; @@ -1096,48 +1106,12 @@ static int __init host1x_drm_init(void) if (err < 0) return err;
- err = platform_driver_register(&tegra_dc_driver); + err = platform_register_drivers(drivers, ARRAY_SIZE(drivers)); if (err < 0) goto unregister_host1x;
- err = platform_driver_register(&tegra_dsi_driver); - if (err < 0) - goto unregister_dc; - - err = platform_driver_register(&tegra_sor_driver); - if (err < 0) - goto unregister_dsi; - - err = platform_driver_register(&tegra_hdmi_driver); - if (err < 0) - goto unregister_sor; - - err = platform_driver_register(&tegra_dpaux_driver); - if (err < 0) - goto unregister_hdmi; - - err = platform_driver_register(&tegra_gr2d_driver); - if (err < 0) - goto unregister_dpaux; - - err = platform_driver_register(&tegra_gr3d_driver); - if (err < 0) - goto unregister_gr2d; - return 0;
-unregister_gr2d: - platform_driver_unregister(&tegra_gr2d_driver); -unregister_dpaux: - platform_driver_unregister(&tegra_dpaux_driver); -unregister_hdmi: - platform_driver_unregister(&tegra_hdmi_driver); -unregister_sor: - platform_driver_unregister(&tegra_sor_driver); -unregister_dsi: - platform_driver_unregister(&tegra_dsi_driver); -unregister_dc: - platform_driver_unregister(&tegra_dc_driver); unregister_host1x: host1x_driver_unregister(&host1x_drm_driver); return err; @@ -1146,13 +1120,7 @@ module_init(host1x_drm_init);
static void __exit host1x_drm_exit(void) { - platform_driver_unregister(&tegra_gr3d_driver); - platform_driver_unregister(&tegra_gr2d_driver); - platform_driver_unregister(&tegra_dpaux_driver); - platform_driver_unregister(&tegra_hdmi_driver); - platform_driver_unregister(&tegra_sor_driver); - platform_driver_unregister(&tegra_dsi_driver); - platform_driver_unregister(&tegra_dc_driver); + platform_unregister_drivers(drivers, ARRAY_SIZE(drivers)); host1x_driver_unregister(&host1x_drm_driver); } module_exit(host1x_drm_exit); diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 112e07d81557..830a7170f4b0 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -277,10 +277,10 @@ void tegra_fb_output_poll_changed(struct drm_device *drm); #endif
extern struct platform_driver tegra_dc_driver; -extern struct platform_driver tegra_dsi_driver; -extern struct platform_driver tegra_sor_driver; extern struct platform_driver tegra_hdmi_driver; +extern struct platform_driver tegra_dsi_driver; extern struct platform_driver tegra_dpaux_driver; +extern struct platform_driver tegra_sor_driver; extern struct platform_driver tegra_gr2d_driver; extern struct platform_driver tegra_gr3d_driver;
From: Thierry Reding treding@nvidia.com
There's no use building the individual drivers as separate modules because they are all only useful if combined into a single DRM/KMS device.
Cc: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/imx/Kconfig | 15 +++++++-------- drivers/gpu/drm/imx/Makefile | 17 +++++++---------- drivers/gpu/drm/imx/dw_hdmi-imx.c | 4 +--- drivers/gpu/drm/imx/imx-drm-core.c | 22 +++++++++++++++++++++- drivers/gpu/drm/imx/imx-drm.h | 6 ++++++ drivers/gpu/drm/imx/imx-ldb.c | 4 +--- drivers/gpu/drm/imx/imx-tve.c | 4 +--- drivers/gpu/drm/imx/ipuv3-crtc.c | 3 +-- drivers/gpu/drm/imx/parallel-display.c | 4 +--- 9 files changed, 46 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig index 2b81a417cf29..999c21ba94b7 100644 --- a/drivers/gpu/drm/imx/Kconfig +++ b/drivers/gpu/drm/imx/Kconfig @@ -11,7 +11,7 @@ config DRM_IMX enable i.MX graphics support
config DRM_IMX_FB_HELPER - tristate "provide legacy framebuffer /dev/fb0" + bool "provide legacy framebuffer /dev/fb0" select DRM_KMS_CMA_HELPER depends on DRM_IMX help @@ -20,13 +20,13 @@ config DRM_IMX_FB_HELPER and also for applications using the legacy framebuffer API
config DRM_IMX_PARALLEL_DISPLAY - tristate "Support for parallel displays" + bool "Support for parallel displays" select DRM_PANEL depends on DRM_IMX select VIDEOMODE_HELPERS
config DRM_IMX_TVE - tristate "Support for TV and VGA displays" + bool "Support for TV and VGA displays" depends on DRM_IMX select REGMAP_MMIO help @@ -34,7 +34,7 @@ config DRM_IMX_TVE found on i.MX53 processors.
config DRM_IMX_LDB - tristate "Support for LVDS displays" + bool "Support for LVDS displays" depends on DRM_IMX && MFD_SYSCON select DRM_PANEL help @@ -42,14 +42,13 @@ config DRM_IMX_LDB found on i.MX53 and i.MX6 processors.
config DRM_IMX_IPUV3 - tristate + bool depends on DRM_IMX depends on IMX_IPUV3_CORE - default y if DRM_IMX=y - default m if DRM_IMX=m + default y
config DRM_IMX_HDMI - tristate "Freescale i.MX DRM HDMI" + bool "Freescale i.MX DRM HDMI" select DRM_DW_HDMI depends on DRM_IMX help diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile index f3ecd8903d97..48b3844928d9 100644 --- a/drivers/gpu/drm/imx/Makefile +++ b/drivers/gpu/drm/imx/Makefile @@ -1,12 +1,9 @@ +imx-drm-y := imx-drm-core.o
-imxdrm-objs := imx-drm-core.o +imx-drm-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o +imx-drm-$(CONFIG_DRM_IMX_TVE) += imx-tve.o +imx-drm-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o +imx-drm-$(CONFIG_DRM_IMX_IPUV3) += ipuv3-crtc.o ipuv3-plane.o +imx-drm-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
-obj-$(CONFIG_DRM_IMX) += imxdrm.o - -obj-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o -obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o -obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o - -imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o -obj-$(CONFIG_DRM_IMX_IPUV3) += imx-ipuv3-crtc.o -obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o +obj-$(CONFIG_DRM_IMX) += imx-drm.o diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c index 644edf65dbe0..eacf32d36c7b 100644 --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c @@ -271,7 +271,7 @@ static int dw_hdmi_imx_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver dw_hdmi_imx_platform_driver = { +struct platform_driver dw_hdmi_imx_platform_driver = { .probe = dw_hdmi_imx_probe, .remove = dw_hdmi_imx_remove, .driver = { @@ -280,8 +280,6 @@ static struct platform_driver dw_hdmi_imx_platform_driver = { }, };
-module_platform_driver(dw_hdmi_imx_platform_driver); - MODULE_AUTHOR("Andy Yan andy.yan@rock-chips.com"); MODULE_AUTHOR("Yakir Yang ykk@rock-chips.com"); MODULE_DESCRIPTION("IMX6 Specific DW-HDMI Driver Extension"); diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 40950e15b759..4b49a7cb0a14 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -636,7 +636,27 @@ static struct platform_driver imx_drm_pdrv = { .of_match_table = imx_drm_dt_ids, }, }; -module_platform_driver(imx_drm_pdrv); + +static struct platform_driver * const drivers[] = { + &ipu_drm_driver, + &imx_ldb_driver, + &imx_pd_driver, + &dw_hdmi_imx_platform_driver, + &imx_tve_driver, + &imx_drm_pdrv, +}; + +static int imx_drm_init(void) +{ + return platform_register_drivers(drivers, ARRAY_SIZE(drivers)); +} +module_init(imx_drm_init); + +static void imx_drm_exit(void) +{ + platform_unregister_drivers(drivers, ARRAY_SIZE(drivers)); +} +module_exit(imx_drm_exit);
MODULE_AUTHOR("Sascha Hauer s.hauer@pengutronix.de"); MODULE_DESCRIPTION("i.MX drm driver core"); diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h index eebf0e2fefd0..87bdbf5cabb1 100644 --- a/drivers/gpu/drm/imx/imx-drm.h +++ b/drivers/gpu/drm/imx/imx-drm.h @@ -53,4 +53,10 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, void imx_drm_connector_destroy(struct drm_connector *connector); void imx_drm_encoder_destroy(struct drm_encoder *encoder);
+extern struct platform_driver ipu_drm_driver; +extern struct platform_driver imx_ldb_driver; +extern struct platform_driver imx_pd_driver; +extern struct platform_driver dw_hdmi_imx_platform_driver; +extern struct platform_driver imx_tve_driver; + #endif /* _IMX_DRM_H_ */ diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index abacc8f67469..3fcb221accd7 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -673,7 +673,7 @@ static int imx_ldb_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver imx_ldb_driver = { +struct platform_driver imx_ldb_driver = { .probe = imx_ldb_probe, .remove = imx_ldb_remove, .driver = { @@ -682,8 +682,6 @@ static struct platform_driver imx_ldb_driver = { }, };
-module_platform_driver(imx_ldb_driver); - MODULE_DESCRIPTION("i.MX LVDS driver"); MODULE_AUTHOR("Sascha Hauer, Pengutronix"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index e671ad369416..cf6d0b171074 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -722,7 +722,7 @@ static const struct of_device_id imx_tve_dt_ids[] = { { /* sentinel */ } };
-static struct platform_driver imx_tve_driver = { +struct platform_driver imx_tve_driver = { .probe = imx_tve_probe, .remove = imx_tve_remove, .driver = { @@ -731,8 +731,6 @@ static struct platform_driver imx_tve_driver = { }, };
-module_platform_driver(imx_tve_driver); - MODULE_DESCRIPTION("i.MX Television Encoder driver"); MODULE_AUTHOR("Philipp Zabel, Pengutronix"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c index a10da8e011c2..af48ffee752f 100644 --- a/drivers/gpu/drm/imx/ipuv3-crtc.c +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c @@ -501,14 +501,13 @@ static int ipu_drm_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver ipu_drm_driver = { +struct platform_driver ipu_drm_driver = { .driver = { .name = "imx-ipuv3-crtc", }, .probe = ipu_drm_probe, .remove = ipu_drm_remove, }; -module_platform_driver(ipu_drm_driver);
MODULE_AUTHOR("Sascha Hauer s.hauer@pengutronix.de"); MODULE_DESCRIPTION(DRIVER_DESC); diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index b4deb9cf9d71..ec61b5753bc6 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -295,7 +295,7 @@ static const struct of_device_id imx_pd_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, imx_pd_dt_ids);
-static struct platform_driver imx_pd_driver = { +struct platform_driver imx_pd_driver = { .probe = imx_pd_probe, .remove = imx_pd_remove, .driver = { @@ -304,8 +304,6 @@ static struct platform_driver imx_pd_driver = { }, };
-module_platform_driver(imx_pd_driver); - MODULE_DESCRIPTION("i.MX parallel display driver"); MODULE_AUTHOR("Sascha Hauer, Pengutronix"); MODULE_LICENSE("GPL");
Hi Thierry,
Am Donnerstag, den 24.09.2015, 19:02 +0200 schrieb Thierry Reding:
From: Thierry Reding treding@nvidia.com
There's no use building the individual drivers as separate modules because they are all only useful if combined into a single DRM/KMS device.
Why not? On an i.MX5 device with VGA(tve) output only, it is nice not to have to load the i.MX6 HDMI driver.
regards Philipp
On Fri, Sep 25, 2015 at 09:16:06AM +0200, Philipp Zabel wrote:
Hi Thierry,
Am Donnerstag, den 24.09.2015, 19:02 +0200 schrieb Thierry Reding:
From: Thierry Reding treding@nvidia.com
There's no use building the individual drivers as separate modules because they are all only useful if combined into a single DRM/KMS device.
Why not? On an i.MX5 device with VGA(tve) output only, it is nice not to have to load the i.MX6 HDMI driver.
I think you gain much less by splitting up than you realize. Compare this from before the series:
$ du -ch drivers/gpu/drm/imx/*.ko 8.0K drivers/gpu/drm/imx/dw_hdmi-imx.ko 16K drivers/gpu/drm/imx/imxdrm.ko 16K drivers/gpu/drm/imx/imx-ipuv3-crtc.ko 12K drivers/gpu/drm/imx/imx-ldb.ko 12K drivers/gpu/drm/imx/imx-tve.ko 8.0K drivers/gpu/drm/imx/parallel-display.ko 72K total
$ du -ch drivers/gpu/drm/sti/*.ko 36K drivers/gpu/drm/sti/sticompositor.ko 12K drivers/gpu/drm/sti/sti_drv.ko 16K drivers/gpu/drm/sti/stidvo.ko 16K drivers/gpu/drm/sti/sti_hda.ko 24K drivers/gpu/drm/sti/stihdmi.ko 8.0K drivers/gpu/drm/sti/sti_hqvdp.ko 12K drivers/gpu/drm/sti/sti_tvout.ko 8.0K drivers/gpu/drm/sti/sti_vtac.ko 8.0K drivers/gpu/drm/sti/sti_vtg.ko 140K total
with this after the series:
$ du -ch drivers/gpu/drm/imx/*.ko 44K drivers/gpu/drm/imx/imx-drm.ko 44K total
$ du -ch drivers/gpu/drm/sti/*.ko 92K drivers/gpu/drm/sti/sti-drm.ko 92K total
There are other things to consider as well, such as the additional memory overhead caused by merely having multiple modules, or all of the exported functions that unnecessarily clutter up the symbol table, and which end up slowing down every symbol lookup.
Thierry
Am Freitag, den 25.09.2015, 14:17 +0200 schrieb Thierry Reding:
I think you gain much less by splitting up than you realize. Compare this from before the series:
$ du -ch drivers/gpu/drm/imx/*.ko 8.0K drivers/gpu/drm/imx/dw_hdmi-imx.ko
Oh right, I didn't realize that this is just the shim. The bulk of the HDMI driver is in bridge/dw_hdmi.ko.
16K drivers/gpu/drm/imx/imxdrm.ko 16K drivers/gpu/drm/imx/imx-ipuv3-crtc.ko 12K drivers/gpu/drm/imx/imx-ldb.ko 12K drivers/gpu/drm/imx/imx-tve.ko 8.0K drivers/gpu/drm/imx/parallel-display.ko 72K total
[...]
with this after the series:
$ du -ch drivers/gpu/drm/imx/*.ko 44K drivers/gpu/drm/imx/imx-drm.ko 44K total
[...]
There are other things to consider as well, such as the additional memory overhead caused by merely having multiple modules, or all of the exported functions that unnecessarily clutter up the symbol table, and which end up slowing down every symbol lookup.
Thanks, can't argue with those numbers. I'll queue patches 3 and 4 for imx-drm.
regards Philipp
Am Freitag, den 25.09.2015, 15:09 +0200 schrieb Philipp Zabel:
Am Freitag, den 25.09.2015, 14:17 +0200 schrieb Thierry Reding:
I think you gain much less by splitting up than you realize. Compare this from before the series:
$ du -ch drivers/gpu/drm/imx/*.ko 8.0K drivers/gpu/drm/imx/dw_hdmi-imx.ko
Oh right, I didn't realize that this is just the shim. The bulk of the HDMI driver is in bridge/dw_hdmi.ko.
16K drivers/gpu/drm/imx/imxdrm.ko 16K drivers/gpu/drm/imx/imx-ipuv3-crtc.ko 12K drivers/gpu/drm/imx/imx-ldb.ko 12K drivers/gpu/drm/imx/imx-tve.ko 8.0K drivers/gpu/drm/imx/parallel-display.ko 72K total
[...]
with this after the series:
$ du -ch drivers/gpu/drm/imx/*.ko 44K drivers/gpu/drm/imx/imx-drm.ko 44K total
[...]
There are other things to consider as well, such as the additional memory overhead caused by merely having multiple modules, or all of the exported functions that unnecessarily clutter up the symbol table, and which end up slowing down every symbol lookup.
Thanks, can't argue with those numbers. I'll queue patches 3 and 4 for imx-drm.
Wait, I won't because they depend on the helper function in patch 1.
Acked-by: Philipp Zabel p.zabel@pengutronix.de
then. Can I get a stable tag so I can solve potential merge conflicts in imx-drm-core.c?
regards Philipp
On Fri, Sep 25, 2015 at 03:13:54PM +0200, Philipp Zabel wrote:
Am Freitag, den 25.09.2015, 15:09 +0200 schrieb Philipp Zabel:
Am Freitag, den 25.09.2015, 14:17 +0200 schrieb Thierry Reding:
I think you gain much less by splitting up than you realize. Compare this from before the series:
$ du -ch drivers/gpu/drm/imx/*.ko 8.0K drivers/gpu/drm/imx/dw_hdmi-imx.ko
Oh right, I didn't realize that this is just the shim. The bulk of the HDMI driver is in bridge/dw_hdmi.ko.
16K drivers/gpu/drm/imx/imxdrm.ko 16K drivers/gpu/drm/imx/imx-ipuv3-crtc.ko 12K drivers/gpu/drm/imx/imx-ldb.ko 12K drivers/gpu/drm/imx/imx-tve.ko 8.0K drivers/gpu/drm/imx/parallel-display.ko 72K total
[...]
with this after the series:
$ du -ch drivers/gpu/drm/imx/*.ko 44K drivers/gpu/drm/imx/imx-drm.ko 44K total
[...]
There are other things to consider as well, such as the additional memory overhead caused by merely having multiple modules, or all of the exported functions that unnecessarily clutter up the symbol table, and which end up slowing down every symbol lookup.
Thanks, can't argue with those numbers. I'll queue patches 3 and 4 for imx-drm.
Wait, I won't because they depend on the helper function in patch 1.
Acked-by: Philipp Zabel p.zabel@pengutronix.de
then. Can I get a stable tag so I can solve potential merge conflicts in imx-drm-core.c?
Let's see what Greg thinks about patch 1. There are a bunch of other drivers that can use these helpers, so it might be best to get the helper merged first and hold off on the depending patches until the next release.
I'll let you know when we've figured out how to merge this.
Thierry
From: Thierry Reding treding@nvidia.com
Now that the driver is monolithic there is no longer any need to export these symbols.
Cc: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/imx/imx-drm-core.c | 12 ------------ 1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 4b49a7cb0a14..e854a019bbd4 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -55,7 +55,6 @@ unsigned int imx_drm_crtc_id(struct imx_drm_crtc *crtc) { return drm_crtc_index(crtc->crtc); } -EXPORT_SYMBOL_GPL(imx_drm_crtc_id);
static void imx_drm_driver_lastclose(struct drm_device *drm) { @@ -118,31 +117,26 @@ int imx_drm_set_bus_format_pins(struct drm_encoder *encoder, u32 bus_format, bus_format, hsync_pin, vsync_pin); return 0; } -EXPORT_SYMBOL_GPL(imx_drm_set_bus_format_pins);
int imx_drm_set_bus_format(struct drm_encoder *encoder, u32 bus_format) { return imx_drm_set_bus_format_pins(encoder, bus_format, 2, 3); } -EXPORT_SYMBOL_GPL(imx_drm_set_bus_format);
int imx_drm_crtc_vblank_get(struct imx_drm_crtc *imx_drm_crtc) { return drm_crtc_vblank_get(imx_drm_crtc->crtc); } -EXPORT_SYMBOL_GPL(imx_drm_crtc_vblank_get);
void imx_drm_crtc_vblank_put(struct imx_drm_crtc *imx_drm_crtc) { drm_crtc_vblank_put(imx_drm_crtc->crtc); } -EXPORT_SYMBOL_GPL(imx_drm_crtc_vblank_put);
void imx_drm_handle_vblank(struct imx_drm_crtc *imx_drm_crtc) { drm_crtc_handle_vblank(imx_drm_crtc->crtc); } -EXPORT_SYMBOL_GPL(imx_drm_handle_vblank);
static int imx_drm_enable_vblank(struct drm_device *drm, unsigned int pipe) { @@ -204,13 +198,11 @@ void imx_drm_connector_destroy(struct drm_connector *connector) drm_connector_unregister(connector); drm_connector_cleanup(connector); } -EXPORT_SYMBOL_GPL(imx_drm_connector_destroy);
void imx_drm_encoder_destroy(struct drm_encoder *encoder) { drm_encoder_cleanup(encoder); } -EXPORT_SYMBOL_GPL(imx_drm_encoder_destroy);
static void imx_drm_output_poll_changed(struct drm_device *drm) { @@ -387,7 +379,6 @@ err_register: kfree(imx_drm_crtc); return ret; } -EXPORT_SYMBOL_GPL(imx_drm_add_crtc);
/* * imx_drm_remove_crtc - remove a crtc @@ -405,7 +396,6 @@ int imx_drm_remove_crtc(struct imx_drm_crtc *imx_drm_crtc)
return 0; } -EXPORT_SYMBOL_GPL(imx_drm_remove_crtc);
int imx_drm_encoder_parse_of(struct drm_device *drm, struct drm_encoder *encoder, struct device_node *np) @@ -428,7 +418,6 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
return 0; } -EXPORT_SYMBOL_GPL(imx_drm_encoder_parse_of);
/* * @node: device tree node containing encoder input ports @@ -458,7 +447,6 @@ int imx_drm_encoder_get_mux_id(struct device_node *node,
return -EINVAL; } -EXPORT_SYMBOL_GPL(imx_drm_encoder_get_mux_id);
static const struct drm_ioctl_desc imx_drm_ioctls[] = { /* none so far */
From: Thierry Reding treding@nvidia.com
There's no use building the individual drivers as separate modules because they are all only useful if combined into a single DRM/KMS device.
Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/sti/Makefile | 21 +++++++++------------ drivers/gpu/drm/sti/sti_compositor.c | 4 +--- drivers/gpu/drm/sti/sti_drv.c | 24 +++++++++++++++++++++++- drivers/gpu/drm/sti/sti_drv.h | 9 +++++++++ drivers/gpu/drm/sti/sti_dvo.c | 2 -- drivers/gpu/drm/sti/sti_hda.c | 2 -- drivers/gpu/drm/sti/sti_hdmi.c | 2 -- drivers/gpu/drm/sti/sti_hqvdp.c | 2 -- drivers/gpu/drm/sti/sti_tvout.c | 2 -- drivers/gpu/drm/sti/sti_vtac.c | 2 -- drivers/gpu/drm/sti/sti_vtg.c | 2 -- 11 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/sti/Makefile b/drivers/gpu/drm/sti/Makefile index e27490b492a5..b8057620b3b3 100644 --- a/drivers/gpu/drm/sti/Makefile +++ b/drivers/gpu/drm/sti/Makefile @@ -1,26 +1,23 @@ -sticompositor-y := \ +sti-drm-y := \ sti_mixer.o \ sti_gdp.o \ sti_vid.o \ sti_cursor.o \ sti_compositor.o \ sti_crtc.o \ - sti_plane.o - -stihdmi-y := sti_hdmi.o \ + sti_plane.o \ + sti_crtc.o \ + sti_plane.o \ + sti_hdmi.o \ sti_hdmi_tx3g0c55phy.o \ sti_hdmi_tx3g4c28phy.o \ - -stidvo-y := sti_dvo.o \ - sti_awg_utils.o - -obj-$(CONFIG_DRM_STI) = \ + sti_dvo.o \ + sti_awg_utils.o \ sti_vtg.o \ sti_vtac.o \ - stihdmi.o \ sti_hda.o \ sti_tvout.o \ - sticompositor.o \ sti_hqvdp.o \ - stidvo.o \ sti_drv.o + +obj-$(CONFIG_DRM_STI) = sti-drm.o diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index c652627b1bca..afed2171beb9 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -263,7 +263,7 @@ static int sti_compositor_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver sti_compositor_driver = { +struct platform_driver sti_compositor_driver = { .driver = { .name = "sti-compositor", .of_match_table = compositor_of_match, @@ -272,8 +272,6 @@ static struct platform_driver sti_compositor_driver = { .remove = sti_compositor_remove, };
-module_platform_driver(sti_compositor_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index 9f85988a43ce..66f6e9864c99 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -287,7 +287,29 @@ static struct platform_driver sti_platform_driver = { }, };
-module_platform_driver(sti_platform_driver); +static struct platform_driver * const drivers[] = { + &sti_tvout_driver, + &sti_vtac_driver, + &sti_hqvdp_driver, + &sti_hdmi_driver, + &sti_hda_driver, + &sti_dvo_driver, + &sti_vtg_driver, + &sti_compositor_driver, + &sti_platform_driver, +}; + +static int sti_drm_init(void) +{ + return platform_register_drivers(drivers, ARRAY_SIZE(drivers)); +} +module_init(sti_drm_init); + +static void sti_drm_exit(void) +{ + platform_unregister_drivers(drivers, ARRAY_SIZE(drivers)); +} +module_exit(sti_drm_exit);
MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h index 9372f69e1859..30ddc20841c3 100644 --- a/drivers/gpu/drm/sti/sti_drv.h +++ b/drivers/gpu/drm/sti/sti_drv.h @@ -32,4 +32,13 @@ struct sti_private { } commit; };
+extern struct platform_driver sti_tvout_driver; +extern struct platform_driver sti_vtac_driver; +extern struct platform_driver sti_hqvdp_driver; +extern struct platform_driver sti_hdmi_driver; +extern struct platform_driver sti_hda_driver; +extern struct platform_driver sti_dvo_driver; +extern struct platform_driver sti_vtg_driver; +extern struct platform_driver sti_compositor_driver; + #endif diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c index d141d645bd13..a619aa9e688d 100644 --- a/drivers/gpu/drm/sti/sti_dvo.c +++ b/drivers/gpu/drm/sti/sti_dvo.c @@ -557,8 +557,6 @@ struct platform_driver sti_dvo_driver = { .remove = sti_dvo_remove, };
-module_platform_driver(sti_dvo_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c index 598cd78b0b16..d71abe33722e 100644 --- a/drivers/gpu/drm/sti/sti_hda.c +++ b/drivers/gpu/drm/sti/sti_hda.c @@ -784,8 +784,6 @@ struct platform_driver sti_hda_driver = { .remove = sti_hda_remove, };
-module_platform_driver(sti_hda_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index 09e29e43423e..43954a212bb9 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -901,8 +901,6 @@ struct platform_driver sti_hdmi_driver = { .remove = sti_hdmi_remove, };
-module_platform_driver(sti_hdmi_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index 09d86be4f73f..348c7c58f385 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1090,8 +1090,6 @@ struct platform_driver sti_hqvdp_driver = { .remove = sti_hqvdp_remove, };
-module_platform_driver(sti_hqvdp_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index c1aac8e66fb5..c8a4c5dae2b6 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -735,8 +735,6 @@ struct platform_driver sti_tvout_driver = { .remove = sti_tvout_remove, };
-module_platform_driver(sti_tvout_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_vtac.c b/drivers/gpu/drm/sti/sti_vtac.c index 97bcdac23ae1..b1eb0d77630d 100644 --- a/drivers/gpu/drm/sti/sti_vtac.c +++ b/drivers/gpu/drm/sti/sti_vtac.c @@ -216,8 +216,6 @@ struct platform_driver sti_vtac_driver = { .remove = sti_vtac_remove, };
-module_platform_driver(sti_vtac_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sti/sti_vtg.c b/drivers/gpu/drm/sti/sti_vtg.c index 4d8a918db6f5..d8bd8b76b1fa 100644 --- a/drivers/gpu/drm/sti/sti_vtg.c +++ b/drivers/gpu/drm/sti/sti_vtg.c @@ -406,8 +406,6 @@ struct platform_driver sti_vtg_driver = { .remove = vtg_remove, };
-module_platform_driver(sti_vtg_driver); - MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
Hi Thierry,
Ack-by: Vincent Abriou vincent.abriou@st.com
BR Vicnent
On 09/24/2015 07:02 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
There's no use building the individual drivers as separate modules because they are all only useful if combined into a single DRM/KMS device.
Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Signed-off-by: Thierry Reding treding@nvidia.com
drivers/gpu/drm/sti/Makefile | 21 +++++++++------------ drivers/gpu/drm/sti/sti_compositor.c | 4 +--- drivers/gpu/drm/sti/sti_drv.c | 24 +++++++++++++++++++++++- drivers/gpu/drm/sti/sti_drv.h | 9 +++++++++ drivers/gpu/drm/sti/sti_dvo.c | 2 -- drivers/gpu/drm/sti/sti_hda.c | 2 -- drivers/gpu/drm/sti/sti_hdmi.c | 2 -- drivers/gpu/drm/sti/sti_hqvdp.c | 2 -- drivers/gpu/drm/sti/sti_tvout.c | 2 -- drivers/gpu/drm/sti/sti_vtac.c | 2 -- drivers/gpu/drm/sti/sti_vtg.c | 2 -- 11 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/sti/Makefile b/drivers/gpu/drm/sti/Makefile index e27490b492a5..b8057620b3b3 100644 --- a/drivers/gpu/drm/sti/Makefile +++ b/drivers/gpu/drm/sti/Makefile @@ -1,26 +1,23 @@ -sticompositor-y := \ +sti-drm-y := \ sti_mixer.o \ sti_gdp.o \ sti_vid.o \ sti_cursor.o \ sti_compositor.o \ sti_crtc.o \
- sti_plane.o
-stihdmi-y := sti_hdmi.o \
- sti_plane.o \
- sti_crtc.o \
- sti_plane.o \
- sti_hdmi.o \ sti_hdmi_tx3g0c55phy.o \ sti_hdmi_tx3g4c28phy.o \
-stidvo-y := sti_dvo.o \
- sti_awg_utils.o
-obj-$(CONFIG_DRM_STI) = \
- sti_dvo.o \
- sti_awg_utils.o \ sti_vtg.o \ sti_vtac.o \
- stihdmi.o \ sti_hda.o \ sti_tvout.o \
- sticompositor.o \ sti_hqvdp.o \
- stidvo.o \ sti_drv.o
+obj-$(CONFIG_DRM_STI) = sti-drm.o diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index c652627b1bca..afed2171beb9 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -263,7 +263,7 @@ static int sti_compositor_remove(struct platform_device *pdev) return 0; }
-static struct platform_driver sti_compositor_driver = { +struct platform_driver sti_compositor_driver = { .driver = { .name = "sti-compositor", .of_match_table = compositor_of_match, @@ -272,8 +272,6 @@ static struct platform_driver sti_compositor_driver = { .remove = sti_compositor_remove, };
-module_platform_driver(sti_compositor_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index 9f85988a43ce..66f6e9864c99 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -287,7 +287,29 @@ static struct platform_driver sti_platform_driver = { }, };
-module_platform_driver(sti_platform_driver); +static struct platform_driver * const drivers[] = {
- &sti_tvout_driver,
- &sti_vtac_driver,
- &sti_hqvdp_driver,
- &sti_hdmi_driver,
- &sti_hda_driver,
- &sti_dvo_driver,
- &sti_vtg_driver,
- &sti_compositor_driver,
- &sti_platform_driver,
+};
+static int sti_drm_init(void) +{
- return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
+} +module_init(sti_drm_init);
+static void sti_drm_exit(void) +{
- platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
+} +module_exit(sti_drm_exit);
MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h index 9372f69e1859..30ddc20841c3 100644 --- a/drivers/gpu/drm/sti/sti_drv.h +++ b/drivers/gpu/drm/sti/sti_drv.h @@ -32,4 +32,13 @@ struct sti_private { } commit; };
+extern struct platform_driver sti_tvout_driver; +extern struct platform_driver sti_vtac_driver; +extern struct platform_driver sti_hqvdp_driver; +extern struct platform_driver sti_hdmi_driver; +extern struct platform_driver sti_hda_driver; +extern struct platform_driver sti_dvo_driver; +extern struct platform_driver sti_vtg_driver; +extern struct platform_driver sti_compositor_driver;
- #endif
diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c index d141d645bd13..a619aa9e688d 100644 --- a/drivers/gpu/drm/sti/sti_dvo.c +++ b/drivers/gpu/drm/sti/sti_dvo.c @@ -557,8 +557,6 @@ struct platform_driver sti_dvo_driver = { .remove = sti_dvo_remove, };
-module_platform_driver(sti_dvo_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c index 598cd78b0b16..d71abe33722e 100644 --- a/drivers/gpu/drm/sti/sti_hda.c +++ b/drivers/gpu/drm/sti/sti_hda.c @@ -784,8 +784,6 @@ struct platform_driver sti_hda_driver = { .remove = sti_hda_remove, };
-module_platform_driver(sti_hda_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index 09e29e43423e..43954a212bb9 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -901,8 +901,6 @@ struct platform_driver sti_hdmi_driver = { .remove = sti_hdmi_remove, };
-module_platform_driver(sti_hdmi_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index 09d86be4f73f..348c7c58f385 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1090,8 +1090,6 @@ struct platform_driver sti_hqvdp_driver = { .remove = sti_hqvdp_remove, };
-module_platform_driver(sti_hqvdp_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index c1aac8e66fb5..c8a4c5dae2b6 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -735,8 +735,6 @@ struct platform_driver sti_tvout_driver = { .remove = sti_tvout_remove, };
-module_platform_driver(sti_tvout_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_vtac.c b/drivers/gpu/drm/sti/sti_vtac.c index 97bcdac23ae1..b1eb0d77630d 100644 --- a/drivers/gpu/drm/sti/sti_vtac.c +++ b/drivers/gpu/drm/sti/sti_vtac.c @@ -216,8 +216,6 @@ struct platform_driver sti_vtac_driver = { .remove = sti_vtac_remove, };
-module_platform_driver(sti_vtac_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/sti/sti_vtg.c b/drivers/gpu/drm/sti/sti_vtg.c index 4d8a918db6f5..d8bd8b76b1fa 100644 --- a/drivers/gpu/drm/sti/sti_vtg.c +++ b/drivers/gpu/drm/sti/sti_vtg.c @@ -406,8 +406,6 @@ struct platform_driver sti_vtg_driver = { .remove = vtg_remove, };
-module_platform_driver(sti_vtg_driver);
- MODULE_AUTHOR("Benjamin Gaignard benjamin.gaignard@st.com"); MODULE_DESCRIPTION("STMicroelectronics SoC DRM driver"); MODULE_LICENSE("GPL");
From: Thierry Reding treding@nvidia.com
None of these exported symbols are used outside of the drm-sti driver, so there is no reason to export them.
Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/sti/sti_crtc.c | 3 --- drivers/gpu/drm/sti/sti_mixer.c | 1 - drivers/gpu/drm/sti/sti_plane.c | 3 --- drivers/gpu/drm/sti/sti_vtg.c | 6 ------ 4 files changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c index 87bb4096b9d7..342d8a617c6f 100644 --- a/drivers/gpu/drm/sti/sti_crtc.c +++ b/drivers/gpu/drm/sti/sti_crtc.c @@ -318,7 +318,6 @@ int sti_crtc_enable_vblank(struct drm_device *dev, unsigned int pipe)
return 0; } -EXPORT_SYMBOL(sti_crtc_enable_vblank);
void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) { @@ -339,7 +338,6 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) compo->mixer[pipe]->pending_event = NULL; } } -EXPORT_SYMBOL(sti_crtc_disable_vblank);
static struct drm_crtc_funcs sti_crtc_funcs = { .set_config = drm_atomic_helper_set_config, @@ -360,7 +358,6 @@ bool sti_crtc_is_main(struct drm_crtc *crtc)
return false; } -EXPORT_SYMBOL(sti_crtc_is_main);
int sti_crtc_init(struct drm_device *drm_dev, struct sti_mixer *mixer, struct drm_plane *primary, struct drm_plane *cursor) diff --git a/drivers/gpu/drm/sti/sti_mixer.c b/drivers/gpu/drm/sti/sti_mixer.c index 0182e9365004..4c18b50d71c5 100644 --- a/drivers/gpu/drm/sti/sti_mixer.c +++ b/drivers/gpu/drm/sti/sti_mixer.c @@ -58,7 +58,6 @@ const char *sti_mixer_to_str(struct sti_mixer *mixer) return "<UNKNOWN MIXER>"; } } -EXPORT_SYMBOL(sti_mixer_to_str);
static inline u32 sti_mixer_reg_read(struct sti_mixer *mixer, u32 reg_id) { diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c index d5c5e91f2956..2e5c751910c5 100644 --- a/drivers/gpu/drm/sti/sti_plane.c +++ b/drivers/gpu/drm/sti/sti_plane.c @@ -42,7 +42,6 @@ const char *sti_plane_to_str(struct sti_plane *plane) return "<UNKNOWN PLANE>"; } } -EXPORT_SYMBOL(sti_plane_to_str);
static void sti_plane_destroy(struct drm_plane *drm_plane) { @@ -108,7 +107,6 @@ void sti_plane_init_property(struct sti_plane *plane, plane->drm_plane.base.id, sti_plane_to_str(plane), plane->zorder); } -EXPORT_SYMBOL(sti_plane_init_property);
struct drm_plane_funcs sti_plane_helpers_funcs = { .update_plane = drm_atomic_helper_update_plane, @@ -119,4 +117,3 @@ struct drm_plane_funcs sti_plane_helpers_funcs = { .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, }; -EXPORT_SYMBOL(sti_plane_helpers_funcs); diff --git a/drivers/gpu/drm/sti/sti_vtg.c b/drivers/gpu/drm/sti/sti_vtg.c index d8bd8b76b1fa..d56630c60039 100644 --- a/drivers/gpu/drm/sti/sti_vtg.c +++ b/drivers/gpu/drm/sti/sti_vtg.c @@ -110,7 +110,6 @@ struct sti_vtg *of_vtg_find(struct device_node *np) } return NULL; } -EXPORT_SYMBOL(of_vtg_find);
static void vtg_reset(struct sti_vtg *vtg) { @@ -242,7 +241,6 @@ void sti_vtg_set_config(struct sti_vtg *vtg, else vtg_enable_irq(vtg); } -EXPORT_SYMBOL(sti_vtg_set_config);
/** * sti_vtg_get_line_number @@ -265,7 +263,6 @@ u32 sti_vtg_get_line_number(struct drm_display_mode mode, int y)
return start_line + y; } -EXPORT_SYMBOL(sti_vtg_get_line_number);
/** * sti_vtg_get_pixel_number @@ -281,7 +278,6 @@ u32 sti_vtg_get_pixel_number(struct drm_display_mode mode, int x) { return mode.htotal - mode.hsync_start + x; } -EXPORT_SYMBOL(sti_vtg_get_pixel_number);
int sti_vtg_register_client(struct sti_vtg *vtg, struct notifier_block *nb, struct drm_crtc *crtc) @@ -292,7 +288,6 @@ int sti_vtg_register_client(struct sti_vtg *vtg, struct notifier_block *nb, vtg->crtc = crtc; return raw_notifier_chain_register(&vtg->notifier_list, nb); } -EXPORT_SYMBOL(sti_vtg_register_client);
int sti_vtg_unregister_client(struct sti_vtg *vtg, struct notifier_block *nb) { @@ -301,7 +296,6 @@ int sti_vtg_unregister_client(struct sti_vtg *vtg, struct notifier_block *nb)
return raw_notifier_chain_unregister(&vtg->notifier_list, nb); } -EXPORT_SYMBOL(sti_vtg_unregister_client);
static irqreturn_t vtg_irq_thread(int irq, void *arg) {
Hi Thierry,
Ack-by: Vincent Abriou vincent.abriou@st.com
BR Vincent
On 09/24/2015 07:02 PM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
None of these exported symbols are used outside of the drm-sti driver, so there is no reason to export them.
Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Signed-off-by: Thierry Reding treding@nvidia.com
drivers/gpu/drm/sti/sti_crtc.c | 3 --- drivers/gpu/drm/sti/sti_mixer.c | 1 - drivers/gpu/drm/sti/sti_plane.c | 3 --- drivers/gpu/drm/sti/sti_vtg.c | 6 ------ 4 files changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c index 87bb4096b9d7..342d8a617c6f 100644 --- a/drivers/gpu/drm/sti/sti_crtc.c +++ b/drivers/gpu/drm/sti/sti_crtc.c @@ -318,7 +318,6 @@ int sti_crtc_enable_vblank(struct drm_device *dev, unsigned int pipe)
return 0; } -EXPORT_SYMBOL(sti_crtc_enable_vblank);
void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) { @@ -339,7 +338,6 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) compo->mixer[pipe]->pending_event = NULL; } } -EXPORT_SYMBOL(sti_crtc_disable_vblank);
static struct drm_crtc_funcs sti_crtc_funcs = { .set_config = drm_atomic_helper_set_config, @@ -360,7 +358,6 @@ bool sti_crtc_is_main(struct drm_crtc *crtc)
return false; } -EXPORT_SYMBOL(sti_crtc_is_main);
int sti_crtc_init(struct drm_device *drm_dev, struct sti_mixer *mixer, struct drm_plane *primary, struct drm_plane *cursor) diff --git a/drivers/gpu/drm/sti/sti_mixer.c b/drivers/gpu/drm/sti/sti_mixer.c index 0182e9365004..4c18b50d71c5 100644 --- a/drivers/gpu/drm/sti/sti_mixer.c +++ b/drivers/gpu/drm/sti/sti_mixer.c @@ -58,7 +58,6 @@ const char *sti_mixer_to_str(struct sti_mixer *mixer) return "<UNKNOWN MIXER>"; } } -EXPORT_SYMBOL(sti_mixer_to_str);
static inline u32 sti_mixer_reg_read(struct sti_mixer *mixer, u32 reg_id) { diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c index d5c5e91f2956..2e5c751910c5 100644 --- a/drivers/gpu/drm/sti/sti_plane.c +++ b/drivers/gpu/drm/sti/sti_plane.c @@ -42,7 +42,6 @@ const char *sti_plane_to_str(struct sti_plane *plane) return "<UNKNOWN PLANE>"; } } -EXPORT_SYMBOL(sti_plane_to_str);
static void sti_plane_destroy(struct drm_plane *drm_plane) { @@ -108,7 +107,6 @@ void sti_plane_init_property(struct sti_plane *plane, plane->drm_plane.base.id, sti_plane_to_str(plane), plane->zorder); } -EXPORT_SYMBOL(sti_plane_init_property);
struct drm_plane_funcs sti_plane_helpers_funcs = { .update_plane = drm_atomic_helper_update_plane, @@ -119,4 +117,3 @@ struct drm_plane_funcs sti_plane_helpers_funcs = { .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, }; -EXPORT_SYMBOL(sti_plane_helpers_funcs); diff --git a/drivers/gpu/drm/sti/sti_vtg.c b/drivers/gpu/drm/sti/sti_vtg.c index d8bd8b76b1fa..d56630c60039 100644 --- a/drivers/gpu/drm/sti/sti_vtg.c +++ b/drivers/gpu/drm/sti/sti_vtg.c @@ -110,7 +110,6 @@ struct sti_vtg *of_vtg_find(struct device_node *np) } return NULL; } -EXPORT_SYMBOL(of_vtg_find);
static void vtg_reset(struct sti_vtg *vtg) { @@ -242,7 +241,6 @@ void sti_vtg_set_config(struct sti_vtg *vtg, else vtg_enable_irq(vtg); } -EXPORT_SYMBOL(sti_vtg_set_config);
/**
- sti_vtg_get_line_number
@@ -265,7 +263,6 @@ u32 sti_vtg_get_line_number(struct drm_display_mode mode, int y)
return start_line + y; } -EXPORT_SYMBOL(sti_vtg_get_line_number);
/**
- sti_vtg_get_pixel_number
@@ -281,7 +278,6 @@ u32 sti_vtg_get_pixel_number(struct drm_display_mode mode, int x) { return mode.htotal - mode.hsync_start + x; } -EXPORT_SYMBOL(sti_vtg_get_pixel_number);
int sti_vtg_register_client(struct sti_vtg *vtg, struct notifier_block *nb, struct drm_crtc *crtc) @@ -292,7 +288,6 @@ int sti_vtg_register_client(struct sti_vtg *vtg, struct notifier_block *nb, vtg->crtc = crtc; return raw_notifier_chain_register(&vtg->notifier_list, nb); } -EXPORT_SYMBOL(sti_vtg_register_client);
int sti_vtg_unregister_client(struct sti_vtg *vtg, struct notifier_block *nb) { @@ -301,7 +296,6 @@ int sti_vtg_unregister_client(struct sti_vtg *vtg, struct notifier_block *nb)
return raw_notifier_chain_unregister(&vtg->notifier_list, nb); } -EXPORT_SYMBOL(sti_vtg_unregister_client);
static irqreturn_t vtg_irq_thread(int irq, void *arg) {
On Thu, 24 Sep 2015, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
Documentation/driver-model/platform.txt | 11 ++++++ drivers/base/platform.c | 60 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 5 +++ 3 files changed, 76 insertions(+)
diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt index 07795ec51cde..e80468738ba9 100644 --- a/Documentation/driver-model/platform.txt +++ b/Documentation/driver-model/platform.txt @@ -63,6 +63,17 @@ runtime memory footprint: int platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *))
+Kernel modules can be composed of several platform drivers. The platform core +provides helpers to register and unregister an array of drivers:
- int platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count);- void platform_unregister_drivers(struct platform_driver * const *drivers,
unsigned int count);+If one of the drivers fails to register, all drivers registered up to that +point will be unregistered in reverse order.
Device Enumeration
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..b7d7987fda97 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,66 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle); +/** + * platform_register_drivers - register an array of platform drivers + * @drivers: an array of drivers to register + * @count: the number of drivers to register + * + * Registers platform drivers specified by an array. On failure to register a + * driver, all previously registered drivers will be unregistered. Callers of + * this API should use platform_unregister_drivers() to unregister drivers in + * the reverse order. + * + * Returns: 0 on success or a negative error code on failure. + */ +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + unsigned int i; + int err; + + for (i = 0; i < count; i++) { + pr_debug("registering platform driver %ps\n", drivers[i]); + + err = platform_driver_register(drivers[i]); + if (err < 0) { + pr_err("failed to register platform driver %ps: %d\n", + drivers[i], err); + goto error; + } + } + + return 0; + +error: + while (i--) { + pr_debug("unregistering platform driver %ps\n", drivers[i]); + platform_driver_unregister(drivers[i]); + }
This will call platform_driver_unregister() on the driver that failed, but not the first driver.
You should probably make i an int, and use while (--i >= 0).
BR, Jani.
- return err;
+} +EXPORT_SYMBOL_GPL(platform_register_drivers);
+/**
- platform_unregister_drivers - unregister an array of platform drivers
- @drivers: an array of drivers to unregister
- @count: the number of drivers to unregister
- Unegisters platform drivers specified by an array. This is typically used
- to complement an earlier call to platform_register_drivers(). Drivers are
- unregistered in the reverse order in which they were registered.
- */
+void platform_unregister_drivers(struct platform_driver * const *drivers,
unsigned int count)+{
- while (count--) {
pr_debug("unregistering platform driver %ps\n", drivers[count]);platform_driver_unregister(drivers[count]);- }
+} +EXPORT_SYMBOL_GPL(platform_unregister_drivers);
/* modalias support enables more hands-off userspace setup:
- (a) environment variable lets new-style hotplug events work once system is
fully running: "modprobe $MODALIAS"diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index bba08f44cc97..0c9f16bfdd99 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -270,6 +270,11 @@ extern struct platform_device *__platform_create_bundle( struct resource *res, unsigned int n_res, const void *data, size_t size, struct module *module);
+int platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count);+void platform_unregister_drivers(struct platform_driver * const *drivers,
unsigned int count);/* early platform driver interface */ struct early_platform_driver { const char *class_str; -- 2.5.0
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Sep 25, 2015 at 01:27:28PM +0300, Jani Nikula wrote:
On Thu, 24 Sep 2015, Thierry Reding thierry.reding@gmail.com wrote:
[...]
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
[...]
+/**
- platform_register_drivers - register an array of platform drivers
- @drivers: an array of drivers to register
- @count: the number of drivers to register
- Registers platform drivers specified by an array. On failure to register a
- driver, all previously registered drivers will be unregistered. Callers of
- this API should use platform_unregister_drivers() to unregister drivers in
- the reverse order.
- Returns: 0 on success or a negative error code on failure.
- */
+int platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count)+{
- unsigned int i;
- int err;
- for (i = 0; i < count; i++) {
pr_debug("registering platform driver %ps\n", drivers[i]);err = platform_driver_register(drivers[i]);if (err < 0) {pr_err("failed to register platform driver %ps: %d\n",drivers[i], err);goto error;}- }
- return 0;
+error:
- while (i--) {
pr_debug("unregistering platform driver %ps\n", drivers[i]);platform_driver_unregister(drivers[i]);- }
This will call platform_driver_unregister() on the driver that failed, but not the first driver.
You should probably make i an int, and use while (--i >= 0).
Actually it won't. I was especially careful and even tested this with one driver by instrumenting platform_driver_register() to return failure at various points in the sequence.
This works fine.
Thierry
On Fri, 25 Sep 2015, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Sep 25, 2015 at 01:27:28PM +0300, Jani Nikula wrote:
On Thu, 24 Sep 2015, Thierry Reding thierry.reding@gmail.com wrote:
[...]
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
[...]
+/**
- platform_register_drivers - register an array of platform drivers
- @drivers: an array of drivers to register
- @count: the number of drivers to register
- Registers platform drivers specified by an array. On failure to register a
- driver, all previously registered drivers will be unregistered. Callers of
- this API should use platform_unregister_drivers() to unregister drivers in
- the reverse order.
- Returns: 0 on success or a negative error code on failure.
- */
+int platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count)+{
- unsigned int i;
- int err;
- for (i = 0; i < count; i++) {
pr_debug("registering platform driver %ps\n", drivers[i]);err = platform_driver_register(drivers[i]);if (err < 0) {pr_err("failed to register platform driver %ps: %d\n",drivers[i], err);goto error;}- }
- return 0;
+error:
- while (i--) {
pr_debug("unregistering platform driver %ps\n", drivers[i]);platform_driver_unregister(drivers[i]);- }
This will call platform_driver_unregister() on the driver that failed, but not the first driver.
You should probably make i an int, and use while (--i >= 0).
Actually it won't. I was especially careful and even tested this with one driver by instrumenting platform_driver_register() to return failure at various points in the sequence.
This works fine.
You are right, of course. What was I thinking. My kingdom for an excuse!
BR, Jani.
On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
Documentation/driver-model/platform.txt | 11 ++++++ drivers/base/platform.c | 60 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 5 +++ 3 files changed, 76 insertions(+)
Hi Greg,
In addition to patches 2-6 I have about two dozen patches across various subsystems that make use of this. I didn't want to spam everyone with all of them before you've given an indication about what you think about this patch.
The diffstat of the conversions I did is this:
drivers/crypto/n2_core.c | 17 +++++++---------- drivers/edac/mpc85xx_edac.c | 16 ++++++++-------- drivers/edac/mv64x60_edac.c | 39 +++++++++++---------------------------- drivers/gpio/gpio-mpc5200.c | 17 +++++++---------- drivers/gpu/drm/armada/armada_drv.c | 16 +++++++--------- drivers/gpu/drm/exynos/exynos_drm_drv.c | 42 ++++++++---------------------------------- drivers/gpu/drm/omapdrm/omap_drv.c | 24 +++++++----------------- drivers/gpu/host1x/dev.c | 17 +++++++---------- drivers/input/misc/sparcspkr.c | 18 +++++++----------- drivers/iommu/msm_iommu_dev.c | 25 +++++++------------------ drivers/leds/leds-sunfire.c | 23 +++++++---------------- drivers/mfd/sta2x11-mfd.c | 36 ++++++++++-------------------------- drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++------- drivers/net/ethernet/broadcom/bcm63xx_enet.c | 28 ++++++++-------------------- drivers/net/ethernet/freescale/fec_mpc52xx.c | 22 +++++++++------------- drivers/net/ethernet/marvell/mv643xx_eth.c | 19 +++++++------------ drivers/pinctrl/pinctrl-adi2.c | 24 ++++++++---------------- drivers/pinctrl/pinctrl-at91.c | 14 +++++++------- drivers/regulator/lp8788-ldo.c | 16 +++++++--------- drivers/regulator/wm831x-dcdc.c | 31 +++++++++---------------------- drivers/regulator/wm831x-ldo.c | 27 ++++++++------------------- drivers/tty/serial/mpsc.c | 19 ++++++++----------- drivers/usb/gadget/udc/dummy_hcd.c | 17 ++++++++--------- drivers/video/fbdev/s3c2410fb.c | 15 +++++++-------- 24 files changed, 186 insertions(+), 350 deletions(-)
That's not too thrilling but in many cases this fixes a potential bug if a driver fails to register and the code doesn't properly unregister any previously registered drivers.
Anyway, if you think this is worth merging, how would you like to go about it? Do you want Acked-bys on all the patches and merge them through your tree? Perhaps the easiest would be to just merge this patch and then take the other patches through the maintainer trees after the core patch has landed?
Thierry
diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt index 07795ec51cde..e80468738ba9 100644 --- a/Documentation/driver-model/platform.txt +++ b/Documentation/driver-model/platform.txt @@ -63,6 +63,17 @@ runtime memory footprint: int platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *))
+Kernel modules can be composed of several platform drivers. The platform core +provides helpers to register and unregister an array of drivers:
- int platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count);- void platform_unregister_drivers(struct platform_driver * const *drivers,
unsigned int count);+If one of the drivers fails to register, all drivers registered up to that +point will be unregistered in reverse order.
Device Enumeration
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..b7d7987fda97 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,66 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle); +/** + * platform_register_drivers - register an array of platform drivers + * @drivers: an array of drivers to register + * @count: the number of drivers to register + * + * Registers platform drivers specified by an array. On failure to register a + * driver, all previously registered drivers will be unregistered. Callers of + * this API should use platform_unregister_drivers() to unregister drivers in + * the reverse order. + * + * Returns: 0 on success or a negative error code on failure. + */ +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + unsigned int i; + int err; + + for (i = 0; i < count; i++) { + pr_debug("registering platform driver %ps\n", drivers[i]); + + err = platform_driver_register(drivers[i]); + if (err < 0) { + pr_err("failed to register platform driver %ps: %d\n", + drivers[i], err); + goto error; + } + } + + return 0; + +error: + while (i--) { + pr_debug("unregistering platform driver %ps\n", drivers[i]); + platform_driver_unregister(drivers[i]); + } + + return err; +} +EXPORT_SYMBOL_GPL(platform_register_drivers); + +/** + * platform_unregister_drivers - unregister an array of platform drivers + * @drivers: an array of drivers to unregister + * @count: the number of drivers to unregister + * + * Unegisters platform drivers specified by an array. This is typically used + * to complement an earlier call to platform_register_drivers(). Drivers are + * unregistered in the reverse order in which they were registered. + */ +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + while (count--) { + pr_debug("unregistering platform driver %ps\n", drivers[count]); + platform_driver_unregister(drivers[count]); + } +} +EXPORT_SYMBOL_GPL(platform_unregister_drivers); + /* modalias support enables more hands-off userspace setup: * (a) environment variable lets new-style hotplug events work once system is * fully running: "modprobe $MODALIAS" diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index bba08f44cc97..0c9f16bfdd99 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -270,6 +270,11 @@ extern struct platform_device *__platform_create_bundle( struct resource *res, unsigned int n_res, const void *data, size_t size, struct module *module); +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count); +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count); + /* early platform driver interface */ struct early_platform_driver { const char *class_str; -- 2.5.0
On Fri, Sep 25, 2015 at 04:29:44PM +0200, Thierry Reding wrote:
On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
Documentation/driver-model/platform.txt | 11 ++++++ drivers/base/platform.c | 60 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 5 +++ 3 files changed, 76 insertions(+)
Hi Greg,
In addition to patches 2-6 I have about two dozen patches across various subsystems that make use of this. I didn't want to spam everyone with all of them before you've given an indication about what you think about this patch.
The diffstat of the conversions I did is this:
drivers/crypto/n2_core.c | 17 +++++++---------- drivers/edac/mpc85xx_edac.c | 16 ++++++++-------- drivers/edac/mv64x60_edac.c | 39 +++++++++++---------------------------- drivers/gpio/gpio-mpc5200.c | 17 +++++++---------- drivers/gpu/drm/armada/armada_drv.c | 16 +++++++--------- drivers/gpu/drm/exynos/exynos_drm_drv.c | 42 ++++++++---------------------------------- drivers/gpu/drm/omapdrm/omap_drv.c | 24 +++++++----------------- drivers/gpu/host1x/dev.c | 17 +++++++---------- drivers/input/misc/sparcspkr.c | 18 +++++++----------- drivers/iommu/msm_iommu_dev.c | 25 +++++++------------------ drivers/leds/leds-sunfire.c | 23 +++++++---------------- drivers/mfd/sta2x11-mfd.c | 36 ++++++++++-------------------------- drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++------- drivers/net/ethernet/broadcom/bcm63xx_enet.c | 28 ++++++++-------------------- drivers/net/ethernet/freescale/fec_mpc52xx.c | 22 +++++++++------------- drivers/net/ethernet/marvell/mv643xx_eth.c | 19 +++++++------------ drivers/pinctrl/pinctrl-adi2.c | 24 ++++++++---------------- drivers/pinctrl/pinctrl-at91.c | 14 +++++++------- drivers/regulator/lp8788-ldo.c | 16 +++++++--------- drivers/regulator/wm831x-dcdc.c | 31 +++++++++---------------------- drivers/regulator/wm831x-ldo.c | 27 ++++++++------------------- drivers/tty/serial/mpsc.c | 19 ++++++++----------- drivers/usb/gadget/udc/dummy_hcd.c | 17 ++++++++--------- drivers/video/fbdev/s3c2410fb.c | 15 +++++++-------- 24 files changed, 186 insertions(+), 350 deletions(-)
That's not too thrilling but in many cases this fixes a potential bug if a driver fails to register and the code doesn't properly unregister any previously registered drivers.
Anyway, if you think this is worth merging, how would you like to go about it? Do you want Acked-bys on all the patches and merge them through your tree? Perhaps the easiest would be to just merge this patch and then take the other patches through the maintainer trees after the core patch has landed?
The last thing is the easiest, but you have to wait a release cycle for that. Or I can take the whole series if you want to, just cc: the proper subsystem maintainers when you send them. It's up to you.
thanks,
greg k-h
On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
Documentation/driver-model/platform.txt | 11 ++++++ drivers/base/platform.c | 60 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 5 +++ 3 files changed, 76 insertions(+)
diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt index 07795ec51cde..e80468738ba9 100644 --- a/Documentation/driver-model/platform.txt +++ b/Documentation/driver-model/platform.txt @@ -63,6 +63,17 @@ runtime memory footprint: int platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *))
+Kernel modules can be composed of several platform drivers. The platform core +provides helpers to register and unregister an array of drivers:
- int platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count);- void platform_unregister_drivers(struct platform_driver * const *drivers,
unsigned int count);+If one of the drivers fails to register, all drivers registered up to that +point will be unregistered in reverse order.
Device Enumeration
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..b7d7987fda97 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,66 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle); +/** + * platform_register_drivers - register an array of platform drivers + * @drivers: an array of drivers to register + * @count: the number of drivers to register + * + * Registers platform drivers specified by an array. On failure to register a + * driver, all previously registered drivers will be unregistered. Callers of + * this API should use platform_unregister_drivers() to unregister drivers in + * the reverse order. + * + * Returns: 0 on success or a negative error code on failure. + */ +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + unsigned int i; + int err; + + for (i = 0; i < count; i++) { + pr_debug("registering platform driver %ps\n", drivers[i]); + + err = platform_driver_register(drivers[i]);
I notice that this is actually doing the wrong thing because the platform drivers will end up with their .owner field set to NULL because this file is always built-in. I've fixed it up by passing in a struct module *owner into __platform_register_drivers() and pass it on to the __platform_driver_register() function instead.
Thierry
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - properly pass around the owner module
Documentation/driver-model/platform.txt | 14 ++++++++ drivers/base/platform.c | 61 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 8 +++++ 3 files changed, 83 insertions(+)
diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt index 07795ec51cde..e456696cfef2 100644 --- a/Documentation/driver-model/platform.txt +++ b/Documentation/driver-model/platform.txt @@ -63,6 +63,20 @@ runtime memory footprint: int platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *))
+Kernel modules can be composed of several platform drivers. The platform core +provides helpers to register and unregister an array of drivers: + + int __platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count, struct module *owner); + void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count); + +If one of the drivers fails to register, all drivers registered up to that +point will be unregistered in reverse order. Note that there is a convenience +macro that passes THIS_MODULE as owner parameter: + + #define platform_register_driver(drivers, count) +
Device Enumeration ~~~~~~~~~~~~~~~~~~ diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..68c58c43e45a 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,67 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle);
+/** + * __platform_register_drivers - register an array of platform drivers + * @drivers: an array of drivers to register + * @count: the number of drivers to register + * @owner: module owning the drivers + * + * Registers platform drivers specified by an array. On failure to register a + * driver, all previously registered drivers will be unregistered. Callers of + * this API should use platform_unregister_drivers() to unregister drivers in + * the reverse order. + * + * Returns: 0 on success or a negative error code on failure. + */ +int __platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count, struct module *owner) +{ + unsigned int i; + int err; + + for (i = 0; i < count; i++) { + pr_debug("registering platform driver %ps\n", drivers[i]); + + err = __platform_driver_register(drivers[i], owner); + if (err < 0) { + pr_err("failed to register platform driver %ps: %d\n", + drivers[i], err); + goto error; + } + } + + return 0; + +error: + while (i--) { + pr_debug("unregistering platform driver %ps\n", drivers[i]); + platform_driver_unregister(drivers[i]); + } + + return err; +} +EXPORT_SYMBOL_GPL(__platform_register_drivers); + +/** + * platform_unregister_drivers - unregister an array of platform drivers + * @drivers: an array of drivers to unregister + * @count: the number of drivers to unregister + * + * Unegisters platform drivers specified by an array. This is typically used + * to complement an earlier call to platform_register_drivers(). Drivers are + * unregistered in the reverse order in which they were registered. + */ +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + while (count--) { + pr_debug("unregistering platform driver %ps\n", drivers[count]); + platform_driver_unregister(drivers[count]); + } +} +EXPORT_SYMBOL_GPL(platform_unregister_drivers); + /* modalias support enables more hands-off userspace setup: * (a) environment variable lets new-style hotplug events work once system is * fully running: "modprobe $MODALIAS" diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index bba08f44cc97..dc777be5f2e1 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -270,6 +270,14 @@ extern struct platform_device *__platform_create_bundle( struct resource *res, unsigned int n_res, const void *data, size_t size, struct module *module);
+int __platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count, struct module *owner); +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count); + +#define platform_register_drivers(drivers, count) \ + __platform_register_drivers(drivers, count, THIS_MODULE) + /* early platform driver interface */ struct early_platform_driver { const char *class_str;
On Fri, Sep 25, 2015 at 6:29 PM, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..68c58c43e45a 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,67 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle);
+/**
- __platform_register_drivers - register an array of platform drivers
- @drivers: an array of drivers to register
- @count: the number of drivers to register
- @owner: module owning the drivers
- Registers platform drivers specified by an array. On failure to register a
- driver, all previously registered drivers will be unregistered. Callers of
- this API should use platform_unregister_drivers() to unregister drivers in
- the reverse order.
- Returns: 0 on success or a negative error code on failure.
- */
+int __platform_register_drivers(struct platform_driver * const *drivers,
unsigned int count, struct module *owner)+{
unsigned int i;int err;for (i = 0; i < count; i++) {pr_debug("registering platform driver %ps\n", drivers[i]);err = __platform_driver_register(drivers[i], owner);if (err < 0) {pr_err("failed to register platform driver %ps: %d\n",drivers[i], err);
Would platform_unregister_drivers(drivers, i); work?
goto error;}}return 0;+error:
while (i--) {
I think Jani was confused since often idiom is 'while (--i >=0)' is used.
pr_debug("unregistering platform driver %ps\n", drivers[i]);platform_driver_unregister(drivers[i]);}return err;+} +EXPORT_SYMBOL_GPL(__platform_register_drivers);
+/**
- platform_unregister_drivers - unregister an array of platform drivers
- @drivers: an array of drivers to unregister
- @count: the number of drivers to unregister
- Unegisters platform drivers specified by an array. This is typically used
- to complement an earlier call to platform_register_drivers(). Drivers are
- unregistered in the reverse order in which they were registered.
- */
+void platform_unregister_drivers(struct platform_driver * const *drivers,
unsigned int count)+{
while (count--) {pr_debug("unregistering platform driver %ps\n", drivers[count]);platform_driver_unregister(drivers[count]);}+} +EXPORT_SYMBOL_GPL(platform_unregister_drivers);
On Thu, Sep 24, 2015 at 7:02 PM, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
I raised this already on irc but let's do it here too for the record: Eric Anholt has a very similar thing (but in drm only) with the addition of also integrating with the component framework:
http://lists.freedesktop.org/archives/dri-devel/2015-September/090449.html
Having something that works for everyone (so includes msm and can be used for vc4 too) would be great. Adding Eric. -Daniel
Documentation/driver-model/platform.txt | 11 ++++++ drivers/base/platform.c | 60 +++++++++++++++++++++++++++++++++ include/linux/platform_device.h | 5 +++ 3 files changed, 76 insertions(+)
diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt index 07795ec51cde..e80468738ba9 100644 --- a/Documentation/driver-model/platform.txt +++ b/Documentation/driver-model/platform.txt @@ -63,6 +63,17 @@ runtime memory footprint: int platform_driver_probe(struct platform_driver *drv, int (*probe)(struct platform_device *))
+Kernel modules can be composed of several platform drivers. The platform core +provides helpers to register and unregister an array of drivers:
int platform_register_drivers(struct platform_driver * const *drivers,unsigned int count);void platform_unregister_drivers(struct platform_driver * const *drivers,unsigned int count);+If one of the drivers fails to register, all drivers registered up to that +point will be unregistered in reverse order.
Device Enumeration
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index f80aaaf9f610..b7d7987fda97 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -711,6 +711,66 @@ err_out: } EXPORT_SYMBOL_GPL(__platform_create_bundle); +/** + * platform_register_drivers - register an array of platform drivers + * @drivers: an array of drivers to register + * @count: the number of drivers to register + * + * Registers platform drivers specified by an array. On failure to register a + * driver, all previously registered drivers will be unregistered. Callers of + * this API should use platform_unregister_drivers() to unregister drivers in + * the reverse order. + * + * Returns: 0 on success or a negative error code on failure. + */ +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + unsigned int i; + int err; + + for (i = 0; i < count; i++) { + pr_debug("registering platform driver %ps\n", drivers[i]); + + err = platform_driver_register(drivers[i]); + if (err < 0) { + pr_err("failed to register platform driver %ps: %d\n", + drivers[i], err); + goto error; + } + } + + return 0; + +error: + while (i--) { + pr_debug("unregistering platform driver %ps\n", drivers[i]); + platform_driver_unregister(drivers[i]); + } + + return err; +} +EXPORT_SYMBOL_GPL(platform_register_drivers); + +/** + * platform_unregister_drivers - unregister an array of platform drivers + * @drivers: an array of drivers to unregister + * @count: the number of drivers to unregister + * + * Unegisters platform drivers specified by an array. This is typically used + * to complement an earlier call to platform_register_drivers(). Drivers are + * unregistered in the reverse order in which they were registered. + */ +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count) +{ + while (count--) { + pr_debug("unregistering platform driver %ps\n", drivers[count]); + platform_driver_unregister(drivers[count]); + } +} +EXPORT_SYMBOL_GPL(platform_unregister_drivers); + /* modalias support enables more hands-off userspace setup: * (a) environment variable lets new-style hotplug events work once system is * fully running: "modprobe $MODALIAS" diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index bba08f44cc97..0c9f16bfdd99 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -270,6 +270,11 @@ extern struct platform_device *__platform_create_bundle( struct resource *res, unsigned int n_res, const void *data, size_t size, struct module *module); +int platform_register_drivers(struct platform_driver * const *drivers, + unsigned int count); +void platform_unregister_drivers(struct platform_driver * const *drivers, + unsigned int count); + /* early platform driver interface */ struct early_platform_driver { const char *class_str; -- 2.5.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter daniel@ffwll.ch writes:
On Thu, Sep 24, 2015 at 7:02 PM, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Some modules register several sub-drivers. Provide a helper that makes it easy to register and unregister a list of sub-drivers, as well as unwind properly on error.
Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Thierry Reding treding@nvidia.com
I raised this already on irc but let's do it here too for the record: Eric Anholt has a very similar thing (but in drm only) with the addition of also integrating with the component framework:
http://lists.freedesktop.org/archives/dri-devel/2015-September/090449.html
Having something that works for everyone (so includes msm and can be used for vc4 too) would be great. Adding Eric.
I'm not sure if I should be providing a Reviewed-by here, but I like this patch. It obsoletes part of my patch series, and I look forward to using it in my driver.
dri-devel@lists.freedesktop.org
-
Andy Shevchenko -
Daniel Vetter -
Eric Anholt -
Greg Kroah-Hartman -
Jani Nikula -
Philipp Zabel -
Thierry Reding -
Vincent ABRIOU