| Message ID | 20230622101813.3453772-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: typec: nb7vpq904m: add CONFIG_DRM dependency | expand |
Hi, On 22/06/2023 12:18, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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 >>>> 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 <arnd@arndb.de> > --- > drivers/usb/typec/mux/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig > index 8c4d6b8fb75c3..f53ae24b6c048 100644 > --- a/drivers/usb/typec/mux/Kconfig > +++ b/drivers/usb/typec/mux/Kconfig > @@ -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
On Thu, Jun 22, 2023, at 12:39, Neil Armstrong wrote: > Hi, > > On 22/06/2023 12:18, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> 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 >>>>> 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 <arnd@arndb.de> >> --- >> drivers/usb/typec/mux/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig >> index 8c4d6b8fb75c3..f53ae24b6c048 100644 >> --- a/drivers/usb/typec/mux/Kconfig >> +++ b/drivers/usb/typec/mux/Kconfig >> @@ -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 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
Hi, On 22/06/2023 13:24, Arnd Bergmann wrote: > On Thu, Jun 22, 2023, at 12:39, Neil Armstrong wrote: >> Hi, >> >> On 22/06/2023 12:18, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> 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 >>>>>> 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 <arnd@arndb.de> >>> --- >>> drivers/usb/typec/mux/Kconfig | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig >>> index 8c4d6b8fb75c3..f53ae24b6c048 100644 >>> --- a/drivers/usb/typec/mux/Kconfig >>> +++ b/drivers/usb/typec/mux/Kconfig >>> @@ -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 > > 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 > > Arnd
On Thu, Jun 22, 2023, at 17:39, Neil Armstrong wrote: > On 22/06/2023 13:24, Arnd Bergmann wrote: >> On Thu, Jun 22, 2023, at 12:39, Neil Armstrong wrote: >>>> >>>> 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 >> >> 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) 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. > 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
On 22/06/2023 17:57, Arnd Bergmann wrote: > On Thu, Jun 22, 2023, at 17:39, Neil Armstrong wrote: >> On 22/06/2023 13:24, Arnd Bergmann wrote: >>> On Thu, Jun 22, 2023, at 12:39, Neil Armstrong wrote: >>>>> >>>>> 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 >>> >>> 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) > > 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. > >> 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 Ok now I see it > > 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. Yes DRM || DRM=n seems sufficient, but having select DRM_PANEL_BRIDGE if DRM could clarify things. > > Can you send the updated patch, or should I? I'll send an updated patch, Thanks, Neil > > > Arnd
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig index 8c4d6b8fb75c3..f53ae24b6c048 100644 --- a/drivers/usb/typec/mux/Kconfig +++ b/drivers/usb/typec/mux/Kconfig @@ -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