The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111") Signed-off-by: Arnd Bergmann arnd@arndb.de --- arch/arm/mach-s3c64xx/Kconfig | 1 - drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index 459214fa20b4..5ee5ad74a3d6 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -40,7 +40,6 @@ config CPU_S3C6410
config S3C64XX_PL080 def_bool DMADEVICES - select ARM_AMBA select AMBA_PL08X
config S3C64XX_SETUP_SDHCI diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index 309f4fd52de7..e2f393fcc4bf 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -2,6 +2,7 @@ config DRM_PL111 tristate "DRM Support for PL111 CLCD Controller" depends on DRM depends on ARM || ARM64 || COMPILE_TEST + depends on ARM_AMBA depends on COMMON_CLK select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index 97095b1aa961..dce382f97f8c 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -179,7 +179,6 @@ static struct drm_driver pl111_drm_driver = { #endif };
-#ifdef CONFIG_ARM_AMBA static int pl111_amba_probe(struct amba_device *amba_dev, const struct amba_id *id) { @@ -262,7 +261,6 @@ static struct amba_driver pl111_amba_driver = { };
module_amba_driver(pl111_amba_driver); -#endif /* CONFIG_ARM_AMBA */
MODULE_DESCRIPTION(DRIVER_DESC); MODULE_AUTHOR("ARM Ltd.");
Arnd Bergmann arnd@arndb.de writes:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
If I understand the Kconfig, you're effectively disabling the build on COMPILE_TEST && !ARM. I'd rather that we just fix up the warnings, so that people can keep build-testing DRM drivers:
On Mon, May 22, 2017 at 6:23 PM, Eric Anholt eric@anholt.net wrote:
Arnd Bergmann arnd@arndb.de writes:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
If I understand the Kconfig, you're effectively disabling the build on COMPILE_TEST && !ARM. I'd rather that we just fix up the warnings, so that people can keep build-testing DRM drivers:
That works too. When I tried the same, I thought it was a little too silly to have a file that effectively compiles into nothing.
My initial approach did two things differently though:
- use __maybe_unused instead of __used, since
- annotate pl111_amba_driver instead of pl111_drm_driver/pl111_modeset_init, and keep only module_amba_driver() in the #ifdef.
Arnd
Arnd Bergmann arnd@arndb.de writes:
On Mon, May 22, 2017 at 6:23 PM, Eric Anholt eric@anholt.net wrote:
Arnd Bergmann arnd@arndb.de writes:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
If I understand the Kconfig, you're effectively disabling the build on COMPILE_TEST && !ARM. I'd rather that we just fix up the warnings, so that people can keep build-testing DRM drivers:
That works too. When I tried the same, I thought it was a little too silly to have a file that effectively compiles into nothing.
My initial approach did two things differently though:
use __maybe_unused instead of __used, since
annotate pl111_amba_driver instead of pl111_drm_driver/pl111_modeset_init, and keep only module_amba_driver() in the #ifdef.
Oh, yeah, we've eliminated the amba_request_regions() that used to be why the probe had to be under the #ifdef, so your solution would get us better coverage and simpler code. If you could send that patch to dri-devel today, I'll get it applied.
On Tue, May 23, 2017 at 12:08 AM, Eric Anholt eric@anholt.net wrote:
Oh, yeah, we've eliminated the amba_request_regions() that used to be why the probe had to be under the #ifdef, so your solution would get us better coverage and simpler code. If you could send that patch to dri-devel today, I'll get it applied.
Sent now. I'll also send the oneline patch for s3c64xx separately, in case we hit the same circular dependency another time.
Arnd
On Mon, May 22, 2017 at 05:20:08PM +0200, Arnd Bergmann wrote:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111") Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/arm/mach-s3c64xx/Kconfig | 1 - drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index 459214fa20b4..5ee5ad74a3d6 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -40,7 +40,6 @@ config CPU_S3C6410
config S3C64XX_PL080 def_bool DMADEVICES
- select ARM_AMBA
I must admit that I miss how pl111 DRM driver create circular dependency with S3C64XX_PL080 (or AMBA_PL08X?). Maybe it is unrelated change and should be a separate patch?
Best regards, Krzysztof
select AMBA_PL08X
config S3C64XX_SETUP_SDHCI diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index 309f4fd52de7..e2f393fcc4bf 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -2,6 +2,7 @@ config DRM_PL111 tristate "DRM Support for PL111 CLCD Controller" depends on DRM depends on ARM || ARM64 || COMPILE_TEST
- depends on ARM_AMBA depends on COMMON_CLK select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index 97095b1aa961..dce382f97f8c 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -179,7 +179,6 @@ static struct drm_driver pl111_drm_driver = { #endif };
-#ifdef CONFIG_ARM_AMBA static int pl111_amba_probe(struct amba_device *amba_dev, const struct amba_id *id) { @@ -262,7 +261,6 @@ static struct amba_driver pl111_amba_driver = { };
module_amba_driver(pl111_amba_driver); -#endif /* CONFIG_ARM_AMBA */
MODULE_DESCRIPTION(DRIVER_DESC); MODULE_AUTHOR("ARM Ltd."); -- 2.9.0
-- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 22, 2017 at 6:32 PM, Krzysztof Kozlowski krzk@kernel.org wrote:
On Mon, May 22, 2017 at 05:20:08PM +0200, Arnd Bergmann wrote:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111") Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/arm/mach-s3c64xx/Kconfig | 1 - drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index 459214fa20b4..5ee5ad74a3d6 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -40,7 +40,6 @@ config CPU_S3C6410
config S3C64XX_PL080 def_bool DMADEVICES
select ARM_AMBAI must admit that I miss how pl111 DRM driver create circular dependency with S3C64XX_PL080 (or AMBA_PL08X?). Maybe it is unrelated change and should be a separate patch?
When I add 'depends on ARM_AMBA' for pl111, I get this dependency loop:
drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:381: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:369: symbol FB_CYBER2000 depends on FB For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:72: symbol DRM_KMS_FB_HELPER is selected by DRM_KMS_CMA_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:137: symbol DRM_KMS_CMA_HELPER is selected by DRM_PL111 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on ARM_AMBA For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/amba/Kconfig:1: symbol ARM_AMBA is selected by S3C64XX_PL080 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" arch/arm/mach-s3c64xx/Kconfig:42: symbol S3C64XX_PL080 default value contains DMADEVICES For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/dma/Kconfig:5: symbol DMADEVICES is selected by SND_SOC_SH4_SIU For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:29: symbol SND_SOC_SH4_SIU is selected by SND_SIU_MIGOR For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:59: symbol SND_SIU_MIGOR depends on I2C
We could break the loop at any of those places, but the S3C64XX_PL080 symbol seemed the easiest way since the 'select' there is completely unnecessary and it directly relates to the change I made in PL111.
Arnd
On Mon, May 22, 2017 at 11:37 PM, Arnd Bergmann arnd@arndb.de wrote:
On Mon, May 22, 2017 at 6:32 PM, Krzysztof Kozlowski krzk@kernel.org wrote:
On Mon, May 22, 2017 at 05:20:08PM +0200, Arnd Bergmann wrote:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111") Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/arm/mach-s3c64xx/Kconfig | 1 - drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index 459214fa20b4..5ee5ad74a3d6 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -40,7 +40,6 @@ config CPU_S3C6410
config S3C64XX_PL080 def_bool DMADEVICES
select ARM_AMBAI must admit that I miss how pl111 DRM driver create circular dependency with S3C64XX_PL080 (or AMBA_PL08X?). Maybe it is unrelated change and should be a separate patch?
When I add 'depends on ARM_AMBA' for pl111, I get this dependency loop:
drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:381: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:369: symbol FB_CYBER2000 depends on FB For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:72: symbol DRM_KMS_FB_HELPER is selected by DRM_KMS_CMA_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:137: symbol DRM_KMS_CMA_HELPER is selected by DRM_PL111 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on ARM_AMBA For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/amba/Kconfig:1: symbol ARM_AMBA is selected by S3C64XX_PL080 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" arch/arm/mach-s3c64xx/Kconfig:42: symbol S3C64XX_PL080 default value contains DMADEVICES For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/dma/Kconfig:5: symbol DMADEVICES is selected by SND_SOC_SH4_SIU For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:29: symbol SND_SOC_SH4_SIU is selected by SND_SIU_MIGOR For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:59: symbol SND_SIU_MIGOR depends on I2C
We could break the loop at any of those places, but the S3C64XX_PL080 symbol seemed the easiest way since the 'select' there is completely unnecessary and it directly relates to the change I made in PL111.
Assuming ARM_AMBA is unnecessary here, then it should be unnecessary in other platforms as well but we have it (git grep 'select ARM_AMBA'). The ARM_AMBA is a dependency of AMBA_PL08X which is selected by S3C64xx... so IMHO this is needed.
Best regards, Krzysztof
On Tue, May 23, 2017 at 1:17 PM, Krzysztof Kozlowski krzk@kernel.org wrote:
On Mon, May 22, 2017 at 11:37 PM, Arnd Bergmann arnd@arndb.de wrote:
On Mon, May 22, 2017 at 6:32 PM, Krzysztof Kozlowski krzk@kernel.org wrote:
On Mon, May 22, 2017 at 05:20:08PM +0200, Arnd Bergmann wrote:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111") Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/arm/mach-s3c64xx/Kconfig | 1 - drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index 459214fa20b4..5ee5ad74a3d6 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -40,7 +40,6 @@ config CPU_S3C6410
config S3C64XX_PL080 def_bool DMADEVICES
select ARM_AMBAI must admit that I miss how pl111 DRM driver create circular dependency with S3C64XX_PL080 (or AMBA_PL08X?). Maybe it is unrelated change and should be a separate patch?
When I add 'depends on ARM_AMBA' for pl111, I get this dependency loop:
drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:381: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:369: symbol FB_CYBER2000 depends on FB For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:72: symbol DRM_KMS_FB_HELPER is selected by DRM_KMS_CMA_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:137: symbol DRM_KMS_CMA_HELPER is selected by DRM_PL111 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on ARM_AMBA For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/amba/Kconfig:1: symbol ARM_AMBA is selected by S3C64XX_PL080 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" arch/arm/mach-s3c64xx/Kconfig:42: symbol S3C64XX_PL080 default value contains DMADEVICES For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/dma/Kconfig:5: symbol DMADEVICES is selected by SND_SOC_SH4_SIU For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:29: symbol SND_SOC_SH4_SIU is selected by SND_SIU_MIGOR For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:59: symbol SND_SIU_MIGOR depends on I2C
We could break the loop at any of those places, but the S3C64XX_PL080 symbol seemed the easiest way since the 'select' there is completely unnecessary and it directly relates to the change I made in PL111.
Assuming ARM_AMBA is unnecessary here, then it should be unnecessary in other platforms as well but we have it (git grep 'select ARM_AMBA'). The ARM_AMBA is a dependency of AMBA_PL08X which is selected by S3C64xx... so IMHO this is needed.
I guess I should have explained it in more detail: S3C64XX_PL080 is set to 'ARCH_S3C64XX && DMADEVICES', and ARCH_S3C64XX selects ARM_AMBA, so the 'select ARM_AMBA' for S3C64XX_PL080 is redundant. We still need it, but it's already guaranteed to be enabled.
Arnd
On Tue, May 23, 2017 at 1:22 PM, Arnd Bergmann arnd@arndb.de wrote:
On Tue, May 23, 2017 at 1:17 PM, Krzysztof Kozlowski krzk@kernel.org wrote:
On Mon, May 22, 2017 at 11:37 PM, Arnd Bergmann arnd@arndb.de wrote:
On Mon, May 22, 2017 at 6:32 PM, Krzysztof Kozlowski krzk@kernel.org wrote:
On Mon, May 22, 2017 at 05:20:08PM +0200, Arnd Bergmann wrote:
The driver is written in a way to enable compile-testing without CONFIG_ARM_AMBA, but it just causes needless warnings:
drivers/gpu/drm/pl111/pl111_drv.c:149:26: error: 'pl111_drm_driver' defined but not used [-Werror=unused-variable] drivers/gpu/drm/pl111/pl111_drv.c:81:12: error: 'pl111_modeset_init' defined but not used [-Werror=unused-function]
This removes the #ifdef instead, and adds a dependency on ARM_AMBA to only let us build the driver when the base support is enabled.
Unfortunately, this requires removing one redundant 'select ARM_AMBA' line from mach-s3c64xx to avoid a circular dependency.
It might be good to allow manually enabling ARM_AMBA when COMPILE_TEST is turned on, but that should be a separate patch and may cause other build regressions.
Fixes: bed41005e617 ("drm/pl111: Initial drm/kms driver for pl111") Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/arm/mach-s3c64xx/Kconfig | 1 - drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_drv.c | 2 -- 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig index 459214fa20b4..5ee5ad74a3d6 100644 --- a/arch/arm/mach-s3c64xx/Kconfig +++ b/arch/arm/mach-s3c64xx/Kconfig @@ -40,7 +40,6 @@ config CPU_S3C6410
config S3C64XX_PL080 def_bool DMADEVICES
select ARM_AMBAI must admit that I miss how pl111 DRM driver create circular dependency with S3C64XX_PL080 (or AMBA_PL08X?). Maybe it is unrelated change and should be a separate patch?
When I add 'depends on ARM_AMBA' for pl111, I get this dependency loop:
drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:381: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:369: symbol FB_CYBER2000 depends on FB For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:72: symbol DRM_KMS_FB_HELPER is selected by DRM_KMS_CMA_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:137: symbol DRM_KMS_CMA_HELPER is selected by DRM_PL111 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on ARM_AMBA For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/amba/Kconfig:1: symbol ARM_AMBA is selected by S3C64XX_PL080 For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" arch/arm/mach-s3c64xx/Kconfig:42: symbol S3C64XX_PL080 default value contains DMADEVICES For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/dma/Kconfig:5: symbol DMADEVICES is selected by SND_SOC_SH4_SIU For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:29: symbol SND_SOC_SH4_SIU is selected by SND_SIU_MIGOR For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" sound/soc/sh/Kconfig:59: symbol SND_SIU_MIGOR depends on I2C
We could break the loop at any of those places, but the S3C64XX_PL080 symbol seemed the easiest way since the 'select' there is completely unnecessary and it directly relates to the change I made in PL111.
Assuming ARM_AMBA is unnecessary here, then it should be unnecessary in other platforms as well but we have it (git grep 'select ARM_AMBA'). The ARM_AMBA is a dependency of AMBA_PL08X which is selected by S3C64xx... so IMHO this is needed.
I guess I should have explained it in more detail: S3C64XX_PL080 is set to 'ARCH_S3C64XX && DMADEVICES', and ARCH_S3C64XX selects ARM_AMBA, so the 'select ARM_AMBA' for S3C64XX_PL080 is redundant. We still need it, but it's already guaranteed to be enabled.
Ah, yes, I missed the S3C64XX_PL080. Looks fine. For the s3c64xx: Acked-by: Krzysztof Kozlowski krzk@kernel.org
Best regards, Krzysztof
dri-devel@lists.freedesktop.org
-
Arnd Bergmann -
Eric Anholt -
Krzysztof Kozlowski