Hi Stephen,
On Mon, Dec 5, 2011 at 1:33 PM, Stephen Warren <swar...@nvidia.com> wrote:
> 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 <swar...@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.
At the moment the fdts are in the U-Boot tree, so we should be able to
change them at the same time. But of course when we update the fdt we
need to update the U-Boot code. Is there any alternative other than
doing nothing until the kernel decides everything?
>
>>>> +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.
At present we do:
'usb start 0'
'usb start 1'
to select between the ports. There is a patch in the works to change
that but it hasn't gone upstream yet, or at least wasn't accepted.
There is a problem with adopting that approach in general - e.g.
*which* UART you write to has a big effect on whether the user sees
any output.
>
> (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)
Yes it is - in fact I added that property.
Regards,
Simon
>
> --
> nvpublic
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
