2018-08-13 21:51:05

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n

The renesas UDC implementation now calls usb_of_get_companion_dev(),
which is only defined when CONFIG_USB is enabled:

drivers/usb/gadget/udc/renesas_usb3.o: In function `renesas_usb3_probe':
renesas_usb3.c:(.text+0xa34): undefined reference to `usb_of_get_companion_dev'

To avoid the build error, we need two changes:

- usb_of_get_companion_dev must be stubbed out when CONFIG_USB is
disabled, so it the udc driver can be built for a gadget-only
mode kernel
- With CONFIG_USB=m, we must not attempt to have USB_RENESAS_USB3
built-in, so we need a soft dependency on USB.

Fixes: 39facfa01c9f ("usb: gadget: udc: renesas_usb3: Add register of usb role switch")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/usb/gadget/udc/Kconfig | 1 +
include/linux/usb/of.h | 6 +++++-
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 0a16cbd4e528..663a8bd67a7b 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -193,6 +193,7 @@ config USB_RENESAS_USB3
tristate 'Renesas USB3.0 Peripheral controller'
depends on ARCH_RENESAS || COMPILE_TEST
depends on EXTCON
+ depends on USB || !USB
select USB_ROLE_SWITCH
help
Renesas USB3.0 Peripheral controller is a USB peripheral controller
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index dba55ccb9b53..733339218466 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -23,7 +23,6 @@ struct device_node *usb_of_get_device_node(struct usb_device *hub, int port1);
bool usb_of_has_combined_node(struct usb_device *udev);
struct device_node *usb_of_get_interface_node(struct usb_device *udev,
u8 config, u8 ifnum);
-struct device *usb_of_get_companion_dev(struct device *dev);
#else
static inline enum usb_dr_mode
of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
@@ -53,6 +52,11 @@ usb_of_get_interface_node(struct usb_device *udev, u8 config, u8 ifnum)
{
return NULL;
}
+#endif
+
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB)
+struct device *usb_of_get_companion_dev(struct device *dev);
+#else
static inline struct device *usb_of_get_companion_dev(struct device *dev)
{
return NULL;
--
2.18.0



2018-08-14 13:59:37

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n

On Mon, 13 Aug 2018, Arnd Bergmann wrote:

> The renesas UDC implementation now calls usb_of_get_companion_dev(),
> which is only defined when CONFIG_USB is enabled:
>
> drivers/usb/gadget/udc/renesas_usb3.o: In function `renesas_usb3_probe':
> renesas_usb3.c:(.text+0xa34): undefined reference to `usb_of_get_companion_dev'
>
> To avoid the build error, we need two changes:
>
> - usb_of_get_companion_dev must be stubbed out when CONFIG_USB is
> disabled, so it the udc driver can be built for a gadget-only
> mode kernel
> - With CONFIG_USB=m, we must not attempt to have USB_RENESAS_USB3
> built-in, so we need a soft dependency on USB.
>
> Fixes: 39facfa01c9f ("usb: gadget: udc: renesas_usb3: Add register of usb role switch")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/usb/gadget/udc/Kconfig | 1 +
> include/linux/usb/of.h | 6 +++++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 0a16cbd4e528..663a8bd67a7b 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> tristate 'Renesas USB3.0 Peripheral controller'
> depends on ARCH_RENESAS || COMPILE_TEST
> depends on EXTCON
> + depends on USB || !USB

Is this some weird standard idiom? It looks really strange.

Alan Stern


2018-08-14 14:28:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n

On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <[email protected]> wrote:
> On Mon, 13 Aug 2018, Arnd Bergmann wrote:
> > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > index 0a16cbd4e528..663a8bd67a7b 100644
> > --- a/drivers/usb/gadget/udc/Kconfig
> > +++ b/drivers/usb/gadget/udc/Kconfig
> > @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> > tristate 'Renesas USB3.0 Peripheral controller'
> > depends on ARCH_RENESAS || COMPILE_TEST
> > depends on EXTCON
> > + depends on USB || !USB
>
> Is this some weird standard idiom? It looks really strange.

Yes, and yes.

A less common way to write it is

depends on (USB != m) || m

which some people prefer, but I find even weirder.

Arnd

2018-08-20 03:25:38

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n

Hi Arnd,

> From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM
>
> On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <[email protected]> wrote:
> > On Mon, 13 Aug 2018, Arnd Bergmann wrote:
> > > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > > index 0a16cbd4e528..663a8bd67a7b 100644
> > > --- a/drivers/usb/gadget/udc/Kconfig
> > > +++ b/drivers/usb/gadget/udc/Kconfig
> > > @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> > > tristate 'Renesas USB3.0 Peripheral controller'
> > > depends on ARCH_RENESAS || COMPILE_TEST
> > > depends on EXTCON
> > > + depends on USB || !USB
> >
> > Is this some weird standard idiom? It looks really strange.
>
> Yes, and yes.
>
> A less common way to write it is
>
> depends on (USB != m) || m
>
> which some people prefer, but I find even weirder.

Thank you for the patch!

On other thread, John mentions gadget-side drivers should not depend on host-side [1].
So, I submitted a patch today [2]. What do you think about my submitted patch?

[1]
https://marc.info/?l=linux-usb&m=153433776202861&w=2

[2]
https://patchwork.kernel.org/patch/10569847/

Best regards,
Yoshihiro Shimoda

> Arnd

2018-08-20 11:23:56

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n

On Mon, Aug 20, 2018 at 5:22 AM Yoshihiro Shimoda
<[email protected]> wrote:
> > From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM
> >
> > On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <[email protected]> wrote:
> > > On Mon, 13 Aug 2018, Arnd Bergmann wrote:
> > > > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > > > index 0a16cbd4e528..663a8bd67a7b 100644
> > > > --- a/drivers/usb/gadget/udc/Kconfig
> > > > +++ b/drivers/usb/gadget/udc/Kconfig
> > > > @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> > > > tristate 'Renesas USB3.0 Peripheral controller'
> > > > depends on ARCH_RENESAS || COMPILE_TEST
> > > > depends on EXTCON
> > > > + depends on USB || !USB
> > >
> > > Is this some weird standard idiom? It looks really strange.
> >
> > Yes, and yes.
> >
> > A less common way to write it is
> >
> > depends on (USB != m) || m
> >
> > which some people prefer, but I find even weirder.
>
> Thank you for the patch!
>
> On other thread, John mentions gadget-side drivers should not depend on host-side [1].
> So, I submitted a patch today [2]. What do you think about my submitted patch?
>
> [1]
> https://marc.info/?l=linux-usb&m=153433776202861&w=2

I don't really think this is a big issue, as you can still build the driver
with CONFIG_USB=n, so it's not a strict dependency.

> [2]
> https://patchwork.kernel.org/patch/10569847/

On the other hand, your patch looks nice and is somewhat
less confusing, so you have my Ack on that.

Arnd

2018-08-21 02:00:32

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n

Hi Arnd,

> From: Arnd Bergmann, Sent: Monday, August 20, 2018 8:22 PM
>
> On Mon, Aug 20, 2018 at 5:22 AM Yoshihiro Shimoda
> <[email protected]> wrote:
> > > From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM
> > >
> > > On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <[email protected]> wrote:
> > > > On Mon, 13 Aug 2018, Arnd Bergmann wrote:
> > > > > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > > > > index 0a16cbd4e528..663a8bd67a7b 100644
> > > > > --- a/drivers/usb/gadget/udc/Kconfig
> > > > > +++ b/drivers/usb/gadget/udc/Kconfig
> > > > > @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> > > > > tristate 'Renesas USB3.0 Peripheral controller'
> > > > > depends on ARCH_RENESAS || COMPILE_TEST
> > > > > depends on EXTCON
> > > > > + depends on USB || !USB
> > > >
> > > > Is this some weird standard idiom? It looks really strange.
> > >
> > > Yes, and yes.
> > >
> > > A less common way to write it is
> > >
> > > depends on (USB != m) || m
> > >
> > > which some people prefer, but I find even weirder.
> >
> > Thank you for the patch!
> >
> > On other thread, John mentions gadget-side drivers should not depend on host-side [1].
> > So, I submitted a patch today [2]. What do you think about my submitted patch?
> >
> > [1]
> > https://marc.info/?l=linux-usb&m=153433776202861&w=2
>
> I don't really think this is a big issue, as you can still build the driver
> with CONFIG_USB=n, so it's not a strict dependency.

I got it.

> > [2]
> > https://patchwork.kernel.org/patch/10569847/
>
> On the other hand, your patch looks nice and is somewhat
> less confusing, so you have my Ack on that.

Thank you very much for your Ack!
I'll reply by myself to record your Ack on that.

Best regards,
Yoshihiro Shimoda

> Arnd