linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
@ 2018-08-13 21:48 Arnd Bergmann
  2018-08-14 13:58 ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2018-08-13 21:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Arnd Bergmann, Felipe Balbi, Geert Uytterhoeven,
	Yoshihiro Shimoda, Johan Hovold, linux-usb, linux-kernel

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 <arnd@arndb.de>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
  2018-08-13 21:48 [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n Arnd Bergmann
@ 2018-08-14 13:58 ` Alan Stern
  2018-08-14 14:26   ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2018-08-14 13:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Kroah-Hartman, Felipe Balbi, Geert Uytterhoeven,
	Yoshihiro Shimoda, Johan Hovold, linux-usb, linux-kernel

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 <arnd@arndb.de>
> ---
>  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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
  2018-08-14 13:58 ` Alan Stern
@ 2018-08-14 14:26   ` Arnd Bergmann
  2018-08-20  3:22     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2018-08-14 14:26 UTC (permalink / raw)
  To: Alan Stern
  Cc: gregkh, Felipe Balbi, Geert Uytterhoeven, yoshihiro.shimoda.uh,
	Johan Hovold, USB list, Linux Kernel Mailing List

On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <stern@rowland.harvard.edu> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
  2018-08-14 14:26   ` Arnd Bergmann
@ 2018-08-20  3:22     ` Yoshihiro Shimoda
  2018-08-20 11:22       ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-20  3:22 UTC (permalink / raw)
  To: Arnd Bergmann, Alan Stern
  Cc: gregkh, Felipe Balbi, Geert Uytterhoeven, Johan Hovold, USB list,
	Linux Kernel Mailing List

Hi Arnd,

> From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM
> 
> On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <stern@rowland.harvard.edu> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
  2018-08-20  3:22     ` Yoshihiro Shimoda
@ 2018-08-20 11:22       ` Arnd Bergmann
  2018-08-21  1:59         ` Yoshihiro Shimoda
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2018-08-20 11:22 UTC (permalink / raw)
  To: yoshihiro.shimoda.uh
  Cc: Alan Stern, gregkh, Felipe Balbi, Geert Uytterhoeven,
	Johan Hovold, USB list, Linux Kernel Mailing List

On Mon, Aug 20, 2018 at 5:22 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM
> >
> > On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <stern@rowland.harvard.edu> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n
  2018-08-20 11:22       ` Arnd Bergmann
@ 2018-08-21  1:59         ` Yoshihiro Shimoda
  0 siblings, 0 replies; 6+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-21  1:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alan Stern, gregkh, Felipe Balbi, Geert Uytterhoeven,
	Johan Hovold, USB list, Linux Kernel Mailing List

Hi Arnd,

> From: Arnd Bergmann, Sent: Monday, August 20, 2018 8:22 PM
> 
> On Mon, Aug 20, 2018 at 5:22 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Arnd Bergmann, Sent: Tuesday, August 14, 2018 11:27 PM
> > >
> > > On Tue, Aug 14, 2018 at 3:58 PM Alan Stern <stern@rowland.harvard.edu> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-21  1:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-13 21:48 [PATCH] usb: hide usb_of_get_companion_dev for CONFIG_USB=n Arnd Bergmann
2018-08-14 13:58 ` Alan Stern
2018-08-14 14:26   ` Arnd Bergmann
2018-08-20  3:22     ` Yoshihiro Shimoda
2018-08-20 11:22       ` Arnd Bergmann
2018-08-21  1:59         ` Yoshihiro Shimoda

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).