Re: [PATCH] drm: rcar-du: fix linker undefined references

[Randy Dunlap <rdunlap@infradead.org>]

On 4/23/21 2:56 PM, Randy Dunlap wrote:
> On 4/23/21 2:46 PM, Laurent Pinchart wrote:
>> Hi Randy,
>>
>> Thank you for the patch.
>>
>> On Fri, Apr 23, 2021 at 02:37:27PM -0700, Randy Dunlap wrote:
>>> When DRM_RCAR_DU=y and DRM_RCAR_LVDS=m, there are several build errors
>>> as reported by 'kernel test robot'. These can be corrected by changing
>>> "imply" to "select".
>>>
>>> In looking at this, the same problem (build errors) happens when
>>> DRM_RCAR_DU=y and DRM_RCAR_CMM=m, so again change the "imply" to
>>> "select" here as well.
>>>
>>> These 2 changes fix the following 8 build/link errors:
>>>
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
>>> `rcar_du_crtc_atomic_enable':
>>> rcar_du_crtc.c:(.text+0x1be8): undefined reference to 
>>> `rcar_lvds_clk_enable'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
>>> `rcar_du_crtc_atomic_disable':
>>> rcar_du_crtc.c:(.text+0x2438): undefined reference to 
>>> `rcar_lvds_clk_disable'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_drv.o: in function 
>>> `rcar_du_init':
>>> rcar_du_drv.c:(.init.text+0x14): undefined reference to 
>>> `rcar_du_of_init'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_encoder.o: in 
>>> function `rcar_du_encoder_init':
>>> rcar_du_encoder.c:(.text+0x1d4): undefined reference to 
>>> `rcar_lvds_dual_link'
>>>
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
>>> `rcar_du_cmm_setup':
>>> rcar_du_crtc.c:(.text+0x380): undefined reference to `rcar_cmm_setup'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
>>> `rcar_du_crtc_atomic_enable':
>>> rcar_du_crtc.c:(.text+0x1c08): undefined reference to `rcar_cmm_enable'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_crtc.o: in function 
>>> `rcar_du_crtc_atomic_disable':
>>> rcar_du_crtc.c:(.text+0x231c): undefined reference to `rcar_cmm_disable'
>>> aarch64-linux-ld: drivers/gpu/drm/rcar-du/rcar_du_kms.o: in function 
>>> `rcar_du_modeset_init':
>>> rcar_du_kms.c:(.text+0xd08): undefined reference to `rcar_cmm_init'
>>>
>>> Fixes: e08e934d6c28 ("drm: rcar-du: Add support for CMM")
>>> Fixes: 02f2b30032c1 ("drm: rcar-du: lvds: Add API to enable/disable 
>>> clock output")
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: linux-renesas-soc@vger.kernel.org
>>> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>
>>> ---
>>>   drivers/gpu/drm/rcar-du/Kconfig |    4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> --- linux-next-20210420.orig/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ linux-next-20210420/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -4,8 +4,8 @@ config DRM_RCAR_DU
>>>       depends on DRM && OF
>>>       depends on ARM || ARM64
>>>       depends on ARCH_RENESAS || COMPILE_TEST
>>> -    imply DRM_RCAR_CMM
>>> -    imply DRM_RCAR_LVDS
>>> +    select DRM_RCAR_CMM
>>> +    select DRM_RCAR_LVDS
>>
>> No all platforms that integrate a DU have CMM and/or LVDS support, so we
>> shouldn't select the automatically.
>>
>> Would
>>
>>     depends on DRM_RCAR_CMM || DRM_RCAR_CMM=n
>>     depends on DRM_RCAR_LVDS || DRM_RCAR_LVDS=n
>>
> 
> Looks good; I like it, but kconfig does not:
> 
> $ xbuild_arm64.sh  oldconfig
> make[1]: Entering directory 
> '/home/rdunlap/lnx/next/linux-next-20210420/ARM64'
>    GEN     Makefile
> drivers/gpu/drm/rcar-du/Kconfig:2:error: recursive dependency detected!
> drivers/gpu/drm/rcar-du/Kconfig:2:    symbol DRM_RCAR_DU depends on 
> DRM_RCAR_CMM
> drivers/gpu/drm/rcar-du/Kconfig:17:    symbol DRM_RCAR_CMM depends on 
> DRM_RCAR_DU
> For a resolution refer to Documentation/kbuild/kconfig-language.rst
> subsection "Kconfig recursive dependency limitations"
> 
>> work ? I thought that's what "imply" meant, but it seems I got it wrong.
>>
>>>       select DRM_KMS_HELPER
>>>       select DRM_KMS_CMA_HELPER
>>>       select DRM_GEM_CMA_HELPER
>>

There is some documentation (kconfig-language.rst) that says that when a 
driver Kconfig is using mixed builtin and loadable modules, the driver 
should be using IS_REACHABLE(). The example also uses "imply", so maybe
we should/could try something like that:
If CMM is REACHBLE(), make the calls.
If LVDS is READABLE(), make the calls.

I can look into that some more if you want me to.
Or I'll be happy to let you do so. :)

Thanks.