[PATCH 1/2] fbdev: don't select I2C directly

Subsystems: framebuffer layer, the rest

STALE3111d

9 messages, 7 authors, 2018-02-06 · open the first message on its own page

[PATCH 1/2] fbdev: don't select I2C directly

From: Arnd Bergmann <arnd@arndb.de>
Date: 2018-01-15 16:14:47

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

[PATCH 2/2] drm/msm/adreno: fix nvmem related link error

From: Arnd Bergmann <arnd@arndb.de>
Date: 2018-01-15 16:15:03

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

Re: [Freedreno] [PATCH 2/2] drm/msm/adreno: fix nvmem related link error

From: Jordan Crouse <hidden>
Date: 2018-01-17 21:21:12

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 <redacted>
quoted hunk
---
 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

Re: [PATCH 2/2] drm/msm/adreno: fix nvmem related link error

From: Rob Clark <hidden>
Date: 2018-01-17 21:52:49

On Mon, Jan 15, 2018 at 11:14 AM, Arnd Bergmann [off-list ref] 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

quoted hunk
---
 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

Re: [PATCH 2/2] drm/msm/adreno: fix nvmem related link error

From: Lucas Stach <l.stach@pengutronix.de>
Date: 2018-01-18 10:15:36

Am Mittwoch, den 17.01.2018, 16:52 -0500 schrieb Rob Clark:
On Mon, Jan 15, 2018 at 11:14 AM, Arnd Bergmann [off-list ref]
wrote:
quoted
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

quoted
---
 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

Re: [PATCH 1/2] fbdev: don't select I2C directly

From: Bartlomiej Zolnierkiewicz <hidden>
Date: 2018-02-01 16:14:19

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')..
quoted hunk
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

Re: [PATCH 1/2] fbdev: don't select I2C directly

From: Randy Dunlap <hidden>
Date: 2018-02-02 00:21:27

On 02/01/2018 08:14 AM, Bartlomiej Zolnierkiewicz wrote:
On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote:
quoted
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

quoted
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

Re: [PATCH 1/2] fbdev: don't select I2C directly

From: Arnd Bergmann <arnd@arndb.de>
Date: 2018-02-02 12:00:01

On Fri, Feb 2, 2018 at 1:21 AM, Randy Dunlap [off-list ref] wrote:
On 02/01/2018 08:14 AM, Bartlomiej Zolnierkiewicz wrote:
quoted
On Monday, January 15, 2018 05:14:04 PM Arnd Bergmann wrote:
quoted
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

Re: [PATCH 1/2] fbdev: don't select I2C directly

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2018-02-06 11:07:11

On Fri, Feb 2, 2018 at 12:59 PM, Arnd Bergmann [off-list ref] wrote:
On Fri, Feb 2, 2018 at 1:21 AM, Randy Dunlap [off-list ref] wrote:
quoted
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?
quoted hunk
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
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help