[v2] usb: gadget: fsl: Re-enable driver for ARM SoCs

Message ID 20210610034957.93376-1-joel@jms.id.au
State New
Headers show
Series
  • [v2] usb: gadget: fsl: Re-enable driver for ARM SoCs
Related show

Commit Message

Joel Stanley June 10, 2021, 3:49 a.m.
The commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")
dropped the ARCH_MXC dependency from USB_FSL_USB2, leaving it depending
solely on FSL_SOC.

FSL_SOC is powerpc only; it was briefly available on ARM in 2014 but was
removed by commit cfd074ad8600 ("ARM: imx: temporarily remove
CONFIG_SOC_FSL from LS1021A"). Therefore the driver can no longer be
enabled on ARM platforms.

This appears to be a mistake as arm64's ARCH_LAYERSCAPE and arm32
SOC_LS1021A SoCs use this symbol. It's enabled in these defconfigs:

arch/arm/configs/imx_v6_v7_defconfig:CONFIG_USB_FSL_USB2=y
arch/arm/configs/multi_v7_defconfig:CONFIG_USB_FSL_USB2=y
arch/powerpc/configs/mgcoge_defconfig:CONFIG_USB_FSL_USB2=y
arch/powerpc/configs/mpc512x_defconfig:CONFIG_USB_FSL_USB2=y

To fix, expand the dependencies so USB_FSL_USB2 can be enabled on the
ARM platforms, and with COMPILE_TEST.

Fixes: a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")
Signed-off-by: Joel Stanley <joel@jms.id.au>

---
v2: Fix spelling of ARCH_LAYERSCAPE
---
 drivers/usb/gadget/udc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.30.2

Comments

Joel Stanley June 11, 2021, 2:01 a.m. | #1
On Thu, 10 Jun 2021 at 03:50, Joel Stanley <joel@jms.id.au> wrote:
>

> The commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")

> dropped the ARCH_MXC dependency from USB_FSL_USB2, leaving it depending

> solely on FSL_SOC.

>

> FSL_SOC is powerpc only; it was briefly available on ARM in 2014 but was

> removed by commit cfd074ad8600 ("ARM: imx: temporarily remove

> CONFIG_SOC_FSL from LS1021A"). Therefore the driver can no longer be

> enabled on ARM platforms.

>

> This appears to be a mistake as arm64's ARCH_LAYERSCAPE and arm32

> SOC_LS1021A SoCs use this symbol. It's enabled in these defconfigs:

>

> arch/arm/configs/imx_v6_v7_defconfig:CONFIG_USB_FSL_USB2=y

> arch/arm/configs/multi_v7_defconfig:CONFIG_USB_FSL_USB2=y

> arch/powerpc/configs/mgcoge_defconfig:CONFIG_USB_FSL_USB2=y

> arch/powerpc/configs/mpc512x_defconfig:CONFIG_USB_FSL_USB2=y

>

> To fix, expand the dependencies so USB_FSL_USB2 can be enabled on the

> ARM platforms, and with COMPILE_TEST.


This didn't work out, as there's some missing clock initialisation
calls that were removed when fsl_mxc_udc was deleted:

arm-linux-gnueabi-ld: drivers/usb/gadget/udc/fsl_udc_core.o: in
function `fsl_udc_remove':
fsl_udc_core.c:(.text+0xc88): undefined reference to `fsl_udc_clk_release'
arm-linux-gnueabi-ld: drivers/usb/gadget/udc/fsl_udc_core.o: in
function `fsl_udc_probe':
fsl_udc_core.c:(.text+0x1c44): undefined reference to `fsl_udc_clk_init'
arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x1dcc): undefined
reference to `fsl_udc_clk_finalize'
arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x1fe8): undefined
reference to `fsl_udc_clk_release'

We could add them back, but it's not clear if this driver is still
maintained. I'm looking to the NXP people for input here.

Stephen has dropped the patch from today's -next. I suggest we drop
(or revert?) this patch from the USB tree while a solution is found.

Cheers,

Joel

>

> Fixes: a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")

> Signed-off-by: Joel Stanley <joel@jms.id.au>

> ---

> v2: Fix spelling of ARCH_LAYERSCAPE

> ---

>  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 8c614bb86c66..7348acbdc560 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

> +       depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST

>         help

>            Some of Freescale PowerPC and i.MX processors have a High Speed

>            Dual-Role(DR) USB controller, which supports device mode.

> --

> 2.30.2

>
Leo Li June 11, 2021, 2:11 a.m. | #2
> -----Original Message-----

> From: Joel Stanley <joel@jms.id.au>

> Sent: Thursday, June 10, 2021 9:01 PM

> To: Fabio Estevam <festevam@gmail.com>; Peter Chen

> <peter.chen@nxp.com>; Arnd Bergmann <arnd@arndb.de>; Felipe Balbi

> <balbi@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Leo Li

> <leoyang.li@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Stephen

> Rothwell <sfr@canb.auug.org.au>

> Cc: linux-usb@vger.kernel.org; Linux ARM <linux-arm-

> kernel@lists.infradead.org>

> Subject: Re: [PATCH v2] usb: gadget: fsl: Re-enable driver for ARM SoCs

> 

> On Thu, 10 Jun 2021 at 03:50, Joel Stanley <joel@jms.id.au> wrote:

> >

> > The commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the

> > driver") dropped the ARCH_MXC dependency from USB_FSL_USB2,

> leaving it

> > depending solely on FSL_SOC.

> >

> > FSL_SOC is powerpc only; it was briefly available on ARM in 2014 but

> > was removed by commit cfd074ad8600 ("ARM: imx: temporarily remove

> > CONFIG_SOC_FSL from LS1021A"). Therefore the driver can no longer be

> > enabled on ARM platforms.

> >

> > This appears to be a mistake as arm64's ARCH_LAYERSCAPE and arm32

> > SOC_LS1021A SoCs use this symbol. It's enabled in these defconfigs:

> >

> > arch/arm/configs/imx_v6_v7_defconfig:CONFIG_USB_FSL_USB2=y

> > arch/arm/configs/multi_v7_defconfig:CONFIG_USB_FSL_USB2=y

> > arch/powerpc/configs/mgcoge_defconfig:CONFIG_USB_FSL_USB2=y

> > arch/powerpc/configs/mpc512x_defconfig:CONFIG_USB_FSL_USB2=y

> >

> > To fix, expand the dependencies so USB_FSL_USB2 can be enabled on the

> > ARM platforms, and with COMPILE_TEST.

> 

> This didn't work out, as there's some missing clock initialisation calls that were

> removed when fsl_mxc_udc was deleted:

> 

> arm-linux-gnueabi-ld: drivers/usb/gadget/udc/fsl_udc_core.o: in function

> `fsl_udc_remove':

> fsl_udc_core.c:(.text+0xc88): undefined reference to `fsl_udc_clk_release'

> arm-linux-gnueabi-ld: drivers/usb/gadget/udc/fsl_udc_core.o: in function

> `fsl_udc_probe':

> fsl_udc_core.c:(.text+0x1c44): undefined reference to `fsl_udc_clk_init'

> arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x1dcc): undefined reference to

> `fsl_udc_clk_finalize'

> arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x1fe8): undefined reference to

> `fsl_udc_clk_release'

> 

> We could add them back, but it's not clear if this driver is still maintained. I'm

> looking to the NXP people for input here.


Adding Ran Wang to the loop.  I think we will maintain the driver to keep the legacy platforms working but will not add new users of this driver.

> 

> Stephen has dropped the patch from today's -next. I suggest we drop (or

> revert?) this patch from the USB tree while a solution is found.

> 

> Cheers,

> 

> Joel

> 

> >

> > Fixes: a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")

> > Signed-off-by: Joel Stanley <joel@jms.id.au>

> > ---

> > v2: Fix spelling of ARCH_LAYERSCAPE

> > ---

> >  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 8c614bb86c66..7348acbdc560

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

> > +       depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A ||

> > + COMPILE_TEST

> >         help

> >            Some of Freescale PowerPC and i.MX processors have a High Speed

> >            Dual-Role(DR) USB controller, which supports device mode.

> > --

> > 2.30.2

> >
Greg KH June 11, 2021, 7:22 a.m. | #3
On Fri, Jun 11, 2021 at 02:11:27AM +0000, Leo Li wrote:
> 

> 

> > -----Original Message-----

> > From: Joel Stanley <joel@jms.id.au>

> > Sent: Thursday, June 10, 2021 9:01 PM

> > To: Fabio Estevam <festevam@gmail.com>; Peter Chen

> > <peter.chen@nxp.com>; Arnd Bergmann <arnd@arndb.de>; Felipe Balbi

> > <balbi@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Leo Li

> > <leoyang.li@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Stephen

> > Rothwell <sfr@canb.auug.org.au>

> > Cc: linux-usb@vger.kernel.org; Linux ARM <linux-arm-

> > kernel@lists.infradead.org>

> > Subject: Re: [PATCH v2] usb: gadget: fsl: Re-enable driver for ARM SoCs

> > 

> > On Thu, 10 Jun 2021 at 03:50, Joel Stanley <joel@jms.id.au> wrote:

> > >

> > > The commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the

> > > driver") dropped the ARCH_MXC dependency from USB_FSL_USB2,

> > leaving it

> > > depending solely on FSL_SOC.

> > >

> > > FSL_SOC is powerpc only; it was briefly available on ARM in 2014 but

> > > was removed by commit cfd074ad8600 ("ARM: imx: temporarily remove

> > > CONFIG_SOC_FSL from LS1021A"). Therefore the driver can no longer be

> > > enabled on ARM platforms.

> > >

> > > This appears to be a mistake as arm64's ARCH_LAYERSCAPE and arm32

> > > SOC_LS1021A SoCs use this symbol. It's enabled in these defconfigs:

> > >

> > > arch/arm/configs/imx_v6_v7_defconfig:CONFIG_USB_FSL_USB2=y

> > > arch/arm/configs/multi_v7_defconfig:CONFIG_USB_FSL_USB2=y

> > > arch/powerpc/configs/mgcoge_defconfig:CONFIG_USB_FSL_USB2=y

> > > arch/powerpc/configs/mpc512x_defconfig:CONFIG_USB_FSL_USB2=y

> > >

> > > To fix, expand the dependencies so USB_FSL_USB2 can be enabled on the

> > > ARM platforms, and with COMPILE_TEST.

> > 

> > This didn't work out, as there's some missing clock initialisation calls that were

> > removed when fsl_mxc_udc was deleted:

> > 

> > arm-linux-gnueabi-ld: drivers/usb/gadget/udc/fsl_udc_core.o: in function

> > `fsl_udc_remove':

> > fsl_udc_core.c:(.text+0xc88): undefined reference to `fsl_udc_clk_release'

> > arm-linux-gnueabi-ld: drivers/usb/gadget/udc/fsl_udc_core.o: in function

> > `fsl_udc_probe':

> > fsl_udc_core.c:(.text+0x1c44): undefined reference to `fsl_udc_clk_init'

> > arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x1dcc): undefined reference to

> > `fsl_udc_clk_finalize'

> > arm-linux-gnueabi-ld: fsl_udc_core.c:(.text+0x1fe8): undefined reference to

> > `fsl_udc_clk_release'

> > 

> > We could add them back, but it's not clear if this driver is still maintained. I'm

> > looking to the NXP people for input here.

> 

> Adding Ran Wang to the loop.  I think we will maintain the driver to keep the legacy platforms working but will not add new users of this driver.


Now reverted in my tree.

thanks,

greg k-h

Patch

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 8c614bb86c66..7348acbdc560 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
+	depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
 	help
 	   Some of Freescale PowerPC and i.MX processors have a High Speed
 	   Dual-Role(DR) USB controller, which supports device mode.