Skip to contentSkip to conversation

usb: typec: nb7vpq904m: add CONFIG_DRM dependency

PATCH In review Arnd Bergmann 3 participants 6 messages Jun 22, 2023

Subsystems (2)
USB USB Type-C class
lore
Share & tools
lore

Arnd Bergmann

author
#0 lore raw

PATCH 1/1 usb: typec: nb7vpq904m: add CONFIG_DRM dependency

From: Arnd Bergmann <[email protected]>

This driver calls directly into DRM functions and fails to link if that is disabled:

ld.lld: error: undefined symbol: devm_drm_bridge_add ld.lld: error: undefined symbol: devm_drm_of_get_bridge

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

referenced by nb7vpq904m.c
drivers/usb/typec/mux/nb7vpq904m.o:(nb7vpq904m_probe) in archive vmlinux.a

Fixes 88d8f3ac9c67e ("usb: typec: add support for the nb7vpq904m Type-C Linear Redriver")
Signed-off-by Arnd Bergmann <[email protected]>
---
 drivers/usb/typec/mux/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/mux/Kconfig +1 -1
index 8c4d6b8fb75c3..f53ae24b6c048 100644
@@ -37,7 +37,7 @@ config TYPEC_MUX_INTEL_PMC
 
 config TYPEC_MUX_NB7VPQ904M
 	tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
-	depends on I2C
+	depends on I2C && DRM
 	select REGMAP_I2C
 	help
 	  Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C
Signature · 1 line
2.39.2
#1 lore raw

Hi,

Quoted 21 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

This driver calls directly into DRM functions and fails to link if that is disabled:

ld.lld: error: undefined symbol: devm_drm_bridge_add ld.lld: error: undefined symbol: devm_drm_of_get_bridge

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

referenced by nb7vpq904m.c
drivers/usb/typec/mux/nb7vpq904m.o:(nb7vpq904m_probe) in archive vmlinux.a

Fixes 88d8f3ac9c67e ("usb: typec: add support for the nb7vpq904m Type-C Linear Redriver")
Signed-off-by Arnd Bergmann <[email protected]>
---
  drivers/usb/typec/mux/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/mux/Kconfig +0 -0
index 8c4d6b8fb75c3..f53ae24b6c048 100644

drivers/usb/typec/mux/Kconfig config TYPEC_MUX_INTEL_PMC · line 43

Quoted 8 lines
drivers/usb/typec/mux/Kconfig +1 -1
@@ -37,7 +37,7 @@ config TYPEC_MUX_INTEL_PMC
 
  config TYPEC_MUX_NB7VPQ904M
  	tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
-	depends on I2C
+	depends on I2C && DRM
  	select REGMAP_I2C
  	help
  	  Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C

I think it could be :

+ depends on DRM || DRM=n
+ select DRM_PANEL_BRIDGE if DRM

Neil

#2 lore raw
Quoted 35 lines · Neil Armstrong

Hi,

Quoted 21 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

This driver calls directly into DRM functions and fails to link if that is disabled:

ld.lld: error: undefined symbol: devm_drm_bridge_add ld.lld: error: undefined symbol: devm_drm_of_get_bridge

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

referenced by nb7vpq904m.c
drivers/usb/typec/mux/nb7vpq904m.o:(nb7vpq904m_probe) in archive vmlinux.a

Fixes 88d8f3ac9c67e ("usb: typec: add support for the nb7vpq904m Type-C Linear Redriver")
Signed-off-by Arnd Bergmann <[email protected]>
---
  drivers/usb/typec/mux/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/mux/Kconfig +0 -0
index 8c4d6b8fb75c3..f53ae24b6c048 100644

drivers/usb/typec/mux/Kconfig config TYPEC_MUX_INTEL_PMC · line 43

Quoted 8 lines
drivers/usb/typec/mux/Kconfig +1 -1
@@ -37,7 +37,7 @@ config TYPEC_MUX_INTEL_PMC
 
  config TYPEC_MUX_NB7VPQ904M
  	tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
-	depends on I2C
+	depends on I2C && DRM
  	select REGMAP_I2C
  	help
  	  Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C

I think it could be :

Review

Quoted 2 lines
(quoted diff) +2 -0
+	depends on DRM || DRM=n
+	select DRM_PANEL_BRIDGE if DRM

As far as I can tell, this would only avoid the link error against devm_drm_of_get_bridge(), but not the one against devm_drm_bridge_add(), which is defined in drm.ko through drivers/gpu/drm/drm_bridge.c.

Arnd

#3 lore raw

Hi,

Quoted 36 lines · Arnd Bergmann
Quoted 35 lines · Neil Armstrong

Hi,

Quoted 21 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

This driver calls directly into DRM functions and fails to link if that is disabled:

ld.lld: error: undefined symbol: devm_drm_bridge_add ld.lld: error: undefined symbol: devm_drm_of_get_bridge

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

referenced by nb7vpq904m.c
drivers/usb/typec/mux/nb7vpq904m.o:(nb7vpq904m_probe) in archive vmlinux.a

Fixes 88d8f3ac9c67e ("usb: typec: add support for the nb7vpq904m Type-C Linear Redriver")
Signed-off-by Arnd Bergmann <[email protected]>
---
   drivers/usb/typec/mux/Kconfig | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/mux/Kconfig +0 -0
index 8c4d6b8fb75c3..f53ae24b6c048 100644

drivers/usb/typec/mux/Kconfig config TYPEC_MUX_INTEL_PMC · line 43

Quoted 8 lines
drivers/usb/typec/mux/Kconfig +1 -1
@@ -37,7 +37,7 @@ config TYPEC_MUX_INTEL_PMC
 
   config TYPEC_MUX_NB7VPQ904M
   	tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
-	depends on I2C
+	depends on I2C && DRM
   	select REGMAP_I2C
   	help
   	  Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C

I think it could be :

Quoted 7 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	depends on DRM || DRM=n
+	select DRM_PANEL_BRIDGE if DRM

As far as I can tell, this would only avoid the link error against devm_drm_of_get_bridge(), but not the one against devm_drm_bridge_add(), which is defined in drm.ko through drivers/gpu/drm/drm_bridge.c.

I'm trying to reproduce such situation, but so fail I fail.

In the driver there's a guard to avoid calling into DRM functions when disabled:
#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_PANEL_BRIDGE)

so I wonder which kind on config leaded to that since CONFIG_DRM_PANEL_BRIDGE is only enabled when DRM_PANEL and DRM are.

Neil

Quoted 2 lines

Arnd

#4 lore raw
Quoted 21 lines · Neil Armstrong
Quoted 12 lines · Arnd Bergmann
Quoted 11 lines · Neil Armstrong

drivers/usb/typec/mux/Kconfig config TYPEC_MUX_INTEL_PMC · line 43

Quoted 8 lines

config TYPEC_MUX_NB7VPQ904M
tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"

drivers/usb/typec/mux/Kconfig +1 -1
-	depends on I2C
+	depends on I2C && DRM
   	select REGMAP_I2C
   	help
   	  Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C

I think it could be :

Quoted 7 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	depends on DRM || DRM=n
+	select DRM_PANEL_BRIDGE if DRM

As far as I can tell, this would only avoid the link error against devm_drm_of_get_bridge(), but not the one against devm_drm_bridge_add(), which is defined in drm.ko through drivers/gpu/drm/drm_bridge.c.

Quoted 5 lines

I'm trying to reproduce such situation, but so fail I fail.

In the driver there's a guard to avoid calling into DRM functions when disabled:
#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_PANEL_BRIDGE)

Ah, you are right. I got confused because the check is in header file for devm_drm_of_get_bridge(), but not for devm_drm_bridge_add(), which has the check in the source file as you point out.

Quoted 2 lines

so I wonder which kind on config leaded to that since CONFIG_DRM_PANEL_BRIDGE is only enabled when DRM_PANEL and DRM are.

I only saw the original issue with

CONFIG_DRM=m
CONFIG_DRM_PANEL=y
CONFIG_DRM_BRIDGE=y
CONFIG_DRM_PANEL_BRIDGE=y
CONFIG_TYPEC_MUX_NB7VPQ904M=y

and since CONFIG_DRM_PANEL_BRIDGE already depends on CONFIG_DRM, I think that is the only one that can go wrong, so your suggestion of

   depends on DRM || DRM=n

should be sufficient. I see that DRM_PANEL, DRM_BRIDGE and DRM_PANEL_BRIDGE are now always =y whenever DRM is enabled, so I don't think the 'select CONFIG_DRM_PANEL_BRIDGE' serves any purpose any more, but it's also harmless if you think it helps for clarification.

Can you send the updated patch, or should I?

Arnd

#5 lore raw
Quoted 36 lines · Arnd Bergmann
Quoted 21 lines · Neil Armstrong
Quoted 12 lines · Arnd Bergmann
Quoted 11 lines · Neil Armstrong

drivers/usb/typec/mux/Kconfig config TYPEC_MUX_INTEL_PMC · line 43

Quoted 8 lines

config TYPEC_MUX_NB7VPQ904M
tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"

drivers/usb/typec/mux/Kconfig +1 -1
-	depends on I2C
+	depends on I2C && DRM
    	select REGMAP_I2C
    	help
    	  Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C

I think it could be :

Quoted 7 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	depends on DRM || DRM=n
+	select DRM_PANEL_BRIDGE if DRM

As far as I can tell, this would only avoid the link error against devm_drm_of_get_bridge(), but not the one against devm_drm_bridge_add(), which is defined in drm.ko through drivers/gpu/drm/drm_bridge.c.

Quoted 5 lines

I'm trying to reproduce such situation, but so fail I fail.

In the driver there's a guard to avoid calling into DRM functions when disabled:
#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_PANEL_BRIDGE)

Ah, you are right. I got confused because the check is in header file for devm_drm_of_get_bridge(), but not for devm_drm_bridge_add(), which has the check in the source file as you point out.

Quoted 2 lines

so I wonder which kind on config leaded to that since CONFIG_DRM_PANEL_BRIDGE is only enabled when DRM_PANEL and DRM are.

Quoted 7 lines

I only saw the original issue with

CONFIG_DRM=m
CONFIG_DRM_PANEL=y
CONFIG_DRM_BRIDGE=y
CONFIG_DRM_PANEL_BRIDGE=y
CONFIG_TYPEC_MUX_NB7VPQ904M=y

Ok now I see it

Quoted 5 lines

and since CONFIG_DRM_PANEL_BRIDGE already depends on CONFIG_DRM, I think that is the only one that can go wrong, so your suggestion of

Quoted 7 lines
    depends on DRM || DRM=n

should be sufficient. I see that DRM_PANEL, DRM_BRIDGE and DRM_PANEL_BRIDGE are now always =y whenever DRM is enabled, so I don't think the 'select CONFIG_DRM_PANEL_BRIDGE' serves any purpose any more, but it's also harmless if you think it helps for clarification.

Yes DRM || DRM=n seems sufficient, but having select DRM_PANEL_BRIDGE if DRM could clarify things.

Quoted 2 lines

Can you send the updated patch, or should I?

I'll send an updated patch,

Thanks,
Neil

Quoted 3 lines

Arnd