On 11/23/2011 08:54 PM, Simon Glass wrote: > This adds basic support for the Tegra2 USB controller. Board files should > call board_usb_init() to set things up.
Just a very brief review: > +/* Put the port into host mode (this only works for USB1) */ > +static void set_host_mode(struct usb_ctlr *usbctlr) > +{ > + /* Check whether remote host from USB1 is driving VBus */ > + if (readl(&usbctlr->phy_vbus_sensors) & VBUS_VLD_STS) > + return; > + > + /* > + * If not driving, we set GPIO USB1_VBus_En. Seaboard platform uses > + * PAD SLXK (GPIO D.00) as USB1_VBus_En Config as GPIO > + */ > + gpio_direction_output(GPIO_PD0, 1); The GPIO to use here needs to be parameterized; there's no reason to believe it'll be the same on all boards (or even that boards will have any such GPIO). > +/* Put our ports into host mode */ > +void usb_set_host_mode(void) > +{ > + if (host_dev_ctlr) > + set_host_mode(host_dev_ctlr); > +} Why not just call set_host_mode() directly from board_usb_init()? > +#ifndef CONFIG_OF_CONTROL > +static int probe_port(struct usb_ctlr *usbctlr, const int params[]) > +{ > + enum periph_id id; > + int utmi = 0; > + > + /* > + * Get the periph id. Port 1 has an internal transceiver, port 3 is > + * external > + */ > + switch ((u32)usbctlr) { > + case TEGRA_USB1_BASE: > + id = PERIPH_ID_USBD; > + break; > + > + case TEGRA_USB3_BASE: > + id = PERIPH_ID_USB3; > + utmi = 1; > + break; > + > + default: > + printf("tegrausb: probe_port: no such port %p\n", usbctlr); > + return -1; > + } What about the other port (USB2)? > +#ifdef CONFIG_OF_CONTROL > +int fdt_decode_usb(const void *blob, int node, unsigned osc_frequency_mhz, > + struct fdt_usb *config) > +{ > + int clk_node = 0, rate; > + > + /* Find the parameters for our oscillator frequency */ > + do { > + clk_node = fdt_node_offset_by_compatible(blob, clk_node, > + "nvidia,tegra20-usbparams"); > + if (clk_node < 0) { > + debug("Cannot find USB params for clock %u", > + osc_frequency_mhz); > + return -FDT_ERR_NOTFOUND; > + } > + rate = fdtdec_get_int(blob, clk_node, "osc-frequency", 0); > + } while (rate != osc_frequency_mhz); > + > + config->reg = (struct usb_ctlr *)fdtdec_get_addr(blob, node, "reg"); > + config->host_mode = fdtdec_get_bool(blob, node, "support-host-mode"); Property "support-host-mode" isn't something that's supported by the kernel binding, and needs discussion/ack there. > + config->utmi = fdtdec_lookup_phandle(blob, node, "utmi") >= 0; In the kernel DT binding, this is property "phy_type" with legal values "utmi" or "ulpi." > + config->enabled = fdtdec_get_is_enabled(blob, node); > + config->periph_id = fdtdec_get_int(blob, node, "periph-id", -1); periph-id is a U-Boot specific concept, not HW description. The DT shouldn't contain that value. > + if (config->periph_id == -1) > + return -FDT_ERR_NOTFOUND; > + > + return fdtdec_get_int_array(blob, clk_node, "params", config->params, > + PARAM_COUNT); Property "params" (which should probably be named something better anyway) isn't something that's supported by the kernel binding, and needs discussion/ack there. Instead, I suggest following the kernel's approach for now - don't specify these PHY parameters in the binding, but rather just apply the defaults, which are apparently suitable for all the boards supported by the mainline kernel at least. > +int board_usb_init(const void *blob) > +{ > +#ifdef CONFIG_OF_CONTROL > + struct fdt_usb config; > + int clk_done = 0; > + int node, upto = 0; > + unsigned osc_freq = clock_get_rate(CLOCK_ID_OSC); > + > + do { > + node = fdtdec_next_alias(blob, "usb", > + COMPAT_NVIDIA_TEGRA20_USB, &upto); Why only initialize USB controllers with aliases? Surely this should enumerate all nodes with a specific compatible flag? > + /* The first port we find gets to set the clocks */ Ick. > + /* Set up our two ports */ > +#ifdef CONFIG_TEGRA_USB1_HOST > + host_dev_ctlr = (struct usb_ctlr *)TEGRA_USB1_BASE; > +#endif That port is always the host/device switchable controller. Why not always make that assignment? The issue here is probably that set_host_mode() isn't suitable for all boards. The solution seems to be to fix that. -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot