[U-Boot] [PATCH 5/8] tegra: usb: move controller init into start_port

Lucas Stach
2012-10-30T13:54:29Z

Am Dienstag, den 30.10.2012, 06:48 -0700 schrieb Simon Glass:
> Hi Lucas,
>
> On Tue, Oct 30, 2012 at 6:37 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > Am Dienstag, den 30.10.2012, 06:27 -0700 schrieb Simon Glass:
> >> Hi Lucas,
> >>
> >> On Tue, Oct 30, 2012 at 2:22 AM, Lucas Stach <dev@lynxeye.de> wrote:
> >> > There is no need to init a USB controller before the upper layers indicate
> >> > that they are actually going to use it.
> >> >
> >> > board_usb_init now only parses the device tree and sets up the common pll.
> >> >
> >> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> >> > ---
> >> > arch/arm/cpu/armv7/tegra20/usb.c | 47 +++++++++++++++-------------------------
> >> > 1 Datei ge?ndert, 18 Zeilen hinzugef?gt(+), 29 Zeilen entfernt(-)
> >> >
> >> > diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
> >> > index cf800b1..e372b8b 100644
> >> > --- a/arch/arm/cpu/armv7/tegra20/usb.c
> >> > +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> >> > @@ -417,44 +417,29 @@ static int init_ulpi_usb_controller(struct fdt_usb *config)
> >> > }
> >> > #endif
> >> >
> >> > -/**
> >> > - * Add a new USB port to the list of available ports.
> >> > - *
> >> > - * @param config USB port configuration
> >> > - * @return 0 if ok, -1 if error (too many ports)
> >> > - */
> >> > -static int add_port(struct fdt_usb *config)
> >> > +int tegrausb_start_port(int portnum, u32 *hccr, u32 *hcor)
> >> > {
> >> > - if (port_count == USB_PORTS_MAX) {
> >> > - printf("tegrausb: Cannot register more than %d ports\n",
> >> > - USB_PORTS_MAX);
> >> > + struct fdt_usb *config;
> >> > + struct usb_ctlr *usbctlr;
> >> > +
> >> > + if (portnum >= port_count)
> >> > return -1;
> >> > - }
> >> > +
> >> > + config = &port[portnum];
> >> >
> >> > if (config->utmi && init_utmi_usb_controller(config)) {
> >> > - printf("tegrausb: Cannot init port\n");
> >> > + printf("tegrausb: Cannot init port %d\n", portnum);
> >> > return -1;
> >> > }
> >> >
> >> > if (config->ulpi && init_ulpi_usb_controller(config)) {
> >> > - printf("tegrausb: Cannot init port\n");
> >> > + printf("tegrausb: Cannot init port %d\n", portnum);
> >> > return -1;
> >> > }
> >> >
> >> > - port[port_count++] = *config;
> >> > -
> >> > - return 0;
> >> > -}
> >> > + set_host_mode(config);
> >>
> >> This is good, but now I think you will re-init the USB peripheral at
> >> every 'usb start'. Perhaps you should remember whether it has been
> >> inited and only do it the first time?
> >
> > I have to look this up, but the upper USB layers should not call those
> > lowlevel init functions repeatedly unless explicitly asked for it
> > through a "usb reset" or the like. If it actually does so it's a bug in
> > the upper layer and should not be fixed up in the lowlevel functions.
>
> Perhaps, but you have to write your code in the environment that
> exists. At present usb_lowlevel_init() is called on every 'usb start'
> (and ehci_hcd_init() from that).
>
After all this is open source and I would rather spin a patch to fix
this at the right spot if we do the wrong thing, than having to cope
with the bug at a lower level. Even with bug present we are not failing
in any severe way, we are just wasting time bringing up a controller
which is already up.

Regards,
Lucas