diff mbox series

[RFC,4/6] drm/bridge/sii8620: fix extcon dependency

Message ID 20200408202711.1198966-5-arnd@arndb.de
State New
Headers show
Series Regressions for "imply" behavior change | expand

Commit Message

Arnd Bergmann April 8, 2020, 8:27 p.m. UTC 
Using 'imply' does not work here, it still cause the same build
failure:

arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_remove':
sil-sii8620.c:(.text+0x1b8): undefined reference to `extcon_unregister_notifier'
arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_probe':
sil-sii8620.c:(.text+0x27e8): undefined reference to `extcon_find_edev_by_node'
arm-linux-gnueabi-ld: sil-sii8620.c:(.text+0x2870): undefined reference to `extcon_register_notifier'
arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_extcon_work':
sil-sii8620.c:(.text+0x2908): undefined reference to `extcon_get_state'

I tried the usual 'depends on EXTCON || !EXTCON' logic, but that caused
a circular Kconfig dependency. Using IS_REACHABLE() is ugly but works.

Fixes: 7a109673899b ("drm/bridge/sii8620: add Kconfig dependency on extcon")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/gpu/drm/bridge/Kconfig       | 1 -
 drivers/gpu/drm/bridge/sil-sii8620.c | 5 +++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.26.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Andrzej Hajda April 10, 2020, 6:56 a.m. UTC | #1 
On 08.04.2020 22:27, Arnd Bergmann wrote:
> Using 'imply' does not work here, it still cause the same build

> failure:

>

> arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_remove':

> sil-sii8620.c:(.text+0x1b8): undefined reference to `extcon_unregister_notifier'

> arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_probe':

> sil-sii8620.c:(.text+0x27e8): undefined reference to `extcon_find_edev_by_node'

> arm-linux-gnueabi-ld: sil-sii8620.c:(.text+0x2870): undefined reference to `extcon_register_notifier'

> arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_extcon_work':

> sil-sii8620.c:(.text+0x2908): undefined reference to `extcon_get_state'

>

> I tried the usual 'depends on EXTCON || !EXTCON' logic, but that caused

> a circular Kconfig dependency. Using IS_REACHABLE() is ugly but works.


'depends on EXTCON || !EXTCON' seems to be proper solution, maybe would be better to try to solve circular dependencies issue.

Regards
Andrzej


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Arnd Bergmann April 14, 2020, 3:04 p.m. UTC | #2 
On Fri, Apr 10, 2020 at 8:56 AM Andrzej Hajda <a.hajda@samsung.com> wrote:
>

>

> On 08.04.2020 22:27, Arnd Bergmann wrote:

> > Using 'imply' does not work here, it still cause the same build

> > failure:

> >

> > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_remove':

> > sil-sii8620.c:(.text+0x1b8): undefined reference to `extcon_unregister_notifier'

> > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_probe':

> > sil-sii8620.c:(.text+0x27e8): undefined reference to `extcon_find_edev_by_node'

> > arm-linux-gnueabi-ld: sil-sii8620.c:(.text+0x2870): undefined reference to `extcon_register_notifier'

> > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_extcon_work':

> > sil-sii8620.c:(.text+0x2908): undefined reference to `extcon_get_state'

> >

> > I tried the usual 'depends on EXTCON || !EXTCON' logic, but that caused

> > a circular Kconfig dependency. Using IS_REACHABLE() is ugly but works.

>

> 'depends on EXTCON || !EXTCON' seems to be proper solution, maybe would be better to try to solve circular dependencies issue.


I agree that would be nice, but I failed to come to a proper solution
here. FWIW, there
is one circular dependency that I managed to avoid by changing all
drivers that select FB_DDC
to depend on I2C rather than selecting it:

drivers/i2c/Kconfig:8:error: recursive dependency detected!
drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC
drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER
drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620
drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON
drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER
drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY
drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by
HID_BATTERY_STRENGTH
drivers/hid/Kconfig:29: symbol HID_BATTERY_STRENGTH depends on HID
drivers/hid/Kconfig:8: symbol HID is selected by I2C_HID
drivers/hid/i2c-hid/Kconfig:5: symbol I2C_HID depends on I2C

After that, Kconfig crashes with a segfault:

drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER
drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620
drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON
drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER
drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY
drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by HID_ASUS
drivers/hid/Kconfig:150: symbol HID_ASUS depends on LEDS_CLASS
drivers/leds/Kconfig:17: symbol LEDS_CLASS depends on NEW_LEDS
drivers/leds/Kconfig:9: symbol NEW_LEDS is selected by SENSORS_APPLESMC
drivers/hwmon/Kconfig:327: symbol SENSORS_APPLESMC depends on HWMON
drivers/hwmon/Kconfig:6: symbol HWMON is selected by EEEPC_LAPTOP
drivers/platform/x86/Kconfig:260: symbol EEEPC_LAPTOP depends on ACPI_VIDEO
make[3]: *** [/git/arm-soc/scripts/kconfig/Makefile:71: randconfig]
Segmentation fault (core dumped)

After changing EEEPC_LAPTOP and THINKPAD_ACPI to 'depends on HWMON' instead of
'select HWMON', I get this one:

drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER
drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620
drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON
drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER
drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY
drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by HID_ASUS
drivers/hid/Kconfig:150: symbol HID_ASUS depends on LEDS_CLASS
drivers/leds/Kconfig:17: symbol LEDS_CLASS depends on NEW_LEDS
drivers/leds/Kconfig:9: symbol NEW_LEDS is selected by BACKLIGHT_ADP8860
drivers/video/backlight/Kconfig:316: symbol BACKLIGHT_ADP8860 depends
on BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:143: symbol BACKLIGHT_CLASS_DEVICE is
selected by FB_BACKLIGHT
drivers/video/fbdev/Kconfig:187: symbol FB_BACKLIGHT depends on FB

Changing all drivers that select 'FB_BACKLIGHT' or 'BACKLIGHT_CLASS_DEVICE' to
'depends on BACKLIGHT_CLASS_DEVICE' gets it to build.

The steps each seem reasonable, in particular since they mostly clean
up the legacy
fbdev drivers to what they should have done anyway, but it is quite
invasive in the end.
Any other ideas?

       Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter April 14, 2020, 3:37 p.m. UTC | #3 
On Tue, Apr 14, 2020 at 5:05 PM Arnd Bergmann <arnd@arndb.de> wrote:
>

> On Fri, Apr 10, 2020 at 8:56 AM Andrzej Hajda <a.hajda@samsung.com> wrote:

> >

> >

> > On 08.04.2020 22:27, Arnd Bergmann wrote:

> > > Using 'imply' does not work here, it still cause the same build

> > > failure:

> > >

> > > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_remove':

> > > sil-sii8620.c:(.text+0x1b8): undefined reference to `extcon_unregister_notifier'

> > > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_probe':

> > > sil-sii8620.c:(.text+0x27e8): undefined reference to `extcon_find_edev_by_node'

> > > arm-linux-gnueabi-ld: sil-sii8620.c:(.text+0x2870): undefined reference to `extcon_register_notifier'

> > > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_extcon_work':

> > > sil-sii8620.c:(.text+0x2908): undefined reference to `extcon_get_state'

> > >

> > > I tried the usual 'depends on EXTCON || !EXTCON' logic, but that caused

> > > a circular Kconfig dependency. Using IS_REACHABLE() is ugly but works.

> >

> > 'depends on EXTCON || !EXTCON' seems to be proper solution, maybe would be better to try to solve circular dependencies issue.

>

> I agree that would be nice, but I failed to come to a proper solution

> here. FWIW, there

> is one circular dependency that I managed to avoid by changing all

> drivers that select FB_DDC

> to depend on I2C rather than selecting it:

>

> drivers/i2c/Kconfig:8:error: recursive dependency detected!

> drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC

> drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB

> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER

> drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER

> drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620

> drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON

> drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER

> drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY

> drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by

> HID_BATTERY_STRENGTH

> drivers/hid/Kconfig:29: symbol HID_BATTERY_STRENGTH depends on HID

> drivers/hid/Kconfig:8: symbol HID is selected by I2C_HID

> drivers/hid/i2c-hid/Kconfig:5: symbol I2C_HID depends on I2C

>

> After that, Kconfig crashes with a segfault:

>

> drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!

> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER

> drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER

> drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620

> drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON

> drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER

> drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY

> drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by HID_ASUS

> drivers/hid/Kconfig:150: symbol HID_ASUS depends on LEDS_CLASS

> drivers/leds/Kconfig:17: symbol LEDS_CLASS depends on NEW_LEDS

> drivers/leds/Kconfig:9: symbol NEW_LEDS is selected by SENSORS_APPLESMC

> drivers/hwmon/Kconfig:327: symbol SENSORS_APPLESMC depends on HWMON

> drivers/hwmon/Kconfig:6: symbol HWMON is selected by EEEPC_LAPTOP

> drivers/platform/x86/Kconfig:260: symbol EEEPC_LAPTOP depends on ACPI_VIDEO

> make[3]: *** [/git/arm-soc/scripts/kconfig/Makefile:71: randconfig]

> Segmentation fault (core dumped)

>

> After changing EEEPC_LAPTOP and THINKPAD_ACPI to 'depends on HWMON' instead of

> 'select HWMON', I get this one:

>

> drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!

> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER

> drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER

> drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620

> drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON

> drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER

> drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY

> drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by HID_ASUS

> drivers/hid/Kconfig:150: symbol HID_ASUS depends on LEDS_CLASS

> drivers/leds/Kconfig:17: symbol LEDS_CLASS depends on NEW_LEDS

> drivers/leds/Kconfig:9: symbol NEW_LEDS is selected by BACKLIGHT_ADP8860

> drivers/video/backlight/Kconfig:316: symbol BACKLIGHT_ADP8860 depends

> on BACKLIGHT_CLASS_DEVICE

> drivers/video/backlight/Kconfig:143: symbol BACKLIGHT_CLASS_DEVICE is

> selected by FB_BACKLIGHT

> drivers/video/fbdev/Kconfig:187: symbol FB_BACKLIGHT depends on FB

>

> Changing all drivers that select 'FB_BACKLIGHT' or 'BACKLIGHT_CLASS_DEVICE' to

> 'depends on BACKLIGHT_CLASS_DEVICE' gets it to build.

>

> The steps each seem reasonable, in particular since they mostly clean

> up the legacy

> fbdev drivers to what they should have done anyway, but it is quite

> invasive in the end.

> Any other ideas?


Adding Jani, since iirc he looked at the entire backlight Kconfig
story before. I think there's some nonsense going on where in some
cases you don't get reasonable dummy functions where it just doesn't
make sense. Or something like that.

At least the entire select vs depends on backlight sounds eerily familiar.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jani Nikula April 15, 2020, 6:58 a.m. UTC | #4 
On Tue, 14 Apr 2020, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Apr 14, 2020 at 5:05 PM Arnd Bergmann <arnd@arndb.de> wrote:

>>

>> On Fri, Apr 10, 2020 at 8:56 AM Andrzej Hajda <a.hajda@samsung.com> wrote:

>> >

>> >

>> > On 08.04.2020 22:27, Arnd Bergmann wrote:

>> > > Using 'imply' does not work here, it still cause the same build

>> > > failure:

>> > >

>> > > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_remove':

>> > > sil-sii8620.c:(.text+0x1b8): undefined reference to `extcon_unregister_notifier'

>> > > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_probe':

>> > > sil-sii8620.c:(.text+0x27e8): undefined reference to `extcon_find_edev_by_node'

>> > > arm-linux-gnueabi-ld: sil-sii8620.c:(.text+0x2870): undefined reference to `extcon_register_notifier'

>> > > arm-linux-gnueabi-ld: drivers/gpu/drm/bridge/sil-sii8620.o: in function `sii8620_extcon_work':

>> > > sil-sii8620.c:(.text+0x2908): undefined reference to `extcon_get_state'

>> > >

>> > > I tried the usual 'depends on EXTCON || !EXTCON' logic, but that caused

>> > > a circular Kconfig dependency. Using IS_REACHABLE() is ugly but works.

>> >

>> > 'depends on EXTCON || !EXTCON' seems to be proper solution, maybe would be better to try to solve circular dependencies issue.

>>

>> I agree that would be nice, but I failed to come to a proper solution

>> here. FWIW, there

>> is one circular dependency that I managed to avoid by changing all

>> drivers that select FB_DDC

>> to depend on I2C rather than selecting it:

>>

>> drivers/i2c/Kconfig:8:error: recursive dependency detected!

>> drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC

>> drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB

>> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER

>> drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER

>> drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620

>> drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON

>> drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER

>> drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY

>> drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by

>> HID_BATTERY_STRENGTH

>> drivers/hid/Kconfig:29: symbol HID_BATTERY_STRENGTH depends on HID

>> drivers/hid/Kconfig:8: symbol HID is selected by I2C_HID

>> drivers/hid/i2c-hid/Kconfig:5: symbol I2C_HID depends on I2C

>>

>> After that, Kconfig crashes with a segfault:

>>

>> drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!

>> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER

>> drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER

>> drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620

>> drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON

>> drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER

>> drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY

>> drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by HID_ASUS

>> drivers/hid/Kconfig:150: symbol HID_ASUS depends on LEDS_CLASS

>> drivers/leds/Kconfig:17: symbol LEDS_CLASS depends on NEW_LEDS

>> drivers/leds/Kconfig:9: symbol NEW_LEDS is selected by SENSORS_APPLESMC

>> drivers/hwmon/Kconfig:327: symbol SENSORS_APPLESMC depends on HWMON

>> drivers/hwmon/Kconfig:6: symbol HWMON is selected by EEEPC_LAPTOP

>> drivers/platform/x86/Kconfig:260: symbol EEEPC_LAPTOP depends on ACPI_VIDEO

>> make[3]: *** [/git/arm-soc/scripts/kconfig/Makefile:71: randconfig]

>> Segmentation fault (core dumped)

>>

>> After changing EEEPC_LAPTOP and THINKPAD_ACPI to 'depends on HWMON' instead of

>> 'select HWMON', I get this one:

>>

>> drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!

>> drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER

>> drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER

>> drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by DRM_SIL_SII8620

>> drivers/gpu/drm/bridge/Kconfig:89: symbol DRM_SIL_SII8620 depends on EXTCON

>> drivers/extcon/Kconfig:2: symbol EXTCON is selected by CHARGER_MANAGER

>> drivers/power/supply/Kconfig:482: symbol CHARGER_MANAGER depends on POWER_SUPPLY

>> drivers/power/supply/Kconfig:2: symbol POWER_SUPPLY is selected by HID_ASUS

>> drivers/hid/Kconfig:150: symbol HID_ASUS depends on LEDS_CLASS

>> drivers/leds/Kconfig:17: symbol LEDS_CLASS depends on NEW_LEDS

>> drivers/leds/Kconfig:9: symbol NEW_LEDS is selected by BACKLIGHT_ADP8860

>> drivers/video/backlight/Kconfig:316: symbol BACKLIGHT_ADP8860 depends

>> on BACKLIGHT_CLASS_DEVICE

>> drivers/video/backlight/Kconfig:143: symbol BACKLIGHT_CLASS_DEVICE is

>> selected by FB_BACKLIGHT

>> drivers/video/fbdev/Kconfig:187: symbol FB_BACKLIGHT depends on FB

>>

>> Changing all drivers that select 'FB_BACKLIGHT' or 'BACKLIGHT_CLASS_DEVICE' to

>> 'depends on BACKLIGHT_CLASS_DEVICE' gets it to build.

>>

>> The steps each seem reasonable, in particular since they mostly clean

>> up the legacy

>> fbdev drivers to what they should have done anyway, but it is quite

>> invasive in the end.

>> Any other ideas?

>

> Adding Jani, since iirc he looked at the entire backlight Kconfig

> story before. I think there's some nonsense going on where in some

> cases you don't get reasonable dummy functions where it just doesn't

> make sense. Or something like that.

>

> At least the entire select vs depends on backlight sounds eerily familiar.


TL;DR I'd ack a patch switching to depends.

I'm all for switching to "depends on BACKLIGHT_CLASS_DEVICE" throughout,
and I've sent patches to do just that years ago. Alas it was met with
opposition because if you have BACKLIGHT_CLASS_DEVICE=n as a starting
point, you won't even see your driver that depends on it in menuconfig.

I do think the menuconfig usability issue is a big part of the problem
here. You can't find and enable the options you want easily if the
dependencies aren't met. And you can't easily enable the dependencies
either. Select is an answer to that question. It's the wrong answer, but
it's *an* answer.

On another level, a lot of the problems could be avoided if we took heed
of the note in Documentation/kbuild/kconfig-language.rst:

  Note:
	select should be used with care. select will force
	a symbol to a value without visiting the dependencies.
	By abusing select you are able to select a symbol FOO even
	if FOO depends on BAR that is not set.
	In general use select only for non-visible symbols
	(no prompts anywhere) and for symbols with no dependencies.
	That will limit the usefulness but on the other hand avoid
	the illegal configurations all over.

Another part of the problem is that there is no warning or lint option
or anything to warn about this usage in config. You can easily add more
of these, and it'll break later in subtle ways. And we'll just keep
repeating the same discussion over and over again, some folks fixing
issues in kconfig, some folks using IS_REACHABLE() and eventually
hitting different kinds of problems. (Personally, I wouldn't very easily
accept a patch adding IS_REACHABLE() use in i915.)


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index aaed2347ace9..78e5ba06acff 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -90,7 +90,6 @@  config DRM_SIL_SII8620
 	tristate "Silicon Image SII8620 HDMI/MHL bridge"
 	depends on OF
 	select DRM_KMS_HELPER
-	imply EXTCON
 	depends on RC_CORE || !RC_CORE
 	help
 	  Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 92acd336aa89..94b6c38e6855 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2330,7 +2330,8 @@  static int sii8620_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = sii8620_extcon_init(ctx);
+	if (IS_REACHABLE(CONFIG_EXTCON))
+		ret = sii8620_extcon_init(ctx);
 	if (ret < 0) {
 		dev_err(ctx->dev, "failed to initialize EXTCON\n");
 		return ret;
@@ -2352,7 +2353,7 @@  static int sii8620_remove(struct i2c_client *client)
 {
 	struct sii8620 *ctx = i2c_get_clientdata(client);
 
-	if (ctx->extcon) {
+	if (IS_REACHABLE(CONFIG_EXTCON) && ctx->extcon) {
 		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
 					   &ctx->extcon_nb);
 		flush_work(&ctx->extcon_wq);