patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

[gregkh]

This is a note to let you know that I've just added the patch titled

    Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"

to my usb git tree which can be found at
    git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will hopefully also be merged in Linus's tree for the
next -rc kernel release.

If you have any questions about this process, please let me know.


From abd062886cd103196b4f26cf735c3a3619dec76b Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Fri, 11 Jun 2021 09:18:47 +0200
Subject: Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"

This reverts commit e0e8b6abe8c862229ba00cdd806e8598cdef00bb.

Turns out this breaks the build.  We had numerous reports of problems
from linux-next and 0-day about this not working properly, so revert it
for now until it can be figured out properly.

The build errors are:
	arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x29d4): undefined reference to `fsl_udc_clk_finalize'
	arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x2ba8): undefined reference to `fsl_udc_clk_release'
	fsl_udc_core.c:(.text+0x2848): undefined reference to `fsl_udc_clk_init'
	fsl_udc_core.c:(.text+0xe88): undefined reference to `fsl_udc_clk_release'

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Reported-by: kernel test robot <lkp@intel.com>
Fixes: e0e8b6abe8c8 ("usb: gadget: fsl: Re-enable driver for ARM SoCs")
Cc: stable <stable@vger.kernel.org>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Leo Li <leoyang.li@nxp.com>
Cc: Peter Chen <peter.chen@nxp.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Ran Wang <ran.wang_1@nxp.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/gadget/udc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 7348acbdc560..8c614bb86c66 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -90,7 +90,7 @@ config USB_BCM63XX_UDC
 
 config USB_FSL_USB2
 	tristate "Freescale Highspeed USB DR Peripheral Controller"
-	depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
+	depends on FSL_SOC
 	help
 	   Some of Freescale PowerPC and i.MX processors have a High Speed
 	   Dual-Role(DR) USB controller, which supports device mode.
-- 
2.32.0

Re: patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

[Arnd Bergmann]

On Fri, Jun 11, 2021 at 9:22 AM <gregkh@linuxfoundation.org> wrote:
>
> Turns out this breaks the build.  We had numerous reports of problems
> from linux-next and 0-day about this not working properly, so revert it
> for now until it can be figured out properly.
>
> The build errors are:
>         arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x29d4): undefined reference to `fsl_udc_clk_finalize'
>         arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x2ba8): undefined reference to `fsl_udc_clk_release'
>         fsl_udc_core.c:(.text+0x2848): undefined reference to `fsl_udc_clk_init'
>         fsl_udc_core.c:(.text+0xe88): undefined reference to `fsl_udc_clk_release'
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: e0e8b6abe8c8 ("usb: gadget: fsl: Re-enable driver for ARM SoCs")

Adding Fabio and Guennadi to Cc.

I can see that the missing symbols were in a driver that got removed in commit
a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver").

If CONFIG_ARCH_MXC is disabled, these are stubbed out in the header file.
These were added a long time ago by Guennadi Liakhovetski 54e4026b64a9
("USB: gadget: Add i.MX3x support to the fsl_usb2_udc driver"). I also
see that this patch added  a few #ifdef CONFIG_ARCH_MXC checks to the
driver that still remain today. This is clearly broken as it must be possible
to use the same driver module on both SOC_LS1021A and i.MX using
a runtime check.

I also don't see any i.MX variant actually using this driver, but instead see
the dts files declaring fsl,imx27-usb devices, which bind to the
drivers/usb/chipidea/ci_hdrc_imx.c driver. Is this one of those cases
where we have two separate drivers for the same hardware, or is this
for a different device?

        Arnd

Re: patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

[Fabio Estevam]

Hi Arnd,

On Fri, Jun 11, 2021 at 5:30 AM Arnd Bergmann <arnd@arndb.de> wrote:

> Adding Fabio and Guennadi to Cc.
>
> I can see that the missing symbols were in a driver that got removed in commit
> a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver").
>
> If CONFIG_ARCH_MXC is disabled, these are stubbed out in the header file.
> These were added a long time ago by Guennadi Liakhovetski 54e4026b64a9
> ("USB: gadget: Add i.MX3x support to the fsl_usb2_udc driver"). I also
> see that this patch added  a few #ifdef CONFIG_ARCH_MXC checks to the
> driver that still remain today. This is clearly broken as it must be possible
> to use the same driver module on both SOC_LS1021A and i.MX using
> a runtime check.
>
> I also don't see any i.MX variant actually using this driver, but instead see
> the dts files declaring fsl,imx27-usb devices, which bind to the
> drivers/usb/chipidea/ci_hdrc_imx.c driver. Is this one of those cases
> where we have two separate drivers for the same hardware, or is this
> for a different device?

Exactly. The USB IP on several i.MX devices comes from ChipIdea.

Prior to using devicetree, we had the fsl_mxc_udc driver to handle the
gadget side.

Since i.MX has been converted to a DT-only platform, we no longer need
fsl_mxc_udc, as
drivers/usb/chipidea is used nowadays.

Re: patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

[Arnd Bergmann]

On Fri, Jun 11, 2021 at 4:51 PM Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Jun 11, 2021 at 5:30 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> > Adding Fabio and Guennadi to Cc.
> >
> > I can see that the missing symbols were in a driver that got removed in commit
> > a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver").
> >
> > If CONFIG_ARCH_MXC is disabled, these are stubbed out in the header file.
> > These were added a long time ago by Guennadi Liakhovetski 54e4026b64a9
> > ("USB: gadget: Add i.MX3x support to the fsl_usb2_udc driver"). I also
> > see that this patch added  a few #ifdef CONFIG_ARCH_MXC checks to the
> > driver that still remain today. This is clearly broken as it must be possible
> > to use the same driver module on both SOC_LS1021A and i.MX using
> > a runtime check.
> >
> > I also don't see any i.MX variant actually using this driver, but instead see
> > the dts files declaring fsl,imx27-usb devices, which bind to the
> > drivers/usb/chipidea/ci_hdrc_imx.c driver. Is this one of those cases
> > where we have two separate drivers for the same hardware, or is this
> > for a different device?
>
> Exactly. The USB IP on several i.MX devices comes from ChipIdea.
>
> Prior to using devicetree, we had the fsl_mxc_udc driver to handle the
> gadget side.
>
> Since i.MX has been converted to a DT-only platform, we no longer need
> fsl_mxc_udc, as
> drivers/usb/chipidea is used nowadays.

Ok, good, so the simples solution I suppose is to remove the remaining
bits that Guennadi added when he wrote the removed driver in
54e4026b64a9, and then re-apply Joel's patch.

Alternatively, it might be possible change the chipidea driver to work
on ls1021a and ls1012a, assuming that they use the same hardware
block as i.MX.

Either way, it would be good to test the changes on at least one of these
two platforms.

        Arnd

RE: patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

[Guennadi Liakhovetski]

The fsl_mxc_udc driver was originally developed as a part of a DENX
project, adding Marek to CC to have them check internally what their
preferences and requirements might be.

Thanks
Guennadi

On Fri, 11 Jun 2021, Leo Li wrote:

> > -----Original Message-----
> > From: Arnd Bergmann <arnd@arndb.de>
> > Sent: Friday, June 11, 2021 10:43 AM
> > To: Fabio Estevam <festevam@gmail.com>
> > Cc: gregkh <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>;
> > Joel Stanley <joel@jms.id.au>; Leo Li <leoyang.li@nxp.com>; kbuild test
> > robot <lkp@intel.com>; Peter Chen <peter.chen@nxp.com>; Ran Wang
> > <ran.wang_1@nxp.com>; Stephen Rothwell <sfr@canb.auug.org.au>;
> > Shawn Guo <shawnguo@kernel.org>; # 3.4.x <stable@vger.kernel.org>;
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Subject: Re: patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs""
> > added to usb-linus
> >
> > On Fri, Jun 11, 2021 at 4:51 PM Fabio Estevam <festevam@gmail.com> wrote:
> > > On Fri, Jun 11, 2021 at 5:30 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > > Adding Fabio and Guennadi to Cc.
> > > >
> > > > I can see that the missing symbols were in a driver that got removed
> > > > in commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver").
> > > >
> > > > If CONFIG_ARCH_MXC is disabled, these are stubbed out in the header
> > file.
> > > > These were added a long time ago by Guennadi Liakhovetski
> > > > 54e4026b64a9
> > > > ("USB: gadget: Add i.MX3x support to the fsl_usb2_udc driver"). I
> > > > also see that this patch added  a few #ifdef CONFIG_ARCH_MXC checks
> > > > to the driver that still remain today. This is clearly broken as it
> > > > must be possible to use the same driver module on both SOC_LS1021A
> > > > and i.MX using a runtime check.
> > > >
> > > > I also don't see any i.MX variant actually using this driver, but
> > > > instead see the dts files declaring fsl,imx27-usb devices, which
> > > > bind to the drivers/usb/chipidea/ci_hdrc_imx.c driver. Is this one
> > > > of those cases where we have two separate drivers for the same
> > > > hardware, or is this for a different device?
> > >
> > > Exactly. The USB IP on several i.MX devices comes from ChipIdea.
> > >
> > > Prior to using devicetree, we had the fsl_mxc_udc driver to handle the
> > > gadget side.
> > >
> > > Since i.MX has been converted to a DT-only platform, we no longer need
> > > fsl_mxc_udc, as drivers/usb/chipidea is used nowadays.
> >
> > Ok, good, so the simples solution I suppose is to remove the remaining bits
> > that Guennadi added when he wrote the removed driver in 54e4026b64a9,
> > and then re-apply Joel's patch.
>
> I can provide a patch for this.
>
> >
> > Alternatively, it might be possible change the chipidea driver to work on
> > ls1021a and ls1012a, assuming that they use the same hardware block as i.MX.
>
> It is also used on many legacy FSL PowerPC SoCs.  I agree with the direction, but it does require some effort to make sure it works on all these legacy platforms.  I think Ran Wang had tried to do that, but not completed due to bandwidth issue.
>
> >
> > Either way, it would be good to test the changes on at least one of these two
> > platforms.
> >
> >         Arnd
>

Re: patch "Revert "usb: gadget: fsl: Re-enable driver for ARM SoCs"" added to usb-linus

[Fabio Estevam]

Hi Guennadi,

On Sat, Jun 12, 2021 at 5:38 AM Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
>
> The fsl_mxc_udc driver was originally developed as a part of a DENX
> project, adding Marek to CC to have them check internally what their
> preferences and requirements might be.

Li Yang has already fixed the problem:
https://lore.kernel.org/lkml/20210612003128.372238-1-leoyang.li@nxp.com/