| Message ID | 20230924192604.3262187-6-jernej.skrabec@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/sun4i: dw-hdmi: Fix initialization & refactor | expand |
Hi Jernej, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.6-rc3 next-20230921] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jernej-Skrabec/drm-sun4i-dw-hdmi-Deinit-PHY-in-fail-path/20230925-032818 base: linus/master patch link: https://lore.kernel.org/r/20230924192604.3262187-6-jernej.skrabec%40gmail.com patch subject: [PATCH 5/7] drm/sun4i: dw-hdmi: Split driver registration config: parisc-randconfig-002-20230925 (https://download.01.org/0day-ci/archive/20230925/202309251030.rZTXXyFE-lkp@intel.com/config) compiler: hppa-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230925/202309251030.rZTXXyFE-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309251030.rZTXXyFE-lkp@intel.com/ All errors (new ones prefixed by >>): hppa-linux-ld: drivers/gpu/drm/sun4i/sun8i_hdmi_phy.o: in function `sun8i_hdmi_phy_driver_init': >> (.init.text+0x0): multiple definition of `init_module'; drivers/gpu/drm/sun4i/sun8i_dw_hdmi.o:(.init.text+0x0): first defined here hppa-linux-ld: drivers/gpu/drm/sun4i/sun8i_hdmi_phy.o: in function `sun8i_hdmi_phy_driver_exit': >> (.exit.text+0x0): multiple definition of `cleanup_module'; drivers/gpu/drm/sun4i/sun8i_dw_hdmi.o:(.exit.text+0x0): first defined here
On Sun, Sep 24, 2023 at 09:26:02PM +0200, Jernej Skrabec wrote: > There is no reason to register two drivers in same place. Using macro > lowers amount of boilerplate code. There's one actually: you can't have several module_init functions in the some module, and both files are compiled into the same module. Maxime
Dne ponedeljek, 25. september 2023 ob 09:47:15 CEST je Maxime Ripard napisal(a): > On Sun, Sep 24, 2023 at 09:26:02PM +0200, Jernej Skrabec wrote: > > There is no reason to register two drivers in same place. Using macro > > lowers amount of boilerplate code. > > There's one actually: you can't have several module_init functions in > the some module, and both files are compiled into the same module. Yeah, I figured as much. However, I think code clean up is good enough reason to add hidden option in Kconfig and extra entry in Makefile (in v2). Do you agree? Best regards, Jernej
Hi Jernej, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.6-rc3 next-20230925] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jernej-Skrabec/drm-sun4i-dw-hdmi-Deinit-PHY-in-fail-path/20230925-032818 base: linus/master patch link: https://lore.kernel.org/r/20230924192604.3262187-6-jernej.skrabec%40gmail.com patch subject: [PATCH 5/7] drm/sun4i: dw-hdmi: Split driver registration config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20230926/202309260027.aNIjQRBI-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230926/202309260027.aNIjQRBI-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309260027.aNIjQRBI-lkp@intel.com/ All errors (new ones prefixed by >>): s390-linux-ld: drivers/gpu/drm/sun4i/sun8i_hdmi_phy.o: in function `sun8i_hdmi_phy_driver_init': >> sun8i_hdmi_phy.c:(.init.text+0x0): multiple definition of `init_module'; drivers/gpu/drm/sun4i/sun8i_dw_hdmi.o:sun8i_dw_hdmi.c:(.init.text+0x0): first defined here s390-linux-ld: drivers/gpu/drm/sun4i/sun8i_hdmi_phy.o: in function `sun8i_hdmi_phy_driver_exit': >> sun8i_hdmi_phy.c:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/gpu/drm/sun4i/sun8i_dw_hdmi.o:sun8i_dw_hdmi.c:(.exit.text+0x0): first defined here
On Mon, Sep 25, 2023 at 05:07:45PM +0200, Jernej Škrabec wrote: > Dne ponedeljek, 25. september 2023 ob 09:47:15 CEST je Maxime Ripard napisal(a): > > On Sun, Sep 24, 2023 at 09:26:02PM +0200, Jernej Skrabec wrote: > > > There is no reason to register two drivers in same place. Using macro > > > lowers amount of boilerplate code. > > > > There's one actually: you can't have several module_init functions in > > the some module, and both files are compiled into the same module. > > Yeah, I figured as much. However, I think code clean up is good enough reason > to add hidden option in Kconfig and extra entry in Makefile (in v2). > > Do you agree? Yeah, I don't know. Adding more modules makes it more difficult to handle (especially autoloading) without a clear argument why. Module_init is simple enough as it is currently, maybe we should just add a comment to make it clearer? Maxime
Dne četrtek, 05. oktober 2023 ob 10:43:14 CEST je Maxime Ripard napisal(a): > On Mon, Sep 25, 2023 at 05:07:45PM +0200, Jernej Škrabec wrote: > > Dne ponedeljek, 25. september 2023 ob 09:47:15 CEST je Maxime Ripard napisal(a): > > > On Sun, Sep 24, 2023 at 09:26:02PM +0200, Jernej Skrabec wrote: > > > > There is no reason to register two drivers in same place. Using macro > > > > lowers amount of boilerplate code. > > > > > > There's one actually: you can't have several module_init functions in > > > the some module, and both files are compiled into the same module. > > > > Yeah, I figured as much. However, I think code clean up is good enough reason > > to add hidden option in Kconfig and extra entry in Makefile (in v2). > > > > Do you agree? > > Yeah, I don't know. Adding more modules makes it more difficult to > handle (especially autoloading) without a clear argument why. > Module_init is simple enough as it is currently, maybe we should just > add a comment to make it clearer? I'll just drop this patch then. While I think autoloading works pretty good these days and cleaner code is nice, it can certainly cause some issues while packaging. Best regards, Jernej
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 93831cdf1917..d93e8ff71aae 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -378,32 +378,7 @@ static struct platform_driver sun8i_dw_hdmi_pltfm_driver = { .of_match_table = sun8i_dw_hdmi_dt_ids, }, }; - -static int __init sun8i_dw_hdmi_init(void) -{ - int ret; - - ret = platform_driver_register(&sun8i_dw_hdmi_pltfm_driver); - if (ret) - return ret; - - ret = platform_driver_register(&sun8i_hdmi_phy_driver); - if (ret) { - platform_driver_unregister(&sun8i_dw_hdmi_pltfm_driver); - return ret; - } - - return ret; -} - -static void __exit sun8i_dw_hdmi_exit(void) -{ - platform_driver_unregister(&sun8i_dw_hdmi_pltfm_driver); - platform_driver_unregister(&sun8i_hdmi_phy_driver); -} - -module_init(sun8i_dw_hdmi_init); -module_exit(sun8i_dw_hdmi_exit); +module_platform_driver(sun8i_dw_hdmi_pltfm_driver); MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec@siol.net>"); MODULE_DESCRIPTION("Allwinner DW HDMI bridge"); diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 18ffc1b4841f..21e010deeb48 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -194,8 +194,6 @@ struct sun8i_dw_hdmi { struct reset_control *rst_ctrl; }; -extern struct platform_driver sun8i_hdmi_phy_driver; - static inline struct sun8i_dw_hdmi * encoder_to_sun8i_dw_hdmi(struct drm_encoder *encoder) { diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 489ea94693ff..f917a979e4a4 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -729,10 +729,11 @@ static int sun8i_hdmi_phy_probe(struct platform_device *pdev) return 0; } -struct platform_driver sun8i_hdmi_phy_driver = { +static struct platform_driver sun8i_hdmi_phy_driver = { .probe = sun8i_hdmi_phy_probe, .driver = { .name = "sun8i-hdmi-phy", .of_match_table = sun8i_hdmi_phy_of_table, }, }; +module_platform_driver(sun8i_hdmi_phy_driver);
There is no reason to register two drivers in same place. Using macro lowers amount of boilerplate code. Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 27 +------------------------- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 -- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 3 ++- 3 files changed, 3 insertions(+), 29 deletions(-)