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

[Simon Glass]

Hi Stephen,

On Fri, Dec 2, 2011 at 12:40 PM, Stephen Warren <swarren at nvidia.com> wrote:
> On 12/02/2011 10:00 AM, Simon Glass wrote:
>> On Fri, Dec 2, 2011 at 8:10 AM, Stephen Warren <swarren at nvidia.com> wrote:
>>> On 12/01/2011 06:51 PM, Simon Glass wrote:
>>>> 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.
>>>>
>>>> It is actually the bit position of the peripheral in the clock
>>>> registers, so arguably a hardware description. U-Boot uses this to
>>>> efficiently manage peripheral clocks, reset, pinmux, etc.
>>>>
>>>> How does the kernel figure out the clock register (etc.) to use with a
>>>> particular peripheral? Also bear in mind that the intent with U-Boot
>>>> is to be a lot more lightweight with these things.
>>>
>>> The DT binding has to be identical though; U-Boot implementation details
>>> aren't supposed to affect the content of the DT.
>>>
>>> Clock bindings are an area of active development. I haven't been
>>> following the progress, but I imagine that the clock controller will
>>> define a node per clock, and the devices that consume the clock will
>>> refer to that node using a phandle. It's then up to the clock controller
>>> driver to extract whatever information it needs from the clock node and
>>> map that to an internal periph-id. It's plausible that a legitimate part
>>> of the clock binding itself is such a periph-id field, but that should
>>> be defined by the clock controller binding, not the peripheral binding.
>>
>> OK, well this is an example of where I would like to run with what we
>> have, and adjust it later when things are finalized in the kernel.
>>
>> I'm not sure about your analysis here actually. The peripherals have a
>> selectable source clock and their own divider from that clock, plus
>> they have bits for enabling their internal clock and reset. The
>> registers for all of these can sort-of be indexed through the
>> peripheral ID. I think with this model you would need to have a
>> separate clock node for every peripheral, with the peripheral node
>> pointing back to that. Perhaps that is what you mean. It means that
>> every peripheral has its own node and then a clock node. It probably
>> won't be too slow to decode.
>
> re: the last-but-one sentence: Yes, I think that's how it'll work.
>
>>>>>> +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 aliases are (I thought) the official way that device trees specify
>>>> device ordering. No we do not enumerate things in U-Boot - there is no
>>>> device model as such. We can do this on Tegra, but still need to know
>>>> the order to use (i.e. which is port 0).
>>>
>>> I don't believe the kernel uses the alias for anything at all right now.
>>> Instead, it enumerates all nodes that match a certain compatible flag,
>>> and instantiates a device for each one it has a driver for. I believe
>>> this mode of operation is pretty implicit in DT itself; it's something
>>> U-Boot should do too.
>>
>> It does this at present with USB. But we want to enumerate the ports
>> and know which is port 0, which is port 1, etc. How does the kernel do
>> that?
>
> I don't think it cares; it just hosts a number of USB ports, and
> peripherals show up on those USB ports. The numbering of the ports is
> entirely arbitrary AFAIK.

OK. For the moment in U-Boot we do care, so I will leave the alias
solution in there for now. I may look later at the patch to suppose a
virtual hub on Tegra.

Regards,
Simon

>
> --
> nvpublic
>