[U-Boot] [PATCH v4 00/10] Improvements for the dwc3_generic driver
Jon Nettleton
jon at solid-run.com
Fri Jan 4 13:54:32 UTC 2019
On Fri, Jan 4, 2019 at 12:22 PM Jon Nettleton <jon at solid-run.com> wrote:
>
> On Fri, Jan 4, 2019 at 11:25 AM Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
> >
> >
> > On 04/01/2019 11:19, Lukasz Majewski wrote:
> > > Hi Jean-Jacques,
> > >
> > >> On 04/01/2019 08:05, Jon Nettleton wrote:
> > >>> On Fri, Dec 7, 2018 at 8:46 AM Jean-Jacques Hiblot
> > >>> <jjhiblot at ti.com> wrote:
> > >>>> On 06/12/2018 21:56, Loic Devulder wrote:
> > >>>>> Hi,
> > >>>>>
> > >>>>> I re-tested this series on Khadas VIM1 and it still fix the USB
> > >>>>> issue I had with 'usb reset' (I already tested v2 patches).
> > >>>>>
> > >>>>> I just have a message and I'm not sure that I had it last time:
> > >>>>> "Error disabling PHY supply
> > >>>>> Can't shutdown USB PHY1 for dwc3 at c9000000"
> > >>>>>
> > >>>>> But USB stack looks ok, I can plug/unplug USB key and do the
> > >>>>> reset, all works as expected.
> > >>>>>
> > >>>>> Since my last test lot of changes has been done in Amlogic SoCs,
> > >>>>> maybe it's because of this?
> > >>>>>
> > >>>>> So if you want you can add my tested-by flag.
> > >>>>> Tested-by: Loic Devulder <ldevulder at suse.de>
> > >>>> Thank you for testing
> > >>>>> On 11/29/18 10:52 AM, Jean-Jacques Hiblot wrote:
> > >>>>>> This series aims at bringing improvements to the dwc3_generic
> > >>>>>> driver so that it can be used by most of the platforms using the
> > >>>>>> dwc3 controller.
> > >>>>>>
> > >>>>>> I tested this on with DRA7 and AM57x platforms for both
> > >>>>>> Peripheral and Host operations. The code to enable DM USB host &
> > >>>>>> dev support for those platforms will be submitted in a separate
> > >>>>>> series.
> > >>>>>>
> > >>>>>> Michal Simek has tested this series:
> > >>>>>> " I have tested it on zcu100 with usb stick, usb to ethernet
> > >>>>>> converter and also dfu.
> > >>>>>> Tested-by: Michal Simek <michal.simek at xilinx.com>"
> > >>>>>>
> > >>>>>> Enhancements:
> > >>>>>> - use separate Kconfig option for DM USB Periphal and DM USB
> > >>>>>> Host. This allow platforms to keep their non-DM USB peripheral
> > >>>>>> code and use the DM USB host.
> > >>>>>> - fixes the bind/probe confusion in dwc3_generic. The probe is
> > >>>>>> done when the USB device is first needed.
> > >>>>>> - handles PHYs when in the peripheral mode. The code to handle
> > >>>>>> the PHYs is shared with the host side
> > >>>>>> - handles clock and reset
> > >>>>>> - bind host controller to the more generic driver 'xhci-dwc3'
> > >>>>>>
> > >>>>>>
> > >>>>>> Changes in v4:
> > >>>>>> - rebased on latest U-Boot
> > >>>>>> - renamed DM_USB_DEV as DM_USB_GADGET
> > >>>>>> - Add a new commit to create a new UCLASS for USB gadget drivers
> > >>>>>>
> > >>>>>> Changes in v3:
> > >>>>>> - fixes bug dwc3_setup_phy(): the phy arrays wasn't returned.
> > >>>>>> This was visible only when the device is removed.
> > >>>>>> - Stub the DWC3 PHY operations if CONFIG_IS_ENABLED(PHY) is
> > >>>>>> false. This fixes all build issues but one (evb-rk3328).
> > >>>>>> - Fix build issue with evb-rk3328 by enabling CONFIG_USB_DWC3.
> > >>>>>> This has little impact on the footprint and should not break the
> > >>>>>> runtime as the xhci-rockchip driver has its own probe function.
> > >>>>>> Nevertheless this was !!! NOT TESTED !!! by lack of hw
> > >>>>>>
> > >>>>>> Changes in v2:
> > >>>>>> - Updated commit log
> > >>>>>> - Fixed typo in thordown.c
> > >>>>>> - select DM_USB_DEV by default for zynqmp platforms
> > >>>>>>
> > >>>>>> Jean-Jacques Hiblot (10):
> > >>>>>> usb: gadget: Do not call board_usb_xxx() directly in USB
> > >>>>>> gadget drivers
> > >>>>>> usb: introduce a separate config option for DM USB device
> > >>>>>> usb: udc: implement DM versions of
> > >>>>>> usb_gadget_initialize()/_release()/_handle_interrupt()
> > >>>>>> dwc3_generic: do not probe the USB device driver when it's
> > >>>>>> bound dwc3: move phy operation to core.c
> > >>>>>> dm: usb: create a new UCLASS ID for USB gadget devices
> > >>>>>> configs: evb-rk3328: Enable CONFIG_USB_DWC3
> > >>>>>> dwc3-generic: Handle the PHYs, the clocks and the reset lines
> > >>>>>> dwc3-generic: Add select_dr_mode operation
> > >>>>>> usb: dwc3: Fix a compilation error with the edison defconfig
> > >>>>>>
> > >>>>>> arch/arm/Kconfig | 2 +
> > >>>>>> board/sunxi/board.c | 2 +-
> > >>>>>> cmd/fastboot.c | 4 +-
> > >>>>>> cmd/rockusb.c | 4 +-
> > >>>>>> cmd/thordown.c | 4 +-
> > >>>>>> cmd/usb_gadget_sdp.c | 4 +-
> > >>>>>> cmd/usb_mass_storage.c | 4 +-
> > >>>>>> common/dfu.c | 6 +-
> > >>>>>> configs/evb-rk3328_defconfig | 1 +
> > >>>>>> drivers/usb/Kconfig | 14 +++
> > >>>>>> drivers/usb/dwc3/Kconfig | 7 +-
> > >>>>>> drivers/usb/dwc3/core.c | 89 ++++++++++++++-
> > >>>>>> drivers/usb/dwc3/dwc3-generic.c | 208
> > >>>>>> ++++++++++++++++++++++++++++--------
> > >>>>>> drivers/usb/dwc3/ep0.c | 2 +-
> > >>>>>> drivers/usb/gadget/ether.c | 40 ++-----
> > >>>>>> drivers/usb/gadget/udc/Makefile | 4 +
> > >>>>>> drivers/usb/gadget/udc/udc-core.c | 3 +-
> > >>>>>> drivers/usb/gadget/udc/udc-uclass.c | 58 ++++++++++
> > >>>>>> drivers/usb/host/xhci-dwc3.c | 95 ++--------------
> > >>>>>> drivers/usb/musb-new/omap2430.c | 2 +-
> > >>>>>> drivers/usb/musb-new/sunxi.c | 2 +-
> > >>>>>> include/dm/uclass-id.h | 1 +
> > >>>>>> include/dwc3-uboot.h | 19 ++++
> > >>>>>> include/linux/usb/gadget.h | 18 ++++ 24 files changed,
> > >>>>>> 401 insertions(+), 192 deletions(-) create mode 100644
> > >>>>>> drivers/usb/gadget/udc/udc-uclass.c
> > >>>> _______________________________________________
> > >>>> U-Boot mailing list
> > >>>> U-Boot at lists.denx.de
> > >>>> https://lists.denx.de/listinfo/u-boot
> > >>> First off thanks for the work. I have imported it into my u-boot
> > >>> tree for the iMX8M and now have both usb host and gadget mode
> > >>> working simultaneously. There is one catch. Currently, the iMX8M
> > >>> codebase does not support clock and power device initialization
> > >>> from DM. I have patched the driver to punt back to the board to do
> > >>> these initializations in the following patch. Yes I know not
> > >>> ideal, but functional. I am wondering if we should support
> > >>> architecture specific implementation quirks for boards that don't
> > >>> fully support DM initialization yet.
> > >> That is not fitting the DM model. I had the same issue and finally
> > >> enabled the clocks in the board_late_init():
> > >> board/ti/dra7xx/evm.c:662:int board_late_init(void)
> > >>
> > >>> I am open to writing something more complete if
> > >>> we decide it is something worth pursuing, otherwise I will carry
> > >>> this patch in my tree for time being.
> > >> The true solution would be to implement the support for clk.
> > > Many other people on the ML (including me) see the urgent need to
> > > implement the DM_CLK for e.g. imx6/5/7/8, etc.
> > >
> > > I think that it would be best to add it clock by clock. The stubs in
> > > board code/files is a dead end, IMHO.
> >
> > I agree 100%.
> >
> > We had a similar discussion for the TI platforms, but due to the
> > specific design of the TI power/clk in Linux (hwmod), we chose to keep
> > the clocks handling in the board file at the moment.
> >
> >
> > >
> > >>> -Jon
> > >>>
> > >>> diff --git a/drivers/usb/dwc3/dwc3-generic.c
> > >>> b/drivers/usb/dwc3/dwc3-generic.c index 6d9be9b245..90d4ff2ea7
> > >>> 100644 --- a/drivers/usb/dwc3/dwc3-generic.c
> > >>> +++ b/drivers/usb/dwc3/dwc3-generic.c
> > >>> @@ -119,6 +119,15 @@ struct dwc3_glue_ops {
> > >>> enum usb_dr_mode mode);
> > >>> };
> > >>>
> > >>> +__weak int board_usb_of_init(struct udevice *dev)
> > >>> +{
> > >>> + return 0;
> > >>> +}
> > >>> +
> > >>> +__weak void board_usb_of_cleanup(struct udevice *dev)
> > >>> +{
> > >>> +}
> > >>> +
> > >>> static int dwc3_glue_bind(struct udevice *parent)
> > >>> {
> > >>> const void *fdt = gd->fdt_blob;
> > >>> @@ -242,6 +251,8 @@ static int dwc3_glue_probe(struct udevice *dev)
> > >>> index++;
> > >>> }
> > >>>
> > >>> + ret = board_usb_of_init(dev);
> > >>> +
> > >>> return 0;
> > >>> }
> > >>>
> > >>> @@ -249,6 +260,8 @@ static int dwc3_glue_remove(struct udevice *dev)
> > >>> {
> > >>> struct dwc3_glue_data *glue = dev_get_platdata(dev);
> > >>>
> > >>> + board_usb_of_cleanup(dev);
> > >>> +
> > >>> reset_release_bulk(&glue->resets);
> > >>>
> > >>> clk_release_bulk(&glue->clks);
> > >>> @@ -257,6 +270,7 @@ static int dwc3_glue_remove(struct udevice *dev)
> > >>> }
> > >>>
> > >
> > >
> > >
> > > Best regards,
> > >
> > > Lukasz Majewski
> > >
> > > --
> > >
> > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
>
> Obviously the proper solution is implementing a proper DM clk driver.
> I generally refrain from just enabling a bunch of clocks and code in
> our board file, since our devices are SOM + carrier, and many
> customers design their own boards and then continue to use our
> bootloader. I am fine carrying the patch until we have the time to
> build the proper infrastructure in mainline.
Quick followup.
Has anyone tried to build this into SPL? I am getting lots of
undefined symbols. undefined reference to `usb_get_dr_mode' ,
undefined reference to `device_bind_driver_to_node' etc.
-Jon
More information about the U-Boot
mailing list