Hi Stephen,
On Wed, Jan 18, 2012 at 4:43 PM, Stephen Warren <swar...@nvidia.com> wrote:
> 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?
Yes I'm pretty sure I can clean this up to make it more fdt-dependent.
Will have a look.
Regards,
Simon
>
> --
> nvpublic
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
