* [PATCH v2 0/2] Fix role-switch selection in type-c drivers
@ 2020-05-20 12:33 Bryan O'Donoghue
2020-05-20 12:33 ` [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x Bryan O'Donoghue
2020-05-20 12:33 ` [PATCH v2 2/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220 Bryan O'Donoghue
0 siblings, 2 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2020-05-20 12:33 UTC (permalink / raw)
To: linux-usb, gregkh, heikki.krogerus; +Cc: Bryan O'Donoghue
A recent change to the typs6598x resulted in a link-time error caught by
kbuild. The first patch fixes that error, however since it is not included
in stable yet, it carries no fixes tag.
In trying to fix the typs6598x error, the first thing I did was try to take
the same fix, for the same error that had been applied to the hd3ss3220
but, I could still configure my kernel and produce the same link-time error
with the tps6598x.
I did a quick grep and saw that everywhere else we were doing "select
USB_ROLE_SWITCH" not "depends on USB_ROLE_SWITCH".
Using select fixes the error for me on tps6598x so I've also rolled the
same fix into the hd3ss3220 because it looks like the right thing to do
there also.
Bryan O'Donoghue (2):
usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220
drivers/usb/typec/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
2020-05-20 12:33 [PATCH v2 0/2] Fix role-switch selection in type-c drivers Bryan O'Donoghue
@ 2020-05-20 12:33 ` Bryan O'Donoghue
2020-05-20 13:17 ` Heikki Krogerus
2020-05-20 12:33 ` [PATCH v2 2/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220 Bryan O'Donoghue
1 sibling, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2020-05-20 12:33 UTC (permalink / raw)
To: linux-usb, gregkh, heikki.krogerus; +Cc: Bryan O'Donoghue
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
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
2020-05-20 12:33 ` [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x Bryan O'Donoghue
@ 2020-05-20 13:17 ` Heikki Krogerus
2020-05-20 13:24 ` Bryan O'Donoghue
0 siblings, 1 reply; 9+ messages in thread
From: Heikki Krogerus @ 2020-05-20 13:17 UTC (permalink / raw)
To: Bryan O'Donoghue; +Cc: linux-usb, gregkh
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,
--
heikki
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
2020-05-20 13:17 ` Heikki Krogerus
@ 2020-05-20 13:24 ` Bryan O'Donoghue
2020-05-20 13:32 ` Bryan O'Donoghue
0 siblings, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2020-05-20 13:24 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: linux-usb, gregkh
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
2020-05-20 13:24 ` Bryan O'Donoghue
@ 2020-05-20 13:32 ` Bryan O'Donoghue
2020-05-20 13:39 ` Heikki Krogerus
0 siblings, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2020-05-20 13:32 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: linux-usb, gregkh
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 ?
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
2020-05-20 13:32 ` Bryan O'Donoghue
@ 2020-05-20 13:39 ` Heikki Krogerus
2020-05-20 15:31 ` Bryan O'Donoghue
0 siblings, 1 reply; 9+ messages in thread
From: Heikki Krogerus @ 2020-05-20 13:39 UTC (permalink / raw)
To: Bryan O'Donoghue; +Cc: linux-usb, gregkh
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,
--
heikki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x
2020-05-20 13:39 ` Heikki Krogerus
@ 2020-05-20 15:31 ` Bryan O'Donoghue
0 siblings, 0 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2020-05-20 15:31 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: linux-usb, gregkh
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220
2020-05-20 12:33 [PATCH v2 0/2] Fix role-switch selection in type-c drivers Bryan O'Donoghue
2020-05-20 12:33 ` [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x Bryan O'Donoghue
@ 2020-05-20 12:33 ` Bryan O'Donoghue
2020-05-20 13:13 ` Heikki Krogerus
1 sibling, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2020-05-20 12:33 UTC (permalink / raw)
To: linux-usb, gregkh, heikki.krogerus; +Cc: Bryan O'Donoghue
kbuild found a compile error with recent additions to the tps6598x to add
role-switch notification support. Digging into that error it turns out we
needed to add "select USB_ROLE_SWITCH" to Kconfig.
Adding "depends on USB_ROLE_SWITCH" as had previously been done for this
driver could still result in a link-time failure.
This patch propagates the tps6598x fix to hd3ss3220.
Fixes: da4b5d18dd94 ("usb: typec: add dependency for TYPEC_HD3SS3220")
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/usb/typec/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index a6cdf4ec0be6..8f84304802f4 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -53,7 +53,7 @@ source "drivers/usb/typec/ucsi/Kconfig"
config TYPEC_HD3SS3220
tristate "TI HD3SS3220 Type-C DRP Port controller driver"
depends on I2C
- depends on USB_ROLE_SWITCH
+ select USB_ROLE_SWITCH
help
Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
controller driver.
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220
2020-05-20 12:33 ` [PATCH v2 2/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220 Bryan O'Donoghue
@ 2020-05-20 13:13 ` Heikki Krogerus
0 siblings, 0 replies; 9+ messages in thread
From: Heikki Krogerus @ 2020-05-20 13:13 UTC (permalink / raw)
To: Bryan O'Donoghue; +Cc: linux-usb, gregkh
On Wed, May 20, 2020 at 01:33:12PM +0100, Bryan O'Donoghue wrote:
> kbuild found a compile error with recent additions to the tps6598x to add
> role-switch notification support. Digging into that error it turns out we
> needed to add "select USB_ROLE_SWITCH" to Kconfig.
>
> Adding "depends on USB_ROLE_SWITCH" as had previously been done for this
> driver could still result in a link-time failure.
>
> This patch propagates the tps6598x fix to hd3ss3220.
>
> Fixes: da4b5d18dd94 ("usb: typec: add dependency for TYPEC_HD3SS3220")
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
NAK.
This is a switch consumer not supplier. The switch may not be needed
on every platform with this driver.
> ---
> drivers/usb/typec/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index a6cdf4ec0be6..8f84304802f4 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -53,7 +53,7 @@ source "drivers/usb/typec/ucsi/Kconfig"
> config TYPEC_HD3SS3220
> tristate "TI HD3SS3220 Type-C DRP Port controller driver"
> depends on I2C
> - depends on USB_ROLE_SWITCH
> + select USB_ROLE_SWITCH
> help
> Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
> controller driver.
thanks,
--
heikki
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-20 15:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 12:33 [PATCH v2 0/2] Fix role-switch selection in type-c drivers Bryan O'Donoghue
2020-05-20 12:33 ` [PATCH v2 1/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for tps6598x Bryan O'Donoghue
2020-05-20 13:17 ` Heikki Krogerus
2020-05-20 13:24 ` Bryan O'Donoghue
2020-05-20 13:32 ` Bryan O'Donoghue
2020-05-20 13:39 ` Heikki Krogerus
2020-05-20 15:31 ` Bryan O'Donoghue
2020-05-20 12:33 ` [PATCH v2 2/2] usb: typec: Ensure USB_ROLE_SWITCH is selected for hd3ss3220 Bryan O'Donoghue
2020-05-20 13:13 ` Heikki Krogerus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).