Re: linux-next: Tree for Jul 27 (drivers/gpu/drm/xlnx/zynqmp-dpsub)

[Randy Dunlap <rdunlap@infradead.org>]

On 7/28/20 3:32 PM, Laurent Pinchart wrote:
> Hi Randy,
> 
> (adding a few people to the CC list to discuss the proposed solution
> below)
> 
> Thanks for the report.
> 
> On Mon, Jul 27, 2020 at 05:49:41PM -0700, Randy Dunlap wrote:
>> On 7/27/20 6:23 AM, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> Changes since 20200724:
>>
>> on x86_64:
>>
>> WARNING: unmet direct dependencies detected for DMA_ENGINE
>>   Depends on [n]: DMADEVICES [=n]
>>   Selected by [m]:
>>   - DRM_ZYNQMP_DPSUB [=m] && HAS_IOMEM [=y] && (ARCH_ZYNQMP || COMPILE_TEST [=y]) && COMMON_CLK [=y] && DRM [=m] && OF [=y]
>>
>> and about 45 "undefined reference" build errors (here's a sample):
>>
>> ld: drivers/misc/mic/scif/scif_dma.o: in function `scif_sync_dma.constprop.11':
>> scif_dma.c:(.text+0x672): undefined reference to `dma_sync_wait'
>> ld: drivers/spi/spi-bcm2835.o: in function `bcm2835_dma_release.isra.9':
>> spi-bcm2835.c:(.text+0xb34): undefined reference to `dma_release_channel'
>> ld: spi-bcm2835.c:(.text+0xc17): undefined reference to `dma_release_channel'
>> ld: drivers/spi/spi-bcm2835.o: in function `bcm2835_dma_init':
>> spi-bcm2835.c:(.text+0xd3c): undefined reference to `dma_request_chan'
>> ld: spi-bcm2835.c:(.text+0xd8b): undefined reference to `dma_request_chan'
>> ld: spi-bcm2835.c:(.text+0xf8a): undefined reference to `dma_get_slave_caps'
>> ld: spi-bcm2835.c:(.text+0x11d0): undefined reference to `dma_get_slave_caps'
>> ld: drivers/spi/spi-ep93xx.o: in function `ep93xx_spi_release_dma':
>> spi-ep93xx.c:(.text+0x1fc): undefined reference to `dma_release_channel'
>> ld: spi-ep93xx.c:(.text+0x220): undefined reference to `dma_release_channel'
>> ERROR: modpost: "dma_release_channel" [drivers/gpu/drm/xlnx/zynqmp-dpsub.ko] undefined!
>>
>>
>>
>> I tried adding
>> 	depends on DMADEVICES
>> to DRM_ZYNQMP_DPSUB
>>
>> but that just gets into messy/ugly Kconfig
>> 	error: recursive dependency detected!
> 
> drivers/i2c/Kconfig:8:error: recursive dependency detected!
> drivers/i2c/Kconfig:8:  symbol I2C is selected by FB_DDC
> drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB
> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
> drivers/gpu/drm/Kconfig:80:     symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER
> drivers/gpu/drm/Kconfig:74:     symbol DRM_KMS_HELPER is selected by DRM_ZYNQMP_DPSUB
> drivers/gpu/drm/xlnx/Kconfig:1: symbol DRM_ZYNQMP_DPSUB depends on DMA_ENGINE
> drivers/dma/Kconfig:44: symbol DMA_ENGINE depends on DMADEVICES
> drivers/dma/Kconfig:6:  symbol DMADEVICES is selected by SND_SOC_SH4_SIU
> sound/soc/sh/Kconfig:30:        symbol SND_SOC_SH4_SIU is selected by SND_SIU_MIGOR
> sound/soc/sh/Kconfig:60:        symbol SND_SIU_MIGOR depends on I2C
> For a resolution refer to Documentation/kbuild/kconfig-language.rst
> subsection "Kconfig recursive dependency limitations"
> 
>> BTW, adding
>> 	select DMADEVICES
>> is not a good solution.  We try very hard not to enable entire
>> subsystems with one driver "select".  (No doubt you can find a
>> few examples that do just that, but it is strongly discouraged.)
> 
> If this isn't allowed, then we need to fix this where used, as that's
> what seems to cause the recursive dependency. Would the following
> patches be acceptable in your opinion ? If so I'll submit them proper.

Yes, they look like they do the right thing and I tested them.

Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks for doing this.

> commit 410e29afd54fd23ee94cd1842b51b7a9e2f96cd8
> Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Date:   Wed Jul 29 01:19:40 2020 +0300
> 
>     treewide: kconfig: Replace 'select' DMAENGINES 'with depends on'
>     
>     Enabling a whole subsystem from a single driver 'select' is frowned
>     upon and won't be accepted in new drivers, that need to use 'depends on'
>     instead. Existing selection of DMAENGINES will then cause circular
>     dependencies. Replace them with a dependency.
>     
>     Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> diff --git a/drivers/rapidio/Kconfig b/drivers/rapidio/Kconfig
> index e4c422d806be..b9f8514909bf 100644
> --- a/drivers/rapidio/Kconfig
> +++ b/drivers/rapidio/Kconfig
> @@ -37,7 +37,7 @@ config RAPIDIO_ENABLE_RX_TX_PORTS
>  config RAPIDIO_DMA_ENGINE
>  	bool "DMA Engine support for RapidIO"
>  	depends on RAPIDIO
> -	select DMADEVICES
> +	depends on DMADEVICES
>  	select DMA_ENGINE
>  	help
>  	  Say Y here if you want to use DMA Engine frameork for RapidIO data
> diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig
> index dc20f0f7080a..ef8a29b9f641 100644
> --- a/sound/soc/sh/Kconfig
> +++ b/sound/soc/sh/Kconfig
> @@ -30,8 +30,8 @@ config SND_SOC_SH4_FSI
>  config SND_SOC_SH4_SIU
>  	tristate
>  	depends on ARCH_SHMOBILE && HAVE_CLK
> +	depends on DMADEVICES
>  	select DMA_ENGINE
> -	select DMADEVICES
>  	select SH_DMAE
>  	select FW_LOADER
>  
> commit 3e68c8fc7a2f3f7992c7fa8b30108d3831c7fb3b
> Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Date:   Wed Jul 29 01:23:32 2020 +0300
> 
>     drm: xlnx: dpsub: Fix DMADEVICES Kconfig dependency
> 
>     The dpsub driver uses the DMA engine API, and thus selects DMA_ENGINE to
>     provide that API. DMA_ENGINE depends on DMADEVICES, which can be
>     deselected by the user, creating a possibly unmet indirect dependency:
> 
>     WARNING: unmet direct dependencies detected for DMA_ENGINE
>       Depends on [n]: DMADEVICES [=n]
>       Selected by [m]:
>       - DRM_ZYNQMP_DPSUB [=m] && HAS_IOMEM [=y] && (ARCH_ZYNQMP || COMPILE_TEST [=y]) && COMMON_CLK [=y] && DRM [=m] && OF [=y]
> 
>     Add a dependency on DMADEVICES to fix this.
> 
>     Reported-by: Randy Dunlap <rdunlap@infradead.org>
>     Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index aa6cd889bd11..b52c6cdfc0b8 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -2,6 +2,7 @@ config DRM_ZYNQMP_DPSUB
>  	tristate "ZynqMP DisplayPort Controller Driver"
>  	depends on ARCH_ZYNQMP || COMPILE_TEST
>  	depends on COMMON_CLK && DRM && OF
> +	depends on DMADEVICES
>  	select DMA_ENGINE
>  	select DRM_GEM_CMA_HELPER
>  	select DRM_KMS_CMA_HELPER
> 
>> Full randconfig file is attached.
>>
>> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> 
> [snip]
> 


-- 
~Randy