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

From: Randy Dunlap
Date: Tue Jul 28 2020 - 18:52:59 EST

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@xxxxxxxxxxxxx>

Thanks for doing this.

> commit 410e29afd54fd23ee94cd1842b51b7a9e2f96cd8
> Author: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
>
> [snip]
>


--
~Randy