[U-Boot] [PATCH v4 16/20] tegra: usb: Add support for Tegra USB peripheral

[Stephen Warren]

On 01/11/2012 09:33 PM, Simon Glass wrote:
> This adds basic support for the Tegra2 USB controller. Board files should
> call board_usb_init() to set things up.

> diff --git a/arch/arm/cpu/armv7/tegra2/usb.c b/arch/arm/cpu/armv7/tegra2/usb.c

> +/* Record which controller can switch from host to device mode */
> +static struct fdt_usb *host_dev_ctlr;

As you'll note from my comments on the DT bindings, it doesn't make
sense to have a single global variable for that; both USB1 and USB3 can
switch at run-time apparently, and USB2 at initialization time.

> +void usbf_reset_controller(enum periph_id id, struct usb_ctlr *usbctlr)
...
> +       /*
> +        * Set USB1_NO_LEGACY_MODE to 1, Registers are accessible under
> +        * base address
> +        */
> +       if (id == PERIPH_ID_USBD)
> +               setbits_le32(&usbctlr->usb1_legacy_ctrl, USB1_NO_LEGACY_MODE);
...
> +       /* Set USB3 to use UTMIP PHY */
> +       if (id == PERIPH_ID_USB3)
> +               setbits_le32(&usbctlr->susp_ctrl, UTMIP_PHY_ENB);
...
> +static void init_usb_controller(enum periph_id id, struct usb_ctlr *usbctlr,
> +               const u32 timing[])
...
> +       /*
> +        * To Use the A Session Valid for cable detection logic, VBUS_WAKEUP
> +        * mux must be switched to actually use a_sess_vld threshold.
> +        */
> +       if (id == PERIPH_ID_USBD) {
> +               clrsetbits_le32(&usbctlr->usb1_legacy_ctrl,
> +                       VBUS_SENSE_CTL_MASK, VBUS_SENSE_CTL_A_SESS_VLD);
> +       }

Uggh. The driver shouldn't really be altering its behaviour based on
knowledge of the instance number. Can those conditions be modified to
test something else instead?

For the first (USBD) condition, it seems like if the registers are
different, we should really have either different compatible values, or
a flag in the DT to indicate this.

For the second (USB3) condition, shouldn't this be testing the phy type
field, not the instance ID?

For the third (USBD) condition, can it test whether there's a VBUS GPIO
defined, or whether an appropriate combination of my proposed
enable-host-mode/enable-device-mode properties are present?

-- 
nvpublic