[U-Boot] [PATCH 10/14] tegra: usb: Add support for USB peripheral

[Stephen Warren]

On 12/02/2011 05:59 PM, Simon Glass wrote:
> Hi Stephen,
> 
> Here are my comments on the rest of your email.
> 
> On Mon, Nov 28, 2011 at 11:21 AM, Stephen Warren <swarren at nvidia.com> wrote:
>> 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.

>>> +       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.
> 
> See my previous comments as to why this is desirable. We can change
> over to a clock-based approach once the kernel implements it.

That will cause backwards-compatibility problems; older FDTs won't work
with newer U-Boots. We should avoid incompatible changes like this if at
all possible.

>>> +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?
> 
> See my other comments - we want to know that port 0 is USB3 on Seaboard.

Why does U-Boot care? Everything should be enumerating which peripherals
happen to appear on the various USB busses, and not care which host
controller they're attached to.

(With the exception of USB port 0 being device-capable, but that should
be configured by an explicit property, not based on aliases or anything
like that)

-- 
nvpublic