* [PATCH 1/2] fbdev: don't select I2C directly @ 2018-01-15 16:14 ` Arnd Bergmann [not found] ` <20180115161431.803248-1-arnd-r2nGTMty4D4@public.gmane.org> 2018-02-01 16:14 ` [PATCH 1/2] fbdev: don't select I2C directly Bartlomiej Zolnierkiewicz 0 siblings, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2018-01-15 16:14 UTC (permalink / raw) To: Rob Clark Cc: linux-fbdev, Arnd Bergmann, Wolfram Sang, Bartlomiej Zolnierkiewicz, linux-kernel, dri-devel, Srinivas Kandagatla, linux-i2c Using a Kconfig 'select' statement for a user-visible symbol that other drivers depend on often causes circular dependencies. A new one showed up when I wanted to add an NVMEM dependency to the DRM_MSM driver: drivers/i2c/Kconfig:7:error: recursive dependency detected! drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC drivers/video/fbdev/Kconfig:390: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 drivers/video/fbdev/Kconfig:378: symbol FB_CYBER2000 depends on FB drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER drivers/gpu/drm/Kconfig:77: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER drivers/gpu/drm/Kconfig:71: symbol DRM_KMS_HELPER is selected by DRM_MSM drivers/gpu/drm/msm/Kconfig:2: symbol DRM_MSM depends on NVMEM drivers/nvmem/Kconfig:1: symbol NVMEM is selected by EEPROM_AT24 drivers/misc/eeprom/Kconfig:3: symbol EEPROM_AT24 depends on I2C Here, the problem is that many fbdev drivers have an i2c based interface and just 'select i2c' for that, while we also select the framebuffer subsystem indirectly from a DRM driver that now depends on i2c. This does away with the 'select' statement and instead uses 'depends on', like almost all I2C users. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/video/fbdev/Kconfig | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 6962b4583fd7..892eb1863100 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -64,7 +64,6 @@ config FB_DDC tristate depends on FB select I2C_ALGOBIT - select I2C default n config FB_BOOT_VESA_SUPPORT @@ -390,6 +389,7 @@ config FB_CYBER2000 config FB_CYBER2000_DDC bool "DDC for CyberPro support" depends on FB_CYBER2000 + depends on I2C=y || I2Cû_CYBER2000 select FB_DDC default y help @@ -634,7 +634,7 @@ config FB_BF537_LQ035 config FB_BFIN_7393 tristate "Blackfin ADV7393 Video encoder" depends on FB && BLACKFIN - select I2C + depends on I2C select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1026,6 +1026,7 @@ config FB_NVIDIA config FB_NVIDIA_I2C bool "Enable DDC Support" depends on FB_NVIDIA + depends on I2C=y || I2Cû_NVIDIA select FB_DDC help This enables I2C support for nVidia Chipsets. This is used @@ -1073,6 +1074,7 @@ config FB_RIVA config FB_RIVA_I2C bool "Enable DDC Support" depends on FB_RIVA + depends on I2C=y || I2Cû_RIVA select FB_DDC help This enables I2C support for nVidia Chipsets. This is used @@ -1102,6 +1104,7 @@ config FB_RIVA_BACKLIGHT config FB_I740 tristate "Intel740 support" depends on FB && PCI + depends on I2C select FB_MODE_HELPERS select FB_CFB_FILLRECT select FB_CFB_COPYAREA @@ -1155,6 +1158,7 @@ config FB_I810_GTF config FB_I810_I2C bool "Enable DDC Support" depends on FB_I810 && FB_I810_GTF + depends on I2C=y || I2Cû_I810 select FB_DDC help @@ -1206,6 +1210,7 @@ config FB_INTEL_DEBUG config FB_INTEL_I2C bool "DDC/I2C for Intel framebuffer support" depends on FB_INTEL + depends on I2C=y || I2Cû_INTEL select FB_DDC default y help @@ -1285,6 +1290,7 @@ config FB_MATROX_G config FB_MATROX_I2C tristate "Matrox I2C support" depends on FB_MATROX + depends on I2C select FB_DDC ---help--- This drivers creates I2C buses which are needed for accessing the @@ -1350,6 +1356,7 @@ config FB_RADEON config FB_RADEON_I2C bool "DDC/I2C for ATI Radeon support" depends on FB_RADEON + depends on I2C=y || I2Cû_RADEON select FB_DDC default y help @@ -1460,6 +1467,7 @@ config FB_S3 config FB_S3_DDC bool "DDC for S3 support" depends on FB_S3 + depends on I2C=y || I2Cû_S3 select FB_DDC default y help @@ -1485,6 +1493,7 @@ config FB_SAVAGE config FB_SAVAGE_I2C bool "Enable DDC2 Support" depends on FB_SAVAGE + depends on I2C=y || I2Cû_SAVAGE select FB_DDC help This enables I2C support for S3 Savage Chipsets. This is used @@ -1629,6 +1638,7 @@ config FB_3DFX_ACCEL config FB_3DFX_I2C bool "Enable DDC/I2C support" depends on FB_3DFX + depends on I2C=y || I2Cû_3DFX select FB_DDC default y help @@ -1669,6 +1679,7 @@ config FB_VT8623 config FB_TRIDENT tristate "Trident/CyberXXX/CyberBlade support" depends on FB && PCI + depends on I2C select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -2299,8 +2310,8 @@ endchoice config FB_MB862XX_I2C bool "Support I2C bus on MB862XX GDC" - depends on FB_MB862XX && I2C - depends on FB_MB862XX=m || I2C=y + depends on FB_MB862XX + depends on I2C=y || I2Cû_MB862XX default y help Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter -- 2.9.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20180115161431.803248-1-arnd-r2nGTMty4D4@public.gmane.org>]
* [PATCH 2/2] drm/msm/adreno: fix nvmem related link error [not found] ` <20180115161431.803248-1-arnd-r2nGTMty4D4@public.gmane.org> @ 2018-01-15 16:14 ` Arnd Bergmann 2018-01-17 21:21 ` [Freedreno] " Jordan Crouse 2018-01-17 21:52 ` Rob Clark 0 siblings, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2018-01-15 16:14 UTC (permalink / raw) To: Rob Clark Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Wolfram Sang, David Airlie, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Bartlomiej Zolnierkiewicz, Archit Taneja, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Bjorn Andersson, Jordan Crouse, Srinivas Kandagatla, linux-i2c-u79uwXL29TY76Z2rM5mHXA, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-kernel-u79uwXL29TY76Z2rM5mHXA When NVMEM is configured as a loadable module, and adreno is built-in, we get a link failure: drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_gpu_init': a5xx_gpu.c:(.text+0x15cc): undefined reference to `nvmem_cell_get' a5xx_gpu.c:(.text+0x15da): undefined reference to `nvmem_cell_read' a5xx_gpu.c:(.text+0x15e4): undefined reference to `nvmem_cell_put' This adds a Kconfig dependency to enforce valid configurations, when NVMEM is a loadable module, adreno now has to also be one. The code seems to deal fine with nvmem being completely disabled, it will just not set the right speed bin then, so we don't need a hard dependency. Fixes: f56d9df656c4 ("drm/msm/adreno: Read the speed bins for a5xx targets") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/msm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 99d39b2aefa6..74fb1c816da9 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -4,6 +4,7 @@ config DRM_MSM depends on DRM depends on ARCH_QCOM || (ARM && COMPILE_TEST) depends on OF && COMMON_CLK + depends on NVMEM || !NVMEM depends on MMU select QCOM_MDT_LOADER if ARCH_QCOM select REGULATOR -- 2.9.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Freedreno] [PATCH 2/2] drm/msm/adreno: fix nvmem related link error 2018-01-15 16:14 ` [PATCH 2/2] drm/msm/adreno: fix nvmem related link error Arnd Bergmann @ 2018-01-17 21:21 ` Jordan Crouse 2018-01-17 21:52 ` Rob Clark 1 sibling, 0 replies; 9+ messages in thread From: Jordan Crouse @ 2018-01-17 21:21 UTC (permalink / raw) To: Arnd Bergmann Cc: Rob Clark, linux-fbdev, Wolfram Sang, David Airlie, linux-arm-msm, Bartlomiej Zolnierkiewicz, Archit Taneja, dri-devel, Bjorn Andersson, Srinivas Kandagatla, linux-i2c, freedreno, linux-kernel On Mon, Jan 15, 2018 at 05:14:05PM +0100, Arnd Bergmann wrote: > When NVMEM is configured as a loadable module, and adreno > is built-in, we get a link failure: > > drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_gpu_init': > a5xx_gpu.c:(.text+0x15cc): undefined reference to `nvmem_cell_get' > a5xx_gpu.c:(.text+0x15da): undefined reference to `nvmem_cell_read' > a5xx_gpu.c:(.text+0x15e4): undefined reference to `nvmem_cell_put' > > This adds a Kconfig dependency to enforce valid configurations, > when NVMEM is a loadable module, adreno now has to also be one. > The code seems to deal fine with nvmem being completely disabled, > it will just not set the right speed bin then, so we don't need > a hard dependency. > > Fixes: f56d9df656c4 ("drm/msm/adreno: Read the speed bins for a5xx targets") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > drivers/gpu/drm/msm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index 99d39b2aefa6..74fb1c816da9 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -4,6 +4,7 @@ config DRM_MSM > depends on DRM > depends on ARCH_QCOM || (ARM && COMPILE_TEST) > depends on OF && COMMON_CLK > + depends on NVMEM || !NVMEM > depends on MMU > select QCOM_MDT_LOADER if ARCH_QCOM > select REGULATOR > -- > 2.9.0 > > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/msm/adreno: fix nvmem related link error 2018-01-15 16:14 ` [PATCH 2/2] drm/msm/adreno: fix nvmem related link error Arnd Bergmann 2018-01-17 21:21 ` [Freedreno] " Jordan Crouse @ 2018-01-17 21:52 ` Rob Clark 2018-01-18 10:15 ` Lucas Stach 1 sibling, 1 reply; 9+ messages in thread From: Rob Clark @ 2018-01-17 21:52 UTC (permalink / raw) To: Arnd Bergmann Cc: Jordan Crouse, Bartlomiej Zolnierkiewicz, dri-devel, Linux Fbdev development list, linux-i2c, Wolfram Sang, Srinivas Kandagatla, David Airlie, Bjorn Andersson, Archit Taneja, linux-arm-msm, freedreno, Linux Kernel Mailing List On Mon, Jan 15, 2018 at 11:14 AM, Arnd Bergmann <arnd@arndb.de> wrote: > When NVMEM is configured as a loadable module, and adreno > is built-in, we get a link failure: > > drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_gpu_init': > a5xx_gpu.c:(.text+0x15cc): undefined reference to `nvmem_cell_get' > a5xx_gpu.c:(.text+0x15da): undefined reference to `nvmem_cell_read' > a5xx_gpu.c:(.text+0x15e4): undefined reference to `nvmem_cell_put' > > This adds a Kconfig dependency to enforce valid configurations, > when NVMEM is a loadable module, adreno now has to also be one. > The code seems to deal fine with nvmem being completely disabled, > it will just not set the right speed bin then, so we don't need > a hard dependency. > > Fixes: f56d9df656c4 ("drm/msm/adreno: Read the speed bins for a5xx targets") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Arnd, beyond randconfig, I guess there are probably two real-world scenarios, both =m (distro) and both =y (android/oe/etc).. Is there a kconfig way to say if nvmem=m then drm_msm must be =n or =m? BR, -R > --- > drivers/gpu/drm/msm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index 99d39b2aefa6..74fb1c816da9 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -4,6 +4,7 @@ config DRM_MSM > depends on DRM > depends on ARCH_QCOM || (ARM && COMPILE_TEST) > depends on OF && COMMON_CLK > + depends on NVMEM || !NVMEM > depends on MMU > select QCOM_MDT_LOADER if ARCH_QCOM > select REGULATOR > -- > 2.9.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/msm/adreno: fix nvmem related link error 2018-01-17 21:52 ` Rob Clark @ 2018-01-18 10:15 ` Lucas Stach 0 siblings, 0 replies; 9+ messages in thread From: Lucas Stach @ 2018-01-18 10:15 UTC (permalink / raw) To: Rob Clark, Arnd Bergmann Cc: Linux Fbdev development list, Wolfram Sang, David Airlie, linux-arm-msm, Bartlomiej Zolnierkiewicz, dri-devel, Bjorn Andersson, Srinivas Kandagatla, linux-i2c, freedreno, Linux Kernel Mailing List Am Mittwoch, den 17.01.2018, 16:52 -0500 schrieb Rob Clark: > On Mon, Jan 15, 2018 at 11:14 AM, Arnd Bergmann <arnd@arndb.de> > wrote: > > When NVMEM is configured as a loadable module, and adreno > > is built-in, we get a link failure: > > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_gpu_init': > > a5xx_gpu.c:(.text+0x15cc): undefined reference to `nvmem_cell_get' > > a5xx_gpu.c:(.text+0x15da): undefined reference to `nvmem_cell_read' > > a5xx_gpu.c:(.text+0x15e4): undefined reference to `nvmem_cell_put' > > > > This adds a Kconfig dependency to enforce valid configurations, > > when NVMEM is a loadable module, adreno now has to also be one. > > The code seems to deal fine with nvmem being completely disabled, > > it will just not set the right speed bin then, so we don't need > > a hard dependency. > > > > Fixes: f56d9df656c4 ("drm/msm/adreno: Read the speed bins for a5xx > > targets") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Arnd, beyond randconfig, I guess there are probably two real-world > scenarios, both =m (distro) and both =y (android/oe/etc).. > > Is there a kconfig way to say if nvmem=m then drm_msm must be =n or > =m? That's exactly what the below patch does. Regards, Lucas > BR, > -R > > > > --- > > drivers/gpu/drm/msm/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/msm/Kconfig > > b/drivers/gpu/drm/msm/Kconfig > > index 99d39b2aefa6..74fb1c816da9 100644 > > --- a/drivers/gpu/drm/msm/Kconfig > > +++ b/drivers/gpu/drm/msm/Kconfig > > @@ -4,6 +4,7 @@ config DRM_MSM > > depends on DRM > > depends on ARCH_QCOM || (ARM && COMPILE_TEST) > > depends on OF && COMMON_CLK > > + depends on NVMEM || !NVMEM > > depends on MMU > > select QCOM_MDT_LOADER if ARCH_QCOM > > select REGULATOR > > -- > > 2.9.0 > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fbdev: don't select I2C directly 2018-01-15 16:14 ` [PATCH 1/2] fbdev: don't select I2C directly Arnd Bergmann [not found] ` <20180115161431.803248-1-arnd-r2nGTMty4D4@public.gmane.org> @ 2018-02-01 16:14 ` Bartlomiej Zolnierkiewicz 2018-02-02 0:21 ` Randy Dunlap 1 sibling, 1 reply; 9+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2018-02-01 16:14 UTC (permalink / raw) To: Arnd Bergmann Cc: Rob Clark, Jordan Crouse, dri-devel, linux-fbdev, linux-i2c, Wolfram Sang, Srinivas Kandagatla, linux-kernel On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote: > Using a Kconfig 'select' statement for a user-visible symbol that other > drivers depend on often causes circular dependencies. A new one showed > up when I wanted to add an NVMEM dependency to the DRM_MSM driver: > > drivers/i2c/Kconfig:7:error: recursive dependency detected! > drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC > drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC > drivers/video/fbdev/Kconfig:390: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 > drivers/video/fbdev/Kconfig:378: symbol FB_CYBER2000 depends on FB > drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER > drivers/gpu/drm/Kconfig:77: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER > drivers/gpu/drm/Kconfig:71: symbol DRM_KMS_HELPER is selected by DRM_MSM > drivers/gpu/drm/msm/Kconfig:2: symbol DRM_MSM depends on NVMEM > drivers/nvmem/Kconfig:1: symbol NVMEM is selected by EEPROM_AT24 > drivers/misc/eeprom/Kconfig:3: symbol EEPROM_AT24 depends on I2C > > Here, the problem is that many fbdev drivers have an i2c based interface > and just 'select i2c' for that, while we also select the framebuffer > subsystem indirectly from a DRM driver that now depends on i2c. > > This does away with the 'select' statement and instead uses 'depends on', > like almost all I2C users. I worry that this change may cause various driver options to no longer be visible to people configuring their kernels and not having I2C already selected. DRM somehow manages to select I2C and I would prefer to be it the same way for fbdev (also looking at the current next tree there are still some drivers that 'select I2C').. > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/video/fbdev/Kconfig | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig > index 6962b4583fd7..892eb1863100 100644 > --- a/drivers/video/fbdev/Kconfig > +++ b/drivers/video/fbdev/Kconfig > @@ -64,7 +64,6 @@ config FB_DDC > tristate > depends on FB > select I2C_ALGOBIT > - select I2C > default n > > config FB_BOOT_VESA_SUPPORT > @@ -390,6 +389,7 @@ config FB_CYBER2000 > config FB_CYBER2000_DDC > bool "DDC for CyberPro support" > depends on FB_CYBER2000 > + depends on I2C=y || I2Cû_CYBER2000 > select FB_DDC > default y > help > @@ -634,7 +634,7 @@ config FB_BF537_LQ035 > config FB_BFIN_7393 > tristate "Blackfin ADV7393 Video encoder" > depends on FB && BLACKFIN > - select I2C > + depends on I2C > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -1026,6 +1026,7 @@ config FB_NVIDIA > config FB_NVIDIA_I2C > bool "Enable DDC Support" > depends on FB_NVIDIA > + depends on I2C=y || I2Cû_NVIDIA > select FB_DDC > help > This enables I2C support for nVidia Chipsets. This is used > @@ -1073,6 +1074,7 @@ config FB_RIVA > config FB_RIVA_I2C > bool "Enable DDC Support" > depends on FB_RIVA > + depends on I2C=y || I2Cû_RIVA > select FB_DDC > help > This enables I2C support for nVidia Chipsets. This is used > @@ -1102,6 +1104,7 @@ config FB_RIVA_BACKLIGHT > config FB_I740 > tristate "Intel740 support" > depends on FB && PCI > + depends on I2C > select FB_MODE_HELPERS > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > @@ -1155,6 +1158,7 @@ config FB_I810_GTF > config FB_I810_I2C > bool "Enable DDC Support" > depends on FB_I810 && FB_I810_GTF > + depends on I2C=y || I2Cû_I810 > select FB_DDC > help > > @@ -1206,6 +1210,7 @@ config FB_INTEL_DEBUG > config FB_INTEL_I2C > bool "DDC/I2C for Intel framebuffer support" > depends on FB_INTEL > + depends on I2C=y || I2Cû_INTEL > select FB_DDC > default y > help > @@ -1285,6 +1290,7 @@ config FB_MATROX_G > config FB_MATROX_I2C > tristate "Matrox I2C support" > depends on FB_MATROX > + depends on I2C > select FB_DDC > ---help--- > This drivers creates I2C buses which are needed for accessing the > @@ -1350,6 +1356,7 @@ config FB_RADEON > config FB_RADEON_I2C > bool "DDC/I2C for ATI Radeon support" > depends on FB_RADEON > + depends on I2C=y || I2Cû_RADEON > select FB_DDC > default y > help > @@ -1460,6 +1467,7 @@ config FB_S3 > config FB_S3_DDC > bool "DDC for S3 support" > depends on FB_S3 > + depends on I2C=y || I2Cû_S3 > select FB_DDC > default y > help > @@ -1485,6 +1493,7 @@ config FB_SAVAGE > config FB_SAVAGE_I2C > bool "Enable DDC2 Support" > depends on FB_SAVAGE > + depends on I2C=y || I2Cû_SAVAGE > select FB_DDC > help > This enables I2C support for S3 Savage Chipsets. This is used > @@ -1629,6 +1638,7 @@ config FB_3DFX_ACCEL > config FB_3DFX_I2C > bool "Enable DDC/I2C support" > depends on FB_3DFX > + depends on I2C=y || I2Cû_3DFX > select FB_DDC > default y > help > @@ -1669,6 +1679,7 @@ config FB_VT8623 > config FB_TRIDENT > tristate "Trident/CyberXXX/CyberBlade support" > depends on FB && PCI > + depends on I2C > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -2299,8 +2310,8 @@ endchoice > > config FB_MB862XX_I2C > bool "Support I2C bus on MB862XX GDC" > - depends on FB_MB862XX && I2C > - depends on FB_MB862XX=m || I2C=y > + depends on FB_MB862XX > + depends on I2C=y || I2Cû_MB862XX > default y > help > Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fbdev: don't select I2C directly 2018-02-01 16:14 ` [PATCH 1/2] fbdev: don't select I2C directly Bartlomiej Zolnierkiewicz @ 2018-02-02 0:21 ` Randy Dunlap 2018-02-02 11:59 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Randy Dunlap @ 2018-02-02 0:21 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz, Arnd Bergmann Cc: Rob Clark, Jordan Crouse, dri-devel, linux-fbdev, linux-i2c, Wolfram Sang, Srinivas Kandagatla, linux-kernel On 02/01/2018 08:14 AM, Bartlomiej Zolnierkiewicz wrote: > On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote: >> Using a Kconfig 'select' statement for a user-visible symbol that other >> drivers depend on often causes circular dependencies. A new one showed >> up when I wanted to add an NVMEM dependency to the DRM_MSM driver: >> >> drivers/i2c/Kconfig:7:error: recursive dependency detected! >> drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC >> drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC >> drivers/video/fbdev/Kconfig:390: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 >> drivers/video/fbdev/Kconfig:378: symbol FB_CYBER2000 depends on FB >> drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER >> drivers/gpu/drm/Kconfig:77: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER >> drivers/gpu/drm/Kconfig:71: symbol DRM_KMS_HELPER is selected by DRM_MSM >> drivers/gpu/drm/msm/Kconfig:2: symbol DRM_MSM depends on NVMEM >> drivers/nvmem/Kconfig:1: symbol NVMEM is selected by EEPROM_AT24 >> drivers/misc/eeprom/Kconfig:3: symbol EEPROM_AT24 depends on I2C >> >> Here, the problem is that many fbdev drivers have an i2c based interface >> and just 'select i2c' for that, while we also select the framebuffer >> subsystem indirectly from a DRM driver that now depends on i2c. >> >> This does away with the 'select' statement and instead uses 'depends on', >> like almost all I2C users. > > I worry that this change may cause various driver options to no longer > be visible to people configuring their kernels and not having I2C already > selected. > > DRM somehow manages to select I2C and I would prefer to be it the same > way for fbdev (also looking at the current next tree there are still > some drivers that 'select I2C').. a. Linus has stated that a driver should not enable an entire subsystem, so depends on would be better than select. If I had the email/patch, I would be glad to Ack it. b. DRM configuration is a mess. You shouldn't want to follow their model. :) c. If I2C is not enabled in the FB menu, someone could just add something like this: comment "Enable I2C to see more driver choices" depends on !I2C >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/video/fbdev/Kconfig | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig >> index 6962b4583fd7..892eb1863100 100644 >> --- a/drivers/video/fbdev/Kconfig >> +++ b/drivers/video/fbdev/Kconfig >> @@ -64,7 +64,6 @@ config FB_DDC >> tristate >> depends on FB >> select I2C_ALGOBIT >> - select I2C >> default n >> >> config FB_BOOT_VESA_SUPPORT >> @@ -390,6 +389,7 @@ config FB_CYBER2000 >> config FB_CYBER2000_DDC >> bool "DDC for CyberPro support" >> depends on FB_CYBER2000 >> + depends on I2C=y || I2Cû_CYBER2000 >> select FB_DDC >> default y >> help >> @@ -634,7 +634,7 @@ config FB_BF537_LQ035 >> config FB_BFIN_7393 >> tristate "Blackfin ADV7393 Video encoder" >> depends on FB && BLACKFIN >> - select I2C >> + depends on I2C >> select FB_CFB_FILLRECT >> select FB_CFB_COPYAREA >> select FB_CFB_IMAGEBLIT >> @@ -1026,6 +1026,7 @@ config FB_NVIDIA >> config FB_NVIDIA_I2C >> bool "Enable DDC Support" >> depends on FB_NVIDIA >> + depends on I2C=y || I2Cû_NVIDIA >> select FB_DDC >> help >> This enables I2C support for nVidia Chipsets. This is used >> @@ -1073,6 +1074,7 @@ config FB_RIVA >> config FB_RIVA_I2C >> bool "Enable DDC Support" >> depends on FB_RIVA >> + depends on I2C=y || I2Cû_RIVA >> select FB_DDC >> help >> This enables I2C support for nVidia Chipsets. This is used >> @@ -1102,6 +1104,7 @@ config FB_RIVA_BACKLIGHT >> config FB_I740 >> tristate "Intel740 support" >> depends on FB && PCI >> + depends on I2C >> select FB_MODE_HELPERS >> select FB_CFB_FILLRECT >> select FB_CFB_COPYAREA >> @@ -1155,6 +1158,7 @@ config FB_I810_GTF >> config FB_I810_I2C >> bool "Enable DDC Support" >> depends on FB_I810 && FB_I810_GTF >> + depends on I2C=y || I2Cû_I810 >> select FB_DDC >> help >> >> @@ -1206,6 +1210,7 @@ config FB_INTEL_DEBUG >> config FB_INTEL_I2C >> bool "DDC/I2C for Intel framebuffer support" >> depends on FB_INTEL >> + depends on I2C=y || I2Cû_INTEL >> select FB_DDC >> default y >> help >> @@ -1285,6 +1290,7 @@ config FB_MATROX_G >> config FB_MATROX_I2C >> tristate "Matrox I2C support" >> depends on FB_MATROX >> + depends on I2C >> select FB_DDC >> ---help--- >> This drivers creates I2C buses which are needed for accessing the >> @@ -1350,6 +1356,7 @@ config FB_RADEON >> config FB_RADEON_I2C >> bool "DDC/I2C for ATI Radeon support" >> depends on FB_RADEON >> + depends on I2C=y || I2Cû_RADEON >> select FB_DDC >> default y >> help >> @@ -1460,6 +1467,7 @@ config FB_S3 >> config FB_S3_DDC >> bool "DDC for S3 support" >> depends on FB_S3 >> + depends on I2C=y || I2Cû_S3 >> select FB_DDC >> default y >> help >> @@ -1485,6 +1493,7 @@ config FB_SAVAGE >> config FB_SAVAGE_I2C >> bool "Enable DDC2 Support" >> depends on FB_SAVAGE >> + depends on I2C=y || I2Cû_SAVAGE >> select FB_DDC >> help >> This enables I2C support for S3 Savage Chipsets. This is used >> @@ -1629,6 +1638,7 @@ config FB_3DFX_ACCEL >> config FB_3DFX_I2C >> bool "Enable DDC/I2C support" >> depends on FB_3DFX >> + depends on I2C=y || I2Cû_3DFX >> select FB_DDC >> default y >> help >> @@ -1669,6 +1679,7 @@ config FB_VT8623 >> config FB_TRIDENT >> tristate "Trident/CyberXXX/CyberBlade support" >> depends on FB && PCI >> + depends on I2C >> select FB_CFB_FILLRECT >> select FB_CFB_COPYAREA >> select FB_CFB_IMAGEBLIT >> @@ -2299,8 +2310,8 @@ endchoice >> >> config FB_MB862XX_I2C >> bool "Support I2C bus on MB862XX GDC" >> - depends on FB_MB862XX && I2C >> - depends on FB_MB862XX=m || I2C=y >> + depends on FB_MB862XX >> + depends on I2C=y || I2Cû_MB862XX >> default y >> help >> Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter -- ~Randy ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fbdev: don't select I2C directly 2018-02-02 0:21 ` Randy Dunlap @ 2018-02-02 11:59 ` Arnd Bergmann 2018-02-06 11:07 ` Geert Uytterhoeven 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2018-02-02 11:59 UTC (permalink / raw) To: Randy Dunlap Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Wolfram Sang, dri-devel, Linux Kernel Mailing List, Srinivas Kandagatla, linux-i2c On Fri, Feb 2, 2018 at 1:21 AM, Randy Dunlap <rdunlap@infradead.org> wrote: > On 02/01/2018 08:14 AM, Bartlomiej Zolnierkiewicz wrote: >> On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote: >>> Using a Kconfig 'select' statement for a user-visible symbol that other >>> drivers depend on often causes circular dependencies. A new one showed >>> up when I wanted to add an NVMEM dependency to the DRM_MSM driver: >>> >>> drivers/i2c/Kconfig:7:error: recursive dependency detected! >>> drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC >>> drivers/video/fbdev/Kconfig:63: symbol FB_DDC is selected by FB_CYBER2000_DDC >>> drivers/video/fbdev/Kconfig:390: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 >>> drivers/video/fbdev/Kconfig:378: symbol FB_CYBER2000 depends on FB >>> drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER >>> drivers/gpu/drm/Kconfig:77: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER >>> drivers/gpu/drm/Kconfig:71: symbol DRM_KMS_HELPER is selected by DRM_MSM >>> drivers/gpu/drm/msm/Kconfig:2: symbol DRM_MSM depends on NVMEM >>> drivers/nvmem/Kconfig:1: symbol NVMEM is selected by EEPROM_AT24 >>> drivers/misc/eeprom/Kconfig:3: symbol EEPROM_AT24 depends on I2C >>> >>> Here, the problem is that many fbdev drivers have an i2c based interface >>> and just 'select i2c' for that, while we also select the framebuffer >>> subsystem indirectly from a DRM driver that now depends on i2c. >>> >>> This does away with the 'select' statement and instead uses 'depends on', >>> like almost all I2C users. >> >> I worry that this change may cause various driver options to no longer >> be visible to people configuring their kernels and not having I2C already >> selected. >> >> DRM somehow manages to select I2C and I would prefer to be it the same >> way for fbdev (also looking at the current next tree there are still >> some drivers that 'select I2C').. > > a. Linus has stated that a driver should not enable an entire subsystem, > so depends on would be better than select. > If I had the email/patch, I would be glad to Ack it. > > b. DRM configuration is a mess. You shouldn't want to follow their model. :) Right, that should also be fixed, so DRM no longer includes I2C ;-) At the moment, DRM is the most common cause for circular dependencies because it has a number of 'select' statements for symbols that otherwise are used with 'depends on'. We should probably address the 'select I2C' portion in there, but also some of the others like: drivers/gpu/drm/Kconfig: select POWER_SUPPLY drivers/gpu/drm/Kconfig: select HWMON drivers/gpu/drm/Kconfig: select FB drivers/gpu/drm/udl/Kconfig: select USB drivers/gpu/drm/sti/Kconfig: select OF drivers/gpu/drm/sti/Kconfig: select RESET_CONTROLLER drivers/gpu/drm/etnaviv/Kconfig: select CMA if HAVE_DMA_CONTIGUOUS drivers/gpu/drm/etnaviv/Kconfig: select TMPFS drivers/gpu/drm/i915/Kconfig.debug: select DEBUG_FS drivers/gpu/drm/i915/Kconfig.debug: select PREEMPT_COUNT drivers/gpu/drm/i915/Kconfig.debug: select TRACING drivers/gpu/drm/i915/Kconfig.debug: select FAULT_INJECTION drivers/gpu/drm/mediatek/Kconfig: select MEMORY drivers/gpu/drm/mediatek/Kconfig: select GENERIC_PHY drivers/gpu/drm/msm/Kconfig: select REGULATOR > c. If I2C is not enabled in the FB menu, someone could just add something like this: > > comment "Enable I2C to see more driver choices" > depends on !I2C I don't think that would address the issue of 'defconfig' builds losing I2C support when it's no longer automatically selection. On x86, this is not an issue, as X86 always enables I2C. For the rest, we could do a hack like this: --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -8,6 +8,7 @@ config I2C tristate "I2C support" select RT_MUTEXES select IRQ_DOMAIN + default DRM || FB ---help--- I2C (pronounce: I-squared-C) is a slow serial bus protocol used in many micro controller applications and developed by Philips. SMBus, which would let all 'defconfig' versions keep working. It's a bit ugly, but at least wouldn't cause other circular dependencies. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fbdev: don't select I2C directly 2018-02-02 11:59 ` Arnd Bergmann @ 2018-02-06 11:07 ` Geert Uytterhoeven 0 siblings, 0 replies; 9+ messages in thread From: Geert Uytterhoeven @ 2018-02-06 11:07 UTC (permalink / raw) To: Arnd Bergmann Cc: Randy Dunlap, Bartlomiej Zolnierkiewicz, Rob Clark, Jordan Crouse, dri-devel, Linux Fbdev development list, Linux I2C, Wolfram Sang, Srinivas Kandagatla, Linux Kernel Mailing List On Fri, Feb 2, 2018 at 12:59 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Fri, Feb 2, 2018 at 1:21 AM, Randy Dunlap <rdunlap@infradead.org> wrote: >> b. DRM configuration is a mess. You shouldn't want to follow their model. :) > > Right, that should also be fixed, so DRM no longer includes I2C ;-) > > At the moment, DRM is the most common cause for circular dependencies > because it has a number of 'select' statements for symbols that otherwise > are used with 'depends on'. We should probably address the 'select I2C' > portion in there, but also some of the others like: > > drivers/gpu/drm/Kconfig: select POWER_SUPPLY > drivers/gpu/drm/Kconfig: select HWMON > drivers/gpu/drm/Kconfig: select FB > drivers/gpu/drm/udl/Kconfig: select USB USB? Seriously? > drivers/gpu/drm/sti/Kconfig: select OF > drivers/gpu/drm/sti/Kconfig: select RESET_CONTROLLER > drivers/gpu/drm/etnaviv/Kconfig: select CMA if HAVE_DMA_CONTIGUOUS > drivers/gpu/drm/etnaviv/Kconfig: select TMPFS > drivers/gpu/drm/i915/Kconfig.debug: select DEBUG_FS > drivers/gpu/drm/i915/Kconfig.debug: select PREEMPT_COUNT > drivers/gpu/drm/i915/Kconfig.debug: select TRACING > drivers/gpu/drm/i915/Kconfig.debug: select FAULT_INJECTION > drivers/gpu/drm/mediatek/Kconfig: select MEMORY > drivers/gpu/drm/mediatek/Kconfig: select GENERIC_PHY > drivers/gpu/drm/msm/Kconfig: select REGULATOR > >> c. If I2C is not enabled in the FB menu, someone could just add something like this: >> >> comment "Enable I2C to see more driver choices" >> depends on !I2C > > I don't think that would address the issue of 'defconfig' builds losing > I2C support when it's no longer automatically selection. On x86, this > is not an issue, as X86 always enables I2C. For the rest, we could > do a hack like this: > > --- a/drivers/i2c/Kconfig > +++ b/drivers/i2c/Kconfig > @@ -8,6 +8,7 @@ config I2C > tristate "I2C support" > select RT_MUTEXES > select IRQ_DOMAIN > + default DRM || FB > ---help--- > I2C (pronounce: I-squared-C) is a slow serial bus protocol used in > many micro controller applications and developed by Philips. SMBus, > > which would let all 'defconfig' versions keep working. It's a bit ugly, but > at least wouldn't cause other circular dependencies. And then we'll have to refresh all defconfigs for machines with FB and without I2C? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-06 11:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20180115161439epcas3p1006f02c4e49f58c30c4aec1b0b11b0a7@epcas3p1.samsung.com>
2018-01-15 16:14 ` [PATCH 1/2] fbdev: don't select I2C directly Arnd Bergmann
[not found] ` <20180115161431.803248-1-arnd-r2nGTMty4D4@public.gmane.org>
2018-01-15 16:14 ` [PATCH 2/2] drm/msm/adreno: fix nvmem related link error Arnd Bergmann
2018-01-17 21:21 ` [Freedreno] " Jordan Crouse
2018-01-17 21:52 ` Rob Clark
2018-01-18 10:15 ` Lucas Stach
2018-02-01 16:14 ` [PATCH 1/2] fbdev: don't select I2C directly Bartlomiej Zolnierkiewicz
2018-02-02 0:21 ` Randy Dunlap
2018-02-02 11:59 ` Arnd Bergmann
2018-02-06 11:07 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).