2017-04-13 12:33:47

by Alexey Brodkin

Subject: [PATCH] usb: Make sure usb/phy/of gets built-in

DWC3 driver uses of_usb_get_phy_mode() which is
implemented in drivers/usb/phy/of.c and in bare minimal
configuration it might not be pulled in kernel binary.

In case of ARC or ARM this could be easily reproduced with
"allnodefconfig" +CONFIG_USB=m +CONFIG_USB_DWC3=m.

On building all ends-up with:
---------------------->8------------------
Kernel: arch/arm/boot/Image is ready
Kernel: arch/arm/boot/zImage is ready
Building modules, stage 2.
MODPOST 5 modules
ERROR: "of_usb_get_phy_mode" [drivers/usb/dwc3/dwc3.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
---------------------->8------------------

Signed-off-by: Alexey Brodkin <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Felix Fietkau <[email protected]>
Cc: Jeremy Kerr <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/Makefile | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/Makefile b/drivers/Makefile
index 2eced9afba53..8f8bdc9e3d29 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_USB_PHY) += usb/
obj-$(CONFIG_USB) += usb/
obj-$(CONFIG_PCI) += usb/
obj-$(CONFIG_USB_GADGET) += usb/
+obj-$(CONFIG_OF) += usb/
obj-$(CONFIG_SERIO) += input/serio/
obj-$(CONFIG_GAMEPORT) += input/gameport/
obj-$(CONFIG_INPUT) += input/
--
2.7.4

2017-04-18 03:15:50

by Frank Rowand

Subject: Re: [PATCH] usb: Make sure usb/phy/of gets built-in

On 04/13/17 05:33, Alexey Brodkin wrote:
> DWC3 driver uses of_usb_get_phy_mode() which is
> implemented in drivers/usb/phy/of.c and in bare minimal
> configuration it might not be pulled in kernel binary.
>
> In case of ARC or ARM this could be easily reproduced with
> "allnodefconfig" +CONFIG_USB=m +CONFIG_USB_DWC3=m.
>
> On building all ends-up with:
> ---------------------->8------------------
> Kernel: arch/arm/boot/Image is ready
> Kernel: arch/arm/boot/zImage is ready
> Building modules, stage 2.
> MODPOST 5 modules
> ERROR: "of_usb_get_phy_mode" [drivers/usb/dwc3/dwc3.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> ---------------------->8------------------
>
> Signed-off-by: Alexey Brodkin <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Nicolas Pitre <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Felipe Balbi <[email protected]>
> Cc: Felix Fietkau <[email protected]>
> Cc: Jeremy Kerr <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 2eced9afba53..8f8bdc9e3d29 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -104,6 +104,7 @@ obj-$(CONFIG_USB_PHY) += usb/
> obj-$(CONFIG_USB) += usb/
> obj-$(CONFIG_PCI) += usb/
> obj-$(CONFIG_USB_GADGET) += usb/

> +obj-$(CONFIG_OF) += usb/

Would CONFIG_USB_SUPPORT make more sense? (And does it work?)


> obj-$(CONFIG_SERIO) += input/serio/
> obj-$(CONFIG_GAMEPORT) += input/gameport/
> obj-$(CONFIG_INPUT) += input/
>

2017-04-18 12:23:30

by Alexey Brodkin

Subject: Re: [PATCH] usb: Make sure usb/phy/of gets built-in

Hello Frank,

On Mon, 2017-04-17 at 20:15 -0700, Frank Rowand wrote:
> On 04/13/17 05:33, Alexey Brodkin wrote:
> >
> > DWC3 driver uses of_usb_get_phy_mode() which is
> > implemented in drivers/usb/phy/of.c and in bare minimal
> > configuration it might not be pulled in kernel binary.
> >
> > In case of ARC or ARM this could be easily reproduced with
> > "allnodefconfig" +CONFIG_USB=m +CONFIG_USB_DWC3=m.
> >
> > On building all ends-up with:
> > ---------------------->8------------------
> >   Kernel: arch/arm/boot/Image is ready
> >   Kernel: arch/arm/boot/zImage is ready
> >   Building modules, stage 2.
> >   MODPOST 5 modules
> > ERROR: "of_usb_get_phy_mode" [drivers/usb/dwc3/dwc3.ko] undefined!
> > make[1]: *** [__modpost] Error 1
> > make: *** [modules] Error 2
> > ---------------------->8------------------
> >
> > Signed-off-by: Alexey Brodkin <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > Cc: Geert Uytterhoeven <[email protected]>
> > Cc: Nicolas Pitre <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Felipe Balbi <[email protected]>
> > Cc: Felix Fietkau <[email protected]>
> > Cc: Jeremy Kerr <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> >  drivers/Makefile | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 2eced9afba53..8f8bdc9e3d29 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -104,6 +104,7 @@ obj-$(CONFIG_USB_PHY) += usb/
> >  obj-$(CONFIG_USB) += usb/
> >  obj-$(CONFIG_PCI) += usb/
> >  obj-$(CONFIG_USB_GADGET) += usb/
>
> >
> > +obj-$(CONFIG_OF) += usb/
>
> Would CONFIG_USB_SUPPORT make more sense? (And does it work?)

Well I'm not really sure here.

The problem was in missing "drivers/usb/phy/of.o" in "drivers/usb/built-in.o".
So I took a look at how components get enabled and saw quite mixed stuff.

In "drivers/usb/phy/Makefile" we see:
------------------------->8---------------------------
obj-$(CONFIG_OF)»       »       »       += of.o
------------------------->8---------------------------

In "drivers/usb/Makefile" we see:
------------------------->8---------------------------
obj-$(CONFIG_USB_SUPPORT)»      += phy/
------------------------->8---------------------------

>From above I may conclude that your proposal should work as well
but it's more a question of which approach is safer and more future proof.

Maybe it even worth adding the following to "drivers/usb/Makefile":
------------------------->8---------------------------
obj-$(CONFIG_USB_SUPPORT)»      += phy/
------------------------->8---------------------------

Any thoughts are more than welcome.

-Alexey

2017-04-24 12:45:01

by Alexey Brodkin

Subject: Re: [PATCH] usb: Make sure usb/phy/of gets built-in

Hello,

On Tue, 2017-04-18 at 12:23 +0000, Alexey Brodkin wrote:
> Hello Frank,
>
> On Mon, 2017-04-17 at 20:15 -0700, Frank Rowand wrote:
> >
> > On 04/13/17 05:33, Alexey Brodkin wrote:
> > >
> > >
> > > DWC3 driver uses of_usb_get_phy_mode() which is
> > > implemented in drivers/usb/phy/of.c and in bare minimal
> > > configuration it might not be pulled in kernel binary.
> > >
> > > In case of ARC or ARM this could be easily reproduced with
> > > "allnodefconfig" +CONFIG_USB=m +CONFIG_USB_DWC3=m.
> > >
> > > On building all ends-up with:
> > > ---------------------->8------------------
> > >   Kernel: arch/arm/boot/Image is ready
> > >   Kernel: arch/arm/boot/zImage is ready
> > >   Building modules, stage 2.
> > >   MODPOST 5 modules
> > > ERROR: "of_usb_get_phy_mode" [drivers/usb/dwc3/dwc3.ko] undefined!
> > > make[1]: *** [__modpost] Error 1
> > > make: *** [modules] Error 2
> > > ---------------------->8------------------
> > >
> > > Signed-off-by: Alexey Brodkin <[email protected]>
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: Masahiro Yamada <[email protected]>
> > > Cc: Geert Uytterhoeven <[email protected]>
> > > Cc: Nicolas Pitre <[email protected]>
> > > Cc: Thomas Gleixner <[email protected]>
> > > Cc: Felipe Balbi <[email protected]>
> > > Cc: Felix Fietkau <[email protected]>
> > > Cc: Jeremy Kerr <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > ---
> > >  drivers/Makefile | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/Makefile b/drivers/Makefile
> > > index 2eced9afba53..8f8bdc9e3d29 100644
> > > --- a/drivers/Makefile
> > > +++ b/drivers/Makefile
> > > @@ -104,6 +104,7 @@ obj-$(CONFIG_USB_PHY) += usb/
> > >  obj-$(CONFIG_USB) += usb/
> > >  obj-$(CONFIG_PCI) += usb/
> > >  obj-$(CONFIG_USB_GADGET) += usb/
> >
> > >
> > >
> > > +obj-$(CONFIG_OF) += usb/
> >
> > Would CONFIG_USB_SUPPORT make more sense? (And does it work?)
>
> Well I'm not really sure here.
>
> The problem was in missing "drivers/usb/phy/of.o" in "drivers/usb/built-in.o".
> So I took a look at how components get enabled and saw quite mixed stuff.
>
> In "drivers/usb/phy/Makefile" we see:
> ------------------------->8---------------------------
> obj-$(CONFIG_OF)»       »       »       += of.o
> ------------------------->8---------------------------
>
> In "drivers/usb/Makefile" we see:
> ------------------------->8---------------------------
> obj-$(CONFIG_USB_SUPPORT)»      += phy/
> ------------------------->8---------------------------
>
> From above I may conclude that your proposal should work as well
> but it's more a question of which approach is safer and more future proof.
>
> Maybe it even worth adding the following to "drivers/usb/Makefile":
> ------------------------->8---------------------------
> obj-$(CONFIG_USB_SUPPORT)»      += phy/
> ------------------------->8---------------------------

I'm wondering if there're any other thoughts on this one?
The patch fixes a real problem and it would be good to get this or similar
fix upstream.

-Alexey