Hi, On Fri, Apr 16, 2021 at 7:32 PM kernel test robot wrote: > > Hi Douglas, > > I love your patch! Yet something to improve: > > [auto build test ERROR on next-20210416] > [cannot apply to wsa/i2c/for-next robh/for-next linus/master v5.12-rc7 v5.12-rc6 v5.12-rc5 v5.12-rc7] > [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] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/drm-Fix-EDID-reading-on-ti-sn65dsi86-solve-some-chicken-and-egg-problems/20210417-064243 > base: 18250b538735142307082e4e99e3ae5c12d44013 > config: x86_64-randconfig-a002-20210416 (attached as .config) > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project f549176ad976caa3e19edd036df9a7e12770af7c) > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # install x86_64 cross compiling tool for clang build > # apt-get install binutils-x86-64-linux-gnu > # https://github.com/0day-ci/linux/commit/a870b6e38fac3e5453e4b74fdfe6eb05c8be7ea7 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Douglas-Anderson/drm-Fix-EDID-reading-on-ti-sn65dsi86-solve-some-chicken-and-egg-problems/20210417-064243 > git checkout a870b6e38fac3e5453e4b74fdfe6eb05c8be7ea7 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > > >> drivers/gpu/drm/bridge/ti-sn65dsi86.c:1308:1: error: redefinition of '__inittest' > module_auxiliary_driver(ti_sn_bridge_driver); > ^ > include/linux/auxiliary_bus.h:71:2: note: expanded from macro 'module_auxiliary_driver' > module_driver(__auxiliary_driver, auxiliary_driver_register, auxiliary_driver_unregister) > ^ > include/linux/device/driver.h:262:3: note: expanded from macro 'module_driver' > } \ > ^ > include/linux/module.h:130:42: note: expanded from macro '\ > module_init' > static inline initcall_t __maybe_unused __inittest(void) \ > ^ > drivers/gpu/drm/bridge/ti-sn65dsi86.c:1190:1: note: previous definition is here > module_auxiliary_driver(ti_sn_gpio_driver); > ^ > include/linux/auxiliary_bus.h:71:2: note: expanded from macro 'module_auxiliary_driver' > module_driver(__auxiliary_driver, auxiliary_driver_register, auxiliary_driver_unregister) > ^ > include/linux/device/driver.h:262:3: note: expanded from macro 'module_driver' > } \ > ^ > include/linux/module.h:130:42: note: expanded from macro '\ > module_init' > static inline initcall_t __maybe_unused __inittest(void) \ Ah, my mistake in individually using these in the same module: module_auxiliary_driver(ti_sn_gpio_driver); module_auxiliary_driver(ti_sn_bridge_driver); module_auxiliary_driver(ti_sn_aux_driver); module_i2c_driver(ti_sn65dsi86_driver); What I had worked fine because I wasn't building as a module. I've fixed this to have a manual init mechanism that will look something like this at the end of the series: --- static int __init ti_sn65dsi86_init(void) { int ret; ret = i2c_add_driver(&ti_sn65dsi86_driver); if (ret) return ret; ret = ti_sn_gpio_register(); if (ret) goto err_main_was_registered; ret = auxiliary_driver_register(&ti_sn_aux_driver); if (ret) goto err_gpio_was_registered; ret = auxiliary_driver_register(&ti_sn_bridge_driver); if (ret) goto err_aux_was_registered; return 0; err_aux_was_registered: auxiliary_driver_unregister(&ti_sn_aux_driver); err_gpio_was_registered: ti_sn_gpio_unregister(); err_main_was_registered: i2c_del_driver(&ti_sn65dsi86_driver); return ret; } module_init(ti_sn65dsi86_init); static void __exit ti_sn65dsi86_exit(void) { auxiliary_driver_unregister(&ti_sn_bridge_driver); auxiliary_driver_unregister(&ti_sn_aux_driver); ti_sn_gpio_unregister(); i2c_del_driver(&ti_sn65dsi86_driver); } module_exit(ti_sn65dsi86_exit); --- With that I can compile as a module and everything works fine with this builtin. I'll plan to spin a v5 with that fix but I'll wait a little bit to see if I get any feedback. If I happen to get drm-misc commit access or can convince someone, I'll try to get the early patches in the series landed so v5 isn't so giant. NOTE: on my system sn65dsi86 doesn't actually work currently when running as a module. That's a pre-existing problem and not one introduced by my patch. Or perhaps, more appropriately, a pre-existing pile of problems that I'm not going to try to tackle. A quick summary: * Part of the problem of making this a module is that I run into the looping I spent a little bit of time looking at in the past [1]. I believe the main MSM graphics driver can't handle itself being builtin but some of the things it needs being in modules. * Part of the problem is fw_devlink. I don't think it understands the circularness of the panel and HPD lines and it seems to get upset unless in permissive mode. * If I try permissive mode and move the whole MSM graphics to a module, I get an error about 'disp_cc_mdss_mdp_clk_src: RCG did not turn on'. Again, this is without my patch series. Those are not small problems and not new, so I'll settle for making sure I continue to compile as a module. [1] https://lore.kernel.org/lkml/20200710230224.2265647-1-dianders@chromium.org/ _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel