| Message ID | 20200520123312.320281-2-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix role-switch selection in type-c drivers | expand |
On Wed, May 20, 2020 at 01:33:11PM +0100, Bryan O'Donoghue wrote: > When I switched on USB role switching for the tps6598x I completely forgot > to add the Kconfig dependency. > > Ensure USB_ROLE_SWITCH is selected to prevent the typs6598x driver being > compiled in but the role-switch driver being compiled as a module, leading > to link error. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/usb/typec/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig > index b4f2aac7ae8a..a6cdf4ec0be6 100644 > --- a/drivers/usb/typec/Kconfig > +++ b/drivers/usb/typec/Kconfig > @@ -64,6 +64,7 @@ config TYPEC_HD3SS3220 > config TYPEC_TPS6598X > tristate "TI TPS6598x USB Power Delivery controller driver" > depends on I2C > + select USB_ROLE_SWITCH > select REGMAP_I2C > help > Say Y or M here if your system has TI TPS65982 or TPS65983 USB Power The same here. You can depend on the class, but you do not select it. Note that we need to be able to handle this situation: CONFIG_TYPEC_TPS6598X=y CONFIG_USB_ROLE_SWITCH=m and this covers that case: depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH thanks,
On 20/05/2020 14:17, Heikki Krogerus wrote:
> depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
Hmm.
That broke for me with a recursive dependency
but this will work
+ depends on REGMAP_I2C
+ depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
On 20/05/2020 14:24, Bryan O'Donoghue wrote: > On 20/05/2020 14:17, Heikki Krogerus wrote: >> depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH > > Hmm. > > That broke for me with a recursive dependency > > but this will work > > + depends on REGMAP_I2C > + depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH > Sorry Heikki. If I make the above change and then do this to switch off where the USB controller in my build is selecting - role switch index d53db520e209..636a5428b47e 100644 --- a/drivers/usb/chipidea/Kconfig +++ b/drivers/usb/chipidea/Kconfig @@ -6,7 +6,6 @@ config USB_CHIPIDEA select EXTCON select RESET_CONTROLLER select USB_ULPI_BUS - select USB_ROLE_SWITCH select USB_TEGRA_PHY if ARCH_TEGRA help it breaks drivers/usb/dwc3/drd.o: In function `dwc3_usb_role_switch_get': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:508: undefined reference to `usb_role_switch_get_drvdata' drivers/usb/dwc3/drd.o: In function `dwc3_usb_role_switch_set': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:484: undefined reference to `usb_role_switch_get_drvdata' drivers/usb/dwc3/drd.o: In function `dwc3_setup_role_switch': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:555: undefined reference to `usb_role_switch_register' drivers/usb/dwc3/drd.o: In function `dwc3_drd_exit': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:628: undefined reference to `usb_role_switch_unregister' drivers/usb/chipidea/core.o: In function `ci_usb_role_switch_get': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:621: undefined reference to `usb_role_switch_get_drvdata' drivers/usb/chipidea/core.o: In function `ci_usb_role_switch_set': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:635: undefined reference to `usb_role_switch_get_drvdata' drivers/usb/chipidea/core.o: In function `ci_hdrc_remove': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:1231: undefined reference to `usb_role_switch_unregister' drivers/usb/chipidea/core.o: In function `ci_hdrc_probe': /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:1210: undefined reference to `usb_role_switch_unregister' /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:1143: undefined reference to `usb_role_switch_register' make[1]: *** [/home/deckard/Development/qualcomm/qlt-kernel/Makefile:1106: vmlinux] Error 1 make[1]: Leaving directory '/home/deckard/Development/qualcomm/qlt-kernel-tools/qlt-kernel/build/square_5.x-tracking' to do what you want to do - shouldn't we have to make all of those "select USB_ROLE_SWITCH" into "depends on USB_ROLE_SWITCH" ? i.e. make all of the consumers depends on instead of selects ?
On Wed, May 20, 2020 at 02:32:31PM +0100, Bryan O'Donoghue wrote: > On 20/05/2020 14:24, Bryan O'Donoghue wrote: > > On 20/05/2020 14:17, Heikki Krogerus wrote: > > > depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH > > > > Hmm. > > > > That broke for me with a recursive dependency > > > > but this will work > > > > + depends on REGMAP_I2C > > + depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH > > > > Sorry Heikki. > > If I make the above change and then do this to switch off where the USB > controller in my build is selecting - role switch > > index d53db520e209..636a5428b47e 100644 > --- a/drivers/usb/chipidea/Kconfig > +++ b/drivers/usb/chipidea/Kconfig > @@ -6,7 +6,6 @@ config USB_CHIPIDEA > select EXTCON > select RESET_CONTROLLER > select USB_ULPI_BUS > - select USB_ROLE_SWITCH > select USB_TEGRA_PHY if ARCH_TEGRA > help That driver is a switch supplier. You should select the class here. > it breaks > > drivers/usb/dwc3/drd.o: In function `dwc3_usb_role_switch_get': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:508: > undefined reference to `usb_role_switch_get_drvdata' > drivers/usb/dwc3/drd.o: In function `dwc3_usb_role_switch_set': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:484: > undefined reference to `usb_role_switch_get_drvdata' > drivers/usb/dwc3/drd.o: In function `dwc3_setup_role_switch': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:555: > undefined reference to `usb_role_switch_register' > drivers/usb/dwc3/drd.o: In function `dwc3_drd_exit': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/dwc3/drd.c:628: > undefined reference to `usb_role_switch_unregister' > drivers/usb/chipidea/core.o: In function `ci_usb_role_switch_get': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:621: > undefined reference to `usb_role_switch_get_drvdata' > drivers/usb/chipidea/core.o: In function `ci_usb_role_switch_set': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:635: > undefined reference to `usb_role_switch_get_drvdata' > drivers/usb/chipidea/core.o: In function `ci_hdrc_remove': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:1231: > undefined reference to `usb_role_switch_unregister' > drivers/usb/chipidea/core.o: In function `ci_hdrc_probe': > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:1210: > undefined reference to `usb_role_switch_unregister' > /home/deckard/Development/qualcomm/qlt-kernel/drivers/usb/chipidea/core.c:1143: > undefined reference to `usb_role_switch_register' > make[1]: *** [/home/deckard/Development/qualcomm/qlt-kernel/Makefile:1106: > vmlinux] Error 1 > make[1]: Leaving directory '/home/deckard/Development/qualcomm/qlt-kernel-tools/qlt-kernel/build/square_5.x-tracking' > > to do what you want to do - shouldn't we have to make all of those "select > USB_ROLE_SWITCH" into "depends on USB_ROLE_SWITCH" ? > > i.e. make all of the consumers depends on instead of selects ? Yes, ideally. thanks,
On 20/05/2020 14:39, Heikki Krogerus wrote: >> to do what you want to do - shouldn't we have to make all of those "select >> USB_ROLE_SWITCH" into "depends on USB_ROLE_SWITCH" ? >> >> i.e. make all of the consumers depends on instead of selects ? > Yes, ideally. > > thanks, I've built the offending x86 version, instead of my more contrived version deselecting USB_ROLE_SWITCH from the chipidea - select USB_ROLE_SWITCH - select REGMAP_I2C + depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH + depends on REGMAP_I2C works - we can look at the depends stuff for the consumers later, I'll send the above now to fix the build. --- bod
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig index b4f2aac7ae8a..a6cdf4ec0be6 100644 --- a/drivers/usb/typec/Kconfig +++ b/drivers/usb/typec/Kconfig @@ -64,6 +64,7 @@ config TYPEC_HD3SS3220 config TYPEC_TPS6598X tristate "TI TPS6598x USB Power Delivery controller driver" depends on I2C + select USB_ROLE_SWITCH select REGMAP_I2C help Say Y or M here if your system has TI TPS65982 or TPS65983 USB Power
When I switched on USB role switching for the tps6598x I completely forgot to add the Kconfig dependency. Ensure USB_ROLE_SWITCH is selected to prevent the typs6598x driver being compiled in but the role-switch driver being compiled as a module, leading to link error. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/usb/typec/Kconfig | 1 + 1 file changed, 1 insertion(+)